Skip to contentSkip to conversation

Migration cleanups for postcopy

PATCH In review Dr. David Alan Gilbert (git) 3 participants 16 messages May 21, 2015 – Jun 3, 2015

lore
Share & tools
lore
#0 lore raw

cover Migration cleanups for postcopy

From: "Dr. David Alan Gilbert" <[email protected]>

Hi,
This is a set of patches that started off life in my postcopy
tree but that are more general. Amit suggested I split them out as a separate series so that the main Postcopy series gets easier to review.

The qemu_get_counted_string and qemu_ram_foreach_block have already been posted separately.

Dave

Dr. David Alan Gilbert (6):
Add qemu_get_counted_string to read a string prefixed by a count byte Split header writing out of qemu_savevm_state_begin
qemu_ram_foreach_block: pass up error value, and down the ramblock
name
Create MigrationIncomingState
Move copy out of qemu_peek_buffer
Move loadvm_handlers into MigrationIncomingState

 arch_init.c                   |  9 ++++----
 exec.c                        | 10 +++++++--
 include/exec/cpu-common.h     |  4 ++--
 include/migration/migration.h | 14 ++++++++++++
 include/migration/qemu-file.h |  5 ++++-
 include/migration/vmstate.h   |  2 ++
 include/qemu/typedefs.h       |  2 ++
 include/sysemu/sysemu.h       |  1 +
 migration/migration.c         | 31 ++++++++++++++++++++++++++
 migration/qemu-file.c         | 29 +++++++++++++++++++-----
 migration/rdma.c              |  4 ++--
 migration/vmstate.c           |  5 +++--
 savevm.c                      | 52 ++++++++++++++++++++++++++-----------------
 trace-events                  |  1 +
 14 files changed, 131 insertions(+), 38 deletions(-)
Signature · 1 line
2.4.1
#1 lore raw

PATCH 1/6 Add qemu_get_counted_string to read a string prefixed by a count byte

From: "Dr. David Alan Gilbert" <[email protected]>

and use it in loadvm_state and ram_load.

Where ever it's used, check the return and error if it failed.

Minor: ram_load was using a 257 byte array for its string, the
maximum length is 255 bytes + 0 terminator, so fix to 256

---
 arch_init.c                   |  9 +++++----
 include/migration/qemu-file.h |  3 +++
 migration/qemu-file.c         | 17 +++++++++++++++++
 savevm.c                      | 11 ++++++-----
 4 files changed, 31 insertions(+), 9 deletions(-)
arch_init.c +5 -4
index 23d3feb..7e97eb1 100644
@@ -1593,13 +1593,14 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
             total_ram_bytes = addr;
             while (!ret && total_ram_bytes) {
                 RAMBlock *block;
-                uint8_t len;
                 char id[256];
                 ram_addr_t length;
 
-                len = qemu_get_byte(f);
-                qemu_get_buffer(f, (uint8_t *)id, len);
-                id[len] = 0;
+                if (!qemu_get_counted_string(f, id)) {
+                    error_report("Failed to read ID string of RAM Block");
+                    ret = -EINVAL;
+                    break;
+                }
                 length = qemu_get_be64(f);
 
                 QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
include/migration/qemu-file.h +3 -0
index a01c5b8..318aa1e 100644
@@ -312,4 +312,7 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t *pv)
 {
     qemu_get_be64s(f, (uint64_t *)pv);
 }
+
+size_t qemu_get_counted_string(QEMUFile *f, char buf[256]);
+
 #endif
migration/qemu-file.c +17 -0
index 2750365..0ef543a 100644
@@ -585,3 +585,20 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src)
     }
     return len;
 }
+
+/*
+ * Get a string whose length is determined by a single preceding byte
+ * A preallocated 256 byte buffer must be passed in.
+ * Returns: len on success and a 0 terminated string in the buffer
+ *          else 0
+ *          (Note a 0 length string will return 0 either way)
+ */
+size_t qemu_get_counted_string(QEMUFile *f, char buf[256])
+{
+    size_t len = qemu_get_byte(f);
+    size_t res = qemu_get_buffer(f, (uint8_t *)buf, len);
+
+    buf[res] = 0;
+
+    return res == len ? res : 0;
+}
savevm.c +6 -5
index 3b0e222..c162dfd 100644
@@ -964,8 +964,7 @@ int qemu_loadvm_state(QEMUFile *f)
     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
         uint32_t instance_id, version_id, section_id;
         SaveStateEntry *se;
-        char idstr[257];
-        int len;
+        char idstr[256];
 
         trace_qemu_loadvm_state_section(section_type);
         switch (section_type) {
@@ -973,9 +972,11 @@ int qemu_loadvm_state(QEMUFile *f)
         case QEMU_VM_SECTION_FULL:
             /* Read section start */
             section_id = qemu_get_be32(f);
-            len = qemu_get_byte(f);
-            qemu_get_buffer(f, (uint8_t *)idstr, len);
-            idstr[len] = 0;
+            if (!qemu_get_counted_string(f, idstr)) {
+                error_report("Unable to read ID string for section %u",
+                            section_id);
+                return -EINVAL;
+            }
             instance_id = qemu_get_be32(f);
             version_id = qemu_get_be32(f);
 
Signature · 1 line
2.4.1
Reviewed-by on v1 1/6
#2 lore raw

Re: [Qemu-devel] [PATCH 1/6] Add qemu_get_counted_string to read a string prefixed by a count byte

Quoted 11 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

and use it in loadvm_state and ram_load.

Where ever it's used, check the return and error if it failed.

Minor: ram_load was using a 257 byte array for its string, the
maximum length is 255 bytes + 0 terminator, so fix to 256

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Reviewed-by on v1 1/6
#3 lore raw

Re: [Qemu-devel] [PATCH 1/6] Add qemu_get_counted_string to read a string prefixed by a count byte

In reply to Dr. David Alan Gilbert (git)
Quoted 11 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

and use it in loadvm_state and ram_load.

Where ever it's used, check the return and error if it failed.

Minor: ram_load was using a 257 byte array for its string, the
maximum length is 255 bytes + 0 terminator, so fix to 256

#4 lore raw

PATCH 2/6 Split header writing out of qemu_savevm_state_begin

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Split qemu_savevm_state_begin to:
qemu_savevm_state_header That writes the initial file header.
qemu_savevm_state_begin That sets up devices and does the first
device pass.

Used later in postcopy.

---
 include/sysemu/sysemu.h |  1 +
 migration/migration.c   |  1 +
 savevm.c                | 11 ++++++++---
 trace-events            |  1 +
 4 files changed, 11 insertions(+), 3 deletions(-)
include/sysemu/sysemu.h +1 -0
index 8a52934..7a1ea91 100644
@@ -84,6 +84,7 @@ void qemu_announce_self(void);
 bool qemu_savevm_state_blocked(Error **errp);
 void qemu_savevm_state_begin(QEMUFile *f,
                              const MigrationParams *params);
+void qemu_savevm_state_header(QEMUFile *f);
 int qemu_savevm_state_iterate(QEMUFile *f);
 void qemu_savevm_state_complete(QEMUFile *f);
 void qemu_savevm_state_cancel(void);
migration/migration.c +1 -0
index 732d229..1660d74 100644
@@ -738,6 +738,7 @@ static void *migration_thread(void *opaque)
     int64_t start_time = initial_time;
     bool old_vm_running = false;
 
+    qemu_savevm_state_header(s->file);
     qemu_savevm_state_begin(s->file, &s->params);
 
     s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
savevm.c +8 -3
index c162dfd..d9f0cca 100644
@@ -616,6 +616,13 @@ bool qemu_savevm_state_blocked(Error **errp)
     return false;
 }
 
+void qemu_savevm_state_header(QEMUFile *f)
+{
+    trace_savevm_state_header();
+    qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
+    qemu_put_be32(f, QEMU_VM_FILE_VERSION);
+}
+
 void qemu_savevm_state_begin(QEMUFile *f,
                              const MigrationParams *params)
 {
@@ -630,9 +637,6 @@ void qemu_savevm_state_begin(QEMUFile *f,
         se->ops->set_params(params, se->opaque);
     }
 
-    qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
-    qemu_put_be32(f, QEMU_VM_FILE_VERSION);
-
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
         int len;
 
@@ -842,6 +846,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
     }
 
     qemu_mutex_unlock_iothread();
+    qemu_savevm_state_header(f);
     qemu_savevm_state_begin(f, &params);
     qemu_mutex_lock_iothread();
 
trace-events +1 -0
index 11387c3..30bcd64 100644
@@ -1174,6 +1174,7 @@ qemu_loadvm_state_section_startfull(uint32_t section_id, const char *idstr, uint
 savevm_section_start(const char *id, unsigned int section_id) "%s, section_id %u"
 savevm_section_end(const char *id, unsigned int section_id, int ret) "%s, section_id %u -> %d"
 savevm_state_begin(void) ""
+savevm_state_header(void) ""
 savevm_state_iterate(void) ""
 savevm_state_complete(void) ""
 savevm_state_cancel(void) ""
Signature · 1 line
2.4.1
Reviewed-by on v1 2/6
#5 lore raw

Re: [Qemu-devel] [PATCH 2/6] Split header writing out of qemu_savevm_state_begin

Quoted 11 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Split qemu_savevm_state_begin to:
qemu_savevm_state_header That writes the initial file header.
qemu_savevm_state_begin That sets up devices and does the first
device pass.

Used later in postcopy.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Reviewed-by on v1 2/6
#6 lore raw

Re: [Qemu-devel] [PATCH 2/6] Split header writing out of qemu_savevm_state_begin

In reply to Dr. David Alan Gilbert (git)
Quoted 11 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

Split qemu_savevm_state_begin to:
qemu_savevm_state_header That writes the initial file header.
qemu_savevm_state_begin That sets up devices and does the first
device pass.

Used later in postcopy.

#7 lore raw

PATCH 3/6 qemu_ram_foreach_block: pass up error value, and down the ramblock name

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

check the return value of the function it calls and error if it's non-0 Fixup qemu_rdma_init_one_block that is the only current caller,
and rdma_add_block the only function it calls using it.

Pass the name of the ramblock to the function; helps in debugging.

---
 exec.c                    | 10 ++++++++--
 include/exec/cpu-common.h |  4 ++--
 migration/rdma.c          |  4 ++--
 3 files changed, 12 insertions(+), 6 deletions(-)
exec.c +8 -2
index e19ab22..9b87644 100644
@@ -3355,14 +3355,20 @@ bool cpu_physical_memory_is_io(hwaddr phys_addr)
     return res;
 }
 
-void qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque)
+int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque)
 {
     RAMBlock *block;
+    int ret = 0;
 
     rcu_read_lock();
     QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
-        func(block->host, block->offset, block->used_length, opaque);
+        ret = func(block->idstr, block->host, block->offset,
+                   block->used_length, opaque);
+        if (ret) {
+            break;
+        }
     }
     rcu_read_unlock();
+    return ret;
 }
 #endif
include/exec/cpu-common.h +2 -2
index 43428bd..de8a720 100644
@@ -126,10 +126,10 @@ void cpu_flush_icache_range(hwaddr start, int len);
 extern struct MemoryRegion io_mem_rom;
 extern struct MemoryRegion io_mem_notdirty;
 
-typedef void (RAMBlockIterFunc)(void *host_addr,
+typedef int (RAMBlockIterFunc)(const char *block_name, void *host_addr,
     ram_addr_t offset, ram_addr_t length, void *opaque);
 
-void qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);
+int qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);
 
 #endif
 
migration/rdma.c +2 -2
index 77e3444..c13ec6b 100644
@@ -570,10 +570,10 @@ static int rdma_add_block(RDMAContext *rdma, void *host_addr,
  * in advanced before the migration starts. This tells us where the RAM blocks
  * are so that we can register them individually.
  */
-static void qemu_rdma_init_one_block(void *host_addr,
+static int qemu_rdma_init_one_block(const char *block_name, void *host_addr,
     ram_addr_t block_offset, ram_addr_t length, void *opaque)
 {
-    rdma_add_block(opaque, host_addr, block_offset, length);
+    return rdma_add_block(opaque, host_addr, block_offset, length);
 }
 
 /*
Signature · 1 line
2.4.1
Reviewed-by on v1 3/6
#8 lore raw

Re: [Qemu-devel] [PATCH 3/6] qemu_ram_foreach_block: pass up error value, and down the ramblock name

Quoted 6 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

check the return value of the function it calls and error if it's non-0 Fixup qemu_rdma_init_one_block that is the only current caller,
and rdma_add_block the only function it calls using it.

Quoted 6 lines

Pass the name of the ramblock to the function; helps in debugging.

#9 lore raw

PATCH 4/6 Create MigrationIncomingState

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

There are currently lots of pieces of incoming migration state scattered around, and postcopy is adding more, and it seems better to try and keep it together.

allocate MIS in process_incoming_migration_co

---
 include/migration/migration.h |  9 +++++++++
 include/qemu/typedefs.h       |  1 +
 migration/migration.c         | 28 ++++++++++++++++++++++++++++
 savevm.c                      |  2 ++
 4 files changed, 40 insertions(+)
include/migration/migration.h +9 -0
index a6e025a..00fb6a0 100644
@@ -42,6 +42,15 @@ struct MigrationParams {
 
 typedef struct MigrationState MigrationState;
 
+/* State for the incoming migration */
+struct MigrationIncomingState {
+    QEMUFile *file;
+};
+
+MigrationIncomingState *migration_incoming_get_current(void);
+MigrationIncomingState *migration_incoming_state_new(QEMUFile *f);
+void migration_incoming_state_destroy(void);
+
 struct MigrationState
 {
     int64_t bandwidth_limit;
include/qemu/typedefs.h +1 -0
index cde3314..74dfad3 100644
@@ -38,6 +38,7 @@ typedef struct MemoryListener MemoryListener;
 typedef struct MemoryMappingList MemoryMappingList;
 typedef struct MemoryRegion MemoryRegion;
 typedef struct MemoryRegionSection MemoryRegionSection;
+typedef struct MigrationIncomingState MigrationIncomingState;
 typedef struct MigrationParams MigrationParams;
 typedef struct Monitor Monitor;
 typedef struct MouseTransformInfo MouseTransformInfo;
migration/migration.c +28 -0
index 1660d74..9cf4743 100644
@@ -53,6 +53,7 @@ static bool deferred_incoming;
    migrations at once.  For now we don't need to add
    dynamic creation of migration */
 
+/* For outgoing */
 MigrationState *migrate_get_current(void)
 {
     static MigrationState current_migration = {
@@ -71,6 +72,28 @@ MigrationState *migrate_get_current(void)
     return &current_migration;
 }
 
+/* For incoming */
+static MigrationIncomingState *mis_current;
+
+MigrationIncomingState *migration_incoming_get_current(void)
+{
+    return mis_current;
+}
+
+MigrationIncomingState *migration_incoming_state_new(QEMUFile* f)
+{
+    mis_current = g_malloc0(sizeof(MigrationIncomingState));
+    mis_current->file = f;
+
+    return mis_current;
+}
+
+void migration_incoming_state_destroy(void)
+{
+    g_free(mis_current);
+    mis_current = NULL;
+}
+
 /*
  * Called on -incoming with a defer: uri.
  * The migration can be started later after any parameters have been
@@ -115,9 +138,14 @@ static void process_incoming_migration_co(void *opaque)
     Error *local_err = NULL;
     int ret;
 
+    migration_incoming_state_new(f);
+
     ret = qemu_loadvm_state(f);
+
     qemu_fclose(f);
     free_xbzrle_decoded_buf();
+    migration_incoming_state_destroy();
+
     if (ret < 0) {
         error_report("load of migration failed: %s", strerror(-ret));
         migrate_decompress_threads_join();
savevm.c +2 -0
index d9f0cca..2aa2d19 100644
@@ -1320,9 +1320,11 @@ int load_vmstate(const char *name)
     }
 
     qemu_system_reset(VMRESET_SILENT);
+    migration_incoming_state_new(f);
     ret = qemu_loadvm_state(f);
 
     qemu_fclose(f);
+    migration_incoming_state_destroy();
     if (ret < 0) {
         error_report("Error %d while loading VM state", ret);
         return ret;
Signature · 1 line
2.4.1
Reviewed-by on v1 4/6
#10 lore raw

Re: [Qemu-devel] [PATCH 4/6] Create MigrationIncomingState

Quoted 10 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

There are currently lots of pieces of incoming migration state scattered around, and postcopy is adding more, and it seems better to try and keep it together.

allocate MIS in process_incoming_migration_co

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Reviewed-by on v1 4/6
#11 lore raw

Re: [Qemu-devel] [PATCH 4/6] Create MigrationIncomingState

In reply to Dr. David Alan Gilbert (git)
Quoted 10 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

There are currently lots of pieces of incoming migration state scattered around, and postcopy is adding more, and it seems better to try and keep it together.

allocate MIS in process_incoming_migration_co

#12 lore raw

PATCH 5/6 Move copy out of qemu_peek_buffer

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

qemu_peek_buffer currently copies the data it reads into a buffer, however a future patch wants access to the buffer without the copy, hence rework to remove the copy to the layer above.

---
 include/migration/qemu-file.h |  2 +-
 migration/qemu-file.c         | 12 +++++++-----
 migration/vmstate.c           |  5 +++--
 3 files changed, 11 insertions(+), 8 deletions(-)
include/migration/qemu-file.h +1 -1
index 318aa1e..4f67d79 100644
@@ -157,7 +157,7 @@ static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
 void qemu_put_be16(QEMUFile *f, unsigned int v);
 void qemu_put_be32(QEMUFile *f, unsigned int v);
 void qemu_put_be64(QEMUFile *f, uint64_t v);
-int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset);
+int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset);
 int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
 ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size,
                                   int level);
migration/qemu-file.c +7 -5
index 0ef543a..965a757 100644
@@ -349,14 +349,14 @@ void qemu_file_skip(QEMUFile *f, int size)
 }
 
 /*
- * Read 'size' bytes from file (at 'offset') into buf without moving the
- * pointer.
+ * Read 'size' bytes from file (at 'offset') without moving the
+ * pointer and set 'buf' to point to that data.
  *
  * It will return size bytes unless there was an error, in which case it will
  * return as many as it managed to read (assuming blocking fd's which
  * all current QEMUFile are)
  */
-int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
+int qemu_peek_buffer(QEMUFile *f, uint8_t **buf, int size, size_t offset)
 {
     int pending;
     int index;
@@ -392,7 +392,7 @@ int qemu_peek_buffer(QEMUFile *f, uint8_t *buf, int size, size_t offset)
         size = pending;
     }
 
-    memcpy(buf, f->buf + index, size);
+    *buf = f->buf + index;
     return size;
 }
 
@@ -411,11 +411,13 @@ int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size)
 
     while (pending > 0) {
         int res;
+        uint8_t *src;
 
-        res = qemu_peek_buffer(f, buf, MIN(pending, IO_BUF_SIZE), 0);
+        res = qemu_peek_buffer(f, &src, MIN(pending, IO_BUF_SIZE), 0);
         if (res == 0) {
             return done;
         }
+        memcpy(buf, src, res);
         qemu_file_skip(f, res);
         buf += res;
         pending -= res;
migration/vmstate.c +3 -2
index e5388f0..a64ebcc 100644
@@ -358,7 +358,7 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
     trace_vmstate_subsection_load(vmsd->name);
 
     while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
-        char idstr[256];
+        char idstr[256], *idstr_ret;
         int ret;
         uint8_t version_id, len, size;
         const VMStateDescription *sub_vmsd;
@@ -369,11 +369,12 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
             trace_vmstate_subsection_load_bad(vmsd->name, "(short)");
             return 0;
         }
-        size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
+        size = qemu_peek_buffer(f, (uint8_t **)&idstr_ret, len, 2);
         if (size != len) {
             trace_vmstate_subsection_load_bad(vmsd->name, "(peek fail)");
             return 0;
         }
+        memcpy(idstr, idstr_ret, size);
         idstr[size] = 0;
 
         if (strncmp(vmsd->name, idstr, strlen(vmsd->name)) != 0) {
Signature · 1 line
2.4.1
Reviewed-by on v1 5/6
#13 lore raw

Re: [Qemu-devel] [PATCH 5/6] Move copy out of qemu_peek_buffer

Quoted 8 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

qemu_peek_buffer currently copies the data it reads into a buffer, however a future patch wants access to the buffer without the copy, hence rework to remove the copy to the layer above.

#14 lore raw

PATCH 6/6 Move loadvm_handlers into MigrationIncomingState

In reply to Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

In postcopy we need the loadvm_handlers to be used in a couple of different instances of the loadvm loop/routine, and thus it can't be local any more.

---
 include/migration/migration.h |  5 +++++
 include/migration/vmstate.h   |  2 ++
 include/qemu/typedefs.h       |  1 +
 migration/migration.c         |  2 ++
 savevm.c                      | 28 ++++++++++++++++------------
 5 files changed, 26 insertions(+), 12 deletions(-)
include/migration/migration.h +5 -0
index 00fb6a0..1754be7 100644
@@ -42,9 +42,14 @@ struct MigrationParams {
 
 typedef struct MigrationState MigrationState;
 
+typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head;
+
 /* State for the incoming migration */
 struct MigrationIncomingState {
     QEMUFile *file;
+
+    /* See savevm.c */
+    LoadStateEntry_Head loadvm_handlers;
 };
 
 MigrationIncomingState *migration_incoming_get_current(void);
include/migration/vmstate.h +2 -0
index bc7616a..6d548af 100644
@@ -812,6 +812,8 @@ extern const VMStateInfo vmstate_info_bitmap;
 
 #define SELF_ANNOUNCE_ROUNDS 5
 
+void loadvm_free_handlers(MigrationIncomingState *mis);
+
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                        void *opaque, int version_id);
 void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
include/qemu/typedefs.h +1 -0
index 74dfad3..6fdcbcd 100644
@@ -31,6 +31,7 @@ typedef struct I2CBus I2CBus;
 typedef struct I2SCodec I2SCodec;
 typedef struct ISABus ISABus;
 typedef struct ISADevice ISADevice;
+typedef struct LoadStateEntry LoadStateEntry;
 typedef struct MACAddr MACAddr;
 typedef struct MachineClass MachineClass;
 typedef struct MachineState MachineState;
migration/migration.c +2 -0
index 9cf4743..7443ad9 100644
@@ -84,12 +84,14 @@ MigrationIncomingState *migration_incoming_state_new(QEMUFile* f)
 {
     mis_current = g_malloc0(sizeof(MigrationIncomingState));
     mis_current->file = f;
+    QLIST_INIT(&mis_current->loadvm_handlers);
 
     return mis_current;
 }
 
 void migration_incoming_state_destroy(void)
 {
+    loadvm_free_handlers(mis_current);
     g_free(mis_current);
     mis_current = NULL;
 }
savevm.c +16 -12
index 2aa2d19..3336523 100644
@@ -927,18 +927,26 @@ static SaveStateEntry *find_se(const char *idstr, int instance_id)
     return NULL;
 }
 
-typedef struct LoadStateEntry {
+struct LoadStateEntry {
     QLIST_ENTRY(LoadStateEntry) entry;
     SaveStateEntry *se;
     int section_id;
     int version_id;
-} LoadStateEntry;
+};
 
-int qemu_loadvm_state(QEMUFile *f)
+void loadvm_free_handlers(MigrationIncomingState *mis)
 {
-    QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
-        QLIST_HEAD_INITIALIZER(loadvm_handlers);
     LoadStateEntry *le, *new_le;
+
+    QLIST_FOREACH_SAFE(le, &mis->loadvm_handlers, entry, new_le) {
+        QLIST_REMOVE(le, entry);
+        g_free(le);
+    }
+}
+
+int qemu_loadvm_state(QEMUFile *f)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
     Error *local_err = NULL;
     uint8_t section_type;
     unsigned int v;
@@ -969,6 +977,7 @@ int qemu_loadvm_state(QEMUFile *f)
     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
         uint32_t instance_id, version_id, section_id;
         SaveStateEntry *se;
+        LoadStateEntry *le;
         char idstr[256];
 
         trace_qemu_loadvm_state_section(section_type);
@@ -1010,7 +1019,7 @@ int qemu_loadvm_state(QEMUFile *f)
             le->se = se;
             le->section_id = section_id;
             le->version_id = version_id;
-            QLIST_INSERT_HEAD(&loadvm_handlers, le, entry);
+            QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
 
             ret = vmstate_load(f, le->se, le->version_id);
             if (ret < 0) {
@@ -1024,7 +1033,7 @@ int qemu_loadvm_state(QEMUFile *f)
             section_id = qemu_get_be32(f);
 
             trace_qemu_loadvm_state_section_partend(section_id);
-            QLIST_FOREACH(le, &loadvm_handlers, entry) {
+            QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
                 if (le->section_id == section_id) {
                     break;
                 }
@@ -1072,11 +1081,6 @@ int qemu_loadvm_state(QEMUFile *f)
     ret = 0;
 
 out:
-    QLIST_FOREACH_SAFE(le, &loadvm_handlers, entry, new_le) {
-        QLIST_REMOVE(le, entry);
-        g_free(le);
-    }
-
     if (ret == 0) {
         /* We may not have a VMDESC section, so ignore relative errors */
         ret = file_error_after_eof;
Signature · 1 line
2.4.1
Reviewed-by on v1 6/6
#15 lore raw

Re: [Qemu-devel] [PATCH 6/6] Move loadvm_handlers into MigrationIncomingState

Quoted 7 lines · Dr. David Alan Gilbert (git)

From: "Dr. David Alan Gilbert" <[email protected]>

In postcopy we need the loadvm_handlers to be used in a couple of different instances of the loadvm loop/routine, and thus it can't be local any more.