On 04/17/2012 01:05 AM, Eric Blake wrote: > The new block copy storage migration sequence requires both the > 'drive-mirror' and 'drive-reopen' monitor commands, which have > been proposed[1] for qemu 1.1. Someday (probably for qemu 1.2), > these commands may also be added to the 'transaction' monitor > command for even more power, but we don't need to worry about > that now. > > [1]https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01630.html > > * 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. > (qemuMonitorJSONDriveMirror, qemuMonitorDriveReopen): New functions. > * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDriveMirror) > (qemuMonitorDriveReopen): Declare them. > * src/qemu/qemu_monitor.c (qemuMonitorDriveMirror) > (qemuMonitorDriveReopen): New passthroughs. > * src/qemu/qemu_monitor.h (qemuMonitorDriveMirror) > (qemuMonitorDriveReopen): Declare them. > --- > > merge 1/18 and 9/18 from v4 > v5: adjust to latest upstream qemu proposal, which requires 'full' > argument and no longer permits interaction with 'transaction' > > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_capabilities.h | 2 + > src/qemu/qemu_monitor.c | 49 ++++++++++++++++++++++++++++++++ > src/qemu/qemu_monitor.h | 11 +++++++ > src/qemu/qemu_monitor_json.c | 63 ++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_monitor_json.h | 18 ++++++++++- > 6 files changed, 143 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index fd4ca4d..0e5dd68 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -159,6 +159,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, > > "block-job-sync", /* 90 */ > "block-job-async", > + "drive-mirror", > + "drive-reopen", > ); > > struct qemu_feature_flags { > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index 6550d59..e6fa519 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -126,6 +126,8 @@ enum qemuCapsFlags { > 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_DRIVE_MIRROR = 92, /* drive-mirror monitor command */ > + QEMU_CAPS_DRIVE_REOPEN = 93, /* drive-reopen monitor command */ > > 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 2f66c46..34c7926 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -2685,6 +2685,31 @@ qemuMonitorDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr actions, > return ret; > } > > +/* Add the drive-mirror action to a transaction. */ Is this comment now incorrect? (You say the new qemu proposal doesn't allow interaction with "transaction") > +int > +qemuMonitorDriveMirror(qemuMonitorPtr mon, > + const char *device, const char *file, > + const char *format, unsigned int flags) > +{ > + int ret = -1; > + > + VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s, flags=%x", > + mon, device, file, format, flags); Should we be concerned about NULL string pointers for args that aren't qualified as ATTRIBUTE_NONNULL? > + > + if (!mon) { I had thought that declaring an arg ATTRIBUTE_NONNULL meant that you didn't need to check for NULL in the code, so when I saw this, I was confused and decided to investigate. I found the following gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308 conveniently with a comment posted by on "Eric Blake" (any relation? :-) So does this mean that all of our ATTRIBUTE_NONNULL declarations are pointless? > + qemuReportError(VIR_ERR_INVALID_ARG, "%s", > + _("monitor must not be NULL")); > + return -1; > + } > + > + if (mon->json) > + ret = qemuMonitorJSONDriveMirror(mon, device, file, format, flags); > + else > + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("drive-mirror requires JSON monitor")); > + return ret; > +} > + > /* Use the transaction QMP command to run atomic snapshot commands. */ > int > qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) > @@ -2701,6 +2726,30 @@ qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) > return ret; > } > > +/* Use the drive-reopen monitor command. */ > +int > +qemuMonitorDriveReopen(qemuMonitorPtr mon, const char *device, > + const char *file, const char *format) > +{ > + int ret = -1; > + > + VIR_DEBUG("mon=%p, device=%s, file=%s, format=%s", > + mon, device, file, format); > + > + if (!mon) { > + qemuReportError(VIR_ERR_INVALID_ARG, "%s", > + _("monitor must not be NULL")); > + return -1; > + } > + > + if (mon->json) > + ret = qemuMonitorJSONDriveReopen(mon, device, file, format); > + else > + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("drive-reopen 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 f3cdcdd..1b4b130 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -510,6 +510,17 @@ int qemuMonitorDiskSnapshot(qemuMonitorPtr mon, > bool reuse); > int qemuMonitorTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) > ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); > +int qemuMonitorDriveMirror(qemuMonitorPtr mon, > + const char *device, > + const char *file, > + const char *format, > + unsigned int flags) > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); > +int qemuMonitorDriveReopen(qemuMonitorPtr mon, > + const char *device, > + const char *file, > + const char *format) > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); > > int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, > const char *cmd, > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index eb58f13..e0ea505 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -987,6 +987,10 @@ qemuMonitorJSONCheckCommands(qemuMonitorPtr mon, > qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC); > else if (STREQ(name, "block-job-cancel")) > qemuCapsSet(qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC); > + else if (STREQ(name, "drive-mirror")) > + qemuCapsSet(qemuCaps, QEMU_CAPS_DRIVE_MIRROR); > + else if (STREQ(name, "drive-reopen")) > + qemuCapsSet(qemuCaps, QEMU_CAPS_DRIVE_REOPEN); > } > > ret = 0; > @@ -3199,6 +3203,38 @@ cleanup: > } > > int > +qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, > + const char *device, const char *file, > + const char *format, unsigned int flags) > +{ > + int ret = -1; > + virJSONValuePtr cmd; > + virJSONValuePtr reply = NULL; > + bool shallow = (flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) != 0; > + bool reuse = (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) != 0; > + > + cmd = qemuMonitorJSONMakeCommand("drive-mirror", > + "s:device", device, > + "s:target", file, > + "b:full", !shallow, > + "s:mode", > + reuse ? "existing" : "absolute-paths", > + format ? "s:format" : NULL, format, > + 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 > qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) > { > int ret = -1; > @@ -3226,6 +3262,33 @@ cleanup: > return ret; > } > > +int > +qemuMonitorJSONDriveReopen(qemuMonitorPtr mon, const char *device, > + const char *file, const char *format) > +{ > + int ret; > + virJSONValuePtr cmd; > + virJSONValuePtr reply = NULL; > + > + cmd = qemuMonitorJSONMakeCommand("drive-reopen", > + "s:device", device, > + "s:new-image-file", file, > + format ? "s:format" : NULL, format, > + 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, > diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h > index aacbb5f..d93c37d 100644 > --- a/src/qemu/qemu_monitor_json.h > +++ b/src/qemu/qemu_monitor_json.h > @@ -230,8 +230,22 @@ int qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, > const char *device, > const char *file, > const char *format, > - bool reuse); > -int qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions); > + bool reuse) > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) > + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); > +int qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); > +int qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, > + const char *device, > + const char *file, > + const char *format, > + unsigned int flags) > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); > +int qemuMonitorJSONDriveReopen(qemuMonitorPtr mon, > + const char *device, > + const char *file, > + const char *format) > + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); > > int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, > const char *cmd_str, I'll avoid using the "A word" because I don't know the status of qemu upstream, and don't want to confuse patchchecker. It all looks okay to me though, although the ATTRIBUTE_NONNULL stuff was a bit of a revelation (probably I'm the only one who didn't already know about it...). -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list