| Message ID | 20190303145021.2962-8-chen.zhang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Migration/colo: Fix upstream bugs when occur failover | expand |
* Zhang Chen (chen.zhang@intel.com) wrote: > From: Zhang Chen <chen.zhang@intel.com> > > Add the last_colo_mode to save the status after failover. > This patch can solve the issue that user got nothing to call > query_colo_status after failover. I don't understand what this is doing; cna you show me an example of the way the state looks before and after the patch? Dave > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > --- > migration/colo.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c > index d1ae2e6d11..6eba8e06f2 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -38,6 +38,9 @@ > static bool vmstate_loading; > static Notifier packets_compare_notifier; > > +/* User need to know colo mode after COLO failover */ > +static COLOMode last_colo_mode; > + > #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) > > bool migration_in_colo_state(void) > @@ -197,7 +200,10 @@ void colo_do_failover(MigrationState *s) > vm_stop_force_state(RUN_STATE_COLO); > } > > - switch (get_colo_mode()) { > + /* Update last_COLO_mode to avoid unexpectedly exit COLO status */ > + last_colo_mode = get_colo_mode(); > + > + switch (last_colo_mode) { > case COLO_MODE_PRIMARY: > primary_vm_do_failover(); > break; > @@ -263,7 +269,7 @@ COLOStatus *qmp_query_colo_status(Error **errp) > { > COLOStatus *s = g_new0(COLOStatus, 1); > > - s->mode = get_colo_mode(); > + s->mode = last_colo_mode; > > switch (failover_get_state()) { > case FAILOVER_STATUS_NONE: > @@ -515,6 +521,12 @@ static void colo_process_checkpoint(MigrationState *s) > Error *local_err = NULL; > int ret; > > + last_colo_mode = get_colo_mode(); > + if (last_colo_mode != COLO_MODE_PRIMARY) { > + error_report("COLO mode must be COLO_MODE_PRIMARY"); > + return; > + } > + > failover_init_state(); > > s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file); > @@ -682,12 +694,18 @@ void *colo_process_incoming_thread(void *opaque) > Error *local_err = NULL; > int ret; > > - rcu_register_thread(); > - qemu_sem_init(&mis->colo_incoming_sem, 0); > - > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > MIGRATION_STATUS_COLO); > > + last_colo_mode = get_colo_mode(); > + if (last_colo_mode != COLO_MODE_SECONDARY) { > + error_report("COLO mode must be COLO_MODE_SECONDARY"); > + return NULL; > + } > + > + rcu_register_thread(); > + qemu_sem_init(&mis->colo_incoming_sem, 0); > + > failover_init_state(); > > mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); > -- > 2.17.GIT > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
-----Original Message----- From: Dr. David Alan Gilbert [mailto:dgilbert@redhat.com] Sent: Saturday, March 9, 2019 1:46 AM To: Zhang, Chen <chen.zhang@intel.com> Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; Zhang Chen <zhangckid@gmail.com>; Juan Quintela <quintela@redhat.com>; zhanghailiang <zhang.zhanghailiang@huawei.com>; Markus Armbruster <armbru@redhat.com>; Eric Blake <eblake@redhat.com>; qemu-dev <qemu-devel@nongnu.org> Subject: Re: [PATCH V3 7/7] Migration/colo.c: Make user obtain the COLO mode info after failover * Zhang Chen (chen.zhang@intel.com) wrote: > From: Zhang Chen <chen.zhang@intel.com> > > Add the last_colo_mode to save the status after failover. > This patch can solve the issue that user got nothing to call > query_colo_status after failover. I don't understand what this is doing; cna you show me an example of the way the state looks before and after the patch? Sure. In current codes, when we use the " query_colo_status " after failover, the mode field will always return "none". Because after failover we can't use " get_colo_mode()" to get the mode, we need the last_colo_mode args to save the last state. I think we will use this args in new colo feature development. Thanks Zhang Chen Dave > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > --- > migration/colo.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c index > d1ae2e6d11..6eba8e06f2 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -38,6 +38,9 @@ > static bool vmstate_loading; > static Notifier packets_compare_notifier; > > +/* User need to know colo mode after COLO failover */ static COLOMode > +last_colo_mode; > + > #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) > > bool migration_in_colo_state(void) > @@ -197,7 +200,10 @@ void colo_do_failover(MigrationState *s) > vm_stop_force_state(RUN_STATE_COLO); > } > > - switch (get_colo_mode()) { > + /* Update last_COLO_mode to avoid unexpectedly exit COLO status */ > + last_colo_mode = get_colo_mode(); > + > + switch (last_colo_mode) { > case COLO_MODE_PRIMARY: > primary_vm_do_failover(); > break; > @@ -263,7 +269,7 @@ COLOStatus *qmp_query_colo_status(Error **errp) { > COLOStatus *s = g_new0(COLOStatus, 1); > > - s->mode = get_colo_mode(); > + s->mode = last_colo_mode; > > switch (failover_get_state()) { > case FAILOVER_STATUS_NONE: > @@ -515,6 +521,12 @@ static void colo_process_checkpoint(MigrationState *s) > Error *local_err = NULL; > int ret; > > + last_colo_mode = get_colo_mode(); > + if (last_colo_mode != COLO_MODE_PRIMARY) { > + error_report("COLO mode must be COLO_MODE_PRIMARY"); > + return; > + } > + > failover_init_state(); > > s->rp_state.from_dst_file = > qemu_file_get_return_path(s->to_dst_file); > @@ -682,12 +694,18 @@ void *colo_process_incoming_thread(void *opaque) > Error *local_err = NULL; > int ret; > > - rcu_register_thread(); > - qemu_sem_init(&mis->colo_incoming_sem, 0); > - > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > MIGRATION_STATUS_COLO); > > + last_colo_mode = get_colo_mode(); > + if (last_colo_mode != COLO_MODE_SECONDARY) { > + error_report("COLO mode must be COLO_MODE_SECONDARY"); > + return NULL; > + } > + > + rcu_register_thread(); > + qemu_sem_init(&mis->colo_incoming_sem, 0); > + > failover_init_state(); > > mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); > -- > 2.17.GIT > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Zhang, Chen (chen.zhang@intel.com) wrote: > > > > -----Original Message----- > From: Dr. David Alan Gilbert [mailto:dgilbert@redhat.com] > Sent: Saturday, March 9, 2019 1:46 AM > To: Zhang, Chen <chen.zhang@intel.com> > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; Zhang Chen <zhangckid@gmail.com>; Juan Quintela <quintela@redhat.com>; zhanghailiang <zhang.zhanghailiang@huawei.com>; Markus Armbruster <armbru@redhat.com>; Eric Blake <eblake@redhat.com>; qemu-dev <qemu-devel@nongnu.org> > Subject: Re: [PATCH V3 7/7] Migration/colo.c: Make user obtain the COLO mode info after failover > > * Zhang Chen (chen.zhang@intel.com) wrote: > > From: Zhang Chen <chen.zhang@intel.com> > > > > Add the last_colo_mode to save the status after failover. > > This patch can solve the issue that user got nothing to call > > query_colo_status after failover. > > I don't understand what this is doing; cna you show me an example of the way the state looks before and after the patch? > > Sure. > In current codes, when we use the " query_colo_status " after failover, the mode field will always return "none". > Because after failover we can't use " get_colo_mode()" to get the mode, we need the last_colo_mode args to save the last state. > I think we will use this args in new colo feature development. But, after a fail-over, 'none' is correct isn't it? There's no colo protection any more, so it's wrong to say that the VM is either a primary or a secondary. Dave > Thanks > Zhang Chen > > Dave > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > > --- > > migration/colo.c | 28 +++++++++++++++++++++++----- > > 1 file changed, 23 insertions(+), 5 deletions(-) > > > > diff --git a/migration/colo.c b/migration/colo.c index > > d1ae2e6d11..6eba8e06f2 100644 > > --- a/migration/colo.c > > +++ b/migration/colo.c > > @@ -38,6 +38,9 @@ > > static bool vmstate_loading; > > static Notifier packets_compare_notifier; > > > > +/* User need to know colo mode after COLO failover */ static COLOMode > > +last_colo_mode; > > + > > #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) > > > > bool migration_in_colo_state(void) > > @@ -197,7 +200,10 @@ void colo_do_failover(MigrationState *s) > > vm_stop_force_state(RUN_STATE_COLO); > > } > > > > - switch (get_colo_mode()) { > > + /* Update last_COLO_mode to avoid unexpectedly exit COLO status */ > > + last_colo_mode = get_colo_mode(); > > + > > + switch (last_colo_mode) { > > case COLO_MODE_PRIMARY: > > primary_vm_do_failover(); > > break; > > @@ -263,7 +269,7 @@ COLOStatus *qmp_query_colo_status(Error **errp) { > > COLOStatus *s = g_new0(COLOStatus, 1); > > > > - s->mode = get_colo_mode(); > > + s->mode = last_colo_mode; > > > > switch (failover_get_state()) { > > case FAILOVER_STATUS_NONE: > > @@ -515,6 +521,12 @@ static void colo_process_checkpoint(MigrationState *s) > > Error *local_err = NULL; > > int ret; > > > > + last_colo_mode = get_colo_mode(); > > + if (last_colo_mode != COLO_MODE_PRIMARY) { > > + error_report("COLO mode must be COLO_MODE_PRIMARY"); > > + return; > > + } > > + > > failover_init_state(); > > > > s->rp_state.from_dst_file = > > qemu_file_get_return_path(s->to_dst_file); > > @@ -682,12 +694,18 @@ void *colo_process_incoming_thread(void *opaque) > > Error *local_err = NULL; > > int ret; > > > > - rcu_register_thread(); > > - qemu_sem_init(&mis->colo_incoming_sem, 0); > > - > > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > > MIGRATION_STATUS_COLO); > > > > + last_colo_mode = get_colo_mode(); > > + if (last_colo_mode != COLO_MODE_SECONDARY) { > > + error_report("COLO mode must be COLO_MODE_SECONDARY"); > > + return NULL; > > + } > > + > > + rcu_register_thread(); > > + qemu_sem_init(&mis->colo_incoming_sem, 0); > > + > > failover_init_state(); > > > > mis->to_src_file = qemu_file_get_return_path(mis->from_src_file); > > -- > > 2.17.GIT > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
-----Original Message----- From: Dr. David Alan Gilbert [mailto:dgilbert@redhat.com] Sent: Tuesday, March 19, 2019 10:57 PM To: Zhang, Chen <chen.zhang@intel.com> Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; Zhang Chen <zhangckid@gmail.com>; Juan Quintela <quintela@redhat.com>; zhanghailiang <zhang.zhanghailiang@huawei.com>; Markus Armbruster <armbru@redhat.com>; Eric Blake <eblake@redhat.com>; qemu-dev <qemu-devel@nongnu.org> Subject: Re: [PATCH V3 7/7] Migration/colo.c: Make user obtain the COLO mode info after failover * Zhang, Chen (chen.zhang@intel.com) wrote: > > > > -----Original Message----- > From: Dr. David Alan Gilbert [mailto:dgilbert@redhat.com] > Sent: Saturday, March 9, 2019 1:46 AM > To: Zhang, Chen <chen.zhang@intel.com> > Cc: Li Zhijian <lizhijian@cn.fujitsu.com>; Zhang Chen > <zhangckid@gmail.com>; Juan Quintela <quintela@redhat.com>; > zhanghailiang <zhang.zhanghailiang@huawei.com>; Markus Armbruster > <armbru@redhat.com>; Eric Blake <eblake@redhat.com>; qemu-dev > <qemu-devel@nongnu.org> > Subject: Re: [PATCH V3 7/7] Migration/colo.c: Make user obtain the > COLO mode info after failover > > * Zhang Chen (chen.zhang@intel.com) wrote: > > From: Zhang Chen <chen.zhang@intel.com> > > > > Add the last_colo_mode to save the status after failover. > > This patch can solve the issue that user got nothing to call > > query_colo_status after failover. > > I don't understand what this is doing; cna you show me an example of the way the state looks before and after the patch? > > Sure. > In current codes, when we use the " query_colo_status " after failover, the mode field will always return "none". > Because after failover we can't use " get_colo_mode()" to get the mode, we need the last_colo_mode args to save the last state. > I think we will use this args in new colo feature development. But, after a fail-over, 'none' is correct isn't it? There's no colo protection any more, so it's wrong to say that the VM is either a primary or a secondary. Make sense, I will remove this patch and add new qmp command "query_colo_last_mode" to address our needs. Thanks Zhang Chen Dave > Thanks > Zhang Chen > > Dave > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > > --- > > migration/colo.c | 28 +++++++++++++++++++++++----- > > 1 file changed, 23 insertions(+), 5 deletions(-) > > > > diff --git a/migration/colo.c b/migration/colo.c index > > d1ae2e6d11..6eba8e06f2 100644 > > --- a/migration/colo.c > > +++ b/migration/colo.c > > @@ -38,6 +38,9 @@ > > static bool vmstate_loading; > > static Notifier packets_compare_notifier; > > > > +/* User need to know colo mode after COLO failover */ static > > +COLOMode last_colo_mode; > > + > > #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) > > > > bool migration_in_colo_state(void) > > @@ -197,7 +200,10 @@ void colo_do_failover(MigrationState *s) > > vm_stop_force_state(RUN_STATE_COLO); > > } > > > > - switch (get_colo_mode()) { > > + /* Update last_COLO_mode to avoid unexpectedly exit COLO status */ > > + last_colo_mode = get_colo_mode(); > > + > > + switch (last_colo_mode) { > > case COLO_MODE_PRIMARY: > > primary_vm_do_failover(); > > break; > > @@ -263,7 +269,7 @@ COLOStatus *qmp_query_colo_status(Error **errp) { > > COLOStatus *s = g_new0(COLOStatus, 1); > > > > - s->mode = get_colo_mode(); > > + s->mode = last_colo_mode; > > > > switch (failover_get_state()) { > > case FAILOVER_STATUS_NONE: > > @@ -515,6 +521,12 @@ static void colo_process_checkpoint(MigrationState *s) > > Error *local_err = NULL; > > int ret; > > > > + last_colo_mode = get_colo_mode(); > > + if (last_colo_mode != COLO_MODE_PRIMARY) { > > + error_report("COLO mode must be COLO_MODE_PRIMARY"); > > + return; > > + } > > + > > failover_init_state(); > > > > s->rp_state.from_dst_file = > > qemu_file_get_return_path(s->to_dst_file); > > @@ -682,12 +694,18 @@ void *colo_process_incoming_thread(void *opaque) > > Error *local_err = NULL; > > int ret; > > > > - rcu_register_thread(); > > - qemu_sem_init(&mis->colo_incoming_sem, 0); > > - > > migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > > MIGRATION_STATUS_COLO); > > > > + last_colo_mode = get_colo_mode(); > > + if (last_colo_mode != COLO_MODE_SECONDARY) { > > + error_report("COLO mode must be COLO_MODE_SECONDARY"); > > + return NULL; > > + } > > + > > + rcu_register_thread(); > > + qemu_sem_init(&mis->colo_incoming_sem, 0); > > + > > failover_init_state(); > > > > mis->to_src_file = > > qemu_file_get_return_path(mis->from_src_file); > > -- > > 2.17.GIT > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/migration/colo.c b/migration/colo.c index d1ae2e6d11..6eba8e06f2 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -38,6 +38,9 @@ static bool vmstate_loading; static Notifier packets_compare_notifier; +/* User need to know colo mode after COLO failover */ +static COLOMode last_colo_mode; + #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024) bool migration_in_colo_state(void) @@ -197,7 +200,10 @@ void colo_do_failover(MigrationState *s) vm_stop_force_state(RUN_STATE_COLO); } - switch (get_colo_mode()) { + /* Update last_COLO_mode to avoid unexpectedly exit COLO status */ + last_colo_mode = get_colo_mode(); + + switch (last_colo_mode) { case COLO_MODE_PRIMARY: primary_vm_do_failover(); break; @@ -263,7 +269,7 @@ COLOStatus *qmp_query_colo_status(Error **errp) { COLOStatus *s = g_new0(COLOStatus, 1); - s->mode = get_colo_mode(); + s->mode = last_colo_mode; switch (failover_get_state()) { case FAILOVER_STATUS_NONE: @@ -515,6 +521,12 @@ static void colo_process_checkpoint(MigrationState *s) Error *local_err = NULL; int ret; + last_colo_mode = get_colo_mode(); + if (last_colo_mode != COLO_MODE_PRIMARY) { + error_report("COLO mode must be COLO_MODE_PRIMARY"); + return; + } + failover_init_state(); s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file); @@ -682,12 +694,18 @@ void *colo_process_incoming_thread(void *opaque) Error *local_err = NULL; int ret; - rcu_register_thread(); - qemu_sem_init(&mis->colo_incoming_sem, 0); - migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, MIGRATION_STATUS_COLO); + last_colo_mode = get_colo_mode(); + if (last_colo_mode != COLO_MODE_SECONDARY) { + error_report("COLO mode must be COLO_MODE_SECONDARY"); + return NULL; + } + + rcu_register_thread(); + qemu_sem_init(&mis->colo_incoming_sem, 0); + failover_init_state(); mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);