On 04/09/2012 11:52 PM, Eric Blake wrote: > The new block copy storage migration sequence requires both the > 'drive-mirror' action in 'transaction' (present if the 'drive-mirror' > standalone monitor command also exists) and the 'drive-reopen' monitor > command (it would be nice if that were also part of a 'transaction', > but the initial qemu implementation has it standalone only). > > Furthermore, qemu 1.1 will be adding 'block_job_cancel' as an > asynchronous command, but an earlier implementation (that supported > only the qed file format) existed which was synchronous only. If > 'drive-mirror' is added in time, then we can safely use 'drive-mirror' > as the witness of whether 'block_job_cancel' is synchronous or > asynchronous. > > As of this[1] qemu email, both commands have been proposed but not yet > incorporated into the tree; in fact, the implementation I tested with > has changed to match this[2] email that suggested a mandatory > 'full':'bool' argument rather than 'mode':'no-backing-file'. So there > is a risk that qemu 1.1 will have yet another subtly different > implementation. > [1]https://lists.gnu.org/archive/html/qemu-devel/2012-03/msg01524.html > [2]https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg00886.html None of this gives me warm fuzzies :-/ > > * src/qemu/qemu_capabilities.h (QEMU_CAPS_DRIVE_MIRROR) > (QEMU_CAPS_DRIVE_REOPEN): New bits. > * src/qemu/qemu_capabilities.c (qemuCaps): Name them. > * src/qemu/qemu_monitor_json.c (qemuMonitorJSONCheckCommands): Set > them. > (qemuMonitorJSONDiskSnapshot): Fix formatting issues. > --- > src/qemu/qemu_capabilities.c | 3 +++ > src/qemu/qemu_capabilities.h | 2 ++ > src/qemu/qemu_monitor_json.c | 15 ++++++++------- > 3 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 0e09d6d..1938ae4 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", > + > + "drive-mirror", /* 90 */ > + "drive-reopen", > ); > > struct qemu_feature_flags { > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index 78cdbe0..405bf2a 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_DRIVE_MIRROR = 90, /* drive-mirror monitor command */ > + QEMU_CAPS_DRIVE_REOPEN = 91, /* drive-reopen monitor command */ > > QEMU_CAPS_LAST, /* this must always be the last item */ > }; > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index d09d779..c8ed087 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, "drive-mirror")) > + qemuCapsSet(qemuCaps, QEMU_CAPS_DRIVE_MIRROR); > + else if (STREQ(name, "drive-reopen")) > + qemuCapsSet(qemuCaps, QEMU_CAPS_DRIVE_REOPEN); Good optimization (turning the if's into else if's). I notice qemu is inconsistent wrt "-" vs. "_" in command names (yeah, I'm in a snarky mood ;-)) > } > > 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")) { I guess you just snuck this in here on general principle, right? Since there's no functional change, and it's rather minor, I don't have a problem including it. ACK. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list