* [Qemu-devel] [PATCH v3 0/2] Improve info migrate output on destination @ 2017-12-01 12:57 Juan Quintela 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter Juan Quintela 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 2/2] migration: Set the migration tcp port Juan Quintela 0 siblings, 2 replies; 8+ messages in thread From: Juan Quintela @ 2017-12-01 12:57 UTC (permalink / raw) To: qemu-devel; +Cc: dgilbert, lvivier, peterx 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter 2017-12-01 12:57 [Qemu-devel] [PATCH v3 0/2] Improve info migrate output on destination Juan Quintela @ 2017-12-01 12:57 ` Juan Quintela 2017-12-01 18:28 ` Eric Blake 2017-12-05 8:02 ` Peter Xu 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 2/2] migration: Set the migration tcp port Juan Quintela 1 sibling, 2 replies; 8+ messages in thread From: Juan Quintela @ 2017-12-01 12:57 UTC (permalink / raw) To: qemu-devel; +Cc: dgilbert, lvivier, peterx 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter Juan Quintela @ 2017-12-01 18:28 ` Eric Blake 2018-01-04 18:16 ` Juan Quintela 2017-12-05 8:02 ` Peter Xu 1 sibling, 1 reply; 8+ messages in thread From: Eric Blake @ 2017-12-01 18:28 UTC (permalink / raw) To: Juan Quintela, qemu-devel; +Cc: lvivier, dgilbert, peterx 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter 2017-12-01 18:28 ` Eric Blake @ 2018-01-04 18:16 ` Juan Quintela 0 siblings, 0 replies; 8+ messages in thread From: Juan Quintela @ 2018-01-04 18:16 UTC (permalink / raw) To: Eric Blake; +Cc: qemu-devel, lvivier, dgilbert, peterx 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'} } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter Juan Quintela 2017-12-01 18:28 ` Eric Blake @ 2017-12-05 8:02 ` Peter Xu 2017-12-08 11:53 ` Dr. David Alan Gilbert 2018-01-04 18:18 ` Juan Quintela 1 sibling, 2 replies; 8+ messages in thread From: Peter Xu @ 2017-12-05 8:02 UTC (permalink / raw) To: Juan Quintela; +Cc: qemu-devel, dgilbert, lvivier 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter 2017-12-05 8:02 ` Peter Xu @ 2017-12-08 11:53 ` Dr. David Alan Gilbert 2018-01-04 18:18 ` Juan Quintela 1 sibling, 0 replies; 8+ messages in thread From: Dr. David Alan Gilbert @ 2017-12-08 11:53 UTC (permalink / raw) To: Peter Xu; +Cc: Juan Quintela, qemu-devel, lvivier * 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter 2017-12-05 8:02 ` Peter Xu 2017-12-08 11:53 ` Dr. David Alan Gilbert @ 2018-01-04 18:18 ` Juan Quintela 1 sibling, 0 replies; 8+ messages in thread From: Juan Quintela @ 2018-01-04 18:18 UTC (permalink / raw) To: Peter Xu; +Cc: qemu-devel, dgilbert, lvivier 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] migration: Set the migration tcp port 2017-12-01 12:57 [Qemu-devel] [PATCH v3 0/2] Improve info migrate output on destination Juan Quintela 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter Juan Quintela @ 2017-12-01 12:57 ` Juan Quintela 1 sibling, 0 replies; 8+ messages in thread From: Juan Quintela @ 2017-12-01 12:57 UTC (permalink / raw) To: qemu-devel; +Cc: dgilbert, lvivier, peterx 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-01-04 18:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-01 12:57 [Qemu-devel] [PATCH v3 0/2] Improve info migrate output on destination Juan Quintela 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 1/2] migration: Create tcp_port parameter Juan Quintela 2017-12-01 18:28 ` Eric Blake 2018-01-04 18:16 ` Juan Quintela 2017-12-05 8:02 ` Peter Xu 2017-12-08 11:53 ` Dr. David Alan Gilbert 2018-01-04 18:18 ` Juan Quintela 2017-12-01 12:57 ` [Qemu-devel] [PATCH v3 2/2] migration: Set the migration tcp port Juan Quintela
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).