qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v10 03/24] migration: Create tcp_port parameter


From: Daniel P . Berrangé
Subject: Re: [Qemu-devel] [PATCH v10 03/24] migration: Create tcp_port parameter
Date: Wed, 7 Mar 2018 11:38:34 +0000
User-agent: Mutt/1.9.2 (2017-12-15)

On Wed, Mar 07, 2018 at 11:59:49AM +0100, Juan Quintela wrote:
> It will be used to store the uri tcp_port parameter.  This is the only
> parameter than can change and we can need to be able to connect to it.
> 
> Signed-off-by: Juan Quintela <address@hidden>
> 
> --
> 
> This used to be uri parameter, but it has so many troubles to
> reproduce that it don't just make sense.
> ---
>  hmp.c                 |  3 +++
>  migration/migration.c |  8 ++++++++
>  qapi/migration.json   | 19 ++++++++++++++++---
>  3 files changed, 27 insertions(+), 3 deletions(-)

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7f465a1902..b6ef193f47 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -490,6 +490,9 @@
>  #                     and a power of 2
>  #                     (Since 2.11)
>  #
> +# @x-tcp-port: Only used for tcp, to know what the real port is
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -498,7 +501,7 @@
>             'tls-creds', 'tls-hostname', 'max-bandwidth',
>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>             'x-multifd-channels', 'x-multifd-page-count',
> -           'xbzrle-cache-size' ] }
> +           'xbzrle-cache-size', 'x-tcp-port' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -566,6 +569,10 @@
>  #                     needs to be a multiple of the target page size
>  #                     and a power of 2
>  #                     (Since 2.11)
> +#
> +# @x-tcp-port: Only used for tcp, to know what the real port is
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  # TODO either fuse back into MigrationParameters, or make
> @@ -584,7 +591,8 @@
>              '*block-incremental': 'bool',
>              '*x-multifd-channels': 'int',
>              '*x-multifd-page-count': 'int',
> -            '*xbzrle-cache-size': 'size' } }
> +            '*xbzrle-cache-size': 'size',
> +            '*x-tcp-port': 'uint16'} }

This should not exist - this exposes this parameter in migate-set-parameters
as a end user settable property, which is definitely not desirable.

It is only something we should report with 'query-migrate' / 'info migrate'

>  # @migrate-set-parameters:
> @@ -667,6 +675,10 @@
>  #                     needs to be a multiple of the target page size
>  #                     and a power of 2
>  #                     (Since 2.11)
> +#
> +# @x-tcp-port: Only used for tcp, to know what the real port is
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -683,7 +695,8 @@
>              '*block-incremental': 'bool' ,
>              '*x-multifd-channels': 'uint8',
>              '*x-multifd-page-count': 'uint32',
> -            '*xbzrle-cache-size': 'size' } }
> +            '*xbzrle-cache-size': 'size',
> +            '*x-tcp-port': 'uint16'} }

As mentioned in previous review, IMHO we should be reporting the full
socket address, and allow an array of them, since we're going to have
more than one address available. i.e.

   '*socket-address': ['SocketAddress']

It doesn't cover non-socket based URIs, but that's fine, because for those
the mgmt app already knows how the channel is setup. We just need the
array of SocketAddress, because for socket URIs, the hostname, gets turned
into an array of addresses, and the mgmt app can't discover them.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



reply via email to

[Prev in Thread] Current Thread [Next in Thread]