Re: [PATCH v2 1/2] migration: allow multifd for socket protocol only
Li Zhijian wrote: > multifd with unsupported protocol will cause a segment fault. > (gdb) bt > #0 0x563b4a93faf8 in socket_connect (addr=0x0, errp=0x7f7f02675410) at > ../util/qemu-sockets.c:1190 > #1 0x563b4a797a03 in qio_channel_socket_connect_sync > (ioc=0x563b4d16e8c0, addr=0x0, errp=0x7f7f02675410) at > ../io/channel-socket.c:145 > #2 0x563b4a797abf in qio_channel_socket_connect_worker > (task=0x563b4cd86c30, opaque=0x0) at ../io/channel-socket.c:168 > #3 0x563b4a792631 in qio_task_thread_worker (opaque=0x563b4cd86c30) at > ../io/task.c:124 > #4 0x563b4a91da69 in qemu_thread_start (args=0x563b4c44bb80) at > ../util/qemu-thread-posix.c:541 > #5 0x7f7fe9b5b3f9 in ?? () > #6 0x in ?? () > > It's enough to check migrate_multifd_is_allowed() in multifd cleanup() and > multifd setup() though there are so many other places using > migrate_use_multifd(). > > Signed-off-by: Li Zhijian Reviewed-by: Juan Quintela
Re: [PATCH v2 1/2] migration: allow multifd for socket protocol only
kindly ping On 31/07/2021 22:05, Li Zhijian wrote: > multifd with unsupported protocol will cause a segment fault. > (gdb) bt > #0 0x563b4a93faf8 in socket_connect (addr=0x0, errp=0x7f7f02675410) at > ../util/qemu-sockets.c:1190 > #1 0x563b4a797a03 in qio_channel_socket_connect_sync > (ioc=0x563b4d16e8c0, addr=0x0, errp=0x7f7f02675410) at > ../io/channel-socket.c:145 > #2 0x563b4a797abf in qio_channel_socket_connect_worker > (task=0x563b4cd86c30, opaque=0x0) at ../io/channel-socket.c:168 > #3 0x563b4a792631 in qio_task_thread_worker (opaque=0x563b4cd86c30) at > ../io/task.c:124 > #4 0x563b4a91da69 in qemu_thread_start (args=0x563b4c44bb80) at > ../util/qemu-thread-posix.c:541 > #5 0x7f7fe9b5b3f9 in ?? () > #6 0x in ?? () > > It's enough to check migrate_multifd_is_allowed() in multifd cleanup() and > multifd setup() though there are so many other places using > migrate_use_multifd(). > > Signed-off-by: Li Zhijian > --- > migration/migration.c | 4 > migration/multifd.c | 24 ++-- > migration/multifd.h | 2 ++ > 3 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 2d306582ebf..212314541f1 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -456,10 +456,12 @@ static void qemu_start_incoming_migration(const char > *uri, Error **errp) > { > const char *p = NULL; > > +migrate_protocol_allow_multifd(false); /* reset it anyway */ > qapi_event_send_migration(MIGRATION_STATUS_SETUP); > if (strstart(uri, "tcp:", ) || > strstart(uri, "unix:", NULL) || > strstart(uri, "vsock:", NULL)) { > +migrate_protocol_allow_multifd(true); > socket_start_incoming_migration(p ? p : uri, errp); > #ifdef CONFIG_RDMA > } else if (strstart(uri, "rdma:", )) { > @@ -2289,9 +2291,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool > blk, > } > } > > +migrate_protocol_allow_multifd(false); > if (strstart(uri, "tcp:", ) || > strstart(uri, "unix:", NULL) || > strstart(uri, "vsock:", NULL)) { > +migrate_protocol_allow_multifd(true); > socket_start_outgoing_migration(s, p ? p : uri, _err); > #ifdef CONFIG_RDMA > } else if (strstart(uri, "rdma:", )) { > diff --git a/migration/multifd.c b/migration/multifd.c > index ab41590e714..4a4d16d3888 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -531,7 +531,7 @@ void multifd_save_cleanup(void) > { > int i; > > -if (!migrate_use_multifd()) { > +if (!migrate_use_multifd() || !migrate_multifd_is_allowed()) { > return; > } > multifd_send_terminate_threads(NULL); > @@ -864,6 +864,17 @@ cleanup: > multifd_new_send_channel_cleanup(p, sioc, local_err); > } > > +static bool migrate_allow_multifd; > +void migrate_protocol_allow_multifd(bool allow) > +{ > +migrate_allow_multifd = allow; > +} > + > +bool migrate_multifd_is_allowed(void) > +{ > +return migrate_allow_multifd; > +} > + > int multifd_save_setup(Error **errp) > { > int thread_count; > @@ -874,6 +885,11 @@ int multifd_save_setup(Error **errp) > if (!migrate_use_multifd()) { > return 0; > } > +if (!migrate_multifd_is_allowed()) { > +error_setg(errp, "multifd is not supported by current protocol"); > +return -1; > +} > + > s = migrate_get_current(); > thread_count = migrate_multifd_channels(); > multifd_send_state = g_malloc0(sizeof(*multifd_send_state)); > @@ -967,7 +983,7 @@ int multifd_load_cleanup(Error **errp) > { > int i; > > -if (!migrate_use_multifd()) { > +if (!migrate_use_multifd() || !migrate_multifd_is_allowed()) { > return 0; > } > multifd_recv_terminate_threads(NULL); > @@ -1123,6 +1139,10 @@ int multifd_load_setup(Error **errp) > if (!migrate_use_multifd()) { > return 0; > } > +if (!migrate_multifd_is_allowed()) { > +error_setg(errp, "multifd is not supported by current protocol"); > +return -1; > +} > thread_count = migrate_multifd_channels(); > multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state)); > multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count); > diff --git a/migration/multifd.h b/migration/multifd.h > index 8d6751f5ed8..f62a1becd0b 100644 > --- a/migration/multifd.h > +++ b/migration/multifd.h > @@ -13,6 +13,8 @@ > #ifndef QEMU_MIGRATION_MULTIFD_H > #define QEMU_MIGRATION_MULTIFD_H > > +bool migrate_multifd_is_allowed(void); > +void migrate_protocol_allow_multifd(bool allow); > int multifd_save_setup(Error **errp); > void multifd_save_cleanup(void); > int multifd_load_setup(Error **errp);
[PATCH v2 1/2] migration: allow multifd for socket protocol only
multifd with unsupported protocol will cause a segment fault. (gdb) bt #0 0x563b4a93faf8 in socket_connect (addr=0x0, errp=0x7f7f02675410) at ../util/qemu-sockets.c:1190 #1 0x563b4a797a03 in qio_channel_socket_connect_sync (ioc=0x563b4d16e8c0, addr=0x0, errp=0x7f7f02675410) at ../io/channel-socket.c:145 #2 0x563b4a797abf in qio_channel_socket_connect_worker (task=0x563b4cd86c30, opaque=0x0) at ../io/channel-socket.c:168 #3 0x563b4a792631 in qio_task_thread_worker (opaque=0x563b4cd86c30) at ../io/task.c:124 #4 0x563b4a91da69 in qemu_thread_start (args=0x563b4c44bb80) at ../util/qemu-thread-posix.c:541 #5 0x7f7fe9b5b3f9 in ?? () #6 0x in ?? () It's enough to check migrate_multifd_is_allowed() in multifd cleanup() and multifd setup() though there are so many other places using migrate_use_multifd(). Signed-off-by: Li Zhijian --- migration/migration.c | 4 migration/multifd.c | 24 ++-- migration/multifd.h | 2 ++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 2d306582ebf..212314541f1 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -456,10 +456,12 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp) { const char *p = NULL; +migrate_protocol_allow_multifd(false); /* reset it anyway */ qapi_event_send_migration(MIGRATION_STATUS_SETUP); if (strstart(uri, "tcp:", ) || strstart(uri, "unix:", NULL) || strstart(uri, "vsock:", NULL)) { +migrate_protocol_allow_multifd(true); socket_start_incoming_migration(p ? p : uri, errp); #ifdef CONFIG_RDMA } else if (strstart(uri, "rdma:", )) { @@ -2289,9 +2291,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, } } +migrate_protocol_allow_multifd(false); if (strstart(uri, "tcp:", ) || strstart(uri, "unix:", NULL) || strstart(uri, "vsock:", NULL)) { +migrate_protocol_allow_multifd(true); socket_start_outgoing_migration(s, p ? p : uri, _err); #ifdef CONFIG_RDMA } else if (strstart(uri, "rdma:", )) { diff --git a/migration/multifd.c b/migration/multifd.c index ab41590e714..4a4d16d3888 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -531,7 +531,7 @@ void multifd_save_cleanup(void) { int i; -if (!migrate_use_multifd()) { +if (!migrate_use_multifd() || !migrate_multifd_is_allowed()) { return; } multifd_send_terminate_threads(NULL); @@ -864,6 +864,17 @@ cleanup: multifd_new_send_channel_cleanup(p, sioc, local_err); } +static bool migrate_allow_multifd; +void migrate_protocol_allow_multifd(bool allow) +{ +migrate_allow_multifd = allow; +} + +bool migrate_multifd_is_allowed(void) +{ +return migrate_allow_multifd; +} + int multifd_save_setup(Error **errp) { int thread_count; @@ -874,6 +885,11 @@ int multifd_save_setup(Error **errp) if (!migrate_use_multifd()) { return 0; } +if (!migrate_multifd_is_allowed()) { +error_setg(errp, "multifd is not supported by current protocol"); +return -1; +} + s = migrate_get_current(); thread_count = migrate_multifd_channels(); multifd_send_state = g_malloc0(sizeof(*multifd_send_state)); @@ -967,7 +983,7 @@ int multifd_load_cleanup(Error **errp) { int i; -if (!migrate_use_multifd()) { +if (!migrate_use_multifd() || !migrate_multifd_is_allowed()) { return 0; } multifd_recv_terminate_threads(NULL); @@ -1123,6 +1139,10 @@ int multifd_load_setup(Error **errp) if (!migrate_use_multifd()) { return 0; } +if (!migrate_multifd_is_allowed()) { +error_setg(errp, "multifd is not supported by current protocol"); +return -1; +} thread_count = migrate_multifd_channels(); multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state)); multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count); diff --git a/migration/multifd.h b/migration/multifd.h index 8d6751f5ed8..f62a1becd0b 100644 --- a/migration/multifd.h +++ b/migration/multifd.h @@ -13,6 +13,8 @@ #ifndef QEMU_MIGRATION_MULTIFD_H #define QEMU_MIGRATION_MULTIFD_H +bool migrate_multifd_is_allowed(void); +void migrate_protocol_allow_multifd(bool allow); int multifd_save_setup(Error **errp); void multifd_save_cleanup(void); int multifd_load_setup(Error **errp); -- 2.31.1