Google
  Web www.spinics.net

Re: [PATCHv2 01/15] blockjob: add API for async virDomainBlockJobAbort

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Thu, Apr 05, 2012 at 10:36:47PM -0600, Eric Blake wrote:
> From: Adam Litke <agl@xxxxxxxxxx>
> 
> Qemu has changed the semantics of the "block_job_cancel" API.  The original
> qed implementation (pretty much only backported to RHEL 6.2 qemu) was
> synchronous (ie. upon command completion, the operation was guaranteed to
> be completely stopped).  With the new semantics going into qemu 1.1 for
> qcow2, a "block_job_cancel" merely requests that the operation be cancelled
> and an event is triggered once the cancellation request has been honored.
> 
> To adopt the new semantics while preserving compatibility the following
> updates are made to the virDomainBlockJob API:
> 
> A new block job event type VIR_DOMAIN_BLOCK_JOB_CANCELLED is recognized by
> libvirt.  Regardless of the flags used with virDomainBlockJobAbort, this event
> will be raised whenever it is received from qemu.  This event indicates that a
> block job has been successfully cancelled.  For now, libvirt does not try
> to synthesize this event if using an older qemu that did not generate it.
> 
> A new extension flag VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC is added to the
> virDomainBlockJobAbort API.  When enabled, this function will operate
> asynchronously (ie, it can return before the job has actually been cancelled).
> When the API is used in this mode, it is the responsibility of the caller to
> wait for a VIR_DOMAIN_BLOCK_JOB_CANCELLED event or poll via the
> virDomainGetBlockJobInfo API to check the cancellation status; this flag
> is an error if it is not known if the hypervisor supports asynchronous cancel.
> 
> This patch also exposes the new flag through virsh.
> 
> Signed-off-by: Adam Litke <agl@xxxxxxxxxx>
> Cc: Stefan Hajnoczi <stefanha@xxxxxxxxx>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>

Tested-by: Adam Litke <agl@xxxxxxxxxx>

> ---
>  include/libvirt/libvirt.h.in |   10 +++++++++
>  src/libvirt.c                |   10 ++++++++-
>  src/qemu/qemu_monitor_json.c |   42 +++++++++++++++++++++++++++++++------
>  tools/virsh.c                |   46 ++++++++++++++++++++++++++---------------
>  tools/virsh.pod              |    9 +++++--
>  5 files changed, 89 insertions(+), 28 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..af22232 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,14 @@ 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 complete, 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.  Either way, when the job has been cancelled, a
> + * BlockJob event will be emitted, with status VIR_DOMAIN_BLOCK_JOB_CANCELLED.
> + *
>   * Returns -1 in case of failure, 0 when successful.
>   */
>  int virDomainBlockJobAbort(virDomainPtr dom, const char *disk,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 8eeb307..9a8da3a 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -58,13 +58,14 @@ static void qemuMonitorJSONHandleIOError(qemuMonitorPtr mon, virJSONValuePtr dat
>  static void qemuMonitorJSONHandleVNCConnect(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValuePtr data);
> -static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandleSPICEConnect(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandleSPICEInitialize(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandleSPICEDisconnect(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandleTrayChange(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandlePMWakeup(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandlePMSuspend(qemuMonitorPtr mon, virJSONValuePtr data);
> +static void qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon, virJSONValuePtr data);
> +static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONValuePtr data);
> 
>  static struct {
>      const char *type;
> @@ -80,13 +81,14 @@ static struct {
>      { "VNC_CONNECTED", qemuMonitorJSONHandleVNCConnect, },
>      { "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, },
>      { "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, },
> -    { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJob, },
>      { "SPICE_CONNECTED", qemuMonitorJSONHandleSPICEConnect, },
>      { "SPICE_INITIALIZED", qemuMonitorJSONHandleSPICEInitialize, },
>      { "SPICE_DISCONNECTED", qemuMonitorJSONHandleSPICEDisconnect, },
>      { "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, },
>      { "WAKEUP", qemuMonitorJSONHandlePMWakeup, },
>      { "SUSPEND", qemuMonitorJSONHandlePMSuspend, },
> +    { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, },
> +    { "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCanceled, },
>  };
> 
> 
> @@ -754,13 +756,15 @@ static void qemuMonitorJSONHandleSPICEDisconnect(qemuMonitorPtr mon, virJSONValu
>      qemuMonitorJSONHandleGraphics(mon, data, VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT);
>  }
> 
> -static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr data)
> +static void
> +qemuMonitorJSONHandleBlockJobImpl(qemuMonitorPtr mon,
> +                                  virJSONValuePtr data,
> +                                  int event)
>  {
>      const char *device;
>      const char *type_str;
>      int type = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
>      unsigned long long offset, len;
> -    int status = VIR_DOMAIN_BLOCK_JOB_FAILED;
> 
>      if ((device = virJSONValueObjectGetString(data, "device")) == NULL) {
>          VIR_WARN("missing device in block job event");
> @@ -785,11 +789,19 @@ static void qemuMonitorJSONHandleBlockJob(qemuMonitorPtr mon, virJSONValuePtr da
>      if (STREQ(type_str, "stream"))
>          type = VIR_DOMAIN_BLOCK_JOB_TYPE_PULL;
> 
> -    if (offset != 0 && offset == len)
> -        status = VIR_DOMAIN_BLOCK_JOB_COMPLETED;
> +    switch (event) {
> +        case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
> +            /* Make sure the whole device has been processed */
> +            if (offset != len)
> +                event = VIR_DOMAIN_BLOCK_JOB_FAILED;
> +            break;
> +        case VIR_DOMAIN_BLOCK_JOB_FAILED:
> +        case VIR_DOMAIN_BLOCK_JOB_CANCELED:
> +            break;
> +    }
> 
>  out:
> -    qemuMonitorEmitBlockJob(mon, device, type, status);
> +    qemuMonitorEmitBlockJob(mon, device, type, event);
>  }
> 
>  static void
> @@ -832,6 +844,22 @@ qemuMonitorJSONHandlePMSuspend(qemuMonitorPtr mon,
>      qemuMonitorEmitPMSuspend(mon);
>  }
> 
> +static void
> +qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon,
> +                                       virJSONValuePtr data)
> +{
> +    qemuMonitorJSONHandleBlockJobImpl(mon, data,
> +                                      VIR_DOMAIN_BLOCK_JOB_COMPLETED);
> +}
> +
> +static void
> +qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon,
> +                                       virJSONValuePtr data)
> +{
> +    qemuMonitorJSONHandleBlockJobImpl(mon, data,
> +                                      VIR_DOMAIN_BLOCK_JOB_CANCELED);
> +}
> +
>  int
>  qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon,
>                                    const char *cmd_str,
> diff --git a/tools/virsh.c b/tools/virsh.c
> index cfdd040..7180b83 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,23 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
>      virDomainBlockJobInfo info;
>      const char *type;
>      int ret;
> +    bool abortMode = vshCommandOptBool(cmd, "abort");
> +    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"));
>          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 +7646,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;
>  }
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index a60e667..aac72d1 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -686,13 +686,16 @@ 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.  If no mode is chosen, I<--info> is assumed.
> 
>  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.
> -- 
> 1.7.7.6
> 

-- 
Adam Litke <agl@xxxxxxxxxx>
IBM Linux Technology Center

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]

Powered by Linux

Google
  Web www.spinics.net