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

[Michal Prívozník]

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