Re: [PATCH v2 3/3] virsh: Add options for parallel migration

[John Ferlan <jferlan@xxxxxxxxxx>]

On 2/8/19 10:08 AM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
>  tools/virsh-domain.c | 19 +++++++++++++++++++
>  tools/virsh.pod      |  7 +++++++
>  2 files changed, 26 insertions(+)
> 

Should supplying parallel-connections imply parallel since the previous
patch would fail:

+    if (migParams->params[QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS].set &&
+        !(flags & VIR_MIGRATE_PARALLEL)) {
+        virReportError(VIR_ERR_INVALID_ARG, "%s",
+                       _("Turn parallel migration on to tune it"));
+        goto error;
+    }
+

either that or make parallel-connections be an optional parameter to
--parallel using VSH_OFLAG_EMPTY_OK and converting the optionally read
string into an int.


> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 8b20059335..c704faf7e1 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -10561,6 +10561,14 @@ static const vshCmdOptDef opts_migrate[] = {
>       .type = VSH_OT_INT,
>       .help = N_("post-copy migration bandwidth limit in MiB/s")
>      },
> +    {.name = "parallel",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("enable parallel migration")
> +    },
> +    {.name = "parallel-connections",
> +     .type = VSH_OT_INT,
> +     .help = N_("number of connections for parallel migration")
> +    },
>      {.name = NULL}
>  };
>  
> @@ -10766,6 +10774,14 @@ doMigrate(void *opaque)
>              goto save_error;
>      }
>  
> +    if (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;
> +
>      if (vshCommandOptBool(cmd, "live"))
>          flags |= VIR_MIGRATE_LIVE;
>      if (vshCommandOptBool(cmd, "p2p"))
> @@ -10814,6 +10830,9 @@ doMigrate(void *opaque)
>      if (vshCommandOptBool(cmd, "tls"))
>          flags |= VIR_MIGRATE_TLS;
>  
> +    if (vshCommandOptBool(cmd, "parallel"))
> +        flags |= VIR_MIGRATE_PARALLEL;
> +
>      if (flags & VIR_MIGRATE_PEER2PEER || vshCommandOptBool(cmd, "direct")) {
>          if (virDomainMigrateToURI3(dom, desturi, params, nparams, flags) == 0)
>              ret = '0';
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 67edb57b14..7604c7536e 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1904,6 +1904,7 @@ I<domain> I<desturi> [I<migrateuri>] [I<graphicsuri>] [I<listen-address>] [I<dna
>  [I<--comp-xbzrle-cache>] [I<--auto-converge>] [I<auto-converge-initial>]
>  [I<auto-converge-increment>] [I<--persistent-xml> B<file>] [I<--tls>]
>  [I<--postcopy-bandwidth> B<bandwidth>]
> +[I<--parallel> [I<parallel-connections> B<connections>]]

--parallel-connections

>  
>  Migrate domain to another host.  Add I<--live> for live migration; <--p2p>
>  for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled>
> @@ -1997,6 +1998,12 @@ Providing I<--tls> causes the migration to use the host configured TLS setup
>  the migration of the domain. Usage requires proper TLS setup for both source
>  and target.
>  
> +I<--parallel> option will cause migration data to be sent over multiple
> +parallel connections. The number of such connections can be set using
> +I<parallel-connections>. Parallel connections may help with saturating the

--parallel-connections

> +network link between the source and the target and thus speeding up the
> +migration.
> +

[although too many could really be a problem ;-)]

What's here is fine.... If you want to alter to have one option with an
optional parameter, then just repost this patch.

Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>

John

>  Running migration can be canceled by interrupting virsh (usually using
>  C<Ctrl-C>) or by B<domjobabort> command sent from another virsh instance.
>  
>