From: Peter Xu <peterx@redhat.com>
The newly introduced device state buffer can be used for either storing
VFIO's read() raw data, but already also possible to store generic device
states. After noticing that device states may not easily provide a max
buffer size (also the fact that RAM MultiFDPages_t after all also want to
have flexibility on managing offset[] array), it may not be a good idea to
stick with union on MultiFDSendData.. as it won't play well with such
flexibility.
Switch MultiFDSendData to a struct.
It won't consume a lot more space in reality, after all the real buffers
were already dynamically allocated, so it's so far only about the two
structs (pages, device_state) that will be duplicated, but they're small.
With this, we can remove the pretty hard to understand alloc size logic.
Because now we can allocate offset[] together with the SendData, and
properly free it when the SendData is freed.
[MSS: Make sure to clear possible device state payload before freeing
MultiFDSendData, remove placeholders for other patches not included]
Signed-off-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Acked-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/qemu-devel/7b02baba8e6ddb23ef7c349d312b9b631db09d7e.1741124640.git.maciej.szmigiero@oracle.com
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
migration/multifd.h | 15 +++++++++------
migration/multifd-device-state.c | 5 -----
migration/multifd-nocomp.c | 13 ++++++-------
migration/multifd.c | 25 +++++++------------------
4 files changed, 22 insertions(+), 36 deletions(-)
diff --git a/migration/multifd.h b/migration/multifd.h
index aa679d8bbe8358e5cbab0add9c4bf007fa55961b..2d337e7b3b52fd005a6c342fb552975e8f13bdf1 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -115,9 +115,13 @@ typedef struct {
uint32_t num;
/* number of normal pages */
uint32_t normal_num;
+ /*
+ * Pointer to the ramblock. NOTE: it's caller's responsibility to make
+ * sure the pointer is always valid!
+ */
RAMBlock *block;
- /* offset of each page */
- ram_addr_t offset[];
+ /* offset array of each page, managed by multifd */
+ ram_addr_t *offset;
} MultiFDPages_t;
struct MultiFDRecvData {
@@ -140,7 +144,7 @@ typedef enum {
MULTIFD_PAYLOAD_DEVICE_STATE,
} MultiFDPayloadType;
-typedef union MultiFDPayload {
+typedef struct MultiFDPayload {
MultiFDPages_t ram;
MultiFDDeviceState_t device_state;
} MultiFDPayload;
@@ -394,12 +398,11 @@ void multifd_ram_save_cleanup(void);
int multifd_ram_flush_and_sync(QEMUFile *f);
bool multifd_ram_sync_per_round(void);
bool multifd_ram_sync_per_section(void);
-size_t multifd_ram_payload_size(void);
+void multifd_ram_payload_alloc(MultiFDPages_t *pages);
+void multifd_ram_payload_free(MultiFDPages_t *pages);
void multifd_ram_fill_packet(MultiFDSendParams *p);
int multifd_ram_unfill_packet(MultiFDRecvParams *p, Error **errp);
-size_t multifd_device_state_payload_size(void);
-
void multifd_send_data_clear_device_state(MultiFDDeviceState_t *device_state);
void multifd_device_state_send_setup(void);
diff --git a/migration/multifd-device-state.c b/migration/multifd-device-state.c
index e383e75b1a024c1885b41bcc3aee034fd6f71f9d..64d8ca180167b09abf13c9302326404be236717e 100644
--- a/migration/multifd-device-state.c
+++ b/migration/multifd-device-state.c
@@ -20,11 +20,6 @@ static struct {
MultiFDSendData *send_data;
} *multifd_send_device_state;
-size_t multifd_device_state_payload_size(void)
-{
- return sizeof(MultiFDDeviceState_t);
-}
-
void multifd_device_state_send_setup(void)
{
assert(!multifd_send_device_state);
diff --git a/migration/multifd-nocomp.c b/migration/multifd-nocomp.c
index c00804652383ed0496769c134ea2c10e0ae91f18..ffe75256c9fbe459dee4eb4aa8f80ae013ff2d87 100644
--- a/migration/multifd-nocomp.c
+++ b/migration/multifd-nocomp.c
@@ -25,15 +25,14 @@
static MultiFDSendData *multifd_ram_send;
-size_t multifd_ram_payload_size(void)
+void multifd_ram_payload_alloc(MultiFDPages_t *pages)
{
- uint32_t n = multifd_ram_page_count();
+ pages->offset = g_new0(ram_addr_t, multifd_ram_page_count());
+}
- /*
- * We keep an array of page offsets at the end of MultiFDPages_t,
- * add space for it in the allocation.
- */
- return sizeof(MultiFDPages_t) + n * sizeof(ram_addr_t);
+void multifd_ram_payload_free(MultiFDPages_t *pages)
+{
+ g_clear_pointer(&pages->offset, g_free);
}
void multifd_ram_save_setup(void)
diff --git a/migration/multifd.c b/migration/multifd.c
index 3625c9a37c0e046b7a851dfe3d94fefa6887534d..dfb5189f0ea3da97370b81e2dcf5a7f86252ae29 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -105,26 +105,12 @@ struct {
MultiFDSendData *multifd_send_data_alloc(void)
{
- size_t max_payload_size, size_minus_payload;
+ MultiFDSendData *new = g_new0(MultiFDSendData, 1);
- /*
- * MultiFDPages_t has a flexible array at the end, account for it
- * when allocating MultiFDSendData. Use max() in case other types
- * added to the union in the future are larger than
- * (MultiFDPages_t + flex array).
- */
- max_payload_size = MAX(multifd_ram_payload_size(),
- multifd_device_state_payload_size());
- max_payload_size = MAX(max_payload_size, sizeof(MultiFDPayload));
-
- /*
- * Account for any holes the compiler might insert. We can't pack
- * the structure because that misaligns the members and triggers
- * Waddress-of-packed-member.
- */
- size_minus_payload = sizeof(MultiFDSendData) - sizeof(MultiFDPayload);
+ multifd_ram_payload_alloc(&new->u.ram);
+ /* Device state allocates its payload on-demand */
- return g_malloc0(size_minus_payload + max_payload_size);
+ return new;
}
void multifd_send_data_clear(MultiFDSendData *data)
@@ -151,8 +137,11 @@ void multifd_send_data_free(MultiFDSendData *data)
return;
}
+ /* This also free's device state payload */
multifd_send_data_clear(data);
+ multifd_ram_payload_free(&data->u.ram);
+
g_free(data);
}
--
2.48.1