[Qemu-devel] [PATCH 0/5] Create setup/cleanup methods for migration incoming side

[Qemu-devel] [PATCH 0/5] Create setup/cleanup methods for migration incoming side

[Juan Quintela]

Hi

This series make:
- use of cleanup/save methods generic, not only for save_live methods
- create the equivalent methods for the load side (load_setup/cleanup)
- Make ram use this methods to see how/when they are used.

Stefan, Kevin, this were the methods that you asked for the block.c
migration, right?  Please, comment if they are enough for you.

This are on top of the ramstate_dynamic series that I just posted.

Please, review.

Later, Juan.

Juan Quintela (5):
  migration: Rename save_live_setup() to save_setup()
  migration: Rename cleanup() to save_cleanup()
  migration: Create load_setup()/cleanup() methods
  migration: Convert ram to use new load_setup()/load_cleanup()
  migration: Make compression_threads use save/load_setup/cleanup()

 hw/ppc/spapr.c               |  4 +--
 include/migration/register.h |  6 ++--
 migration/block.c            |  4 +--
 migration/migration.c        |  8 ------
 migration/ram.c              | 66 ++++++++++++++++++++++++++++++--------------
 migration/ram.h              |  6 ----
 migration/savevm.c           | 41 +++++++++++++++++++++++----
 7 files changed, 89 insertions(+), 46 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 1/5] migration: Rename save_live_setup() to save_setup()

[Juan Quintela]

We are going to use it now for more than save live regions.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ppc/spapr.c               | 2 +-
 include/migration/register.h | 2 +-
 migration/block.c            | 2 +-
 migration/ram.c              | 2 +-
 migration/savevm.c           | 6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0aadde6..d51a361 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1873,7 +1873,7 @@ static void htab_cleanup(void *opaque)
 }
 
 static SaveVMHandlers savevm_htab_handlers = {
-    .save_live_setup = htab_save_setup,
+    .save_setup = htab_save_setup,
     .save_live_iterate = htab_save_iterate,
     .save_live_complete_precopy = htab_save_complete,
     .cleanup = htab_cleanup,
diff --git a/include/migration/register.h b/include/migration/register.h
index 844afaf..6cb27f4 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -36,7 +36,7 @@ typedef struct SaveVMHandlers {
     int (*save_live_iterate)(QEMUFile *f, void *opaque);
 
     /* This runs outside the iothread lock!  */
-    int (*save_live_setup)(QEMUFile *f, void *opaque);
+    int (*save_setup)(QEMUFile *f, void *opaque);
     void (*save_live_pending)(QEMUFile *f, void *opaque,
                               uint64_t threshold_size,
                               uint64_t *non_postcopiable_pending,
diff --git a/migration/block.c b/migration/block.c
index 14a8a84..976c42e 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -995,7 +995,7 @@ static bool block_is_active(void *opaque)
 }
 
 static SaveVMHandlers savevm_block_handlers = {
-    .save_live_setup = block_save_setup,
+    .save_setup = block_save_setup,
     .save_live_iterate = block_save_iterate,
     .save_live_complete_precopy = block_save_complete,
     .save_live_pending = block_save_pending,
diff --git a/migration/ram.c b/migration/ram.c
index 1164f14..f2c13e2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2597,7 +2597,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
 }
 
 static SaveVMHandlers savevm_ram_handlers = {
-    .save_live_setup = ram_save_setup,
+    .save_setup = ram_save_setup,
     .save_live_iterate = ram_save_iterate,
     .save_live_complete_postcopy = ram_save_complete,
     .save_live_complete_precopy = ram_save_complete,
diff --git a/migration/savevm.c b/migration/savevm.c
index 8489abf..67c9c39 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -606,7 +606,7 @@ int register_savevm_live(DeviceState *dev,
     se->opaque = opaque;
     se->vmsd = NULL;
     /* if this is a live_savem then set is_ram */
-    if (ops->save_live_setup != NULL) {
+    if (ops->save_setup != NULL) {
         se->is_ram = 1;
     }
 
@@ -1000,7 +1000,7 @@ void qemu_savevm_state_begin(QEMUFile *f)
 
     trace_savevm_state_begin();
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
-        if (!se->ops || !se->ops->save_live_setup) {
+        if (!se->ops || !se->ops->save_setup) {
             continue;
         }
         if (se->ops && se->ops->is_active) {
@@ -1010,7 +1010,7 @@ void qemu_savevm_state_begin(QEMUFile *f)
         }
         save_section_header(f, se, QEMU_VM_SECTION_START);
 
-        ret = se->ops->save_live_setup(f, se->opaque);
+        ret = se->ops->save_setup(f, se->opaque);
         save_section_footer(f, se);
         if (ret < 0) {
             qemu_file_set_error(f, ret);
-- 
2.9.4

[Qemu-devel] [PATCH 2/5] migration: Rename cleanup() to save_cleanup()

[Juan Quintela]

We need a cleanup for loads.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ppc/spapr.c               | 2 +-
 include/migration/register.h | 2 +-
 migration/block.c            | 2 +-
 migration/ram.c              | 2 +-
 migration/savevm.c           | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d51a361..0c11613 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1876,7 +1876,7 @@ static SaveVMHandlers savevm_htab_handlers = {
     .save_setup = htab_save_setup,
     .save_live_iterate = htab_save_iterate,
     .save_live_complete_precopy = htab_save_complete,
-    .cleanup = htab_cleanup,
+    .save_cleanup = htab_cleanup,
     .load_state = htab_load,
 };
 
diff --git a/include/migration/register.h b/include/migration/register.h
index 6cb27f4..8e37185 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -21,7 +21,7 @@ typedef struct SaveVMHandlers {
     /* This runs inside the iothread lock.  */
     SaveStateHandler *save_state;
 
-    void (*cleanup)(void *opaque);
+    void (*save_cleanup)(void *opaque);
     int (*save_live_complete_postcopy)(QEMUFile *f, void *opaque);
     int (*save_live_complete_precopy)(QEMUFile *f, void *opaque);
 
diff --git a/migration/block.c b/migration/block.c
index 976c42e..efa0805 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -1000,7 +1000,7 @@ static SaveVMHandlers savevm_block_handlers = {
     .save_live_complete_precopy = block_save_complete,
     .save_live_pending = block_save_pending,
     .load_state = block_load,
-    .cleanup = block_migration_cleanup,
+    .save_cleanup = block_migration_cleanup,
     .is_active = block_is_active,
 };
 
diff --git a/migration/ram.c b/migration/ram.c
index f2c13e2..d3d2ef1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2603,7 +2603,7 @@ static SaveVMHandlers savevm_ram_handlers = {
     .save_live_complete_precopy = ram_save_complete,
     .save_live_pending = ram_save_pending,
     .load_state = ram_load,
-    .cleanup = ram_migration_cleanup,
+    .save_cleanup = ram_migration_cleanup,
 };
 
 void ram_mig_init(void)
diff --git a/migration/savevm.c b/migration/savevm.c
index 67c9c39..0ec2cb0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1242,8 +1242,8 @@ void qemu_savevm_state_cleanup(void)
 
     trace_savevm_state_cleanup();
     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
-        if (se->ops && se->ops->cleanup) {
-            se->ops->cleanup(se->opaque);
+        if (se->ops && se->ops->save_cleanup) {
+            se->ops->save_cleanup(se->opaque);
         }
     }
 }
-- 
2.9.4

[Qemu-devel] [PATCH 3/5] migration: Create load_setup()/cleanup() methods

[Juan Quintela]

We need to do things at load time.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/register.h |  2 ++
 migration/savevm.c           | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/migration/register.h b/include/migration/register.h
index 8e37185..0862bb8 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -42,6 +42,8 @@ typedef struct SaveVMHandlers {
                               uint64_t *non_postcopiable_pending,
                               uint64_t *postcopiable_pending);
     LoadStateHandler *load_state;
+    int (*load_setup)(QEMUFile *f, void *opaque);
+    int (*load_cleanup)(void *opaque);
 } SaveVMHandlers;
 
 int register_savevm(DeviceState *dev,
diff --git a/migration/savevm.c b/migration/savevm.c
index 0ec2cb0..1e5dfad 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1928,6 +1928,31 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
     return 0;
 }
 
+static int qemu_savevm_load_state_begin(QEMUFile *f)
+{
+    SaveStateEntry *se;
+    int ret;
+
+    trace_savevm_state_begin();
+    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
+        if (!se->ops || !se->ops->load_setup) {
+            continue;
+        }
+        if (se->ops && se->ops->is_active) {
+            if (!se->ops->is_active(se->opaque)) {
+                continue;
+            }
+        }
+
+        ret = se->ops->load_setup(f, se->opaque);
+        if (ret < 0) {
+            qemu_file_set_error(f, ret);
+            return ret;
+        }
+    }
+    return 0;
+}
+
 static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
 {
     uint8_t section_type;
@@ -2000,6 +2025,12 @@ int qemu_loadvm_state(QEMUFile *f)
         return -ENOTSUP;
     }
 
+
+    if (qemu_savevm_load_state_begin(f) != 0) {
+        error_report("Load state of one device failed");
+        return -EINVAL;
+    }
+
     if (!savevm_state.skip_configuration || enforce_config_section()) {
         if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
             error_report("Configuration section missing");
-- 
2.9.4

[Qemu-devel] [PATCH 4/5] migration: Convert ram to use new load_setup()/load_cleanup()

[Juan Quintela]

Once there, I rename ram_migration_cleanup() to ram_save_cleanup().
Notice that this is the first pass, and I only passed XBZRLE to the
new scheme.  Moved decoded_buf to inside XBZRLE struct.
As a bonus, I don't have to export xbzrle functions from ram.c.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c |  3 ---
 migration/ram.c       | 52 +++++++++++++++++++++++++++++++++++----------------
 migration/ram.h       |  1 -
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 331cab7..b75aebc 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -350,9 +350,6 @@ static void process_incoming_migration_co(void *opaque)
         migrate_decompress_threads_join();
         exit(EXIT_FAILURE);
     }
-
-    free_xbzrle_decoded_buf();
-
     mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
     qemu_bh_schedule(mis->bh);
 }
diff --git a/migration/ram.c b/migration/ram.c
index d3d2ef1..3349116 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -85,11 +85,10 @@ static struct {
     QemuMutex lock;
     /* it will store a page full of zeros */
     uint8_t *zero_target_page;
+    /* buffer used for XBZRLE decoding */
+    uint8_t *decoded_buf;
 } XBZRLE;
 
-/* buffer used for XBZRLE decoding */
-static uint8_t *xbzrle_decoded_buf;
-
 static void XBZRLE_cache_lock(void)
 {
     if (migrate_use_xbzrle())
@@ -1343,13 +1342,18 @@ uint64_t ram_bytes_total(void)
     return total;
 }
 
-void free_xbzrle_decoded_buf(void)
+static void xbzrle_load_setup(void)
 {
-    g_free(xbzrle_decoded_buf);
-    xbzrle_decoded_buf = NULL;
+    XBZRLE.decoded_buf = g_malloc(TARGET_PAGE_SIZE);
 }
 
-static void ram_migration_cleanup(void *opaque)
+static void xbzrle_load_cleanup(void)
+{
+    g_free(XBZRLE.decoded_buf);
+    XBZRLE.decoded_buf = NULL;
+}
+
+static void ram_save_cleanup(void *opaque)
 {
     RAMState **rsp = opaque;
     RAMBlock *block;
@@ -2068,12 +2072,6 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
 {
     unsigned int xh_len;
     int xh_flags;
-    uint8_t *loaded_data;
-
-    if (!xbzrle_decoded_buf) {
-        xbzrle_decoded_buf = g_malloc(TARGET_PAGE_SIZE);
-    }
-    loaded_data = xbzrle_decoded_buf;
 
     /* extract RLE header */
     xh_flags = qemu_get_byte(f);
@@ -2089,10 +2087,10 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
         return -1;
     }
     /* load data and decode */
-    qemu_get_buffer_in_place(f, &loaded_data, xh_len);
+    qemu_get_buffer_in_place(f, &XBZRLE.decoded_buf, xh_len);
 
     /* decode RLE */
-    if (xbzrle_decode_buffer(loaded_data, xh_len, host,
+    if (xbzrle_decode_buffer(XBZRLE.decoded_buf, xh_len, host,
                              TARGET_PAGE_SIZE) == -1) {
         error_report("Failed to load XBZRLE page - decode error!");
         return -1;
@@ -2296,6 +2294,26 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
 }
 
 /**
+ * ram_load_setup: Setup RAM for migration incoming side
+ *
+ * Returns zero to indicate success and negative for error
+ *
+ * @f: QEMUFile where to receive the data
+ * @opaque: RAMState pointer
+ */
+static int ram_load_setup(QEMUFile *f, void *opaque)
+{
+    xbzrle_load_setup();
+    return 0;
+}
+
+static int ram_load_cleanup(void *opaque)
+{
+    xbzrle_load_cleanup();
+    return 0;
+}
+
+/**
  * ram_postcopy_incoming_init: allocate postcopy data structures
  *
  * Returns 0 for success and negative if there was one error
@@ -2603,7 +2621,9 @@ static SaveVMHandlers savevm_ram_handlers = {
     .save_live_complete_precopy = ram_save_complete,
     .save_live_pending = ram_save_pending,
     .load_state = ram_load,
-    .save_cleanup = ram_migration_cleanup,
+    .save_cleanup = ram_save_cleanup,
+    .load_setup = ram_load_setup,
+    .load_cleanup = ram_load_cleanup,
 };
 
 void ram_mig_init(void)
diff --git a/migration/ram.h b/migration/ram.h
index 9eadc8c..e49dd3f 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -46,7 +46,6 @@ void migrate_decompress_threads_join(void);
 uint64_t ram_pagesize_summary(void);
 int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len);
 void acct_update_position(QEMUFile *f, size_t size, bool zero);
-void free_xbzrle_decoded_buf(void);
 void ram_debug_dump_bitmap(unsigned long *todump, bool expected,
                            unsigned long pages);
 void ram_postcopy_migrated_memory_release(MigrationState *ms);
-- 
2.9.4

[Qemu-devel] [PATCH 5/5] migration: Make compression_threads use save/load_setup/cleanup()

[Juan Quintela]

Once there, be consistent and use
compress_thread_{save,load}_{setup,cleanup}.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c |  5 -----
 migration/ram.c       | 12 ++++++++----
 migration/ram.h       |  5 -----
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index b75aebc..4040b33 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -284,7 +284,6 @@ static void process_incoming_migration_bh(void *opaque)
     } else {
         runstate_set(global_state_get_runstate());
     }
-    migrate_decompress_threads_join();
     /*
      * This must happen after any state changes since as soon as an external
      * observer sees this event they might start to prod at the VM assuming
@@ -347,7 +346,6 @@ static void process_incoming_migration_co(void *opaque)
         migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
                           MIGRATION_STATUS_FAILED);
         error_report("load of migration failed: %s", strerror(-ret));
-        migrate_decompress_threads_join();
         exit(EXIT_FAILURE);
     }
     mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
@@ -358,7 +356,6 @@ void migration_fd_process_incoming(QEMUFile *f)
 {
     Coroutine *co = qemu_coroutine_create(process_incoming_migration_co, f);
 
-    migrate_decompress_threads_create();
     qemu_file_set_blocking(f, false);
     qemu_coroutine_enter(co);
 }
@@ -823,7 +820,6 @@ static void migrate_fd_cleanup(void *opaque)
         }
         qemu_mutex_lock_iothread();
 
-        migrate_compress_threads_join();
         qemu_fclose(s->to_dst_file);
         s->to_dst_file = NULL;
     }
@@ -1974,7 +1970,6 @@ void migrate_fd_connect(MigrationState *s)
         }
     }
 
-    migrate_compress_threads_create();
     qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
                        QEMU_THREAD_JOINABLE);
     s->migration_thread_running = true;
diff --git a/migration/ram.c b/migration/ram.c
index 3349116..e9d97d9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -299,7 +299,7 @@ static inline void terminate_compression_threads(void)
     }
 }
 
-void migrate_compress_threads_join(void)
+static void compress_threads_save_cleanup(void)
 {
     int i, thread_count;
 
@@ -322,7 +322,7 @@ void migrate_compress_threads_join(void)
     comp_param = NULL;
 }
 
-void migrate_compress_threads_create(void)
+static void compress_threads_save_setup(void)
 {
     int i, thread_count;
 
@@ -1383,6 +1383,7 @@ static void ram_save_cleanup(void *opaque)
     }
     XBZRLE_cache_unlock();
     migration_page_queue_free(*rsp);
+    compress_threads_save_cleanup();
     g_free(*rsp);
     *rsp = NULL;
 }
@@ -1915,6 +1916,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     }
 
     rcu_read_unlock();
+    compress_threads_save_setup();
 
     ram_control_before_iterate(f, RAM_CONTROL_SETUP);
     ram_control_after_iterate(f, RAM_CONTROL_SETUP);
@@ -2220,7 +2222,7 @@ static void wait_for_decompress_done(void)
     qemu_mutex_unlock(&decomp_done_lock);
 }
 
-void migrate_decompress_threads_create(void)
+static void compress_threads_load_setup(void)
 {
     int i, thread_count;
 
@@ -2241,7 +2243,7 @@ void migrate_decompress_threads_create(void)
     }
 }
 
-void migrate_decompress_threads_join(void)
+static void compress_threads_load_cleanup(void)
 {
     int i, thread_count;
 
@@ -2304,12 +2306,14 @@ static void decompress_data_with_multi_threads(QEMUFile *f,
 static int ram_load_setup(QEMUFile *f, void *opaque)
 {
     xbzrle_load_setup();
+    compress_threads_load_setup();
     return 0;
 }
 
 static int ram_load_cleanup(void *opaque)
 {
     xbzrle_load_cleanup();
+    compress_threads_load_cleanup();
     return 0;
 }
 
diff --git a/migration/ram.h b/migration/ram.h
index e49dd3f..bfb7b04 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -38,11 +38,6 @@ extern XBZRLECacheStats xbzrle_counters;
 int64_t xbzrle_cache_resize(int64_t new_size);
 uint64_t ram_bytes_total(void);
 
-void migrate_compress_threads_create(void);
-void migrate_compress_threads_join(void);
-void migrate_decompress_threads_create(void);
-void migrate_decompress_threads_join(void);
-
 uint64_t ram_pagesize_summary(void);
 int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len);
 void acct_update_position(QEMUFile *f, size_t size, bool zero);
-- 
2.9.4

Re: [Qemu-devel] [PATCH 0/5] Create setup/cleanup methods for migration incoming side

[Juan Quintela]

Juan Quintela <quintela@redhat.com> wrote:
> Hi

Hi Kevin and Stefan, I intended to cc you on this series.

Is this what you were asking for block layer migration?

Do you need anything else?

Thanks, Juan.


> This series make:
> - use of cleanup/save methods generic, not only for save_live methods
> - create the equivalent methods for the load side (load_setup/cleanup)
> - Make ram use this methods to see how/when they are used.
>
> Stefan, Kevin, this were the methods that you asked for the block.c
> migration, right?  Please, comment if they are enough for you.
>
> This are on top of the ramstate_dynamic series that I just posted.
>
> Please, review.
>
> Later, Juan.
>
> Juan Quintela (5):
>   migration: Rename save_live_setup() to save_setup()
>   migration: Rename cleanup() to save_cleanup()
>   migration: Create load_setup()/cleanup() methods
>   migration: Convert ram to use new load_setup()/load_cleanup()
>   migration: Make compression_threads use save/load_setup/cleanup()
>
>  hw/ppc/spapr.c               |  4 +--
>  include/migration/register.h |  6 ++--
>  migration/block.c            |  4 +--
>  migration/migration.c        |  8 ------
>  migration/ram.c              | 66 ++++++++++++++++++++++++++++++--------------
>  migration/ram.h              |  6 ----
>  migration/savevm.c           | 41 +++++++++++++++++++++++----
>  7 files changed, 89 insertions(+), 46 deletions(-)

Re: [Qemu-devel] [PATCH 3/5] migration: Create load_setup()/cleanup() methods

[Kevin Wolf]

Am 02.06.2017 um 00:15 hat Juan Quintela geschrieben:
> We need to do things at load time.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/migration/register.h |  2 ++
>  migration/savevm.c           | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/include/migration/register.h b/include/migration/register.h
> index 8e37185..0862bb8 100644
> --- a/include/migration/register.h
> +++ b/include/migration/register.h
> @@ -42,6 +42,8 @@ typedef struct SaveVMHandlers {
>                                uint64_t *non_postcopiable_pending,
>                                uint64_t *postcopiable_pending);
>      LoadStateHandler *load_state;
> +    int (*load_setup)(QEMUFile *f, void *opaque);
> +    int (*load_cleanup)(void *opaque);
>  } SaveVMHandlers;

I see that you add code to call load_setup(), but don't you want to call
load_cleanup() somewhere, too?

Kevin

Re: [Qemu-devel] [PATCH 0/5] Create setup/cleanup methods for migration incoming side

[Kevin Wolf]

Am 07.06.2017 um 19:53 hat Juan Quintela geschrieben:
> Juan Quintela <quintela@redhat.com> wrote:
> > Hi
> 
> Hi Kevin and Stefan, I intended to cc you on this series.
> 
> Is this what you were asking for block layer migration?
> 
> Do you need anything else?

I didn't manage to apply this to my local git tree, so my review is
based on scanning the emails.

The important part for us is probably where you call the .cleanup_load
callback. As I commented on the patch, I don't see any place in this
series that calls it. We need it before bdrv_invalidate_cache_all() is
called, I think.

Kevin

Re: [Qemu-devel] [PATCH 3/5] migration: Create load_setup()/cleanup() methods

[Juan Quintela]

Kevin Wolf <kwolf@redhat.com> wrote:
> Am 02.06.2017 um 00:15 hat Juan Quintela geschrieben:
>> We need to do things at load time.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  include/migration/register.h |  2 ++
>>  migration/savevm.c           | 31 +++++++++++++++++++++++++++++++
>>  2 files changed, 33 insertions(+)
>> 
>> diff --git a/include/migration/register.h b/include/migration/register.h
>> index 8e37185..0862bb8 100644
>> --- a/include/migration/register.h
>> +++ b/include/migration/register.h
>> @@ -42,6 +42,8 @@ typedef struct SaveVMHandlers {
>>                                uint64_t *non_postcopiable_pending,
>>                                uint64_t *postcopiable_pending);
>>      LoadStateHandler *load_state;
>> +    int (*load_setup)(QEMUFile *f, void *opaque);
>> +    int (*load_cleanup)(void *opaque);
>>  } SaveVMHandlers;
>
> I see that you add code to call load_setup(), but don't you want to call
> load_cleanup() somewhere, too?

Oops, you are right.

I just sent another version of this series.  They are built on top of my
pull request.  And I added the call to load_cleanup.

Thanks for the review, Juan.