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

[libvirt] [PATCHv5 12/23] blockjob: relax block job behavior when setting speed up front



In qemu, it is possible to call 'migrate_set_speed' prior to
'migrate', and therefore ensure a constant throttling through the
entire migration.  However, this is not possible with
'block-job-set-speed', which fails if a job is not already active.
This means that you can't detect a device that doesn't support
throttling until after you have already started a block job and
let an unknown amount of unthrottled data through.  Aborting the
job upon failure to set the speed seems a bit harsh, since it
would have been nicer to prevent the job from starting in the
first place, rather than letting an unknown amount of data be
processed before detecting the speed failure.  So I propose
relaxing the documentation, and explicitly mentioning that setting
the speed is a best-effort attempt that might be ignored.  On the
other hand, I've also requested that qemu consider adding an
optional parameter to allow setting the speed at the creation of a
block job.

* src/libvirt.c (virDomainBlockPull, virDomainBlockRebase): Update
the documentation.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl)
(qemuDomainBlockCopy): Log but don't fail when speed change fails.
---

v5: new patch; see also this qemu request:
https://lists.gnu.org/archive/html/qemu-devel/2012-04/msg02185.html

 src/libvirt.c          |   12 ++++++++++--
 src/qemu/qemu_driver.c |   18 ++++++++++++------
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index af42d3b..17c7576 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18145,7 +18145,11 @@ error:
  * The maximum bandwidth (in Mbps) that will be used to do the copy can be
  * specified with the bandwidth parameter.  If set to 0, libvirt will choose a
  * suitable default.  Some hypervisors do not support this feature and will
- * return an error if bandwidth is not 0.
+ * return an error if bandwidth is not 0; others will make a best effort
+ * attempt, but silently ignore failure to set the speed.  The actual speed
+ * can be determined with virDomainGetBlockJobInfo(), and setting speed via
+ * virDomainBlockJobSetSpeed() allows more control, at the expense of a race
+ * condition where the job may end before the speed can be set or queried.
  *
  * This is shorthand for virDomainBlockRebase() with a NULL base.
  *
@@ -18263,7 +18267,11 @@ error:
  * The maximum bandwidth (in Mbps) that will be used to do the copy can be
  * specified with the bandwidth parameter.  If set to 0, libvirt will choose a
  * suitable default.  Some hypervisors do not support this feature and will
- * return an error if bandwidth is not 0.
+ * return an error if bandwidth is not 0; others will make a best effort
+ * attempt, but silently ignore failure to set the speed.  The actual speed
+ * can be determined with virDomainGetBlockJobInfo(), and setting speed via
+ * virDomainBlockJobSetSpeed() allows more control, at the expense of a race
+ * condition where the job may end before the speed can be set or queried.
  *
  * When @base is NULL and @flags is 0, this is identical to
  * virDomainBlockPull().
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5c3cea8..aec5c4c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11799,9 +11799,12 @@ 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);
+    if (ret == 0 && mode == BLOCK_JOB_PULL && bandwidth) {
+        /* Setting speed now is best-effort.  */
+        if (qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
+                                BLOCK_JOB_SPEED_INTERNAL, async) < 0)
+            VIR_WARN("failed to set bandwidth for disk %s", disk->dst);
+    }
     qemuDomainObjExitMonitorWithDriver(driver, vm);
     if (ret < 0)
         goto endjob;
@@ -11990,9 +11993,12 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path,
     /* Actually start the mirroring */
     qemuDomainObjEnterMonitorWithDriver(driver, vm);
     ret = qemuMonitorDriveMirror(priv->mon, device, dest, format, flags);
-    if (ret == 0 && bandwidth != 0)
-        ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
-                                  BLOCK_JOB_SPEED_INTERNAL, true);
+    if (ret == 0 && bandwidth != 0) {
+        /* Setting speed now is best-effort.  */
+        if (qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
+                                BLOCK_JOB_SPEED_INTERNAL, true) < 0)
+            VIR_WARN("failed to set bandwidth for disk %s", disk->dst);
+    }
     qemuDomainObjExitMonitorWithDriver(driver, vm);

 endjob:
-- 
1.7.7.6


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