2010/3/17 Daniel P. Berrange <berrange@xxxxxxxxxx>: > On Wed, Mar 17, 2010 at 11:05:35AM +0100, Matthias Bolte wrote: >> 2010/3/17 Jiri Denemark <jdenemar@xxxxxxxxxx>: >> >> > @@ -2794,6 +2799,19 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) >> >> > if (vshCommandOptBool (cmd, "suspend")) >> >> > flags |= VIR_MIGRATE_PAUSED; >> >> > >> >> > + downtime = vshCommandOptFloat(cmd, "downtime", &found); >> >> > + if (found) { >> >> > + unsigned long long nanoseconds = downtime * 1e9; >> >> > + >> >> > + if (nanoseconds <= 0) { >> >> > + vshError(ctl, "%s", _("migrate: Invalid downtime")); >> >> > + goto done; >> >> > + } >> >> > + >> >> > + if (virDomainMigrateSetDowntime(dom, nanoseconds)) >> >> >> >> Is this persistent, or only valid for the next migration? For example, >> >> I do a migration with --downtime and afterwards I do another migration >> >> with the same domain, but this time I don't specify a downtime. Would >> >> the first downtime still apply to the second migration? >> > >> > Actually, this is a very good question since it reveals the API wasn't >> > designed well enough. Current implementation doesn't do much with the value >> > passed to virDomainMigrateSetDowntime, it just sends it to the appropriate >> > hypervisor (if it supports such functionality). In other words, the behavior >> > may differ for different hypervisors. Which doesn't seem to be a good thing >> > for a libvirt's public API. So we should decide whether the effect of calling >> > this API should be one-time or persistent and emulate that behavior for >> > hypervisors which don't support it. We also need flags parameter so that we >> > can change that behavior in the future. >> > >> > Opinions? >> > >> > Jirka >> > >> >> Your patch series haven't been reviewed in detail yet, so some general >> comments about it. >> >> First of all, as with all new public API functions, >> virDomainMigrateSetDowntime should have an 'unsigned int flags' >> parameter. Even if it's currently unused it could come in handy in the >> future. >> >> Maybe the function should be called >> virDomainMigrateSetTolerableDowntime or >> virDomainMigrateSetMaxDowntime, to emphasis that this downtime is an >> upper limit. >> >> There is no getter for the max downtime value, I think there should be one. >> >> Regarding the persistence, if the max downtime is implemented in the >> way of a setter/(getter), it should be persistent. For a per-migration >> option it should be a parameter to the migrate function like >> bandwidth, but we can change the migrate function anymore. Therefore, >> the max downtime should be a persistent option per domain, like the >> autostart flag. > > Conceptually this doesn't make sense. The max downtime parameter is a > live tunable that a management app will set based on the conditions at > the time of migration. Suitable value is dependant on the workload of > the guest (how quickly it is dirtying RAM) and network bandwidth between > hosts (how quickly RAM is transferred). You would only increase the > max downtime if the VM was not able to complete migration promptly > enough. So it shouldn't be persistent, and I don't think there's any > compelling need for a getter either. > > Regards, > Daniel > True. I just had the API POV in mind and forgot about the rest. So we add a new function to configure a per-migration option because we cannot change the existing migrate functions to add a new parameter. Matthias -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list