* [PATCH V2 0/2] migration/colo: Optimize COLO framework code @ 2020-06-04 8:55 Zhang Chen 2020-06-04 8:55 ` [PATCH V2 1/2] migration/colo: Optimize COLO boot code path Zhang Chen 2020-06-04 8:55 ` [PATCH V2 2/2] migration/colo: Update checkpoint time lately Zhang Chen 0 siblings, 2 replies; 6+ messages in thread From: Zhang Chen @ 2020-06-04 8:55 UTC (permalink / raw) To: Dr . David Alan Gilbert, qemu-dev; +Cc: Zhang Chen, Zhanghailiang, Zhang Chen From: Zhang Chen <chen.zhang@intel.com> This series optimize some code of COLO, please review. Zhang Chen (2): migration/colo: Optimize COLO boot code path migration/colo: Update checkpoint time lately migration/colo.c | 7 ++----- migration/migration.c | 17 ++++++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2 1/2] migration/colo: Optimize COLO boot code path 2020-06-04 8:55 [PATCH V2 0/2] migration/colo: Optimize COLO framework code Zhang Chen @ 2020-06-04 8:55 ` Zhang Chen 2020-06-06 19:56 ` Lukas Straub 2020-06-04 8:55 ` [PATCH V2 2/2] migration/colo: Update checkpoint time lately Zhang Chen 1 sibling, 1 reply; 6+ messages in thread From: Zhang Chen @ 2020-06-04 8:55 UTC (permalink / raw) To: Dr . David Alan Gilbert, qemu-dev; +Cc: Zhang Chen, Zhanghailiang, Zhang Chen From: Zhang Chen <chen.zhang@intel.com> No need to reuse MIGRATION_STATUS_ACTIVE boot COLO. Signed-off-by: Zhang Chen <chen.zhang@intel.com> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- migration/colo.c | 2 -- migration/migration.c | 17 ++++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/migration/colo.c b/migration/colo.c index ea7d1e9d4e..91c76789fa 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -670,8 +670,6 @@ void migrate_start_colo_process(MigrationState *s) colo_checkpoint_notify, s); qemu_sem_init(&s->colo_exit_sem, 0); - migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE, - MIGRATION_STATUS_COLO); colo_process_checkpoint(s); qemu_mutex_lock_iothread(); } diff --git a/migration/migration.c b/migration/migration.c index b63ad91d34..7f3f82a715 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2972,7 +2972,10 @@ static void migration_completion(MigrationState *s) goto fail_invalidate; } - if (!migrate_colo_enabled()) { + if (migrate_colo_enabled()) { + migrate_set_state(&s->state, current_active_state, + MIGRATION_STATUS_COLO); + } else { migrate_set_state(&s->state, current_active_state, MIGRATION_STATUS_COMPLETED); } @@ -3304,12 +3307,7 @@ static void migration_iteration_finish(MigrationState *s) migration_calculate_complete(s); runstate_set(RUN_STATE_POSTMIGRATE); break; - - case MIGRATION_STATUS_ACTIVE: - /* - * We should really assert here, but since it's during - * migration, let's try to reduce the usage of assertions. - */ + case MIGRATION_STATUS_COLO: if (!migrate_colo_enabled()) { error_report("%s: critical error: calling COLO code without " "COLO enabled", __func__); @@ -3319,6 +3317,11 @@ static void migration_iteration_finish(MigrationState *s) * Fixme: we will run VM in COLO no matter its old running state. * After exited COLO, we will keep running. */ + case MIGRATION_STATUS_ACTIVE: + /* + * We should really assert here, but since it's during + * migration, let's try to reduce the usage of assertions. + */ s->vm_was_running = true; /* Fallthrough */ case MIGRATION_STATUS_FAILED: -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2 1/2] migration/colo: Optimize COLO boot code path 2020-06-04 8:55 ` [PATCH V2 1/2] migration/colo: Optimize COLO boot code path Zhang Chen @ 2020-06-06 19:56 ` Lukas Straub 2020-06-07 19:42 ` Zhang, Chen 0 siblings, 1 reply; 6+ messages in thread From: Lukas Straub @ 2020-06-06 19:56 UTC (permalink / raw) To: Zhang Chen; +Cc: Zhanghailiang, Zhang Chen, Dr . David Alan Gilbert, qemu-dev [-- Attachment #1: Type: text/plain, Size: 2948 bytes --] On Thu, 4 Jun 2020 16:55:32 +0800 Zhang Chen <chen.zhang@intel.com > wrote: > From: Zhang Chen <chen.zhang@intel.com> > > No need to reuse MIGRATION_STATUS_ACTIVE boot COLO. > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- > migration/colo.c | 2 -- > migration/migration.c | 17 ++++++++++------- > 2 files changed, 10 insertions(+), 9 deletions(-) > > diff --git a/migration/colo.c b/migration/colo.c > index ea7d1e9d4e..91c76789fa 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -670,8 +670,6 @@ void migrate_start_colo_process(MigrationState *s) > colo_checkpoint_notify, s); > > qemu_sem_init(&s->colo_exit_sem, 0); > - migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE, > - MIGRATION_STATUS_COLO); > colo_process_checkpoint(s); > qemu_mutex_lock_iothread(); > } > diff --git a/migration/migration.c b/migration/migration.c > index b63ad91d34..7f3f82a715 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2972,7 +2972,10 @@ static void migration_completion(MigrationState *s) > goto fail_invalidate; > } > > - if (!migrate_colo_enabled()) { > + if (migrate_colo_enabled()) { > + migrate_set_state(&s->state, current_active_state, > + MIGRATION_STATUS_COLO); > + } else { > migrate_set_state(&s->state, current_active_state, > MIGRATION_STATUS_COMPLETED); > } > @@ -3304,12 +3307,7 @@ static void migration_iteration_finish(MigrationState *s) > migration_calculate_complete(s); > runstate_set(RUN_STATE_POSTMIGRATE); > break; > - > - case MIGRATION_STATUS_ACTIVE: > - /* > - * We should really assert here, but since it's during > - * migration, let's try to reduce the usage of assertions. > - */ > + case MIGRATION_STATUS_COLO: > if (!migrate_colo_enabled()) { > error_report("%s: critical error: calling COLO code without " > "COLO enabled", __func__); > @@ -3319,6 +3317,11 @@ static void migration_iteration_finish(MigrationState *s) > * Fixme: we will run VM in COLO no matter its old running state. > * After exited COLO, we will keep running. > */ > + case MIGRATION_STATUS_ACTIVE: > + /* > + * We should really assert here, but since it's during > + * migration, let's try to reduce the usage of assertions. > + */ I think this case should be removed, because arriving here with MIGRATION_STATUS_ACTIVE is invalid. It should be handled by the default case. Regards, Lukas Straub > s->vm_was_running = true; > /* Fallthrough */ > case MIGRATION_STATUS_FAILED: [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH V2 1/2] migration/colo: Optimize COLO boot code path 2020-06-06 19:56 ` Lukas Straub @ 2020-06-07 19:42 ` Zhang, Chen 0 siblings, 0 replies; 6+ messages in thread From: Zhang, Chen @ 2020-06-07 19:42 UTC (permalink / raw) To: Lukas Straub; +Cc: Zhanghailiang, Zhang Chen, Dr . David Alan Gilbert, qemu-dev > -----Original Message----- > From: Lukas Straub <lukasstraub2@web.de> > Sent: Sunday, June 7, 2020 3:57 AM > To: Zhang, Chen <chen.zhang@intel.com> > Cc: Dr . David Alan Gilbert <dgilbert@redhat.com>; qemu-dev <qemu- > devel@nongnu.org>; Zhanghailiang <zhang.zhanghailiang@huawei.com>; > Zhang Chen <zhangckid@gmail.com> > Subject: Re: [PATCH V2 1/2] migration/colo: Optimize COLO boot code path > > On Thu, 4 Jun 2020 16:55:32 +0800 > Zhang Chen <chen.zhang@intel.com > wrote: > > > From: Zhang Chen <chen.zhang@intel.com> > > > > No need to reuse MIGRATION_STATUS_ACTIVE boot COLO. > > > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > > Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > > --- > > migration/colo.c | 2 -- > > migration/migration.c | 17 ++++++++++------- > > 2 files changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/migration/colo.c b/migration/colo.c index > > ea7d1e9d4e..91c76789fa 100644 > > --- a/migration/colo.c > > +++ b/migration/colo.c > > @@ -670,8 +670,6 @@ void migrate_start_colo_process(MigrationState *s) > > colo_checkpoint_notify, s); > > > > qemu_sem_init(&s->colo_exit_sem, 0); > > - migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE, > > - MIGRATION_STATUS_COLO); > > colo_process_checkpoint(s); > > qemu_mutex_lock_iothread(); > > } > > diff --git a/migration/migration.c b/migration/migration.c index > > b63ad91d34..7f3f82a715 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -2972,7 +2972,10 @@ static void migration_completion(MigrationState > *s) > > goto fail_invalidate; > > } > > > > - if (!migrate_colo_enabled()) { > > + if (migrate_colo_enabled()) { > > + migrate_set_state(&s->state, current_active_state, > > + MIGRATION_STATUS_COLO); > > + } else { > > migrate_set_state(&s->state, current_active_state, > > MIGRATION_STATUS_COMPLETED); > > } > > @@ -3304,12 +3307,7 @@ static void > migration_iteration_finish(MigrationState *s) > > migration_calculate_complete(s); > > runstate_set(RUN_STATE_POSTMIGRATE); > > break; > > - > > - case MIGRATION_STATUS_ACTIVE: > > - /* > > - * We should really assert here, but since it's during > > - * migration, let's try to reduce the usage of assertions. > > - */ > > + case MIGRATION_STATUS_COLO: > > if (!migrate_colo_enabled()) { > > error_report("%s: critical error: calling COLO code without " > > "COLO enabled", __func__); @@ -3319,6 > > +3317,11 @@ static void migration_iteration_finish(MigrationState *s) > > * Fixme: we will run VM in COLO no matter its old running state. > > * After exited COLO, we will keep running. > > */ > > + case MIGRATION_STATUS_ACTIVE: > > + /* > > + * We should really assert here, but since it's during > > + * migration, let's try to reduce the usage of assertions. > > + */ > > I think this case should be removed, because arriving here with > MIGRATION_STATUS_ACTIVE is invalid. It should be handled by the default > case. OK, looks good here. I will add this part in V3. Thanks Zhang Chen > > Regards, > Lukas Straub > > > s->vm_was_running = true; > > /* Fallthrough */ > > case MIGRATION_STATUS_FAILED: ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2 2/2] migration/colo: Update checkpoint time lately 2020-06-04 8:55 [PATCH V2 0/2] migration/colo: Optimize COLO framework code Zhang Chen 2020-06-04 8:55 ` [PATCH V2 1/2] migration/colo: Optimize COLO boot code path Zhang Chen @ 2020-06-04 8:55 ` Zhang Chen 2020-06-06 19:53 ` Lukas Straub 1 sibling, 1 reply; 6+ messages in thread From: Zhang Chen @ 2020-06-04 8:55 UTC (permalink / raw) To: Dr . David Alan Gilbert, qemu-dev; +Cc: Zhang Chen, Zhanghailiang, Zhang Chen From: Zhang Chen <chen.zhang@intel.com> Previous operation(like vm_start and replication_start_all) will consume extra time for first forced synchronization, so reduce it in this patch. Signed-off-by: Zhang Chen <chen.zhang@intel.com> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- migration/colo.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/migration/colo.c b/migration/colo.c index 91c76789fa..2b837e1255 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -532,7 +532,6 @@ static void colo_process_checkpoint(MigrationState *s) { QIOChannelBuffer *bioc; QEMUFile *fb = NULL; - int64_t current_time = qemu_clock_get_ms(QEMU_CLOCK_HOST); Error *local_err = NULL; int ret; @@ -581,8 +580,8 @@ static void colo_process_checkpoint(MigrationState *s) qemu_mutex_unlock_iothread(); trace_colo_vm_state_change("stop", "run"); - timer_mod(s->colo_delay_timer, - current_time + s->parameters.x_checkpoint_delay); + timer_mod(s->colo_delay_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) + + s->parameters.x_checkpoint_delay); while (s->state == MIGRATION_STATUS_COLO) { if (failover_get_state() != FAILOVER_STATUS_NONE) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2 2/2] migration/colo: Update checkpoint time lately 2020-06-04 8:55 ` [PATCH V2 2/2] migration/colo: Update checkpoint time lately Zhang Chen @ 2020-06-06 19:53 ` Lukas Straub 0 siblings, 0 replies; 6+ messages in thread From: Lukas Straub @ 2020-06-06 19:53 UTC (permalink / raw) To: Zhang Chen; +Cc: Zhanghailiang, Zhang Chen, Dr . David Alan Gilbert, qemu-dev [-- Attachment #1: Type: text/plain, Size: 575 bytes --] On Thu, 4 Jun 2020 16:55:33 +0800 Zhang Chen <chen.zhang@intel.com > wrote: > From: Zhang Chen <chen.zhang@intel.com> > > Previous operation(like vm_start and replication_start_all) will consume > extra time for first forced synchronization, so reduce it in this patch. > > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > --- Hi, Looks good and works well in my tests. Reviewed-by: Lukas Straub <lukasstraub2@web.de> Tested-by: Lukas Straub <lukasstraub2@web.de> Regards, Lukas Straub [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-07 19:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-06-04 8:55 [PATCH V2 0/2] migration/colo: Optimize COLO framework code Zhang Chen 2020-06-04 8:55 ` [PATCH V2 1/2] migration/colo: Optimize COLO boot code path Zhang Chen 2020-06-06 19:56 ` Lukas Straub 2020-06-07 19:42 ` Zhang, Chen 2020-06-04 8:55 ` [PATCH V2 2/2] migration/colo: Update checkpoint time lately Zhang Chen 2020-06-06 19:53 ` Lukas Straub
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).