[libvirt] [PATCH] Add --downtime option to virsh migrate command

[Matthias Bolte]

2010/3/17 Jiri Denemark <jdenemar at redhat.com>:
>> > @@ -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.

Matthias