Re: [PATCH v6 1/9] migration: introduced 'MigrateAddress' in QAPI for migration wire protocol.

[Markus Armbruster]

Het Gala <het.gala@nutanix.com> writes:

> This patch introduces well defined MigrateAddress struct and its related
> child objects.
>
> The existing argument of 'migrate' and 'migrate-incoming' QAPI - 'uri'
> is of string type. The current migration flow follows double encoding
> scheme for  fetching migration parameters such as 'uri' and this is
> not an ideal design.
>
> Motive for intoducing struct level design is to prevent double encoding
> of QAPI arguments, as Qemu should be able to directly use the QAPI
> arguments without any level of encoding.
>
> Suggested-by: Aravind Retnakaran <aravind.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qapi/migration.json | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 179af0c4d8..e61d25eba2 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1407,6 +1407,51 @@
>  ##
>  { 'command': 'migrate-continue', 'data': {'state': 'MigrationStatus'} }
>  
> +##
> +# @MigrationAddressType:
> +#
> +# The migration stream transport mechanisms.
> +#
> +# @socket: Migrate via socket.
> +#
> +# @exec: Direct the migration stream to another process.
> +#
> +# @rdma: Migrate via RDMA.
> +#
> +# Since 8.1
> +##
> +{ 'enum': 'MigrationAddressType',
> +  'data': ['socket', 'exec', 'rdma'] }
> +
> +##
> +# @MigrationExecCommand:
> +#
> +# @args: list of commands for migraton stream execution to a file.

Typo: migration

> +#
> +# Notes:
> +#
> +# 1. @args[0] needs to be the path to the new program.

@args can't be a "list of commands", as we're spawning just one process.
So what is it?

Digging through the code with the entire series applied...  Member @args
is used in two places:

1. qemu_start_incoming_migration() passes it to
   exec_start_incoming_migration(), which translates it into an array
   and passes it to qio_channel_command_new_spawn().

2. qmp_migrate() passes it to exec_start_outgoing_migration(), which
   does the same.

qio_channel_command_new_spawn() passes it to
g_spawn_async_with_pipes().  A close read of the latter's documentation
leads me to:

* args[0] is the excutable's file name.  As usual, a relative name is
  relative to the QEMU process's current working directory.

* args[1..] are the arguments.

Unlike POSIX interfaces like execv() and posix_spawn(), this doesn't
separate the executable's file name and 0-th argument.

In short, the head of @args is the executable's filename, and the
remainder are the arguments.  The fact that the the executable's
filename is passed as 0-th argument to the child process is detail.

Perhaps this could do:

   ##
   # @MigrationExecCommand:
   #
   # @args: command and arguments to execute.

If we want more detail, perhaps:

   # @args: command (list head) and arguments (list tail) to execute.

Not sure we need it.  Thoughts?

> +#
> +# Since 8.1
> +##
> +{ 'struct': 'MigrationExecCommand',
> +  'data': {'args': [ 'str' ] } }
> +
> +##
> +# @MigrationAddress:
> +#
> +# Migration endpoint configuration.
> +#
> +# Since 8.1
> +##
> +{ 'union': 'MigrationAddress',
> +  'base': { 'transport' : 'MigrationAddressType'},
> +  'discriminator': 'transport',
> +  'data': {
> +    'socket': 'SocketAddress',
> +    'exec': 'MigrationExecCommand',
> +    'rdma': 'InetSocketAddress' } }
> +
>  ##
>  # @migrate:
>  #