On Wed, Apr 11, 2012 at 05:40:35PM -0600, Eric Blake wrote:
> From: Adam Litke <agl@xxxxxxxxxx>
>
> Block job cancellation can take a while. Now that upstream qemu 1.1
> has asynchronous block cancellation, we want to expose that to the user.
> Therefore, the following updates are made to the virDomainBlockJob API:
>
> A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELED is managed by
> libvirt. Regardless of the flags used with virDomainBlockJobAbort, this
> event will be raised: 1. when using synchronous block_job_cancel (the
> event will be synthesized by libvirt), and 2. whenever it is received
> from qemu (via asynchronous block-job-cancel). Note that the event
> may be detected by libvirt even before the virDomainBlockJobAbort
> completes (always true when it is synthesized, but also possible if
> cancellation was fast).
>
> A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC is added to the
> virDomainBlockJobAbort API. When enabled, this function will allow
> (but not require) asynchronous operation (ie, it returns as soon as
> possible, which might be before the job has actually been canceled).
> When the API is used in this mode, it is the responsibility of the
> caller to wait for a VIR_DOMAIN_BLOCK_JOB_CANCELED event or poll via
> the virDomainGetBlockJobInfo API to check the cancellation status.
>
> This patch also exposes the new flag through virsh, and makes virsh
> slightly easier to use (--async implies --abort, and lack of any options
> implies --info), although it leaves the qemu implementation for later
> patches.
>
> Signed-off-by: Adam Litke <agl@xxxxxxxxxx>
> Cc: Stefan Hajnoczi <stefanha@xxxxxxxxx>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> include/libvirt/libvirt.h.in | 10 ++++++++
> src/libvirt.c | 13 ++++++++++-
> tools/virsh.c | 51 ++++++++++++++++++++++++++---------------
> tools/virsh.pod | 35 ++++++++++++++++++++--------
> 4 files changed, 79 insertions(+), 30 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 499dcd4..97ad99d 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1946,6 +1946,15 @@ typedef enum {
> #endif
> } virDomainBlockJobType;
>
> +/**
> + * virDomainBlockJobAbortFlags:
> + *
> + * VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC: Request only, do not wait for completion
> + */
> +typedef enum {
> + VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC = 1 << 0,
> +} virDomainBlockJobAbortFlags;
> +
> /* An iterator for monitoring block job operations */
> typedef unsigned long long virDomainBlockJobCursor;
>
> @@ -3617,6 +3626,7 @@ typedef void (*virConnectDomainEventGraphicsCallback)(virConnectPtr conn,
> typedef enum {
> VIR_DOMAIN_BLOCK_JOB_COMPLETED = 0,
> VIR_DOMAIN_BLOCK_JOB_FAILED = 1,
> + VIR_DOMAIN_BLOCK_JOB_CANCELED = 2,
>
> #ifdef VIR_ENUM_SENTINELS
> VIR_DOMAIN_BLOCK_JOB_LAST
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 16d1fd5..93ec817 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -17902,7 +17902,7 @@ error:
> * virDomainBlockJobAbort:
> * @dom: pointer to domain object
> * @disk: path to the block device, or device shorthand
> - * @flags: extra flags; not used yet, so callers should always pass 0
> + * @flags: bitwise-OR of virDomainBlockJobAbortFlags
> *
> * Cancel the active block job on the given disk.
> *
> @@ -17913,6 +17913,17 @@ error:
> * can be found by calling virDomainGetXMLDesc() and inspecting
> * elements within //domain/devices/disk.
> *
> + * By default, this function performs a synchronous operation and the caller
> + * may assume that the operation has completed when 0 is returned. However,
> + * BlockJob operations may take a long time to cancel, and during this time
> + * further domain interactions may be unresponsive. To avoid this problem,
> + * pass VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC in the @flags argument to enable
> + * asynchronous behavior, returning as soon as possible. When the job has
> + * been canceled, a BlockJob event will be emitted, with status
> + * VIR_DOMAIN_BLOCK_JOB_CANCELED (even if the ABORT_ASYNC flag was not
> + * used); it is also possible to poll virDomainBlockJobInfo() to see if
> + * the job cancellation is still pending.
> + *
> * Returns -1 in case of failure, 0 when successful.
> */
> int virDomainBlockJobAbort(virDomainPtr dom, const char *disk,
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 44514c4..8ef57e0 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -7525,6 +7525,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
> const char *name, *path;
> unsigned long bandwidth = 0;
> int ret = -1;
> + unsigned int flags = 0;
>
> if (!vshConnectionUsability(ctl, ctl->conn))
> goto cleanup;
> @@ -7541,7 +7542,9 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
> }
>
> if (mode == VSH_CMD_BLOCK_JOB_ABORT) {
> - ret = virDomainBlockJobAbort(dom, path, 0);
> + if (vshCommandOptBool(cmd, "async"))
> + flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC;
> + ret = virDomainBlockJobAbort(dom, path, flags);
> } else if (mode == VSH_CMD_BLOCK_JOB_INFO) {
> ret = virDomainGetBlockJobInfo(dom, path, info, 0);
> } else if (mode == VSH_CMD_BLOCK_JOB_SPEED) {
> @@ -7589,20 +7592,25 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
> }
>
> /*
> - * "blockjobinfo" command
> + * "blockjob" command
> */
> static const vshCmdInfo info_block_job[] = {
> - {"help", N_("Manage active block operations.")},
> - {"desc", N_("Manage active block operations.")},
> + {"help", N_("Manage active block operations")},
> + {"desc", N_("Query, adjust speed, or cancel active block operations.")},
> {NULL, NULL}
> };
>
> static const vshCmdOptDef opts_block_job[] = {
> {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> {"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("Fully-qualified path of disk")},
> - {"abort", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("Abort the active job on the specified disk")},
> - {"info", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("Get active job information for the specified disk")},
> - {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE, N_("Set the Bandwidth limit in MB/s")},
> + {"abort", VSH_OT_BOOL, VSH_OFLAG_NONE,
> + N_("Abort the active job on the specified disk")},
> + {"async", VSH_OT_BOOL, VSH_OFLAG_NONE,
> + N_("don't wait for --abort to complete")},
> + {"info", VSH_OT_BOOL, VSH_OFLAG_NONE,
> + N_("Get active job information for the specified disk")},
> + {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE,
> + N_("Set the Bandwidth limit in MB/s")},
> {NULL, 0, 0, NULL}
> };
>
> @@ -7613,19 +7621,24 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
> virDomainBlockJobInfo info;
> const char *type;
> int ret;
> + bool abortMode = (vshCommandOptBool(cmd, "abort") ||
> + vshCommandOptBool(cmd, "async"));
> + bool infoMode = vshCommandOptBool(cmd, "info");
> + bool bandwidth = vshCommandOptBool(cmd, "bandwidth");
>
> - if (vshCommandOptBool (cmd, "abort")) {
> - mode = VSH_CMD_BLOCK_JOB_ABORT;
> - } else if (vshCommandOptBool (cmd, "info")) {
> - mode = VSH_CMD_BLOCK_JOB_INFO;
> - } else if (vshCommandOptBool (cmd, "bandwidth")) {
> - mode = VSH_CMD_BLOCK_JOB_SPEED;
> - } else {
> + if (abortMode + infoMode + bandwidth > 1) {
> vshError(ctl, "%s",
> - _("One of --abort, --info, or --bandwidth is required"));
> + _("conflict between --abort, --info, and --bandwidth modes"));
> return false;
> }
>
> + if (abortMode)
> + mode = VSH_CMD_BLOCK_JOB_ABORT;
> + else if (bandwidth)
> + mode = VSH_CMD_BLOCK_JOB_SPEED;
> + else
> + mode = VSH_CMD_BLOCK_JOB_INFO;
> +
> ret = blockJobImpl(ctl, cmd, &info, mode);
> if (ret < 0)
> return false;
> @@ -7634,13 +7647,13 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
> return true;
>
> if (info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL)
> - type = "Block Pull";
> + type = _("Block Pull");
> else
> - type = "Unknown job";
> + type = _("Unknown job");
>
> print_job_progress(type, info.end - info.cur, info.end);
> if (info.bandwidth != 0)
> - vshPrint(ctl, " Bandwidth limit: %lu MB/s\n", info.bandwidth);
> + vshPrint(ctl, _(" Bandwidth limit: %lu MB/s\n"), info.bandwidth);
> return true;
> }
>
> @@ -17115,8 +17128,8 @@ static const vshCmdDef domManagementCmds[] = {
> {"autostart", cmdAutostart, opts_autostart, info_autostart, 0},
> {"blkdeviotune", cmdBlkdeviotune, opts_blkdeviotune, info_blkdeviotune, 0},
> {"blkiotune", cmdBlkiotune, opts_blkiotune, info_blkiotune, 0},
> - {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0},
> {"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0},
> + {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0},
> {"blockresize", cmdBlockResize, opts_block_resize, info_block_resize, 0},
> {"change-media", cmdChangeMedia, opts_change_media, info_change_media, 0},
> #ifndef WIN32
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index c6e0ff3..5f3d9b1 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -650,7 +650,10 @@ pulled, the disk no longer depends on that portion of the backing chain.
> It pulls data for the entire disk in the background, the process of the
> operation can be checked with B<blockjob>.
>
> -I<path> specifies fully-qualified path of the disk.
> +I<path> specifies fully-qualified path of the disk; it corresponds
> +to a unique target name (<target dev='name'/>) or source file (<source
> +file='name'/>) for one of the disk devices attached to I<domain> (see
> +also B<domblklist> for listing these names).
> I<bandwidth> specifies copying bandwidth limit in Mbps.
>
> =item B<blkdeviotune> I<domain> I<device>
> @@ -687,13 +690,21 @@ Both I<--live> and I<--current> flags may be given, but I<--current> is
> exclusive. If no flag is specified, behavior is different depending
> on hypervisor.
>
> -=item B<blockjob> I<domain> I<path> [I<--abort>] [I<--info>] [I<bandwidth>]
> +=item B<blockjob> I<domain> I<path> { [I<--abort>] [I<--async>] |
> +[I<--info>] | [I<bandwidth>] }
>
> -Manage active block operations.
> +Manage active block operations. There are three modes: I<--info>,
> +I<bandwidth>, and I<--abort>; I<--info> is default except that I<--async>
> +implies I<--abort>.
> +
> +I<path> specifies fully-qualified path of the disk; it corresponds
> +to a unique target name (<target dev='name'/>) or source file (<source
> +file='name'/>) for one of the disk devices attached to I<domain> (see
> +also B<domblklist> for listing these names).
>
> -I<path> specifies fully-qualified path of the disk.
> If I<--abort> is specified, the active job on the specified disk will
> -be aborted.
> +be aborted. If I<--async> is also specified, this command will return
> +immediately, rather than waiting for the cancelation to complete.
> If I<--info> is specified, the active job information on the specified
> disk will be printed.
> I<bandwidth> can be used to set bandwidth limit for the active job.
> @@ -701,11 +712,15 @@ I<bandwidth> can be used to set bandwidth limit for the active job.
> =item B<blockresize> I<domain> I<path> I<size>
>
> Resize a block device of domain while the domain is running, I<path>
> -specifies the absolute path of the block device, I<size> is a scaled
> -integer (see B<NOTES> above) which defaults to KiB (blocks of 1024 bytes)
> -if there is no suffix. You must use a suffix of "B" to get bytes (note
> -that for historical reasons, this differs from B<vol-resize> which
> -defaults to bytes without a suffix).
> +specifies the absolute path of the block device; it corresponds
> +to a unique target name (<target dev='name'/>) or source file (<source
> +file='name'/>) for one of the disk devices attached to I<domain> (see
> +also B<domblklist> for listing these names).
> +
> +I<size> is a scaled integer (see B<NOTES> above) which defaults to KiB
> +(blocks of 1024 bytes) if there is no suffix. You must use a suffix of
> +"B" to get bytes (note that for historical reasons, this differs from
> +B<vol-resize> which defaults to bytes without a suffix).
>
> =item B<dominfo> I<domain-id>
ACK, API change is good, and noted improvements on virsh docs, info
pages and string localization !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list