diff mbox series

[v2,1/2] migration: allow multifd for socket protocol only

Message ID 20210731140552.8818-1-lizhijian@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] migration: allow multifd for socket protocol only | expand

Commit Message

Li Zhijian July 31, 2021, 2:05 p.m. UTC 
multifd with unsupported protocol will cause a segment fault.
(gdb) bt
 #0  0x0000563b4a93faf8 in socket_connect (addr=0x0, errp=0x7f7f02675410) at ../util/qemu-sockets.c:1190
 #1  0x0000563b4a797a03 in qio_channel_socket_connect_sync (ioc=0x563b4d16e8c0, addr=0x0, errp=0x7f7f02675410) at ../io/channel-socket.c:145
 #2  0x0000563b4a797abf in qio_channel_socket_connect_worker (task=0x563b4cd86c30, opaque=0x0) at ../io/channel-socket.c:168
 #3  0x0000563b4a792631 in qio_task_thread_worker (opaque=0x563b4cd86c30) at ../io/task.c:124
 #4  0x0000563b4a91da69 in qemu_thread_start (args=0x563b4c44bb80) at ../util/qemu-thread-posix.c:541
 #5  0x00007f7fe9b5b3f9 in ?? ()
 #6  0x0000000000000000 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 <lizhijian@cn.fujitsu.com>
---
 migration/migration.c |  4 ++++
 migration/multifd.c   | 24 ++++++++++++++++++++++--
 migration/multifd.h   |  2 ++
 3 files changed, 28 insertions(+), 2 deletions(-)

Comments

Zhijian Li (Fujitsu) Aug. 23, 2021, 3:32 a.m. UTC | #1 
kindly ping


On 31/07/2021 22:05, Li Zhijian wrote:
> multifd with unsupported protocol will cause a segment fault.
> (gdb) bt
>   #0  0x0000563b4a93faf8 in socket_connect (addr=0x0, errp=0x7f7f02675410) at ../util/qemu-sockets.c:1190
>   #1  0x0000563b4a797a03 in qio_channel_socket_connect_sync (ioc=0x563b4d16e8c0, addr=0x0, errp=0x7f7f02675410) at ../io/channel-socket.c:145
>   #2  0x0000563b4a797abf in qio_channel_socket_connect_worker (task=0x563b4cd86c30, opaque=0x0) at ../io/channel-socket.c:168
>   #3  0x0000563b4a792631 in qio_task_thread_worker (opaque=0x563b4cd86c30) at ../io/task.c:124
>   #4  0x0000563b4a91da69 in qemu_thread_start (args=0x563b4c44bb80) at ../util/qemu-thread-posix.c:541
>   #5  0x00007f7fe9b5b3f9 in ?? ()
>   #6  0x0000000000000000 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 <lizhijian@cn.fujitsu.com>
> ---
>   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:", &p) ||
>           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:", &p)) {
> @@ -2289,9 +2291,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>           }
>       }
>   
> +    migrate_protocol_allow_multifd(false);
>       if (strstart(uri, "tcp:", &p) ||
>           strstart(uri, "unix:", NULL) ||
>           strstart(uri, "vsock:", NULL)) {
> +        migrate_protocol_allow_multifd(true);
>           socket_start_outgoing_migration(s, p ? p : uri, &local_err);
>   #ifdef CONFIG_RDMA
>       } else if (strstart(uri, "rdma:", &p)) {
> 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);
Juan Quintela Sept. 9, 2021, 7:28 a.m. UTC | #2 
Li Zhijian <lizhijian@cn.fujitsu.com> wrote:
> multifd with unsupported protocol will cause a segment fault.
> (gdb) bt
>  #0  0x0000563b4a93faf8 in socket_connect (addr=0x0, errp=0x7f7f02675410) at ../util/qemu-sockets.c:1190
>  #1 0x0000563b4a797a03 in qio_channel_socket_connect_sync
> (ioc=0x563b4d16e8c0, addr=0x0, errp=0x7f7f02675410) at
> ../io/channel-socket.c:145
>  #2  0x0000563b4a797abf in qio_channel_socket_connect_worker (task=0x563b4cd86c30, opaque=0x0) at ../io/channel-socket.c:168
>  #3  0x0000563b4a792631 in qio_task_thread_worker (opaque=0x563b4cd86c30) at ../io/task.c:124
>  #4  0x0000563b4a91da69 in qemu_thread_start (args=0x563b4c44bb80) at ../util/qemu-thread-posix.c:541
>  #5  0x00007f7fe9b5b3f9 in ?? ()
>  #6  0x0000000000000000 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 <lizhijian@cn.fujitsu.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>
diff mbox series

Patch

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:", &p) ||
         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:", &p)) {
@@ -2289,9 +2291,11 @@  void qmp_migrate(const char *uri, bool has_blk, bool blk,
         }
     }
 
+    migrate_protocol_allow_multifd(false);
     if (strstart(uri, "tcp:", &p) ||
         strstart(uri, "unix:", NULL) ||
         strstart(uri, "vsock:", NULL)) {
+        migrate_protocol_allow_multifd(true);
         socket_start_outgoing_migration(s, p ? p : uri, &local_err);
 #ifdef CONFIG_RDMA
     } else if (strstart(uri, "rdma:", &p)) {
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);