<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 24, 2021 at 11:03 AM Michal Prívozník <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 9/24/21 1:30 AM, Kristina Hanicova wrote:<br>
> Function returned false when everything ended successfully, there<br>
> was a missing check for a return value. This patch fixes that.<br>
> <br>
> Signed-off-by: Kristina Hanicova <<a href="mailto:khanicov@redhat.com" target="_blank">khanicov@redhat.com</a>><br>
> ---<br>
>  tools/virsh-domain.c | 2 +-<br>
>  1 file changed, 1 insertion(+), 1 deletion(-)<br>
> <br>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c<br>
> index f876f30cc5..2730acfba5 100644<br>
> --- a/tools/virsh-domain.c<br>
> +++ b/tools/virsh-domain.c<br>
> @@ -11018,7 +11018,7 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)<br>
>          goto done;<br>
>      }<br>
>  <br>
> -    if (virDomainMigrateSetMaxDowntime(dom, downtime, 0))<br>
> +    if (virDomainMigrateSetMaxDowntime(dom, downtime, 0) < 0)<br>
>          goto done;<br>
>  <br>
>      ret = true;<br>
> <br>
<br>
Technically, there wasn't any bug here. virDomainMigrateSetMaxDowntime()<br>
returns 0 on success and 0 won't evaluate to true thus 'goto done' is<br>
skipped over and ret is set to true.<br>
<br>
And if anything else is returned (currently only -1 is possible) then<br>
i's evaluated to true and control jump to the done label. BUT, your fix<br>
is correct because in libvirt only negative values are treated as<br>
errors. Zero and positive values are treated as success. I'll reword the<br>
commit message a bit.<br>
<br>
Sorry for not realizing this earlier.<br>
<br>
Michal<br>
<br></blockquote><div><br></div><div><br></div><div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">I think the commit message could be rewritten to something as:</span></span></div><div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)"><br></span></span></div><div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)"><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">If there was added a new return value indicating success to the function
</span><br><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">virDomainMigrateSetMaxDowntime</span><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">() in the future, our function would treat it as
</span><br>an error state and would return false (indicating failure). This patch fixes
<br>it, so that the call of the function follows the same pattern as is currently
<br>set in <span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">libvirt</span><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)">.</span><br></span></span></span></div><div><span style="font-family:monospace"><span style="color:rgb(0,0,0);background-color:rgb(255,255,255)"></span><br></span></div></div></div>