[libvirt] [PATCH 1/5] blockjob: add qemu capabilities related to block pull jobs
Daniel Veillard
veillard at redhat.com
Thu Apr 12 01:09:13 UTC 2012
On Wed, Apr 11, 2012 at 05:40:34PM -0600, Eric Blake wrote:
> RHEL 6.2 was released with an early version of block jobs, which only
> worked on the qed file format, where the commands were spelled with
> underscore (contrary to QMP style), and where 'block_job_cancel' was
> synchronous and did not trigger an event.
>
> qemu 1.1 has fixed these short-comings [1][2]: the commands now work on
> multiple file types, are spelled with dash, and 'block-job-cancel' is
> asynchronous and emits an event upon conclusion.
>
> [1]qemu commit 370521a1d6f5537ea7271c119f3fbb7b0fa57063
> [2]https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01248.html
>
> This patch recognizes the new spellings, and fixes virDomainBlockRebase
> to give a graceful error when talking to a too-old qemu on a partial
> rebase attempt.
>
> * src/qemu/qemu_capabilities.h (QEMU_CAPS_BLOCKJOB_SYNC)
> (QEMU_CAPS_BLOCKJOB_ASYNC): New bits.
> * src/qemu/qemu_capabilities.c (qemuCaps): Name them.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Set
> them.
> (qemuMonitorJSONBlockJob): Manage both command names.
> (qemuMonitorJSONDiskSnapshot): Minor formatting fix.
> * src/qemu/qemu_monitor.h (qemuMonitorBlockJob): Alter signature.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockJob): Likewise.
> * src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Pass through
> capability bit.
> * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Update callers.
> ---
> src/qemu/qemu_capabilities.c | 3 ++
> src/qemu/qemu_capabilities.h | 2 +
> src/qemu/qemu_driver.c | 22 ++++++++++++++----
> src/qemu/qemu_monitor.c | 11 +++++---
> src/qemu/qemu_monitor.h | 5 ++-
> src/qemu/qemu_monitor_json.c | 51 +++++++++++++++++++++++-------------------
> src/qemu/qemu_monitor_json.h | 4 ++-
> 7 files changed, 63 insertions(+), 35 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 0e09d6d..fd4ca4d 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -156,6 +156,9 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
> "scsi-disk.channel",
> "scsi-block",
> "transaction",
> +
> + "block-job-sync", /* 90 */
> + "block-job-async",
> );
>
> struct qemu_feature_flags {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 78cdbe0..6550d59 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -124,6 +124,8 @@ enum qemuCapsFlags {
> QEMU_CAPS_SCSI_DISK_CHANNEL = 87, /* Is scsi-disk.channel available? */
> QEMU_CAPS_SCSI_BLOCK = 88, /* -device scsi-block */
> QEMU_CAPS_TRANSACTION = 89, /* transaction monitor command */
> + QEMU_CAPS_BLOCKJOB_SYNC = 90, /* RHEL 6.2 block_job_cancel */
> + QEMU_CAPS_BLOCKJOB_ASYNC = 91, /* qemu 1.1 block-job-cancel */
>
> QEMU_CAPS_LAST, /* this must always be the last item */
> };
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e18e72d..f939a31 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11607,6 +11607,7 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> char *device = NULL;
> int ret = -1;
> + bool async = false;
>
> qemuDriverLock(driver);
> virUUIDFormat(dom->uuid, uuidstr);
> @@ -11616,6 +11617,19 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
> _("no domain with matching uuid '%s'"), uuidstr);
> goto cleanup;
> }
> + priv = vm->privateData;
> + if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
> + async = true;
> + } else if (!qemuCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC)) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("block jobs not supported with this QEMU binary"));
> + goto cleanup;
> + } else if (base) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("partial block pull not supported with this "
> + "QEMU binary"));
> + goto cleanup;
> + }
>
> device = qemuDiskPathToAlias(vm, path, NULL);
> if (!device) {
> @@ -11632,13 +11646,11 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
> }
>
> qemuDomainObjEnterMonitorWithDriver(driver, vm);
> - priv = vm->privateData;
> - /* XXX - add a qemu capability check, since only qemu 1.1 or newer
> - * supports the base argument.
> - * XXX - libvirt should really be tracking the backing file chain
> + /* XXX - libvirt should really be tracking the backing file chain
> * itself, and validating that base is on the chain, rather than
> * relying on qemu to do this. */
> - ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode);
> + ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode,
> + async);
> qemuDomainObjExitMonitorWithDriver(driver, vm);
>
> endjob:
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index e1a8d4c..36f3832 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2773,15 +2773,18 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon,
> const char *base,
> unsigned long bandwidth,
> virDomainBlockJobInfoPtr info,
> - int mode)
> + int mode,
> + bool async)
> {
> int ret = -1;
>
> - VIR_DEBUG("mon=%p, device=%s, base=%s, bandwidth=%lu, info=%p, mode=%o",
> - mon, device, NULLSTR(base), bandwidth, info, mode);
> + VIR_DEBUG("mon=%p, device=%s, base=%s, bandwidth=%lu, info=%p, mode=%o, "
> + "async=%d", mon, device, NULLSTR(base), bandwidth, info, mode,
> + async);
>
> if (mon->json)
> - ret = qemuMonitorJSONBlockJob(mon, device, base, bandwidth, info, mode);
> + ret = qemuMonitorJSONBlockJob(mon, device, base, bandwidth, info, mode,
> + async);
> else
> qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> _("block jobs require JSON monitor"));
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index b480966..dc02964 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -538,8 +538,9 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon,
> const char *back,
> unsigned long bandwidth,
> virDomainBlockJobInfoPtr info,
> - int mode)
> - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
> + int mode,
> + bool async)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
Hum, gcc wasn't complaining on an "ATTRIBUTE_NONNULL(5)" for something
which wasn't a pointer ?
> int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
> const char *protocol,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index d09d779..d4d2c3e 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -939,12 +939,14 @@ qemuMonitorJSONCheckCommands(qemuMonitorPtr mon,
>
> if (STREQ(name, "human-monitor-command"))
> *json_hmp = 1;
> -
> - if (STREQ(name, "system_wakeup"))
> + else if (STREQ(name, "system_wakeup"))
> qemuCapsSet(qemuCaps, QEMU_CAPS_WAKEUP);
> -
> - if (STREQ(name, "transaction"))
> + else if (STREQ(name, "transaction"))
> qemuCapsSet(qemuCaps, QEMU_CAPS_TRANSACTION);
> + else if (STREQ(name, "block_job_cancel"))
> + qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC);
> + else if (STREQ(name, "block-job-cancel"))
> + qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC);
> }
Hum ... seems to me that we always set bits QEMU_CAPS_BLOCKJOB_SYNC
and QEMU_CAPS_BLOCKJOB_ASYNC together, so do you envision cases where
one was set and not the other ? If not why not merge them for the sake
of one less bit to manage ?
> ret = 0;
> @@ -3114,7 +3116,7 @@ qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions,
> const char *device, const char *file,
> const char *format, bool reuse)
> {
> - int ret;
> + int ret = -1;
> virJSONValuePtr cmd;
> virJSONValuePtr reply = NULL;
>
> @@ -3132,14 +3134,13 @@ qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions,
> if (actions) {
> if (virJSONValueArrayAppend(actions, cmd) < 0) {
> virReportOOMError();
> - ret = -1;
> } else {
> ret = 0;
> cmd = NULL;
> }
> } else {
> if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
> - goto cleanup;
> + goto cleanup;
>
> if (qemuMonitorJSONHasError(reply, "CommandNotFound") &&
> qemuMonitorCheckHMP(mon, "snapshot_blkdev")) {
> @@ -3388,39 +3389,43 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
> const char *base,
> unsigned long bandwidth,
> virDomainBlockJobInfoPtr info,
> - int mode)
> + int mode,
> + bool async)
> {
> int ret = -1;
> virJSONValuePtr cmd = NULL;
> virJSONValuePtr reply = NULL;
> const char *cmd_name = NULL;
>
> - if (base && mode != BLOCK_JOB_PULL) {
> + if (base && (mode != BLOCK_JOB_PULL || !async)) {
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> - _("only block pull supports base: %s"), base);
> + _("only modern block pull supports base: %s"), base);
> return -1;
> }
>
> - if (mode == BLOCK_JOB_ABORT) {
> - cmd_name = "block_job_cancel";
> + switch ((BLOCK_JOB_CMD) mode) {
> + case BLOCK_JOB_ABORT:
> + cmd_name = async ? "block-job-cancel" : "block_job_cancel";
> cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL);
> - } else if (mode == BLOCK_JOB_INFO) {
> + break;
> + case BLOCK_JOB_INFO:
> cmd_name = "query-block-jobs";
> cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL);
> - } else if (mode == BLOCK_JOB_SPEED) {
> - cmd_name = "block_job_set_speed";
> + break;
> + case BLOCK_JOB_SPEED:
> + cmd_name = async ? "block-job-set-speed" : "block_job_set_speed";
> cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
> device, "U:value",
> bandwidth * 1024ULL * 1024ULL,
> NULL);
> - } else if (mode == BLOCK_JOB_PULL) {
> - cmd_name = "block_stream";
> - if (base)
> - cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
> - device, "s:base", base, NULL);
> - else
> - cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
> - device, NULL);
> + break;
> + case BLOCK_JOB_PULL:
> + cmd_name = async ? "block-stream" : "block_stream";
> + cmd = qemuMonitorJSONMakeCommand(cmd_name,
> + "s:device", device,
> + base ? "s:base" : NULL, base,
> + NULL);
> + break;
> }
>
> if (!cmd)
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index a0f67aa..aacbb5f 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -253,7 +253,9 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
> const char *base,
> unsigned long bandwidth,
> virDomainBlockJobInfoPtr info,
> - int mode);
> + int mode,
> + bool async)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>
> int qemuMonitorJSONSetLink(qemuMonitorPtr mon,
> const char *name,
Minor point about the extra bit, to me that's fine, ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list