On 10/13/2012 06:00 PM, Eric Blake wrote:
> qemu 1.3 will be adding a 'block-commit' monitor command, per
> qemu.git commit ed61fc1. It matches nicely to the libvirt API
> virDomainBlockCommit.
Hmm. Imagine that. What serendipity. I wonder how that could have
happened. etc.... :-)
>
> * src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCK_COMMIT): New bit.
> * src/qemu/qemu_capabilities.c (qemuCapsProbeQMPCommands): Set it.
> * src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): New prototype.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
> Likewise.
> * src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Implement it.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
> Likewise.
> (qemuMonitorJSONHandleBlockJobImpl)
> (qemuMonitorJSONGetBlockJobInfoOne): Handle new event type.
> ---
>
> Change from v1:
> upstream qemu now has the commit
> recalculate backing chain after commit completes
>
> src/qemu/qemu_capabilities.c | 3 +++
> src/qemu/qemu_capabilities.h | 1 +
> src/qemu/qemu_monitor.c | 30 ++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor.h | 7 +++++++
> src/qemu/qemu_monitor_json.c | 34 ++++++++++++++++++++++++++++++++++
> src/qemu/qemu_monitor_json.h | 7 +++++++
> src/qemu/qemu_process.c | 15 +++++++++------
> 7 files changed, 91 insertions(+), 6 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index fb0fe54..73a12f1 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -187,6 +187,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
> "reboot-timeout", /* 110 */
> "dump-guest-core",
> "seamless-migration",
> + "block-commit",
> );
>
> struct _qemuCaps {
> @@ -1881,6 +1882,8 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
> qemuCapsSet(caps, QEMU_CAPS_SPICE);
> else if (STREQ(name, "query-kvm"))
> qemuCapsSet(caps, QEMU_CAPS_KVM);
> + else if (STREQ(name, "block-commit"))
> + qemuCapsSet(caps, QEMU_CAPS_BLOCK_COMMIT);
> VIR_FREE(name);
> }
> VIR_FREE(commands);
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 5d343c1..6939c45 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -150,6 +150,7 @@ enum qemuCapsFlags {
> QEMU_CAPS_REBOOT_TIMEOUT = 110, /* -boot reboot-timeout */
> QEMU_CAPS_DUMP_GUEST_CORE = 111, /* dump-guest-core-parameter */
> QEMU_CAPS_SEAMLESS_MIGRATION = 112, /* seamless-migration for SPICE */
> + QEMU_CAPS_BLOCK_COMMIT = 113, /* block-commit */
>
> QEMU_CAPS_LAST, /* this must always be the last item */
> };
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 85b0bc2..68c6d3f 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2786,6 +2786,36 @@ qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
> return ret;
> }
>
> +/* Start a block-commit block job. bandwidth is in MB/sec. */
> +int
> +qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
> + const char *top, const char *base,
> + unsigned long bandwidth)
> +{
> + int ret = -1;
> + unsigned long long speed;
> +
> + VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld",
> + mon, device, NULLSTR(top), NULLSTR(base), bandwidth);
> +
> + /* Convert bandwidth MiB to bytes */
> + speed = bandwidth;
> + if (speed > ULLONG_MAX / 1024 / 1024) {
> + virReportError(VIR_ERR_OVERFLOW,
> + _("bandwidth must be less than %llu"),
> + ULLONG_MAX / 1024 / 1024);
> + return -1;
> + }
> + speed <<= 20;
> +
> + if (mon->json)
> + ret = qemuMonitorJSONBlockCommit(mon, device, top, base, speed);
> + else
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("block-commit requires JSON monitor"));
> + return ret;
> +}
> +
> int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
> const char *cmd,
> char **reply,
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 54b3a99..9bdc802 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -521,6 +521,13 @@ int qemuMonitorDiskSnapshot(qemuMonitorPtr mon,
> int qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>
> +int qemuMonitorBlockCommit(qemuMonitorPtr mon,
> + const char *device,
> + const char *top,
> + const char *base,
> + unsigned long bandwidth)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +
> int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
> const char *cmd,
> char **reply,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index bd52ce4..501b338 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -803,6 +803,8 @@ qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon,
>
> if (STREQ(type_str, "stream"))
> type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
> + else if (STREQ(type_str, "commit"))
> + type = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT;
>
> switch ((virConnectDomainEventBlockJobStatus) event) {
> case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
> @@ -3269,6 +3271,36 @@ cleanup:
> return ret;
> }
>
> +/* speed is in bytes/sec */
> +int
> +qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
> + const char *top, const char *base,
> + unsigned long speed)
up above you're using unsigned long long for speed. If
qemuMonitorJSONMakeCommand only accepts unsigned long, shouldn't you be
limiting bandwidth in the caller to ULONG_MAX / 1024 / 1024?
> +{
> + int ret = -1;
> + virJSONValuePtr cmd;
> + virJSONValuePtr reply = NULL;
> +
> + cmd = qemuMonitorJSONMakeCommand("block-commit",
> + "s:device", device,
> + "U:speed", speed,
> + "s:top", top,
> + base ? "s:base" : NULL, base,
> + NULL);
> + if (!cmd)
> + return -1;
> +
> + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
> + goto cleanup;
> + ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> +cleanup:
> + virJSONValueFree(cmd);
> + virJSONValueFree(reply);
> + return ret;
> +}
> +
> +
> int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
> const char *cmd_str,
> char **reply_str,
> @@ -3385,6 +3417,8 @@ static int qemuMonitorJSONGetBlockJobInfoOne(virJSONValuePtr entry,
> }
> if (STREQ(type, "stream"))
> info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
> + else if (STREQ(type, "commit"))
> + info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT;
> else
> info->type = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
>
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index 63b84c6..71bc6aa 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -235,6 +235,13 @@ int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon,
> bool reuse);
> int qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions);
>
> +int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
> + const char *device,
> + const char *top,
> + const char *base,
> + unsigned long bandwidth)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
> +
> int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
> const char *cmd_str,
> char **reply_str,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 58cd8da..3396258 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -909,12 +909,15 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> if (disk) {
> path = disk->src;
> event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
> - /* XXX If we completed a block pull, then recompute the cached
> - * backing chain to match. Better would be storing the chain
> - * ourselves rather than reprobing, but this requires
> - * modifying domain_conf and our XML to fully track the chain
> - * across libvirtd restarts. */
> - if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL &&
> + /* XXX If we completed a block pull or commit, then recompute
> + * the cached backing chain to match. Better would be storing
> + * the chain ourselves rather than reprobing, but this
> + * requires modifying domain_conf and our XML to fully track
> + * the chain across libvirtd restarts. For that matter, if
> + * qemu gains support for committing the active layer, we have
> + * to update disk->src. */
> + if ((type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL ||
> + type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) &&
> status == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
> qemuDomainDetermineDiskChain(driver, disk, true);
> }
ACK dependent on your answer about unsigned long vs. unsigned long long
and "speed".
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list