[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH V2 1/2] migration/colo: Optimize COLO boot code path
From: |
Zhang, Chen |
Subject: |
RE: [PATCH V2 1/2] migration/colo: Optimize COLO boot code path |
Date: |
Sun, 7 Jun 2020 19:42:07 +0000 |
> -----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: