[libvirt] [PATCH V2] Set qemu migration speed unlimited when migrating to file

[Eric Blake]

On 08/04/2011 11:35 AM, Jim Fehlig wrote:
> Discussed previously:
>
> https://www.redhat.com/archives/libvir-list/2011-August/msg00166.html
>
> The qemu migration speed default is 32MiB/s as defined in migration.c
>
> /* Migration speed throttling */
> static int64_t max_throttle = (32<<  20);

Too bad it's not publicly exposed in a header file, in case it ever changes.

>
> There is no reason to throttle migration when targeting a file.  For
> dump and save operations, set migration speed to unlimited prior to
> migration and restore to default value after migration.  Default units
> is MB for migrate_set_speed monitor command, so (INT64_MAX / (1024 * 1024))
> is used for unlimited migration speed.
>
> Tested with both json and text monitors.
>

> +++ b/src/qemu/qemu_migration.c
> @@ -2676,6 +2676,14 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm,
>       virCommandPtr cmd = NULL;
>       int pipeFD[2] = { -1, -1 };
>
> +    /* No need for qemu default of 32MiB/s when migrating to a file.
> +       Default speed unit is MB, so set to unlimited with INT64_MAX / 1M.
> +       Failure to change migration speed is not fatal. */
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
> +        qemuMonitorSetMigrationSpeed(priv->mon, INT64_MAX / (1024 * 1024));
> +        qemuDomainObjExitMonitorWithDriver(driver, vm);
> +    }
> +

This part is now fine.

>       if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)&&
>           (!compressor || pipe(pipeFD) == 0)) {
>           /* All right! We can use fd migration, which means that qemu
> @@ -2783,6 +2791,12 @@ qemuMigrationToFile(struct qemud_driver *driver, virDomainObjPtr vm,
>       ret = 0;
>
>   cleanup:
> +    /* Restore migration speed to 32MiB/s default */
> +    if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
> +        qemuMonitorSetMigrationSpeed(priv->mon, (32<<  20));

If we keep this hunk, I'd like to see this magic number as a #define 
earlier in the file, with the same justification as you gave in your 
commit comment about where it comes from (32Mbps in qemu's migration.c), 
so it becomes easier to replace the number and/or consistently use it 
elsewhere in the code if qemu ever changes.

> +        qemuDomainObjExitMonitorWithDriver(driver, vm);
> +    }

I was first worried that this part means we have more issues.  That is, 
the user can independently call virDomainMigrateSetMaxSpeed, and it 
seems like we should revert back to that value, rather than to the qemu 
default.  But on further thought - guess what?  After you complete 
migration to a file, the qemu process is (supposed to have) ended! 
There's no reason to restore migration speed after this point, because 
there's nothing further you can do with the qemu process; once the 
domain is later restored from the save file, you have created a new qemu 
process which is once again back at the default migration speed.  That 
means we can effectively drop this hunk with no change in behavior.

Meanwhile, it raises independent issues - why do we have a write-only 
interface in virDomainMigrateSetMaxSpeed?  Shouldn't we also be able to 
query the speed currently in use, and shouldn't the domain XML track the 
current migration speed?

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org