[Qemu-devel] [PATCH 0/2] savevm: some improvements benefit COLO's later optimization

[Qemu-devel] [PATCH 0/2] savevm: some improvements benefit COLO's later optimization

[zhanghailiang]

Hi,

This series is split from previous COLO frame version which i post
long time ago.

These two patches are prerequisite for the later COLO optimization.
Since it is independent, I'd like to post it as a single series.

Both of them have been reviewed by Dave before, compared with old
version, I fixed the titles/comments for both of them, and I kept the
reviewed-by tag. (Hi Dave, please give it a glance again, since
it has been a long time ;) ).

Please review. Thanks.


zhanghailiang (2):
  savevm: split save/find loadvm_handlers entry into two helper
    functions
  savevm: Add new helpers to process the different stages of
    loadvm/savevm

 include/sysemu/sysemu.h |  4 +++
 migration/savevm.c      | 96 +++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 85 insertions(+), 15 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/2] savevm: split save/find loadvm_handlers entry into two helper functions

[zhanghailiang]

COLO's checkpoint process is based on migration process,
everytime we do checkpoint we will repeat the process of savevm and loadvm.

So we will call qemu_loadvm_section_start_full() repeatedly, It will
add all migration sections information into loadvm_handlers list everytime,
which will lead to memory leak.

To fix it, we split the process of saving and finding section entry into two
helper functions, we will check if section info was exist in loadvm_handlers
list before save it.

This modifications have no side effect for normal migration.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/savevm.c | 55 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index f9c06e9..92b3d6c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1805,6 +1805,37 @@ void loadvm_free_handlers(MigrationIncomingState *mis)
     }
 }
 
+static LoadStateEntry *loadvm_save_section_entry(MigrationIncomingState *mis,
+                                                 SaveStateEntry *se,
+                                                 uint32_t section_id,
+                                                 uint32_t version_id)
+{
+    LoadStateEntry *le;
+
+    /* Add entry */
+    le = g_malloc0(sizeof(*le));
+
+    le->se = se;
+    le->section_id = section_id;
+    le->version_id = version_id;
+    QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
+    return le;
+}
+
+static LoadStateEntry *loadvm_find_section_entry(MigrationIncomingState *mis,
+                                                 uint32_t section_id)
+{
+    LoadStateEntry *le;
+
+    QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
+        if (le->section_id == section_id) {
+            break;
+        }
+    }
+
+    return le;
+}
+
 static int
 qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
 {
@@ -1847,15 +1878,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
         return -EINVAL;
     }
 
-    /* Add entry */
-    le = g_malloc0(sizeof(*le));
-
-    le->se = se;
-    le->section_id = section_id;
-    le->version_id = version_id;
-    QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
-
-    ret = vmstate_load(f, le->se, le->version_id);
+     /* Check if we have saved this section info before, if not, save it */
+    le = loadvm_find_section_entry(mis, section_id);
+    if (!le) {
+        le = loadvm_save_section_entry(mis, se, section_id, version_id);
+    }
+    ret = vmstate_load(f, se, version_id);
     if (ret < 0) {
         error_report("error while loading state for instance 0x%x of"
                      " device '%s'", instance_id, idstr);
@@ -1878,12 +1906,9 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
     section_id = qemu_get_be32(f);
 
     trace_qemu_loadvm_state_section_partend(section_id);
-    QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
-        if (le->section_id == section_id) {
-            break;
-        }
-    }
-    if (le == NULL) {
+
+    le = loadvm_find_section_entry(mis, section_id);
+    if (!le) {
         error_report("Unknown savevm section %d", section_id);
         return -EINVAL;
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] savevm: Add new helpers to process the different stages of loadvm/savevm

[zhanghailiang]

There are several stages during loadvm/savevm process. In different stage,
migration incoming processes different types of sections.
We want to control these stages more accuracy, it will benefit COLO
performance, we don't have to save type of QEMU_VM_SECTION_START
sections everytime while do checkpoint, besides, we want to separate
the process of saving/loading memory and devices state.

So we add three new helper functions: qemu_loadvm_state_begin(),
qemu_load_device_state() and qemu_savevm_live_state() to achieve
different process during migration.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/sysemu/sysemu.h |  4 ++++
 migration/savevm.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ff8ffb5..d9214bf 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -126,7 +126,11 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
                                            uint64_t *start_list,
                                            uint64_t *length_list);
 
+void qemu_savevm_live_state(QEMUFile *f);
+
 int qemu_loadvm_state(QEMUFile *f);
+int qemu_loadvm_state_begin(QEMUFile *f);
+int qemu_load_device_state(QEMUFile *f);
 
 extern int autostart;
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 92b3d6c..04dee4f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1264,6 +1264,13 @@ done:
     return ret;
 }
 
+void qemu_savevm_live_state(QEMUFile *f)
+{
+    /* save QEMU_VM_SECTION_END section */
+    qemu_savevm_state_complete_precopy(f, true);
+    qemu_put_byte(f, QEMU_VM_EOF);
+}
+
 static int qemu_save_device_state(QEMUFile *f)
 {
     SaveStateEntry *se;
@@ -2064,6 +2071,40 @@ int qemu_loadvm_state(QEMUFile *f)
     return ret;
 }
 
+int qemu_loadvm_state_begin(QEMUFile *f)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+    Error *local_err = NULL;
+    int ret;
+
+    if (qemu_savevm_state_blocked(&local_err)) {
+        error_report_err(local_err);
+        return -EINVAL;
+    }
+    /* Load QEMU_VM_SECTION_START section */
+    ret = qemu_loadvm_state_main(f, mis);
+    if (ret < 0) {
+        error_report("Failed to loadvm begin work: %d", ret);
+    }
+    return ret;
+}
+
+int qemu_load_device_state(QEMUFile *f)
+{
+    MigrationIncomingState *mis = migration_incoming_get_current();
+    int ret;
+
+    /* Load QEMU_VM_SECTION_FULL section */
+    ret = qemu_loadvm_state_main(f, mis);
+    if (ret < 0) {
+        error_report("Failed to load device state: %d", ret);
+        return ret;
+    }
+
+    cpu_synchronize_all_post_init();
+    return 0;
+}
+
 void hmp_savevm(Monitor *mon, const QDict *qdict)
 {
     BlockDriverState *bs, *bs1;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/2] savevm: split save/find loadvm_handlers entry into two helper functions

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> COLO's checkpoint process is based on migration process,
> everytime we do checkpoint we will repeat the process of savevm and loadvm.
> 
> So we will call qemu_loadvm_section_start_full() repeatedly, It will
> add all migration sections information into loadvm_handlers list everytime,
> which will lead to memory leak.
> 
> To fix it, we split the process of saving and finding section entry into two
> helper functions, we will check if section info was exist in loadvm_handlers
> list before save it.
> 
> This modifications have no side effect for normal migration.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/savevm.c | 55 +++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 40 insertions(+), 15 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index f9c06e9..92b3d6c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1805,6 +1805,37 @@ void loadvm_free_handlers(MigrationIncomingState *mis)
>      }
>  }
>  
> +static LoadStateEntry *loadvm_save_section_entry(MigrationIncomingState *mis,

Can you change that to loadvm_add_section_entry please;  it's a bit
confusing using 'save' in a function name that's part of the 'load' path.

Dave

> +                                                 SaveStateEntry *se,
> +                                                 uint32_t section_id,
> +                                                 uint32_t version_id)
> +{
> +    LoadStateEntry *le;
> +
> +    /* Add entry */
> +    le = g_malloc0(sizeof(*le));
> +
> +    le->se = se;
> +    le->section_id = section_id;
> +    le->version_id = version_id;
> +    QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
> +    return le;
> +}
> +
> +static LoadStateEntry *loadvm_find_section_entry(MigrationIncomingState *mis,
> +                                                 uint32_t section_id)
> +{
> +    LoadStateEntry *le;
> +
> +    QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
> +        if (le->section_id == section_id) {
> +            break;
> +        }
> +    }
> +
> +    return le;
> +}
> +
>  static int
>  qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
>  {
> @@ -1847,15 +1878,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
>          return -EINVAL;
>      }
>  
> -    /* Add entry */
> -    le = g_malloc0(sizeof(*le));
> -
> -    le->se = se;
> -    le->section_id = section_id;
> -    le->version_id = version_id;
> -    QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
> -
> -    ret = vmstate_load(f, le->se, le->version_id);
> +     /* Check if we have saved this section info before, if not, save it */
> +    le = loadvm_find_section_entry(mis, section_id);
> +    if (!le) {
> +        le = loadvm_save_section_entry(mis, se, section_id, version_id);
> +    }
> +    ret = vmstate_load(f, se, version_id);
>      if (ret < 0) {
>          error_report("error while loading state for instance 0x%x of"
>                       " device '%s'", instance_id, idstr);
> @@ -1878,12 +1906,9 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
>      section_id = qemu_get_be32(f);
>  
>      trace_qemu_loadvm_state_section_partend(section_id);
> -    QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
> -        if (le->section_id == section_id) {
> -            break;
> -        }
> -    }
> -    if (le == NULL) {
> +
> +    le = loadvm_find_section_entry(mis, section_id);
> +    if (!le) {
>          error_report("Unknown savevm section %d", section_id);
>          return -EINVAL;
>      }
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 2/2] savevm: Add new helpers to process the different stages of loadvm/savevm

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> There are several stages during loadvm/savevm process. In different stage,
> migration incoming processes different types of sections.
> We want to control these stages more accuracy, it will benefit COLO
> performance, we don't have to save type of QEMU_VM_SECTION_START
> sections everytime while do checkpoint, besides, we want to separate
> the process of saving/loading memory and devices state.
> 
> So we add three new helper functions: qemu_loadvm_state_begin(),
> qemu_load_device_state() and qemu_savevm_live_state() to achieve
> different process during migration.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

I don't think this one can go in without the patch that follows which
uses these functions; we don't normally add functions
without the patch that uses them.

Dave

> ---
>  include/sysemu/sysemu.h |  4 ++++
>  migration/savevm.c      | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ff8ffb5..d9214bf 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -126,7 +126,11 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
>                                             uint64_t *start_list,
>                                             uint64_t *length_list);
>  
> +void qemu_savevm_live_state(QEMUFile *f);
> +
>  int qemu_loadvm_state(QEMUFile *f);
> +int qemu_loadvm_state_begin(QEMUFile *f);
> +int qemu_load_device_state(QEMUFile *f);
>  
>  extern int autostart;
>  
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 92b3d6c..04dee4f 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1264,6 +1264,13 @@ done:
>      return ret;
>  }
>  
> +void qemu_savevm_live_state(QEMUFile *f)
> +{
> +    /* save QEMU_VM_SECTION_END section */
> +    qemu_savevm_state_complete_precopy(f, true);
> +    qemu_put_byte(f, QEMU_VM_EOF);
> +}
> +
>  static int qemu_save_device_state(QEMUFile *f)
>  {
>      SaveStateEntry *se;
> @@ -2064,6 +2071,40 @@ int qemu_loadvm_state(QEMUFile *f)
>      return ret;
>  }
>  
> +int qemu_loadvm_state_begin(QEMUFile *f)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    Error *local_err = NULL;
> +    int ret;
> +
> +    if (qemu_savevm_state_blocked(&local_err)) {
> +        error_report_err(local_err);
> +        return -EINVAL;
> +    }
> +    /* Load QEMU_VM_SECTION_START section */
> +    ret = qemu_loadvm_state_main(f, mis);
> +    if (ret < 0) {
> +        error_report("Failed to loadvm begin work: %d", ret);
> +    }
> +    return ret;
> +}
> +
> +int qemu_load_device_state(QEMUFile *f)
> +{
> +    MigrationIncomingState *mis = migration_incoming_get_current();
> +    int ret;
> +
> +    /* Load QEMU_VM_SECTION_FULL section */
> +    ret = qemu_loadvm_state_main(f, mis);
> +    if (ret < 0) {
> +        error_report("Failed to load device state: %d", ret);
> +        return ret;
> +    }
> +
> +    cpu_synchronize_all_post_init();
> +    return 0;
> +}
> +
>  void hmp_savevm(Monitor *mon, const QDict *qdict)
>  {
>      BlockDriverState *bs, *bs1;
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 2/2] savevm: Add new helpers to process the different stages of loadvm/savevm

[Juan Quintela]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
>> There are several stages during loadvm/savevm process. In different stage,
>> migration incoming processes different types of sections.
>> We want to control these stages more accuracy, it will benefit COLO
>> performance, we don't have to save type of QEMU_VM_SECTION_START
>> sections everytime while do checkpoint, besides, we want to separate
>> the process of saving/loading memory and devices state.
>> 
>> So we add three new helper functions: qemu_loadvm_state_begin(),
>> qemu_load_device_state() and qemu_savevm_live_state() to achieve
>> different process during migration.
>> 
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> I don't think this one can go in without the patch that follows which
> uses these functions; we don't normally add functions
> without the patch that uses them.

Agreed.  If you want to add functions, you need new code that use them,
or make old code use them.  It is up to you.

Thanks, Juan.

Re: [Qemu-devel] [PATCH 1/2] savevm: split save/find loadvm_handlers entry into two helper functions

[Hailiang Zhang]

On 2017/1/31 18:04, Dr. David Alan Gilbert wrote:
> * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
>> COLO's checkpoint process is based on migration process,
>> everytime we do checkpoint we will repeat the process of savevm and loadvm.
>>
>> So we will call qemu_loadvm_section_start_full() repeatedly, It will
>> add all migration sections information into loadvm_handlers list everytime,
>> which will lead to memory leak.
>>
>> To fix it, we split the process of saving and finding section entry into two
>> helper functions, we will check if section info was exist in loadvm_handlers
>> list before save it.
>>
>> This modifications have no side effect for normal migration.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>   migration/savevm.c | 55 +++++++++++++++++++++++++++++++++++++++---------------
>>   1 file changed, 40 insertions(+), 15 deletions(-)
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index f9c06e9..92b3d6c 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1805,6 +1805,37 @@ void loadvm_free_handlers(MigrationIncomingState *mis)
>>       }
>>   }
>>
>> +static LoadStateEntry *loadvm_save_section_entry(MigrationIncomingState *mis,
>
> Can you change that to loadvm_add_section_entry please;  it's a bit
> confusing using 'save' in a function name that's part of the 'load' path.
>

OK, will fix it in next version, thanks.

> Dave
>
>> +                                                 SaveStateEntry *se,
>> +                                                 uint32_t section_id,
>> +                                                 uint32_t version_id)
>> +{
>> +    LoadStateEntry *le;
>> +
>> +    /* Add entry */
>> +    le = g_malloc0(sizeof(*le));
>> +
>> +    le->se = se;
>> +    le->section_id = section_id;
>> +    le->version_id = version_id;
>> +    QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
>> +    return le;
>> +}
>> +
>> +static LoadStateEntry *loadvm_find_section_entry(MigrationIncomingState *mis,
>> +                                                 uint32_t section_id)
>> +{
>> +    LoadStateEntry *le;
>> +
>> +    QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
>> +        if (le->section_id == section_id) {
>> +            break;
>> +        }
>> +    }
>> +
>> +    return le;
>> +}
>> +
>>   static int
>>   qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
>>   {
>> @@ -1847,15 +1878,12 @@ qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
>>           return -EINVAL;
>>       }
>>
>> -    /* Add entry */
>> -    le = g_malloc0(sizeof(*le));
>> -
>> -    le->se = se;
>> -    le->section_id = section_id;
>> -    le->version_id = version_id;
>> -    QLIST_INSERT_HEAD(&mis->loadvm_handlers, le, entry);
>> -
>> -    ret = vmstate_load(f, le->se, le->version_id);
>> +     /* Check if we have saved this section info before, if not, save it */
>> +    le = loadvm_find_section_entry(mis, section_id);
>> +    if (!le) {
>> +        le = loadvm_save_section_entry(mis, se, section_id, version_id);
>> +    }
>> +    ret = vmstate_load(f, se, version_id);
>>       if (ret < 0) {
>>           error_report("error while loading state for instance 0x%x of"
>>                        " device '%s'", instance_id, idstr);
>> @@ -1878,12 +1906,9 @@ qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
>>       section_id = qemu_get_be32(f);
>>
>>       trace_qemu_loadvm_state_section_partend(section_id);
>> -    QLIST_FOREACH(le, &mis->loadvm_handlers, entry) {
>> -        if (le->section_id == section_id) {
>> -            break;
>> -        }
>> -    }
>> -    if (le == NULL) {
>> +
>> +    le = loadvm_find_section_entry(mis, section_id);
>> +    if (!le) {
>>           error_report("Unknown savevm section %d", section_id);
>>           return -EINVAL;
>>       }
>> --
>> 1.8.3.1
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
> .
>

Re: [Qemu-devel] [PATCH 2/2] savevm: Add new helpers to process the different stages of loadvm/savevm

[Hailiang Zhang]

On 2017/1/31 18:05, Dr. David Alan Gilbert wrote:
> * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
>> There are several stages during loadvm/savevm process. In different stage,
>> migration incoming processes different types of sections.
>> We want to control these stages more accuracy, it will benefit COLO
>> performance, we don't have to save type of QEMU_VM_SECTION_START
>> sections everytime while do checkpoint, besides, we want to separate
>> the process of saving/loading memory and devices state.
>>
>> So we add three new helper functions: qemu_loadvm_state_begin(),
>> qemu_load_device_state() and qemu_savevm_live_state() to achieve
>> different process during migration.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> I don't think this one can go in without the patch that follows which
> uses these functions; we don't normally add functions
> without the patch that uses them.
>

Yes, this series is prerequisite for the later COLO optimization,
that will be a bigger series, that's why i picked these two patches,
but since it is irrational, I'll post these series with the later one
where we use this functions. Thanks. :)

> Dave
>
>> ---
>>   include/sysemu/sysemu.h |  4 ++++
>>   migration/savevm.c      | 41 +++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 45 insertions(+)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index ff8ffb5..d9214bf 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -126,7 +126,11 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
>>                                              uint64_t *start_list,
>>                                              uint64_t *length_list);
>>
>> +void qemu_savevm_live_state(QEMUFile *f);
>> +
>>   int qemu_loadvm_state(QEMUFile *f);
>> +int qemu_loadvm_state_begin(QEMUFile *f);
>> +int qemu_load_device_state(QEMUFile *f);
>>
>>   extern int autostart;
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 92b3d6c..04dee4f 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1264,6 +1264,13 @@ done:
>>       return ret;
>>   }
>>
>> +void qemu_savevm_live_state(QEMUFile *f)
>> +{
>> +    /* save QEMU_VM_SECTION_END section */
>> +    qemu_savevm_state_complete_precopy(f, true);
>> +    qemu_put_byte(f, QEMU_VM_EOF);
>> +}
>> +
>>   static int qemu_save_device_state(QEMUFile *f)
>>   {
>>       SaveStateEntry *se;
>> @@ -2064,6 +2071,40 @@ int qemu_loadvm_state(QEMUFile *f)
>>       return ret;
>>   }
>>
>> +int qemu_loadvm_state_begin(QEMUFile *f)
>> +{
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +    Error *local_err = NULL;
>> +    int ret;
>> +
>> +    if (qemu_savevm_state_blocked(&local_err)) {
>> +        error_report_err(local_err);
>> +        return -EINVAL;
>> +    }
>> +    /* Load QEMU_VM_SECTION_START section */
>> +    ret = qemu_loadvm_state_main(f, mis);
>> +    if (ret < 0) {
>> +        error_report("Failed to loadvm begin work: %d", ret);
>> +    }
>> +    return ret;
>> +}
>> +
>> +int qemu_load_device_state(QEMUFile *f)
>> +{
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +    int ret;
>> +
>> +    /* Load QEMU_VM_SECTION_FULL section */
>> +    ret = qemu_loadvm_state_main(f, mis);
>> +    if (ret < 0) {
>> +        error_report("Failed to load device state: %d", ret);
>> +        return ret;
>> +    }
>> +
>> +    cpu_synchronize_all_post_init();
>> +    return 0;
>> +}
>> +
>>   void hmp_savevm(Monitor *mon, const QDict *qdict)
>>   {
>>       BlockDriverState *bs, *bs1;
>> --
>> 1.8.3.1
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
> .
>

Re: [Qemu-devel] [PATCH 2/2] savevm: Add new helpers to process the different stages of loadvm/savevm

[Hailiang Zhang]

On 2017/1/31 18:19, Juan Quintela wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
>> * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
>>> There are several stages during loadvm/savevm process. In different stage,
>>> migration incoming processes different types of sections.
>>> We want to control these stages more accuracy, it will benefit COLO
>>> performance, we don't have to save type of QEMU_VM_SECTION_START
>>> sections everytime while do checkpoint, besides, we want to separate
>>> the process of saving/loading memory and devices state.
>>>
>>> So we add three new helper functions: qemu_loadvm_state_begin(),
>>> qemu_load_device_state() and qemu_savevm_live_state() to achieve
>>> different process during migration.
>>>
>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>
>> I don't think this one can go in without the patch that follows which
>> uses these functions; we don't normally add functions
>> without the patch that uses them.
>
> Agreed.  If you want to add functions, you need new code that use them,
> or make old code use them.  It is up to you.
>

Got it, thanks, I'll merge this series with the one where we use them.

> Thanks, Juan.
>
> .
>