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

[Jiri Denemark]

> > @@ -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;
> > +        }
> 
> You are only detecting negative time.  But what about overflow, or if
> downtime was NaN or inf?

Yeah, the test is completely wrong. It's effectively detecting only 0 time,
unsigned cannot really be negative ;)

> > +                else if (opt->type == VSH_OT_FLOAT)
> > +                    /* xgettext:c-format */
> > +                    fmt = _("[--%s <decimal>]");
> 
> <decimal> reminds me of base-10 integers, not floating point.  It looks
> like this is the first time we are accepting floating point; should we
> use <float> or <floating-point> instead as the terminology?

That's probably better but this part will be removed completely in the new
version so we don't have to worry about the terminology.

> > +        res = strtod(arg->data, &end_p);
> 
> Should we be using the gnulib strtod module here?

Why? It doesn't seem to be any better than C89 strtod. Or did I miss anything?

Jirka