[libvirt] [PATCHv5 08/23] blockjob: add qemu capabilities related to block jobs
Laine Stump
laine at laine.org
Wed Apr 18 20:18:52 UTC 2012
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...).
More information about the libvir-list
mailing list