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

[Jim Fehlig]

Eric Blake wrote:
> 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.

Ok.

>
>> +        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.

Yeah, I debated about whether to even add this hunk, but was considering
'virsh dump --live ...' where the  process still exists and could
potentially be migrated to another host later.

>
> 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?

AFAICT, qemu doesn't provide a way to query the speed.  The monitor
would need this addition before we could plumb it in libvirt.

How would you like to proceed?

Regards,
Jim