From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38944) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9x4M-0005MY-E8 for qemu-devel@nongnu.org; Fri, 18 Dec 2015 10:35:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9x4I-000385-UH for qemu-devel@nongnu.org; Fri, 18 Dec 2015 10:35:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35174) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9x4I-00037j-N0 for qemu-devel@nongnu.org; Fri, 18 Dec 2015 10:35:42 -0500 Date: Fri, 18 Dec 2015 15:35:36 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20151218153536.GJ2459@work-vm> References: <1450167779-9960-1-git-send-email-zhang.zhanghailiang@huawei.com> <1450167779-9960-24-git-send-email-zhang.zhanghailiang@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1450167779-9960-24-git-send-email-zhang.zhanghailiang@huawei.com> Subject: Re: [Qemu-devel] [PATCH COLO-Frame v12 23/38] COLO: Implement failover work for Primary VM List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: zhanghailiang Cc: lizhijian@cn.fujitsu.com, quintela@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, amit.shah@redhat.com, hongyang.yang@easystack.cn * zhanghailiang (zhang.zhanghailiang@huawei.com) wrote: > For PVM, if there is failover request from users. > The colo thread will exit the loop while the failover BH does the > cleanup work and resumes VM. > > Signed-off-by: zhanghailiang > Signed-off-by: Li Zhijian > --- > v12: > - Fix error report and remove unnecessary check in primary_vm_do_failover() > (Dave's suggestion) > v11: > - Don't call migration_end() in primary_vm_do_failover(), > The cleanup work will be done in migration_thread(). > - Remove vm_start() in primary_vm_do_failover() which also been done > in migraiton_thread() > v10: > - Call migration_end() in primary_vm_do_failover() > > Signed-off-by: zhanghailiang > --- > include/migration/colo.h | 3 +++ > include/migration/failover.h | 1 + > migration/colo-failover.c | 7 +++++- > migration/colo.c | 54 ++++++++++++++++++++++++++++++++++++++++++-- > 4 files changed, 62 insertions(+), 3 deletions(-) > > diff --git a/include/migration/colo.h b/include/migration/colo.h > index ba27719..0b02e95 100644 > --- a/include/migration/colo.h > +++ b/include/migration/colo.h > @@ -32,4 +32,7 @@ void *colo_process_incoming_thread(void *opaque); > bool migration_incoming_in_colo_state(void); > > COLOMode get_colo_mode(void); > + > +/* failover */ > +void colo_do_failover(MigrationState *s); > #endif > diff --git a/include/migration/failover.h b/include/migration/failover.h > index 882c625..fba3931 100644 > --- a/include/migration/failover.h > +++ b/include/migration/failover.h > @@ -26,5 +26,6 @@ void failover_init_state(void); > int failover_set_state(int old_state, int new_state); > int failover_get_state(void); > void failover_request_active(Error **errp); > +bool failover_request_is_active(void); > > #endif > diff --git a/migration/colo-failover.c b/migration/colo-failover.c > index 1b1be24..0c525da 100644 > --- a/migration/colo-failover.c > +++ b/migration/colo-failover.c > @@ -32,7 +32,7 @@ static void colo_failover_bh(void *opaque) > error_report("Unkown error for failover, old_state=%d", old_state); > return; > } > - /*TODO: Do failover work */ > + colo_do_failover(NULL); > } > > void failover_request_active(Error **errp) > @@ -67,6 +67,11 @@ int failover_get_state(void) > return atomic_read(&failover_state); > } > > +bool failover_request_is_active(void) > +{ > + return ((failover_get_state() != FAILOVER_STATUS_NONE)); You can remove the two sets of brackets. But other than that: Reviewed-by: Dr. David Alan Gilbert > +} > + > void qmp_x_colo_lost_heartbeat(Error **errp) > { > if (get_colo_mode() == COLO_MODE_UNKNOWN) { > diff --git a/migration/colo.c b/migration/colo.c > index 176384e..977c8d8 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -41,6 +41,40 @@ bool migration_incoming_in_colo_state(void) > return mis && (mis->state == MIGRATION_STATUS_COLO); > } > > +static bool colo_runstate_is_stopped(void) > +{ > + return runstate_check(RUN_STATE_COLO) || !runstate_is_running(); > +} > + > +static void primary_vm_do_failover(void) > +{ > + MigrationState *s = migrate_get_current(); > + int old_state; > + > + migrate_set_state(&s->state, MIGRATION_STATUS_COLO, > + MIGRATION_STATUS_COMPLETED); > + > + old_state = failover_set_state(FAILOVER_STATUS_HANDLING, > + FAILOVER_STATUS_COMPLETED); > + if (old_state != FAILOVER_STATUS_HANDLING) { > + error_report("Incorrect state (%d) while doing failover for Primary VM", > + old_state); > + return; > + } > +} > + > +void colo_do_failover(MigrationState *s) > +{ > + /* Make sure vm stopped while failover */ > + if (!colo_runstate_is_stopped()) { > + vm_stop_force_state(RUN_STATE_COLO); > + } > + > + if (get_colo_mode() == COLO_MODE_PRIMARY) { > + primary_vm_do_failover(); > + } > +} > + > static int colo_put_cmd(QEMUFile *f, uint32_t cmd) > { > int ret; > @@ -150,9 +184,22 @@ static int colo_do_checkpoint_transaction(MigrationState *s, > } > > qemu_mutex_lock_iothread(); > + if (failover_request_is_active()) { > + qemu_mutex_unlock_iothread(); > + ret = -1; > + goto out; > + } > vm_stop_force_state(RUN_STATE_COLO); > qemu_mutex_unlock_iothread(); > trace_colo_vm_state_change("run", "stop"); > + /* > + * failover request bh could be called after > + * vm_stop_force_state so we check failover_request_is_active() again. > + */ > + if (failover_request_is_active()) { > + ret = -1; > + goto out; > + } > > /* Disable block migration */ > s->params.blk = 0; > @@ -248,6 +295,11 @@ static void colo_process_checkpoint(MigrationState *s) > trace_colo_vm_state_change("stop", "run"); > > while (s->state == MIGRATION_STATUS_COLO) { > + if (failover_request_is_active()) { > + error_report("failover request"); > + goto out; > + } > + > current_time = qemu_clock_get_ms(QEMU_CLOCK_HOST); > if (current_time - checkpoint_time < > s->parameters[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY]) { > @@ -269,8 +321,6 @@ out: > if (ret < 0) { > error_report("%s: %s", __func__, strerror(-ret)); > } > - migrate_set_state(&s->state, MIGRATION_STATUS_COLO, > - MIGRATION_STATUS_COMPLETED); > > qsb_free(buffer); > buffer = NULL; > -- > 1.8.3.1 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK