[PATCH v2 1/2] virsh: domain: fix mistake in cmdMigrateSetMaxDowntime()

[Kristina Hanicova]

On Fri, Sep 24, 2021 at 11:03 AM Michal Prívozník <mprivozn at redhat.com>
wrote:

> On 9/24/21 1:30 AM, Kristina Hanicova wrote:
> > Function returned false when everything ended successfully, there
> > was a missing check for a return value. This patch fixes that.
> >
> > Signed-off-by: Kristina Hanicova <khanicov at redhat.com>
> > ---
> >  tools/virsh-domain.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index f876f30cc5..2730acfba5 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -11018,7 +11018,7 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const
> vshCmd *cmd)
> >          goto done;
> >      }
> >
> > -    if (virDomainMigrateSetMaxDowntime(dom, downtime, 0))
> > +    if (virDomainMigrateSetMaxDowntime(dom, downtime, 0) < 0)
> >          goto done;
> >
> >      ret = true;
> >
>
> Technically, there wasn't any bug here. virDomainMigrateSetMaxDowntime()
> returns 0 on success and 0 won't evaluate to true thus 'goto done' is
> skipped over and ret is set to true.
>
> And if anything else is returned (currently only -1 is possible) then
> i's evaluated to true and control jump to the done label. BUT, your fix
> is correct because in libvirt only negative values are treated as
> errors. Zero and positive values are treated as success. I'll reword the
> commit message a bit.
>
> Sorry for not realizing this earlier.
>
> Michal
>
>

I think the commit message could be rewritten to something as:

If there was added a new return value indicating success to the function
virDomainMigrateSetMaxDowntime() in the future, our function would treat it
as
an error state and would return false (indicating failure). This patch
fixes
it, so that the call of the function follows the same pattern as is
currently
set in libvirt.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210924/4c0c5298/attachment-0001.htm>