[PATCH 5/5] blockjob: allow for fast-finishing job

[Eric Blake <eblake@xxxxxxxxxx>]

In my testing, I was able to provoke an odd block pull failure:

$ virsh blockpull dom vda --bandwidth 10000
error: Requested operation is not valid: No active operation on device: drive-virtio-disk0

merely by using gdb to artifically wait to do the block job set speed
until after the pull had already finished.  But in reality, that should
be a success, since the pull finished before we had a chance to set
speed.  Furthermore, using a double job lock is annoying; we can alter
the speed as part of the same job that started the block pull for less
likelihood of hitting the speed failure in the first place.

* src/qemu/qemu_monitor.h (BLOCK_JOB_CMD): Add new mode.
* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Move secondary
job call...
(qemuDomainBlockJobImpl): ...here, for fewer locks.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Change
return value on new internal mode.
---
 src/qemu/qemu_driver.c       |   13 +++++--------
 src/qemu/qemu_monitor.h      |    3 ++-
 src/qemu/qemu_monitor_json.c |   31 ++++++++++++++++++++-----------
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bb6089b..e31f407 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11654,6 +11654,9 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
      * relying on qemu to do this.  */
     ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode,
                               async);
+    if (ret == 0 && mode == BLOCK_JOB_PULL && bandwidth)
+        ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
+                                  BLOCK_JOB_SPEED_INTERNAL, async);
     qemuDomainObjExitMonitorWithDriver(driver, vm);
     if (ret < 0)
         goto endjob;
@@ -11750,15 +11753,9 @@ static int
 qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
                       unsigned long bandwidth, unsigned int flags)
 {
-    int ret;
-
     virCheckFlags(0, -1);
-    ret = qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
-                                 BLOCK_JOB_PULL, flags);
-    if (ret == 0 && bandwidth != 0)
-        ret = qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
-                                     BLOCK_JOB_SPEED, flags);
-    return ret;
+    return qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
+                                  BLOCK_JOB_PULL, flags);
 }

 static int
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index dc02964..f3cdcdd 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -530,7 +530,8 @@ typedef enum {
     BLOCK_JOB_ABORT = 0,
     BLOCK_JOB_INFO = 1,
     BLOCK_JOB_SPEED = 2,
-    BLOCK_JOB_PULL = 3,
+    BLOCK_JOB_SPEED_INTERNAL = 3,
+    BLOCK_JOB_PULL = 4,
 } BLOCK_JOB_CMD;

 int qemuMonitorBlockJob(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 242889c..26e41ac 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3452,6 +3452,7 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
         cmd = qemuMonitorJSONMakeCommand(cmd_name, NULL);
         break;
     case BLOCK_JOB_SPEED:
+    case BLOCK_JOB_SPEED_INTERNAL:
         cmd_name = async ? "block-job-set-speed" : "block_job_set_speed";
         cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
                                          device, "U:value",
@@ -3473,22 +3474,30 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
     ret = qemuMonitorJSONCommand(mon, cmd, &reply);

     if (ret == 0 && virJSONValueObjectHasKey(reply, "error")) {
-        if (qemuMonitorJSONHasError(reply, "DeviceNotActive"))
-            qemuReportError(VIR_ERR_OPERATION_INVALID,
-                _("No active operation on device: %s"), device);
-        else if (qemuMonitorJSONHasError(reply, "DeviceInUse"))
+        ret = -1;
+        if (qemuMonitorJSONHasError(reply, "DeviceNotActive")) {
+            /* If a job completes before we get a chance to set the
+             * speed, we don't want to fail the original command.  */
+            if (mode == BLOCK_JOB_SPEED_INTERNAL)
+                ret = 0;
+            else
+                qemuReportError(VIR_ERR_OPERATION_INVALID,
+                                _("No active operation on device: %s"),
+                                device);
+        } else if (qemuMonitorJSONHasError(reply, "DeviceInUse")){
             qemuReportError(VIR_ERR_OPERATION_FAILED,
-                _("Device %s in use"), device);
-        else if (qemuMonitorJSONHasError(reply, "NotSupported"))
+                            _("Device %s in use"), device);
+        } else if (qemuMonitorJSONHasError(reply, "NotSupported")) {
             qemuReportError(VIR_ERR_OPERATION_INVALID,
-                _("Operation is not supported for device: %s"), device);
-        else if (qemuMonitorJSONHasError(reply, "CommandNotFound"))
+                            _("Operation is not supported for device: %s"),
+                            device);
+        } else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
             qemuReportError(VIR_ERR_OPERATION_INVALID,
-                _("Command '%s' is not found"), cmd_name);
-        else
+                            _("Command '%s' is not found"), cmd_name);
+        } else {
             qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                 _("Unexpected error"));
-        ret = -1;
+        }
     }

     if (ret == 0 && mode == BLOCK_JOB_INFO)
-- 
1.7.7.6

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