[libvirt] [PATCH] virsh migrate: Properly check for --parallel-connections

Jiri Denemark posted 1 patch 4 years, 9 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/46a22f8d4bafa5dd713fb7f526de3e16acdecb73.1563545121.git.jdenemar@redhat.com
tools/virsh-domain.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
[libvirt] [PATCH] virsh migrate: Properly check for --parallel-connections
Posted by Jiri Denemark 4 years, 9 months ago
Ever since --parallel-connections option for virsh migrate was
introduced we did not properly check the return value of
vshCommandOptInt. We would set VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS
parameter even if vshCommandOptInt returned 0 (which means
--parallel-connections was not specified) when another int option which
was checked earlier was specified with a nonzero value.

Specifically, running virsh migrate with either
--auto-converge-increment, --auto-converge-initial, --comp-mt-dthreads,
--comp-mt-threads, or --comp-mt-level would set
VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS parameter and if --parallel
option was not used, libvirt would complain

    error: invalid argument: Turn parallel migration on to tune it

even though --parallel-connections option was not used at all.

https://bugzilla.redhat.com/show_bug.cgi?id=1726643

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 tools/virsh-domain.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 828ae30789..2ad73959ec 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10785,13 +10785,14 @@ doMigrate(void *opaque)
             goto save_error;
     }
 
-    if (vshCommandOptInt(ctl, cmd, "parallel-connections", &intOpt) < 0)
+    if ((rv = vshCommandOptInt(ctl, cmd, "parallel-connections", &intOpt)) < 0) {
         goto out;
-    if (intOpt &&
-        virTypedParamsAddInt(&params, &nparams, &maxparams,
-                             VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS,
-                             intOpt) < 0)
-        goto save_error;
+    } else if (rv > 0) {
+        if (virTypedParamsAddInt(&params, &nparams, &maxparams,
+                                 VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS,
+                                 intOpt) < 0)
+            goto save_error;
+    }
 
     if (vshCommandOptBool(cmd, "live"))
         flags |= VIR_MIGRATE_LIVE;
-- 
2.22.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh migrate: Properly check for --parallel-connections
Posted by Michal Privoznik 4 years, 9 months ago
On 7/19/19 4:05 PM, Jiri Denemark wrote:
> Ever since --parallel-connections option for virsh migrate was
> introduced we did not properly check the return value of
> vshCommandOptInt. We would set VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS
> parameter even if vshCommandOptInt returned 0 (which means
> --parallel-connections was not specified) when another int option which
> was checked earlier was specified with a nonzero value.
> 
> Specifically, running virsh migrate with either
> --auto-converge-increment, --auto-converge-initial, --comp-mt-dthreads,
> --comp-mt-threads, or --comp-mt-level would set
> VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS parameter and if --parallel
> option was not used, libvirt would complain
> 
>      error: invalid argument: Turn parallel migration on to tune it
> 
> even though --parallel-connections option was not used at all.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1726643
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>   tools/virsh-domain.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list