Re: [Qemu-devel] [PATCH v4 1/2] migration: Create tcp_port parameter

[Daniel P. Berrange]

On Fri, Jan 05, 2018 at 09:51:08PM +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                 |  7 +++++++
>  migration/migration.c | 10 ++++++++++
>  qapi/migration.json   | 19 ++++++++++++++++---
>  3 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 354baaa6b6..1bb9d5d8f8 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -360,6 +360,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const 
> QDict *qdict)
>          monitor_printf(mon, "%s: %" PRIu64 "\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
>              params->xbzrle_cache_size);
> +        monitor_printf(mon, "%s: %d\n",
> +            MigrationParameter_str(MIGRATION_PARAMETER_TCP_PORT),
> +            params->tcp_port);
>      }
>  
>      qapi_free_MigrationParameters(params);
> @@ -1674,6 +1677,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> QDict *qdict)
>          }
>          p->xbzrle_cache_size = cache_size;
>          break;
> +    case MIGRATION_PARAMETER_TCP_PORT:
> +        p->has_tcp_port = true;
> +        visit_type_uint16(v, param, &p->tcp_port, &err);
> +        break;
>      default:
>          assert(0);
>      }
> diff --git a/migration/migration.c b/migration/migration.c
> index 7a77b193c1..4cdd8ee31b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -522,6 +522,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
> **errp)
>      params->x_multifd_page_count = s->parameters.x_multifd_page_count;
>      params->has_xbzrle_cache_size = true;
>      params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
> +    params->has_tcp_port = true;
> +    params->tcp_port = s->parameters.tcp_port;
>  
>      return params;
>  }
> @@ -923,6 +925,9 @@ static void 
> migrate_params_test_apply(MigrateSetParameters *params,
>      if (params->has_xbzrle_cache_size) {
>          dest->xbzrle_cache_size = params->xbzrle_cache_size;
>      }
> +    if (params->has_tcp_port) {
> +        dest->tcp_port = params->tcp_port;
> +    }
>  }
>  
>  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> @@ -995,6 +1000,9 @@ static void migrate_params_apply(MigrateSetParameters 
> *params, Error **errp)
>          s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
>          xbzrle_cache_resize(params->xbzrle_cache_size, errp);
>      }
> +    if (params->has_tcp_port) {
> +        s->parameters.tcp_port = params->tcp_port;
> +    }
>  }
>  
>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> @@ -2470,6 +2478,8 @@ static Property migration_properties[] = {
>      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>                        parameters.xbzrle_cache_size,
>                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> +    DEFINE_PROP_UINT16("tcp-port", MigrationState,
> +                       parameters.tcp_port, 0),
>  
>      /* Migration capabilities */
>      DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 70e7b677ef..2be1400249 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -501,6 +501,9 @@
>  #                     and a power of 2
>  #                     (Since 2.11)
>  #
> +# @tcp-port: Only used for tcp, to know what the real port is
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -509,7 +512,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', 'tcp-port' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -577,6 +580,10 @@
>  #                     needs to be a multiple of the target page size
>  #                     and a power of 2
>  #                     (Since 2.11)
> +#
> +# @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
> @@ -595,7 +602,8 @@
>              '*block-incremental': 'bool',
>              '*x-multifd-channels': 'int',
>              '*x-multifd-page-count': 'int',
> -            '*xbzrle-cache-size': 'size' } }
> +            '*xbzrle-cache-size': 'size',
> +            '*tcp-port': 'uint16'} }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -678,6 +686,10 @@
>  #                     needs to be a multiple of the target page size
>  #                     and a power of 2
>  #                     (Since 2.11)
> +#
> +# @tcp-port: Only used for tcp, to know what the real port is
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -694,7 +706,8 @@
>              '*block-incremental': 'bool' ,
>              '*x-multifd-channels': 'uint8',
>              '*x-multifd-page-count': 'uint32',
> -            '*xbzrle-cache-size': 'size' } }
> +            '*xbzrle-cache-size': 'size',
> +            '*tcp-port': 'uint16'} }

I think we should have an array of SocketAddress structs here, not merely
the port.

In the near future I'll be sending patches to allow migration code to listen
on multiple sockets concurrently, which is neccessary to correctly support
IPv4+IPv6 dual stack.  This will also make QEMU correctly honour DNS names
which resolve to multiple IP address when a host has mutiple NICs.

eg when you listen on  "foobar.example.com" you might end up having 1, 2,
3, 4, or more IP addresses being listened on. It would be useful to know
exactly which IPv4 and IPv6 addresses were actually listened on, for a
given hostname passed in the original URI.

If we report an array of SocketAddress structs, then we'll solve both your
problem of reporting the port number, and the more general problem of
reporting IP addresses at the same time in a general manner.


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 :|