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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list