[v2,11/17] migration/multifd: Add an explicit MultiFDSendData destructor

Commit Message

Maciej S. Szmigiero Aug. 27, 2024, 5:54 p.m. UTC

From: "Maciej S. Szmigiero" <[e-mail]>

This way if there are fields there that needs explicit disposal (like, for
example, some attached buffers) they will be handled appropriately.

Add a related assert to multifd_set_payload_type() in order to make sure
that this function is only used to fill a previously empty MultiFDSendData
with some payload, not the other way around.

Signed-off-by: Maciej S. Szmigiero <[e-mail]>
---
 migration/multifd-nocomp.c |  3 +--
 migration/multifd.c        | 31 ++++++++++++++++++++++++++++---
 migration/multifd.h        |  5 +++++
 3 files changed, 34 insertions(+), 5 deletions(-)

Comments

Fabiano Rosas Aug. 30, 2024, 1:12 p.m. UTC | #1

"Maciej S. Szmigiero" <[e-mail]> writes:

> From: "Maciej S. Szmigiero" <[e-mail]>
>
> This way if there are fields there that needs explicit disposal (like, for
> example, some attached buffers) they will be handled appropriately.
>
> Add a related assert to multifd_set_payload_type() in order to make sure
> that this function is only used to fill a previously empty MultiFDSendData
> with some payload, not the other way around.
>
> Signed-off-by: Maciej S. Szmigiero <[e-mail]>

Reviewed-by: Fabiano Rosas <[e-mail]>

Patch

diff --git a/migration/multifd-nocomp.c b/migration/multifd-nocomp.c
index 53ea9f9c8371..39eb77c9b3b7 100644
--- a/migration/multifd-nocomp.c
+++ b/migration/multifd-nocomp.c
@@ -40,8 +40,7 @@  void multifd_ram_save_setup(void)
 
 void multifd_ram_save_cleanup(void)
 {
-    g_free(multifd_ram_send);
-    multifd_ram_send = NULL;
+    g_clear_pointer(&multifd_ram_send, multifd_send_data_free);
 }
 
 static void multifd_set_file_bitmap(MultiFDSendParams *p)
diff --git a/migration/multifd.c b/migration/multifd.c
index b25789dde0b3..a74e8a5cc891 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -119,6 +119,32 @@  MultiFDSendData *multifd_send_data_alloc(void)
     return g_malloc0(size_minus_payload + max_payload_size);
 }
 
+void multifd_send_data_clear(MultiFDSendData *data)
+{
+    if (multifd_payload_empty(data)) {
+        return;
+    }
+
+    switch (data->type) {
+    default:
+        /* Nothing to do */
+        break;
+    }
+
+    data->type = MULTIFD_PAYLOAD_NONE;
+}
+
+void multifd_send_data_free(MultiFDSendData *data)
+{
+    if (!data) {
+        return;
+    }
+
+    multifd_send_data_clear(data);
+
+    g_free(data);
+}
+
 static bool multifd_use_packets(void)
 {
     return !migrate_mapped_ram();
@@ -506,8 +532,7 @@  static bool multifd_send_cleanup_channel(MultiFDSendParams *p, Error **errp)
     qemu_sem_destroy(&p->sem_sync);
     g_free(p->name);
     p->name = NULL;
-    g_free(p->data);
-    p->data = NULL;
+    g_clear_pointer(&p->data, multifd_send_data_free);
     p->packet_len = 0;
     g_free(p->packet);
     p->packet = NULL;
@@ -671,7 +696,7 @@  static void *multifd_send_thread(void *opaque)
                        p->next_packet_size + p->packet_len);
 
             p->next_packet_size = 0;
-            multifd_set_payload_type(p->data, MULTIFD_PAYLOAD_NONE);
+            multifd_send_data_clear(p->data);
 
             /*
              * Making sure p->data is published before saying "we're
diff --git a/migration/multifd.h b/migration/multifd.h
index a8f3e4838c01..a0853622153e 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -139,6 +139,9 @@  static inline bool multifd_payload_empty(MultiFDSendData *data)
 static inline void multifd_set_payload_type(MultiFDSendData *data,
                                             MultiFDPayloadType type)
 {
+    assert(multifd_payload_empty(data));
+    assert(type != MULTIFD_PAYLOAD_NONE);
+
     data->type = type;
 }
 
@@ -288,6 +291,8 @@  static inline void multifd_send_prepare_header(MultiFDSendParams *p)
 void multifd_channel_connect(MultiFDSendParams *p, QIOChannel *ioc);
 bool multifd_send(MultiFDSendData **send_data);
 MultiFDSendData *multifd_send_data_alloc(void);
+void multifd_send_data_clear(MultiFDSendData *data);
+void multifd_send_data_free(MultiFDSendData *data);
 
 static inline uint32_t multifd_ram_page_size(void)
 {