Re: [Qemu-devel] [PATCH COLO-Frame v11 32/39] COLO: Separate the process of saving/loading ram and device state

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2015/11/27 13:10, Li Zhijian wrote:
> On 11/24/2015 05:25 PM, zhanghailiang wrote:
>> We separate the process of saving/loading ram and device state when do checkpoint,
>> we add new helpers for save/load ram/device. With this change, we can directly
>> transfer ram from master to slave without using QEMUSizeBuffer as assistant,
>> which also reduce the size of extra memory been used during checkpoint.
>>
>> Besides, we move the colo_flush_ram_cache to the proper position after the
>> above change.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> ---
>> v11:
>> - Remove load configuration section in qemu_loadvm_state_begin()
>> ---
>>   include/sysemu/sysemu.h |   6 +++
>>   migration/colo.c        |  43 ++++++++++++----
>>   migration/ram.c         |   5 --
>>   migration/savevm.c      | 132 ++++++++++++++++++++++++++++++++++++++++++++++--
>>   4 files changed, 168 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 91eeda3..5deae53 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -133,7 +133,13 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const char *name,
>>                                              uint64_t *start_list,
>>                                              uint64_t *length_list);
>> +int qemu_save_ram_precopy(QEMUFile *f);
>> +int qemu_save_device_state(QEMUFile *f);
>> +
>>   int qemu_loadvm_state(QEMUFile *f);
>> +int qemu_loadvm_state_begin(QEMUFile *f);
>> +int qemu_load_ram_state(QEMUFile *f);
>> +int qemu_load_device_state(QEMUFile *f);
>>   typedef enum DisplayType
>>   {
>> diff --git a/migration/colo.c b/migration/colo.c
>> index 3866e86..f7f349b 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -247,21 +247,32 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>>           goto out;
>>       }
>> +    ret = colo_ctl_put(s->to_dst_file, COLO_COMMAND_VMSTATE_SEND, 0);
>> +    if (ret < 0) {
>> +        goto out;
>> +    }
>>       /* Disable block migration */
>>       s->params.blk = 0;
>>       s->params.shared = 0;
>> -    qemu_savevm_state_header(trans);
>> -    qemu_savevm_state_begin(trans, &s->params);
>> -    qemu_mutex_lock_iothread();
>> -    qemu_savevm_state_complete_precopy(trans, false);
>> -    qemu_mutex_unlock_iothread();
>> -
>> -    qemu_fflush(trans);
>> +    qemu_savevm_state_begin(s->to_dst_file, &s->params);
>> +    ret = qemu_file_get_error(s->to_dst_file);
>> +    if (ret < 0) {
>> +        error_report("save vm state begin error\n");
>> +        goto out;
>> +    }
>> -    ret = colo_ctl_put(s->to_dst_file, COLO_COMMAND_VMSTATE_SEND, 0);
>> +    qemu_mutex_lock_iothread();
>> +    /* Note: device state is saved into buffer */
>> +    ret = qemu_save_device_state(trans);
>>       if (ret < 0) {
>> +        error_report("save device state error\n");
>> +        qemu_mutex_unlock_iothread();
>>           goto out;
>>       }
>> +    qemu_fflush(trans);
>> +    qemu_save_ram_precopy(s->to_dst_file);
>> +    qemu_mutex_unlock_iothread();
>> +
>>       /* we send the total size of the vmstate first */
>>       size = qsb_get_length(buffer);
>>       ret = colo_ctl_put(s->to_dst_file, COLO_COMMAND_VMSTATE_SIZE, size);
>> @@ -548,6 +559,16 @@ void *colo_process_incoming_thread(void *opaque)
>>               goto out;
>>           }
>> +        ret = qemu_loadvm_state_begin(mis->from_src_file);
>> +        if (ret < 0) {
>> +            error_report("load vm state begin error, ret=%d", ret);
>> +            goto out;
>> +        }
>> +        ret = qemu_load_ram_state(mis->from_src_file);
>> +        if (ret < 0) {
>> +            error_report("load ram state error");
>> +            goto out;
>> +        }
>>           /* read the VM state total size first */
>>           ret = colo_ctl_get(mis->from_src_file,
>>                              COLO_COMMAND_VMSTATE_SIZE, &value);
>> @@ -580,8 +601,10 @@ void *colo_process_incoming_thread(void *opaque)
>>           qemu_mutex_lock_iothread();
>>           qemu_system_reset(VMRESET_SILENT);
>>           vmstate_loading = true;
>> -        if (qemu_loadvm_state(fb) < 0) {
>> -            error_report("COLO: loadvm failed");
>> +        colo_flush_ram_cache();
>> +        ret = qemu_load_device_state(fb);
>> +        if (ret < 0) {
>> +            error_report("COLO: load device state failed\n");
>>               vmstate_loading = false;
>>               qemu_mutex_unlock_iothread();
>>               goto out;
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 4f37144..06a738b 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2448,7 +2448,6 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>>        * be atomic
>>        */
>>       bool postcopy_running = postcopy_state_get() >= POSTCOPY_INCOMING_LISTENING;
>> -    bool need_flush = false;
>>       seq_iter++;
>> @@ -2483,7 +2482,6 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>>               /* After going into COLO, we should load the Page into colo_cache */
>>               if (ram_cache_enable) {
>>                   host = colo_cache_from_block_offset(block, addr);
>> -                need_flush = true;
>>               } else {
>>                   host = host_from_ram_block_offset(block, addr);
>>               }
>> @@ -2578,9 +2576,6 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>>       rcu_read_unlock();
>> -    if (!ret  && ram_cache_enable && need_flush) {
>> -        colo_flush_ram_cache();
>> -    }
>>       DPRINTF("Completed load of VM with exit code %d seq iteration "
>>               "%" PRIu64 "\n", ret, seq_iter);
>>       return ret;
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index c7c26d8..949caf0 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -50,6 +50,7 @@
>>   #include "qemu/iov.h"
>>   #include "block/snapshot.h"
>>   #include "block/qapi.h"
>> +#include "migration/colo.h"
>>   #ifndef ETH_P_RARP
>> @@ -923,6 +924,10 @@ void qemu_savevm_state_begin(QEMUFile *f,
>>               break;
>>           }
>>       }
>> +    if (migration_in_colo_state()) {
>> +        qemu_put_byte(f, QEMU_VM_EOF);
>> +        qemu_fflush(f);
>> +    }
>>   }
>>   /*
>> @@ -1192,13 +1197,44 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>>       return ret;
>>   }
>> -static int qemu_save_device_state(QEMUFile *f)
>> +int qemu_save_ram_precopy(QEMUFile *f)
>>   {
>>       SaveStateEntry *se;
>> +    int ret = 0;
>> -    qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
>> -    qemu_put_be32(f, QEMU_VM_FILE_VERSION);
>> +    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> +        if (!se->ops || !se->ops->save_live_complete_precopy) {
>> +            continue;
>> +        }
>> +        if (se->ops && se->ops->is_active) {
>> +            if (!se->ops->is_active(se->opaque)) {
>> +                continue;
>> +            }
>> +        }
>> +        trace_savevm_section_start(se->idstr, se->section_id);
>> +        save_section_header(f, se, QEMU_VM_SECTION_END);
>> +
>> +        ret = se->ops->save_live_complete_precopy(f, se->opaque);
>> +        trace_savevm_section_end(se->idstr, se->section_id, ret);
>> +        save_section_footer(f, se);
>> +        if (ret < 0) {
>> +            qemu_file_set_error(f, ret);
>> +            return ret;
>> +        }
>> +    }
>> +    qemu_put_byte(f, QEMU_VM_EOF);
>> +
>> +    return 0;
>> +}
>> +
>> +int qemu_save_device_state(QEMUFile *f)
>> +{
>> +    SaveStateEntry *se;
>> +
>> +    if (!migration_in_colo_state()) {
>> +        qemu_savevm_state_header(f);
>> +    }
>>       cpu_synchronize_all_states();
>>       QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> @@ -1938,6 +1974,96 @@ int qemu_loadvm_state(QEMUFile *f)
>>       return ret;
>>   }
>> +int qemu_loadvm_state_begin(QEMUFile *f)
>> +{
>> +    uint8_t section_type;
>> +    int ret = -1;
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +
>> +    if (!mis) {
>> +        error_report("qemu_loadvm_state_begin");
>> +        return -EINVAL;
>> +    }
>> +    /* CleanUp */
>> +    loadvm_free_handlers(mis);
>> +
>> +    if (qemu_savevm_state_blocked(NULL)) {
>> +        return -EINVAL;
>> +    }
>> +
>> +    while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
>> +        if (section_type != QEMU_VM_SECTION_START) {
>> +            error_report("QEMU_VM_SECTION_START");
>> +            ret = -EINVAL;
>> +            goto out;
>> +        }
>> +        ret = qemu_loadvm_section_start_full(f, mis);
>> +        if (ret < 0) {
>> +            goto out;
>> +        }
>> +    }
>> +    ret = qemu_file_get_error(f);
>> +    if (ret == 0) {
>> +        return 0;
>> +     }
>> +out:
>> +    return ret;
>> +}
>> +
>> +int qemu_load_ram_state(QEMUFile *f)
>> +{
>> +    uint8_t section_type;
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +    int ret = -1;
>> +
>> +    while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
>> +        if (section_type != QEMU_VM_SECTION_PART &&
>> +            section_type != QEMU_VM_SECTION_END) {
>> +            error_report("load ram state, not get "
>> +                         "QEMU_VM_SECTION_FULL or QEMU_VM_SECTION_END");
>> +            return -EINVAL;
>> +        }
>> +        ret = qemu_loadvm_section_part_end(f, mis);
>> +        if (ret < 0) {
>> +            goto out;
>> +        }
>> +    }
>> +    ret = qemu_file_get_error(f);
>> +    if (ret == 0) {
>> +        return 0;
>> +     }
>> +out:
>> +    return ret;
>> +}
>> +
>> +int qemu_load_device_state(QEMUFile *f)
>> +{
>> +    uint8_t section_type;
>> +    MigrationIncomingState *mis = migration_incoming_get_current();
>> +    int ret = -1;
>> +
>> +    while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
>> +        if (section_type != QEMU_VM_SECTION_FULL) {
>> +            error_report("load device state error: "
>> +                         "Not get QEMU_VM_SECTION_FULL");
>> +            return -EINVAL;
>> +        }
>> +         ret = qemu_loadvm_section_start_full(f, mis);
>> +         if (ret < 0) {
>> +            goto out;
>> +         }
>
> coding sytle：ａbove 4 lines are not aligned
>

>> +    }
>> +
>> +     ret = qemu_file_get_error(f);
>> +
>> +    cpu_synchronize_all_post_init();
>> +     if (ret == 0) {
>> +        return 0;
>> +     }
>
> aligned issue
>

I will fix them all in next version, thanks.


> thanks
> Li
>
>> +out:
>> +    return ret;
>> +}
>> +
>>   void hmp_savevm(Monitor *mon, const QDict *qdict)
>>   {
>>       BlockDriverState *bs, *bs1;
>
>
>
>
> .
>