| Message ID | 20180103054043.25719-2-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/11] migration: assert colo instead of check | expand |
Peter Xu <peterx@redhat.com> wrote: > When reaching here if we are still "active" it means we must be in colo > state. Assert it instead of check it in if condition. I don't think so. > Finally I want to use "switch" here rather than lots of complicated if > clauses. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > migration/migration.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 4de3b551fe..0ee4b4c27c 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2309,7 +2309,8 @@ static void *migration_thread(void *opaque) > } > runstate_set(RUN_STATE_POSTMIGRATE); > } else { > - if (s->state == MIGRATION_STATUS_ACTIVE && enable_colo) { > + if (s->state == MIGRATION_STATUS_ACTIVE) { We want to run this code iff: - we are in ACTIVE state - we are using colo We can be doing a normal migration, with colo compliled in, but not enabled, no? Later, Juan. > + assert(enable_colo); > migrate_start_colo_process(s); > qemu_savevm_state_cleanup(); > /*
On Wed, Jan 03, 2018 at 09:38:52AM +0100, Juan Quintela wrote: > Peter Xu <peterx@redhat.com> wrote: > > When reaching here if we are still "active" it means we must be in colo > > state. Assert it instead of check it in if condition. > > I don't think so. > > > Finally I want to use "switch" here rather than lots of complicated if > > clauses. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > --- > > migration/migration.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/migration/migration.c b/migration/migration.c > > index 4de3b551fe..0ee4b4c27c 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -2309,7 +2309,8 @@ static void *migration_thread(void *opaque) > > } > > runstate_set(RUN_STATE_POSTMIGRATE); > > } else { > > - if (s->state == MIGRATION_STATUS_ACTIVE && enable_colo) { > > + if (s->state == MIGRATION_STATUS_ACTIVE) { > > We want to run this code iff: > - we are in ACTIVE state > - we are using colo > > We can be doing a normal migration, with colo compliled in, but not > enabled, no? If COLO is not enabled (even if it is compiled in), IMHO we won't reach this state. Note that we have this in migration_completion(): if (!migrate_colo_enabled()) { migrate_set_state(&s->state, current_active_state, MIGRATION_STATUS_COMPLETED); } That's where we kept the ACTIVE state, and that should be the only place. And, this is exactly what this patch is going to do - I want to remove COLO hacks if possible. Thanks,
diff --git a/migration/migration.c b/migration/migration.c index 4de3b551fe..0ee4b4c27c 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2309,7 +2309,8 @@ static void *migration_thread(void *opaque) } runstate_set(RUN_STATE_POSTMIGRATE); } else { - if (s->state == MIGRATION_STATUS_ACTIVE && enable_colo) { + if (s->state == MIGRATION_STATUS_ACTIVE) { + assert(enable_colo); migrate_start_colo_process(s); qemu_savevm_state_cleanup(); /*
When reaching here if we are still "active" it means we must be in colo state. Assert it instead of check it in if condition. Finally I want to use "switch" here rather than lots of complicated if clauses. Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)