Re: [libvirt] [PATCH 1/5] blockjob: add qemu capabilities related to block pull jobs

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);

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); }

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,