| Message ID | 1456108832-24212-30-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* zhanghailiang (zhang.zhanghailiang@huawei.com) 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 > QEMUSizeBufferas 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> > --- > v14: > - split two new patches from this patch > - Some minor fixes from Dave > v13: > - Re-use some existed helper functions to realize saving/loading > ram and device. > v11: > - Remove load configuration section in qemu_loadvm_state_begin() > --- > migration/colo.c | 48 ++++++++++++++++++++++++++++++++++++++---------- > migration/ram.c | 5 ----- > migration/savevm.c | 5 +++++ > 3 files changed, 43 insertions(+), 15 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c > index 16bada6..300fa54 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -288,21 +288,37 @@ static int colo_do_checkpoint_transaction(MigrationState *s, > goto out; > } > > + colo_put_cmd(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, &local_err); > + if (local_err) { > + 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_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"); > + goto out; > + } > + > qemu_mutex_lock_iothread(); > - qemu_savevm_state_complete_precopy(trans, false); > + /* > + * Only save VM's live state, which not including device state. > + * TODO: We may need a timeout mechanism to prevent COLO process > + * to be blocked here. > + */ > + qemu_savevm_live_state(s->to_dst_file); > + /* Note: device state is saved into buffer */ > + ret = qemu_save_device_state(trans); > qemu_mutex_unlock_iothread(); Yes, I still worry a little about what can hang under that lock, but I think it's the best we've got at the moment; we probably need to understand what the rules are about what actually needs the lock! Other than that, Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > - > - qemu_fflush(trans); > - > - colo_put_cmd(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, &local_err); > - if (local_err) { > + if (ret < 0) { > + error_report("Save device state error"); > goto out; > } > + qemu_fflush(trans); > + > /* we send the total size of the vmstate first */ > size = qsb_get_length(buffer); > colo_put_cmd_value(s->to_dst_file, COLO_MESSAGE_VMSTATE_SIZE, > @@ -573,6 +589,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_loadvm_state_main(mis->from_src_file, mis); > + if (ret < 0) { > + error_report("Load VM's live state (ram) error"); > + goto out; > + } > /* read the VM state total size first */ > value = colo_get_cmd_value(mis->from_src_file, > COLO_MESSAGE_VMSTATE_SIZE, &local_err); > @@ -605,8 +631,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"); > qemu_mutex_unlock_iothread(); > goto out; > } > diff --git a/migration/ram.c b/migration/ram.c > index 891f3b2..8f416d5 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2465,7 +2465,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++; > > @@ -2500,7 +2499,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); > } > @@ -2594,9 +2592,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 60c7b57..1551fbb 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); > + } > } > > /* > -- > 1.8.3.1 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 2016/2/26 21:16, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) 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 >> QEMUSizeBufferas 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> >> --- >> v14: >> - split two new patches from this patch >> - Some minor fixes from Dave >> v13: >> - Re-use some existed helper functions to realize saving/loading >> ram and device. >> v11: >> - Remove load configuration section in qemu_loadvm_state_begin() >> --- >> migration/colo.c | 48 ++++++++++++++++++++++++++++++++++++++---------- >> migration/ram.c | 5 ----- >> migration/savevm.c | 5 +++++ >> 3 files changed, 43 insertions(+), 15 deletions(-) >> >> diff --git a/migration/colo.c b/migration/colo.c >> index 16bada6..300fa54 100644 >> --- a/migration/colo.c >> +++ b/migration/colo.c >> @@ -288,21 +288,37 @@ static int colo_do_checkpoint_transaction(MigrationState *s, >> goto out; >> } >> >> + colo_put_cmd(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, &local_err); >> + if (local_err) { >> + 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_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"); >> + goto out; >> + } >> + >> qemu_mutex_lock_iothread(); >> - qemu_savevm_state_complete_precopy(trans, false); >> + /* >> + * Only save VM's live state, which not including device state. >> + * TODO: We may need a timeout mechanism to prevent COLO process >> + * to be blocked here. >> + */ >> + qemu_savevm_live_state(s->to_dst_file); >> + /* Note: device state is saved into buffer */ >> + ret = qemu_save_device_state(trans); >> qemu_mutex_unlock_iothread(); > > Yes, I still worry a little about what can hang under that lock, but I think Hmm, we have some other places in COLO taking this lock too. Some of them are OK with holding it. But holding it while sending/receiving date from other side in COLO is dangerous. One solution is to apply timeout for QEMUFile operation, But it is not a good choice. > it's the best we've got at the moment; we probably need to understand what > the rules are about what actually needs the lock! > Yes, this is another way to solve the problem. Don't hold the lock while sending/receiving date. For qemu_savevm_live_state() here, IMHO, we can send the remained RAM data without holding the global lock. Here we hold the lock, because we call cpu_synchronize_all_states() in it. (I'm not sure ...) Thanks, Hailiang > Other than that, > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >> - >> - qemu_fflush(trans); >> - >> - colo_put_cmd(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, &local_err); >> - if (local_err) { >> + if (ret < 0) { >> + error_report("Save device state error"); >> goto out; >> } >> + qemu_fflush(trans); >> + >> /* we send the total size of the vmstate first */ >> size = qsb_get_length(buffer); >> colo_put_cmd_value(s->to_dst_file, COLO_MESSAGE_VMSTATE_SIZE, >> @@ -573,6 +589,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_loadvm_state_main(mis->from_src_file, mis); >> + if (ret < 0) { >> + error_report("Load VM's live state (ram) error"); >> + goto out; >> + } >> /* read the VM state total size first */ >> value = colo_get_cmd_value(mis->from_src_file, >> COLO_MESSAGE_VMSTATE_SIZE, &local_err); >> @@ -605,8 +631,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"); >> qemu_mutex_unlock_iothread(); >> goto out; >> } >> diff --git a/migration/ram.c b/migration/ram.c >> index 891f3b2..8f416d5 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -2465,7 +2465,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++; >> >> @@ -2500,7 +2499,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); >> } >> @@ -2594,9 +2592,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 60c7b57..1551fbb 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); >> + } >> } >> >> /* >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >
diff --git a/migration/colo.c b/migration/colo.c index 16bada6..300fa54 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -288,21 +288,37 @@ static int colo_do_checkpoint_transaction(MigrationState *s, goto out; } + colo_put_cmd(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, &local_err); + if (local_err) { + 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_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"); + goto out; + } + qemu_mutex_lock_iothread(); - qemu_savevm_state_complete_precopy(trans, false); + /* + * Only save VM's live state, which not including device state. + * TODO: We may need a timeout mechanism to prevent COLO process + * to be blocked here. + */ + qemu_savevm_live_state(s->to_dst_file); + /* Note: device state is saved into buffer */ + ret = qemu_save_device_state(trans); qemu_mutex_unlock_iothread(); - - qemu_fflush(trans); - - colo_put_cmd(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, &local_err); - if (local_err) { + if (ret < 0) { + error_report("Save device state error"); goto out; } + qemu_fflush(trans); + /* we send the total size of the vmstate first */ size = qsb_get_length(buffer); colo_put_cmd_value(s->to_dst_file, COLO_MESSAGE_VMSTATE_SIZE, @@ -573,6 +589,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_loadvm_state_main(mis->from_src_file, mis); + if (ret < 0) { + error_report("Load VM's live state (ram) error"); + goto out; + } /* read the VM state total size first */ value = colo_get_cmd_value(mis->from_src_file, COLO_MESSAGE_VMSTATE_SIZE, &local_err); @@ -605,8 +631,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"); qemu_mutex_unlock_iothread(); goto out; } diff --git a/migration/ram.c b/migration/ram.c index 891f3b2..8f416d5 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -2465,7 +2465,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++; @@ -2500,7 +2499,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); } @@ -2594,9 +2592,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 60c7b57..1551fbb 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); + } } /*