diff mbox series

[2/3] migration/migration.c: Avoid COLO boot in postcopy migration

Message ID 20211231055935.1878503-3-chen.zhang@intel.com
State New
Headers show
Series Some minor fixes for migration states | expand

Commit Message

Zhang, Chen Dec. 31, 2021, 5:59 a.m. UTC
COLO dose not support postcopy migration and remove the Fixme.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 migration/migration.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Dr. David Alan Gilbert Jan. 19, 2022, 7:40 p.m. UTC | #1
* Zhang Chen (chen.zhang@intel.com) wrote:
> COLO dose not support postcopy migration and remove the Fixme.


'does' not 'dose'

> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  migration/migration.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 2afa77da03..3fac9c67ca 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3230,7 +3230,11 @@ static void migration_completion(MigrationState *s)
>          goto fail_invalidate;
>      }
>  
> -    if (!migrate_colo_enabled()) {
> +    if (migrate_colo_enabled() && s->state == MIGRATION_STATUS_ACTIVE) {
> +        /* COLO dose not support postcopy */
> +        migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> +                          MIGRATION_STATUS_COLO);

I'm a bit confused; where were we setting the source state to COLO
before - I can't find it!

Dave

> +    } else {
>          migrate_set_state(&s->state, current_active_state,
>                            MIGRATION_STATUS_COMPLETED);
>      }
> @@ -3621,10 +3625,6 @@ static void migration_iteration_finish(MigrationState *s)
>                           "COLO enabled", __func__);
>          }
>          migrate_start_colo_process(s);
> -        /*
> -         * Fixme: we will run VM in COLO no matter its old running state.
> -         * After exited COLO, we will keep running.
> -         */
>           /* Fallthrough */
>      case MIGRATION_STATUS_ACTIVE:
>          /*
> -- 
> 2.25.1
>
Zhang, Chen Jan. 20, 2022, 2:53 p.m. UTC | #2
> -----Original Message-----
> From: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Sent: Thursday, January 20, 2022 3:41 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Juan Quintela <quintela@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Subject: Re: [PATCH 2/3] migration/migration.c: Avoid COLO boot in postcopy
> migration
> 
> * Zhang Chen (chen.zhang@intel.com) wrote:
> > COLO dose not support postcopy migration and remove the Fixme.
> 
> 
> 'does' not 'dose'
> 

Yes, typo.

> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  migration/migration.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c index
> > 2afa77da03..3fac9c67ca 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -3230,7 +3230,11 @@ static void migration_completion(MigrationState
> *s)
> >          goto fail_invalidate;
> >      }
> >
> > -    if (!migrate_colo_enabled()) {
> > +    if (migrate_colo_enabled() && s->state ==
> MIGRATION_STATUS_ACTIVE) {
> > +        /* COLO dose not support postcopy */
> > +        migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
> > +                          MIGRATION_STATUS_COLO);
> 
> I'm a bit confused; where were we setting the source state to COLO before -
> I can't find it!

Yes, please read this mail:
https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg03525.html

After that I found I missed something, so this patch disabled postcopy migration and try to fix it.

Thanks
Chen

> 
> Dave
> 
> > +    } else {
> >          migrate_set_state(&s->state, current_active_state,
> >                            MIGRATION_STATUS_COMPLETED);
> >      }
> > @@ -3621,10 +3625,6 @@ static void
> migration_iteration_finish(MigrationState *s)
> >                           "COLO enabled", __func__);
> >          }
> >          migrate_start_colo_process(s);
> > -        /*
> > -         * Fixme: we will run VM in COLO no matter its old running state.
> > -         * After exited COLO, we will keep running.
> > -         */
> >           /* Fallthrough */
> >      case MIGRATION_STATUS_ACTIVE:
> >          /*
> > --
> > 2.25.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela Jan. 26, 2022, 7:30 p.m. UTC | #3
Zhang Chen <chen.zhang@intel.com> wrote:
> COLO dose not support postcopy migration and remove the Fixme.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

fixed the typo.
Zhang, Chen Jan. 27, 2022, 2:03 a.m. UTC | #4
> -----Original Message-----
> From: Juan Quintela <quintela@redhat.com>
> Sent: Thursday, January 27, 2022 3:30 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Subject: Re: [PATCH 2/3] migration/migration.c: Avoid COLO boot in postcopy
> migration
> 
> Zhang Chen <chen.zhang@intel.com> wrote:
> > COLO dose not support postcopy migration and remove the Fixme.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> fixed the typo.

OK, already updated V2 for this series.

Thanks
Chen
diff mbox series

Patch

diff --git a/migration/migration.c b/migration/migration.c
index 2afa77da03..3fac9c67ca 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3230,7 +3230,11 @@  static void migration_completion(MigrationState *s)
         goto fail_invalidate;
     }
 
-    if (!migrate_colo_enabled()) {
+    if (migrate_colo_enabled() && s->state == MIGRATION_STATUS_ACTIVE) {
+        /* COLO dose not support postcopy */
+        migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
+                          MIGRATION_STATUS_COLO);
+    } else {
         migrate_set_state(&s->state, current_active_state,
                           MIGRATION_STATUS_COMPLETED);
     }
@@ -3621,10 +3625,6 @@  static void migration_iteration_finish(MigrationState *s)
                          "COLO enabled", __func__);
         }
         migrate_start_colo_process(s);
-        /*
-         * Fixme: we will run VM in COLO no matter its old running state.
-         * After exited COLO, we will keep running.
-         */
          /* Fallthrough */
     case MIGRATION_STATUS_ACTIVE:
         /*