From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45202) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUFLj-0005xF-Bn for qemu-devel@nongnu.org; Fri, 12 Feb 2016 10:09:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aUFLg-00022N-2h for qemu-devel@nongnu.org; Fri, 12 Feb 2016 10:09:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57440) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aUFLf-00022H-Pp for qemu-devel@nongnu.org; Fri, 12 Feb 2016 10:09:32 -0500 Date: Fri, 12 Feb 2016 15:09:24 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20160212150924.GD2411@work-vm> References: <1454750932-7556-1-git-send-email-zhang.zhanghailiang@huawei.com> <1454750932-7556-25-git-send-email-zhang.zhanghailiang@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1454750932-7556-25-git-send-email-zhang.zhanghailiang@huawei.com> 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: zhanghailiang 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 * 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. 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? > + 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