Message ID | 20190305181602.9051-11-dgilbert@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [PULL,01/21] migration: Fix cancel state | expand |
On 3/5/19 12:15 PM, Dr. David Alan Gilbert (git) wrote: > From: Juan Quintela <quintela@redhat.com> > > It will be used to store the uri parameters. We want this only for > tcp, so we don't set it for other uris. We need it to know what port > is migration running. > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Signed-off-by: Juan Quintela <quintela@redhat.com> > > -- > This was not the usual '---' divider, and hence: > This used to be uri parameter, but it has so many troubles to > reproduce that it don't just make sense. > > This used to be a port parameter. I was asked to move to > SocketAddress, done. > I also merged the setting of the migration tcp port in this one > because now I need to free the address, and this makes it easier. > This used to be x-socket-address with a single direction, now it is a > list of addresses. > Move SocketAddress_to_str here. I used to try to generalize the one > in chardev/char-socket.c, but it is not worth it. > > Free string (eric) > Handle VSOCK address nicely (not that migration can use them yet). > Remove useless breaks (dave) > rename socket_address to socket_address_list to avoid confusion > Update to 4.0 (eric) > Put a comment indicating that there is a problem on the qapi > generator (markus). ...all this got included, even if it was perhaps not intended. > Message-Id: <20190227105128.1655-3-quintela@redhat.com> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> At any rate, it certainly looks odd to split *-by: tags by so much text. > +++ b/qapi/sockets.json > @@ -152,3 +152,21 @@ > 'unix': 'UnixSocketAddress', > 'vsock': 'VsockSocketAddress', > 'fd': 'String' } } > + > +## > +# @DummyStruct: > +# > +# Both block-core and migration needs SocketAddressList > +# I am open to comments about how to share it > +# > +# @dummy-list: A dummy list > +# > +# FIXME: This shouldn't be needed, but this struct has two users, and > +# current qapi generator generates it on the 1st place that uses it, > +# so the second user don't see it. Putting it here it is seen in both > +# sides. If Markus' pull request lands first, we don't need this. https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg01185.html
Eric Blake <eblake@redhat.com> writes: > On 3/5/19 12:15 PM, Dr. David Alan Gilbert (git) wrote: >> From: Juan Quintela <quintela@redhat.com> >> >> It will be used to store the uri parameters. We want this only for >> tcp, so we don't set it for other uris. We need it to know what port >> is migration running. >> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> >> -- >> > > This was not the usual '---' divider, and hence: > >> This used to be uri parameter, but it has so many troubles to >> reproduce that it don't just make sense. >> >> This used to be a port parameter. I was asked to move to >> SocketAddress, done. >> I also merged the setting of the migration tcp port in this one >> because now I need to free the address, and this makes it easier. >> This used to be x-socket-address with a single direction, now it is a >> list of addresses. >> Move SocketAddress_to_str here. I used to try to generalize the one >> in chardev/char-socket.c, but it is not worth it. >> >> Free string (eric) >> Handle VSOCK address nicely (not that migration can use them yet). >> Remove useless breaks (dave) >> rename socket_address to socket_address_list to avoid confusion >> Update to 4.0 (eric) >> Put a comment indicating that there is a problem on the qapi >> generator (markus). > > ...all this got included, even if it was perhaps not intended. > >> Message-Id: <20190227105128.1655-3-quintela@redhat.com> >> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > At any rate, it certainly looks odd to split *-by: tags by so much text. > >> +++ b/qapi/sockets.json >> @@ -152,3 +152,21 @@ >> 'unix': 'UnixSocketAddress', >> 'vsock': 'VsockSocketAddress', >> 'fd': 'String' } } >> + >> +## >> +# @DummyStruct: >> +# >> +# Both block-core and migration needs SocketAddressList >> +# I am open to comments about how to share it >> +# >> +# @dummy-list: A dummy list >> +# >> +# FIXME: This shouldn't be needed, but this struct has two users, and >> +# current qapi generator generates it on the 1st place that uses it, >> +# so the second user don't see it. Putting it here it is seen in both >> +# sides. > > If Markus' pull request lands first, we don't need this. > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg01185.html It did: merge commit a3e3b0a7bd5. Please drop this hunk, either in a respin or in a follow-up patch. A respin would give you a chance to clean up the commit message.
* Markus Armbruster (armbru@redhat.com) wrote: > Eric Blake <eblake@redhat.com> writes: > > > On 3/5/19 12:15 PM, Dr. David Alan Gilbert (git) wrote: > >> From: Juan Quintela <quintela@redhat.com> > >> > >> It will be used to store the uri parameters. We want this only for > >> tcp, so we don't set it for other uris. We need it to know what port > >> is migration running. > >> > >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >> Signed-off-by: Juan Quintela <quintela@redhat.com> > >> > >> -- > >> > > > > This was not the usual '---' divider, and hence: > > > >> This used to be uri parameter, but it has so many troubles to > >> reproduce that it don't just make sense. > >> > >> This used to be a port parameter. I was asked to move to > >> SocketAddress, done. > >> I also merged the setting of the migration tcp port in this one > >> because now I need to free the address, and this makes it easier. > >> This used to be x-socket-address with a single direction, now it is a > >> list of addresses. > >> Move SocketAddress_to_str here. I used to try to generalize the one > >> in chardev/char-socket.c, but it is not worth it. > >> > >> Free string (eric) > >> Handle VSOCK address nicely (not that migration can use them yet). > >> Remove useless breaks (dave) > >> rename socket_address to socket_address_list to avoid confusion > >> Update to 4.0 (eric) > >> Put a comment indicating that there is a problem on the qapi > >> generator (markus). > > > > ...all this got included, even if it was perhaps not intended. > > > >> Message-Id: <20190227105128.1655-3-quintela@redhat.com> > >> > >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > > At any rate, it certainly looks odd to split *-by: tags by so much text. > > > >> +++ b/qapi/sockets.json > >> @@ -152,3 +152,21 @@ > >> 'unix': 'UnixSocketAddress', > >> 'vsock': 'VsockSocketAddress', > >> 'fd': 'String' } } > >> + > >> +## > >> +# @DummyStruct: > >> +# > >> +# Both block-core and migration needs SocketAddressList > >> +# I am open to comments about how to share it > >> +# > >> +# @dummy-list: A dummy list > >> +# > >> +# FIXME: This shouldn't be needed, but this struct has two users, and > >> +# current qapi generator generates it on the 1st place that uses it, > >> +# so the second user don't see it. Putting it here it is seen in both > >> +# sides. > > > > If Markus' pull request lands first, we don't need this. > > https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg01185.html > > It did: merge commit a3e3b0a7bd5. Please drop this hunk, either in a > respin or in a follow-up patch. > > A respin would give you a chance to clean up the commit message. OK, respin coming up. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/hmp.c b/hmp.c index 5f13b16e24..c2ad3f8251 100644 --- a/hmp.c +++ b/hmp.c @@ -166,6 +166,27 @@ void hmp_info_mice(Monitor *mon, const QDict *qdict) qapi_free_MouseInfoList(mice_list); } +static char *SocketAddress_to_str(SocketAddress *addr) +{ + switch (addr->type) { + case SOCKET_ADDRESS_TYPE_INET: + return g_strdup_printf("tcp:%s:%s", + addr->u.inet.host, + addr->u.inet.port); + case SOCKET_ADDRESS_TYPE_UNIX: + return g_strdup_printf("unix:%s", + addr->u.q_unix.path); + case SOCKET_ADDRESS_TYPE_FD: + return g_strdup_printf("fd:%s", addr->u.fd.str); + case SOCKET_ADDRESS_TYPE_VSOCK: + return g_strdup_printf("tcp:%s:%s", + addr->u.vsock.cid, + addr->u.vsock.port); + default: + return g_strdup("unknown address type"); + } +} + void hmp_info_migrate(Monitor *mon, const QDict *qdict) { MigrationInfo *info; @@ -306,6 +327,18 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) g_free(str); visit_free(v); } + if (info->has_socket_address) { + SocketAddressList *addr; + + monitor_printf(mon, "socket address: [\n"); + + for (addr = info->socket_address; addr; addr = addr->next) { + char *s = SocketAddress_to_str(addr->value); + monitor_printf(mon, "\t%s\n", s); + g_free(s); + } + monitor_printf(mon, "]\n"); + } qapi_free_MigrationInfo(info); qapi_free_MigrationCapabilityStatusList(caps); } diff --git a/migration/migration.c b/migration/migration.c index 4ff195c901..501319aa14 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -31,6 +31,8 @@ #include "migration/vmstate.h" #include "block/block.h" #include "qapi/error.h" +#include "qapi/clone-visitor.h" +#include "qapi/qapi-visit-sockets.h" #include "qapi/qapi-commands-migration.h" #include "qapi/qapi-events-migration.h" #include "qapi/qmp/qerror.h" @@ -207,6 +209,11 @@ void migration_incoming_state_destroy(void) } qemu_event_reset(&mis->main_thread_load_event); + + if (mis->socket_address_list) { + qapi_free_SocketAddressList(mis->socket_address_list); + mis->socket_address_list = NULL; + } } static void migrate_generate_event(int new_state) @@ -322,6 +329,17 @@ void migration_incoming_enable_colo(void) migration_colo_enabled = true; } +void migrate_add_address(SocketAddress *address) +{ + MigrationIncomingState *mis = migration_incoming_get_current(); + SocketAddressList *addrs; + + addrs = g_new0(SocketAddressList, 1); + addrs->next = mis->socket_address_list; + mis->socket_address_list = addrs; + addrs->value = QAPI_CLONE(SocketAddress, address); +} + void qemu_start_incoming_migration(const char *uri, Error **errp) { const char *p; @@ -1003,6 +1021,12 @@ static void fill_destination_migration_info(MigrationInfo *info) { MigrationIncomingState *mis = migration_incoming_get_current(); + if (mis->socket_address_list) { + info->has_socket_address = true; + info->socket_address = + QAPI_CLONE(SocketAddressList, mis->socket_address_list); + } + switch (mis->state) { case MIGRATION_STATUS_NONE: return; diff --git a/migration/migration.h b/migration/migration.h index 81c261941d..99e99e56bd 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -84,6 +84,9 @@ struct MigrationIncomingState { bool postcopy_recover_triggered; QemuSemaphore postcopy_pause_sem_dst; QemuSemaphore postcopy_pause_sem_fault; + + /* List of listening socket addresses */ + SocketAddressList *socket_address_list; }; MigrationIncomingState *migration_incoming_get_current(void); @@ -305,6 +308,7 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value); void dirty_bitmap_mig_before_vm_start(void); void init_dirty_bitmap_incoming_migration(void); +void migrate_add_address(SocketAddress *address); int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque); diff --git a/migration/socket.c b/migration/socket.c index f4c8174400..239527fb1f 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" @@ -177,6 +178,7 @@ static void socket_start_incoming_migration(SocketAddress *saddr, Error **errp) { QIONetListener *listener = qio_net_listener_new(); + size_t i; qio_net_listener_set_name(listener, "migration-socket-listener"); @@ -189,6 +191,15 @@ static void socket_start_incoming_migration(SocketAddress *saddr, socket_accept_incoming_migration, NULL, NULL, g_main_context_get_thread_default()); + + for (i = 0; i < listener->nsioc; i++) { + SocketAddress *address = + qio_channel_socket_get_local_address(listener->sioc[i], errp); + if (!address) { + return; + } + migrate_add_address(address); + } } void tcp_start_incoming_migration(const char *host_port, Error **errp) diff --git a/qapi/migration.json b/qapi/migration.json index eab87340b2..6bd7fd3f1a 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -6,6 +6,7 @@ ## { 'include': 'common.json' } +{ 'include': 'sockets.json' } ## # @MigrationStats: @@ -199,6 +200,8 @@ # @compression: migration compression statistics, only returned if compression # feature is on and status is 'active' or 'completed' (Since 3.1) # +# @socket-address: Only used for tcp, to know what the real port is (Since 4.0) +# # Since: 0.14.0 ## { 'struct': 'MigrationInfo', @@ -213,7 +216,8 @@ '*error-desc': 'str', '*postcopy-blocktime' : 'uint32', '*postcopy-vcpu-blocktime': ['uint32'], - '*compression': 'CompressionStats'} } + '*compression': 'CompressionStats', + '*socket-address': ['SocketAddress'] } } ## # @query-migrate: diff --git a/qapi/sockets.json b/qapi/sockets.json index fc81d8d5e8..6eeda4a9d6 100644 --- a/qapi/sockets.json +++ b/qapi/sockets.json @@ -152,3 +152,21 @@ 'unix': 'UnixSocketAddress', 'vsock': 'VsockSocketAddress', 'fd': 'String' } } + +## +# @DummyStruct: +# +# Both block-core and migration needs SocketAddressList +# I am open to comments about how to share it +# +# @dummy-list: A dummy list +# +# FIXME: This shouldn't be needed, but this struct has two users, and +# current qapi generator generates it on the 1st place that uses it, +# so the second user don't see it. Putting it here it is seen in both +# sides. +# +# Since: 4.0 +## +{ 'struct': 'DummyStruct', + 'data': { 'dummy-list': ['SocketAddress'] } }