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