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(a)redhat.com&gt; > --- > 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

Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com&gt; --- tools/virsh-domain.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2730acfba5..cfa0016296 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11006,25 +11006,19 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; unsigned long long downtime = 0; - bool ret = false; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; if (vshCommandOptULongLong(ctl, cmd, "downtime", &downtime) < 0) - goto done; + return false; + if (downtime < 1) { vshError(ctl, "%s", _("migrate: Invalid downtime")); - goto done; + return false; } - if (virDomainMigrateSetMaxDowntime(dom, downtime, 0) < 0) - goto done; - - ret = true; - - done: - return ret; + return virDomainMigrateSetMaxDowntime(dom, downtime, 0) == 0; }