From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45657) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVYxa-0007ro-Gt for qemu-devel@nongnu.org; Tue, 16 Feb 2016 01:18:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVYxW-000437-8r for qemu-devel@nongnu.org; Tue, 16 Feb 2016 01:18:06 -0500 Received: from szxga01-in.huawei.com ([58.251.152.64]:31149) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVYxV-00042H-8D for qemu-devel@nongnu.org; Tue, 16 Feb 2016 01:18:02 -0500 References: <1454750932-7556-1-git-send-email-zhang.zhanghailiang@huawei.com> <1454750932-7556-25-git-send-email-zhang.zhanghailiang@huawei.com> <20160212150924.GD2411@work-vm> From: Hailiang Zhang Message-ID: <56C2BEE5.5030603@huawei.com> Date: Tue, 16 Feb 2016 14:17:09 +0800 MIME-Version: 1.0 In-Reply-To: <20160212150924.GD2411@work-vm> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH COLO-Frame v14 24/40] COLO: Process shutdown command for VM in COLO state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: xiecl.fnst@cn.fujitsu.com, lizhijian@cn.fujitsu.com, quintela@redhat.com, armbru@redhat.com, yunhong.jiang@intel.com, eddie.dong@intel.com, peter.huangpeng@huawei.com, qemu-devel@nongnu.org, arei.gonglei@huawei.com, stefanha@redhat.com, Paolo Bonzini , amit.shah@redhat.com, zhangchen.fnst@cn.fujitsu.com, hongyang.yang@easystack.cn On 2016/2/12 23:09, Dr. David Alan Gilbert wrote: > * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: >> If VM is in COLO FT state, we should do some extra work before normal shutdown >> process. SVM will ignore the shutdown command if this command is issued directly >> to it, PVM will send the shutdown command to SVM if it gets this command. >> >> Cc: Paolo Bonzini >> Signed-off-by: zhanghailiang >> Signed-off-by: Li Zhijian > > Reviewed-by: Dr. David Alan Gilbert > > although one question below. > Thanks! > Dave > >> --- >> v14: >> - Remove 'colo_shutdown' variable, use colo_shutdown_request directly >> v13: >> - Move COLO shutdown related codes to colo.c file (Dave's suggestion) >> --- >> include/migration/colo.h | 2 ++ >> include/sysemu/sysemu.h | 3 +++ >> migration/colo.c | 42 ++++++++++++++++++++++++++++++++++++++++-- >> qapi-schema.json | 4 +++- >> stubs/migration-colo.c | 5 +++++ >> vl.c | 19 ++++++++++++++++--- >> 6 files changed, 69 insertions(+), 6 deletions(-) >> >> diff --git a/include/migration/colo.h b/include/migration/colo.h >> index e32eef4..919b135 100644 >> --- a/include/migration/colo.h >> +++ b/include/migration/colo.h >> @@ -35,4 +35,6 @@ COLOMode get_colo_mode(void); >> >> /* failover */ >> void colo_do_failover(MigrationState *s); >> + >> +bool colo_shutdown(void); >> #endif >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index 3bb8897..91eeda3 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -52,6 +52,8 @@ typedef enum WakeupReason { >> QEMU_WAKEUP_REASON_OTHER, >> } WakeupReason; >> >> +extern int colo_shutdown_requested; >> + >> void qemu_system_reset_request(void); >> void qemu_system_suspend_request(void); >> void qemu_register_suspend_notifier(Notifier *notifier); >> @@ -59,6 +61,7 @@ void qemu_system_wakeup_request(WakeupReason reason); >> void qemu_system_wakeup_enable(WakeupReason reason, bool enabled); >> void qemu_register_wakeup_notifier(Notifier *notifier); >> void qemu_system_shutdown_request(void); >> +void qemu_system_shutdown_request_core(void); >> void qemu_system_powerdown_request(void); >> void qemu_register_powerdown_notifier(Notifier *notifier); >> void qemu_system_debug_request(void); >> diff --git a/migration/colo.c b/migration/colo.c >> index 515d561..92be985 100644 >> --- a/migration/colo.c >> +++ b/migration/colo.c >> @@ -330,6 +330,18 @@ static int colo_do_checkpoint_transaction(MigrationState *s, >> goto out; >> } >> >> + if (colo_shutdown_requested) { >> + colo_put_cmd(s->to_dst_file, COLO_MESSAGE_GUEST_SHUTDOWN, &local_err); >> + if (local_err) { >> + goto out; >> + } > > I wonder what actually happens in this case? Does it eventually shutdown? > No, the qemu will not exit from the main loop, it seems that, we should ignore the error here, and go on the shutdown process. I will fix it in next version. >> + qemu_fflush(s->to_dst_file); >> + colo_shutdown_requested = 0; >> + qemu_system_shutdown_request_core(); >> + /* Fix me: Just let the colo thread exit ? */ >> + qemu_thread_exit(0); >> + } >> + >> ret = 0; >> /* Resume primary guest */ >> qemu_mutex_lock_iothread(); >> @@ -390,8 +402,9 @@ static void colo_process_checkpoint(MigrationState *s) >> } >> >> current_time = qemu_clock_get_ms(QEMU_CLOCK_HOST); >> - if (current_time - checkpoint_time < >> - s->parameters[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY]) { >> + if ((current_time - checkpoint_time < >> + s->parameters[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY]) && >> + !colo_shutdown_requested) { >> int64_t delay_ms; >> >> delay_ms = s->parameters[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY] - >> @@ -465,6 +478,15 @@ static void colo_wait_handle_cmd(QEMUFile *f, int *checkpoint_request, >> case COLO_MESSAGE_CHECKPOINT_REQUEST: >> *checkpoint_request = 1; >> break; >> + case COLO_MESSAGE_GUEST_SHUTDOWN: >> + qemu_mutex_lock_iothread(); >> + vm_stop_force_state(RUN_STATE_COLO); >> + qemu_system_shutdown_request_core(); >> + qemu_mutex_unlock_iothread(); >> + /* the main thread will exit and terminate the whole >> + * process, do we need some cleanup? >> + */ >> + qemu_thread_exit(0); >> default: >> *checkpoint_request = 0; >> error_setg(errp, "Got unknown COLO command: %d", cmd); >> @@ -636,3 +658,19 @@ out: >> >> return NULL; >> } >> + >> +bool colo_shutdown(void) >> +{ >> + /* >> + * if in colo mode, we need do some significant work before respond >> + * to the shutdown request. >> + */ >> + if (migration_incoming_in_colo_state()) { >> + return true; /* primary's responsibility */ >> + } >> + if (migration_in_colo_state()) { >> + colo_shutdown_requested = 1; >> + return true; >> + } >> + return false; >> +} >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 7fec696..4d8ba04 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -752,12 +752,14 @@ >> # >> # @vmstate-loaded: VM's state has been loaded by SVM. >> # >> +# @guest-shutdown: shutdown require from PVM to SVM >> +# >> # Since: 2.6 >> ## >> { 'enum': 'COLOMessage', >> 'data': [ 'checkpoint-ready', 'checkpoint-request', 'checkpoint-reply', >> 'vmstate-send', 'vmstate-size', 'vmstate-received', >> - 'vmstate-loaded' ] } >> + 'vmstate-loaded', 'guest-shutdown' ] } >> >> ## >> # @COLOMode >> diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c >> index a6cd6e5..1996cd9 100644 >> --- a/stubs/migration-colo.c >> +++ b/stubs/migration-colo.c >> @@ -43,3 +43,8 @@ void qmp_x_colo_lost_heartbeat(Error **errp) >> " with --enable-colo option in order to support" >> " COLO feature"); >> } >> + >> +bool colo_shutdown(void) >> +{ >> + return false; >> +} >> diff --git a/vl.c b/vl.c >> index f16fe2d..90da8d1 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -1631,6 +1631,8 @@ static NotifierList wakeup_notifiers = >> NOTIFIER_LIST_INITIALIZER(wakeup_notifiers); >> static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE); >> >> +int colo_shutdown_requested; >> + >> int qemu_shutdown_requested_get(void) >> { >> return shutdown_requested; >> @@ -1762,7 +1764,10 @@ void qemu_system_guest_panicked(void) >> void qemu_system_reset_request(void) >> { >> if (no_reboot) { >> - shutdown_requested = 1; >> + qemu_system_shutdown_request(); >> + if (!shutdown_requested) {/* colo handle it ? */ >> + return; >> + } >> } else { >> reset_requested = 1; >> } >> @@ -1835,14 +1840,22 @@ void qemu_system_killed(int signal, pid_t pid) >> qemu_notify_event(); >> } >> >> -void qemu_system_shutdown_request(void) >> +void qemu_system_shutdown_request_core(void) >> { >> - trace_qemu_system_shutdown_request(); >> replay_shutdown_request(); >> shutdown_requested = 1; >> qemu_notify_event(); >> } >> >> +void qemu_system_shutdown_request(void) >> +{ >> + trace_qemu_system_shutdown_request(); >> + if (colo_shutdown()) { >> + return; >> + } >> + qemu_system_shutdown_request_core(); >> +} >> + >> static void qemu_system_powerdown(void) >> { >> qapi_event_send_powerdown(&error_abort); >> -- >> 1.8.3.1 >> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > . >