[Qemu-devel] [PATCH v3 0/2] Improve info migrate output on destination

[Qemu-devel] [PATCH v3 0/2] Improve info migrate output on destination

[Juan Quintela]

Hi

On top of my previous migration-for-2.11 patches.
- We don't create an uri parameter, just too complicated and not
  needed.
- So we cerate a tcp_port parameter, easier, and everybody knows what
  it means.
- Several patches reviewed integrated in for-2.11.

Please, review.

Later, Juan.

[v2]
- dropped the qio patch, get the address later with qio_socket_get_local_address
 (danp)
- set the uri with all the options (danp found it)
- rebase on top of latest rc

Please review.

Later, Juan.

[v1]
This series (on top of my last pull requset) do:

- Make update port for address work for port 0.
  Right now we only update the _port_ that we ask for, not the one that we receive.
- Print global options as everything else (i.e. one by line, and on/off)
- Move addr cleanup to the place where we created it
- Create an uri migration parameter
- Fill the migrate parameter and set it.
- make the uri parameter optional for migrate

Please review.

Later, Juan.

Juan Quintela (2):
  migration: Create tcp_port parameter
  migration: Set the migration tcp port

 hmp.c                 |  7 +++++++
 migration/migration.c | 20 ++++++++++++++++++++
 migration/migration.h |  2 ++
 migration/socket.c    | 35 ++++++++++++++++++++++++++++++-----
 qapi/migration.json   | 19 ++++++++++++++++---
 5 files changed, 75 insertions(+), 8 deletions(-)

-- 
2.14.3

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

[Juan Quintela]

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 <quintela@redhat.com>

--

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 1727c9c003..6387d6d517 100644
--- a/hmp.c
+++ b/hmp.c
@@ -345,6 +345,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);
@@ -1659,6 +1662,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 19917a4b5b..abc02d4efd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -517,6 +517,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;
 }
@@ -884,6 +886,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)
@@ -956,6 +961,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)
@@ -2422,6 +2430,8 @@ static Property migration_properties[] = {
     DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
                       parameters.xbzrle_cache_size,
                       DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
+    DEFINE_PROP_UINT16("x-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 4cd3d13158..e2a1d86216 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -488,6 +488,9 @@
 #                     and a power of 2
 #                     (Since 2.11)
 #
+# @tcp-port: Only used for tcp, to know what is the real port
+#                     (Since 2.12)
+#
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
@@ -496,7 +499,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:
@@ -564,6 +567,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 is the real port
+#                     (Since 2.12)
+#
 # Since: 2.4
 ##
 # TODO either fuse back into MigrationParameters, or make
@@ -582,7 +589,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:
@@ -665,6 +673,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 is the real port
+#                     (Since 2.12)
+#
 # Since: 2.4
 ##
 { 'struct': 'MigrationParameters',
@@ -681,7 +693,8 @@
             '*block-incremental': 'bool' ,
             '*x-multifd-channels': 'uint8',
             '*x-multifd-page-count': 'uint32',
-            '*xbzrle-cache-size': 'size' } }
+            '*xbzrle-cache-size': 'size',
+            '*tcp-port': 'uint16'} }
 
 ##
 # @query-migrate-parameters:
-- 
2.14.3

[Qemu-devel] [PATCH v3 2/2] migration: Set the migration tcp port

[Juan Quintela]

We can set the port parameter as zero.  This patch lets us know what
port the system was choosen for us.  Now we can migrate to this place.

Signed-off-by: Juan Quintela <quintela@redhat.com>

--

This was migrate_set_uri(), but as we only need the tcp_port, change
to that one.
---
 migration/migration.c | 10 ++++++++++
 migration/migration.h |  2 ++
 migration/socket.c    | 35 ++++++++++++++++++++++++++++++-----
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index abc02d4efd..b3e396a382 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -240,6 +240,16 @@ void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char *rbname,
     }
 }
 
+void migrate_set_port(const uint16_t port, Error **errp)
+{
+    MigrateSetParameters p = {
+        .has_tcp_port = true,
+        .tcp_port = port,
+    };
+
+    qmp_migrate_set_parameters(&p, errp);
+}
+
 void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p;
diff --git a/migration/migration.h b/migration/migration.h
index 663415fe48..53153a9eca 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -210,4 +210,6 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
 void migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* rbname,
                               ram_addr_t start, size_t len);
 
+void migrate_set_port(const uint16_t port, Error **errp);
+
 #endif
diff --git a/migration/socket.c b/migration/socket.c
index 3a8232dd2d..248a798543 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -15,6 +15,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 
 #include "qemu-common.h"
 #include "qemu/error-report.h"
@@ -162,17 +163,24 @@ out:
 }
 
 
-static void socket_start_incoming_migration(SocketAddress *saddr,
-                                            Error **errp)
+static SocketAddress *socket_start_incoming_migration(SocketAddress *saddr,
+                                                      Error **errp)
 {
     QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    SocketAddress *address;
 
     qio_channel_set_name(QIO_CHANNEL(listen_ioc),
                          "migration-socket-listener");
 
     if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
         object_unref(OBJECT(listen_ioc));
-        return;
+        return NULL;
+    }
+
+    address = qio_channel_socket_get_local_address(listen_ioc, errp);
+    if (address < 0) {
+        object_unref(OBJECT(listen_ioc));
+        return NULL;
     }
 
     qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
@@ -180,14 +188,28 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
                           socket_accept_incoming_migration,
                           listen_ioc,
                           (GDestroyNotify)object_unref);
+    return address;
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
 {
     Error *err = NULL;
     SocketAddress *saddr = tcp_build_address(host_port, &err);
+
     if (!err) {
-        socket_start_incoming_migration(saddr, &err);
+        SocketAddress *address = socket_start_incoming_migration(saddr, &err);
+
+        if (address) {
+            unsigned long long port;
+
+            if (parse_uint_full(address->u.inet.port, &port, 10) < 0) {
+                error_setg(errp, "error parsing port in '%s'",
+                           address->u.inet.port);
+            } else {
+                migrate_set_port(port, errp);
+            }
+            qapi_free_SocketAddress(address);
+        }
     }
     qapi_free_SocketAddress(saddr);
     error_propagate(errp, err);
@@ -196,6 +218,9 @@ void tcp_start_incoming_migration(const char *host_port, Error **errp)
 void unix_start_incoming_migration(const char *path, Error **errp)
 {
     SocketAddress *saddr = unix_build_address(path);
-    socket_start_incoming_migration(saddr, errp);
+    SocketAddress *address;
+
+    address = socket_start_incoming_migration(saddr, errp);
+    qapi_free_SocketAddress(address);
     qapi_free_SocketAddress(saddr);
 }
-- 
2.14.3

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

[Eric Blake]

On 12/01/2017 06:57 AM, 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 <quintela@redhat.com>
> 
> --
> 

> @@ -2422,6 +2430,8 @@ static Property migration_properties[] = {
>       DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>                         parameters.xbzrle_cache_size,
>                         DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> +    DEFINE_PROP_UINT16("x-tcp-port", MigrationState,
> +                       parameters.tcp_port, 0),

Why is this one experimental when others are not,

>   
>       /* Migration capabilities */
>       DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 4cd3d13158..e2a1d86216 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -488,6 +488,9 @@
>   #                     and a power of 2
>   #                     (Since 2.11)
>   #
> +# @tcp-port: Only used for tcp, to know what is the real port

s/what is the real port/what the real port is/

> +#                     (Since 2.12)
> +#

Especially since it is not experimental here?

> @@ -564,6 +567,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 is the real port

same wording tweak

> +#                     (Since 2.12)
> +#
>   # Since: 2.4
>   ##
>   # TODO either fuse back into MigrationParameters, or make
> @@ -582,7 +589,8 @@
>               '*block-incremental': 'bool',
>               '*x-multifd-channels': 'int',
>               '*x-multifd-page-count': 'int',
> -            '*xbzrle-cache-size': 'size' } }
> +            '*xbzrle-cache-size': 'size' ,

the space before comma looks unusual (although it's harmless)

> +            '*tcp-port': 'uint16'} }


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

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

[Peter Xu]

On Fri, Dec 01, 2017 at 01:57:49PM +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 <quintela@redhat.com>
> 
> --
> 
> 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 1727c9c003..6387d6d517 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -345,6 +345,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);
> @@ -1659,6 +1662,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);

Should we allow user to set this parameter?  Or it's just used for
query, only?  (AFAIU from next patch, this is only used for query)

> +        break;
>      default:
>          assert(0);
>      }
> diff --git a/migration/migration.c b/migration/migration.c
> index 19917a4b5b..abc02d4efd 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -517,6 +517,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;
>  }
> @@ -884,6 +886,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)
> @@ -956,6 +961,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)
> @@ -2422,6 +2430,8 @@ static Property migration_properties[] = {
>      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>                        parameters.xbzrle_cache_size,
>                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> +    DEFINE_PROP_UINT16("x-tcp-port", MigrationState,
> +                       parameters.tcp_port, 0),

Same question here...  If it's only for querying, why allow user to
specify it?

Thanks,

>  
>      /* Migration capabilities */
>      DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 4cd3d13158..e2a1d86216 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -488,6 +488,9 @@
>  #                     and a power of 2
>  #                     (Since 2.11)
>  #
> +# @tcp-port: Only used for tcp, to know what is the real port
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -496,7 +499,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:
> @@ -564,6 +567,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 is the real port
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  # TODO either fuse back into MigrationParameters, or make
> @@ -582,7 +589,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:
> @@ -665,6 +673,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 is the real port
> +#                     (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -681,7 +693,8 @@
>              '*block-incremental': 'bool' ,
>              '*x-multifd-channels': 'uint8',
>              '*x-multifd-page-count': 'uint32',
> -            '*xbzrle-cache-size': 'size' } }
> +            '*xbzrle-cache-size': 'size',
> +            '*tcp-port': 'uint16'} }
>  
>  ##
>  # @query-migrate-parameters:
> -- 
> 2.14.3
> 

-- 
Peter Xu

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

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Fri, Dec 01, 2017 at 01:57:49PM +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 <quintela@redhat.com>
> > 
> > --
> > 
> > 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 1727c9c003..6387d6d517 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -345,6 +345,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);
> > @@ -1659,6 +1662,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);
> 
> Should we allow user to set this parameter?  Or it's just used for
> query, only?  (AFAIU from next patch, this is only used for query)

Same question; it makes me wonder if this field should actually go in
MigrationStats; the problem with that is it's currently only shows the
source side; even though at times people have suggested adding
destination side data to it.

Dave

> > +        break;
> >      default:
> >          assert(0);
> >      }
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 19917a4b5b..abc02d4efd 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -517,6 +517,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;
> >  }
> > @@ -884,6 +886,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)
> > @@ -956,6 +961,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)
> > @@ -2422,6 +2430,8 @@ static Property migration_properties[] = {
> >      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
> >                        parameters.xbzrle_cache_size,
> >                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
> > +    DEFINE_PROP_UINT16("x-tcp-port", MigrationState,
> > +                       parameters.tcp_port, 0),
> 
> Same question here...  If it's only for querying, why allow user to
> specify it?
> 
> Thanks,
> 
> >  
> >      /* Migration capabilities */
> >      DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 4cd3d13158..e2a1d86216 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -488,6 +488,9 @@
> >  #                     and a power of 2
> >  #                     (Since 2.11)
> >  #
> > +# @tcp-port: Only used for tcp, to know what is the real port
> > +#                     (Since 2.12)
> > +#
> >  # Since: 2.4
> >  ##
> >  { 'enum': 'MigrationParameter',
> > @@ -496,7 +499,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:
> > @@ -564,6 +567,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 is the real port
> > +#                     (Since 2.12)
> > +#
> >  # Since: 2.4
> >  ##
> >  # TODO either fuse back into MigrationParameters, or make
> > @@ -582,7 +589,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:
> > @@ -665,6 +673,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 is the real port
> > +#                     (Since 2.12)
> > +#
> >  # Since: 2.4
> >  ##
> >  { 'struct': 'MigrationParameters',
> > @@ -681,7 +693,8 @@
> >              '*block-incremental': 'bool' ,
> >              '*x-multifd-channels': 'uint8',
> >              '*x-multifd-page-count': 'uint32',
> > -            '*xbzrle-cache-size': 'size' } }
> > +            '*xbzrle-cache-size': 'size',
> > +            '*tcp-port': 'uint16'} }
> >  
> >  ##
> >  # @query-migrate-parameters:
> > -- 
> > 2.14.3
> > 
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[Juan Quintela]

Eric Blake <eblake@redhat.com> wrote:
> On 12/01/2017 06:57 AM, 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 <quintela@redhat.com>
>>
>> --
>>
>
>> @@ -2422,6 +2430,8 @@ static Property migration_properties[] = {
>>       DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>>                         parameters.xbzrle_cache_size,
>>                         DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
>> +    DEFINE_PROP_UINT16("x-tcp-port", MigrationState,
>> +                       parameters.tcp_port, 0),
>
> Why is this one experimental when others are not,

changing.

Actually, because I don't know if everybody wants this.

>
>>         /* Migration capabilities */
>>       DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 4cd3d13158..e2a1d86216 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -488,6 +488,9 @@
>>   #                     and a power of 2
>>   #                     (Since 2.11)
>>   #
>> +# @tcp-port: Only used for tcp, to know what is the real port
>
> s/what is the real port/what the real port is/

Changed.

>> +#                     (Since 2.12)
>> +#
>
> Especially since it is not experimental here?
>
>> @@ -564,6 +567,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 is the real port
>
> same wording tweak

Changed..

>> +#                     (Since 2.12)
>> +#
>>   # Since: 2.4
>>   ##
>>   # TODO either fuse back into MigrationParameters, or make
>> @@ -582,7 +589,8 @@
>>               '*block-incremental': 'bool',
>>               '*x-multifd-channels': 'int',
>>               '*x-multifd-page-count': 'int',
>> -            '*xbzrle-cache-size': 'size' } }
>> +            '*xbzrle-cache-size': 'size' ,
>
> the space before comma looks unusual (although it's harmless)


Changed.

Thanks.

>> +            '*tcp-port': 'uint16'} }

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

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:

>> @@ -1659,6 +1662,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);
>
> Should we allow user to set this parameter?  Or it's just used for
> query, only?  (AFAIU from next patch, this is only used for query)

Only for query.  Really it is only needed for tcp, due to the wierd way
that ports are allocated to allow for test running in parallel safely.

>
>> +        break;
>>      default:
>>          assert(0);
>>      }
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 19917a4b5b..abc02d4efd 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -517,6 +517,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;
>>  }
>> @@ -884,6 +886,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)
>> @@ -956,6 +961,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)
>> @@ -2422,6 +2430,8 @@ static Property migration_properties[] = {
>>      DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
>>                        parameters.xbzrle_cache_size,
>>                        DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
>> +    DEFINE_PROP_UINT16("x-tcp-port", MigrationState,
>> +                       parameters.tcp_port, 0),
>
> Same question here...  If it's only for querying, why allow user to
> specify it?

You are right.

Because *I* changed the x-uri bits into this.  and for x-uri it made
sense.

And I just did the conversion, witohut too much thinking.

Later, Juan.