[PATCH V3 0/3] migration/colo: Optimize COLO framework code

[PATCH V3 0/3] migration/colo: Optimize COLO framework code

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

This series optimize some code of COLO, please review.

Zhang Chen (3):
  migration/colo: Optimize COLO boot code path
  migration/colo: Update checkpoint time lately
  migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in
    migration_iteration_finish

 migration/colo.c      |  7 ++-----
 migration/migration.c | 12 +++++-------
 2 files changed, 7 insertions(+), 12 deletions(-)

-- 
2.17.1

[PATCH V3 1/3] migration/colo: Optimize COLO boot code path

[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..9059238e3d 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__);
@@ -3321,6 +3319,11 @@ static void migration_iteration_finish(MigrationState *s)
          */
         s->vm_was_running = true;
         /* Fallthrough */
+    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_FAILED:
     case MIGRATION_STATUS_CANCELLED:
     case MIGRATION_STATUS_CANCELLING:
-- 
2.17.1

[PATCH V3 2/3] migration/colo: Update checkpoint time lately

[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>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Tested-by: Lukas Straub <lukasstraub2@web.de>
---
 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

[PATCH V3 3/3] migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in migration_iteration_finish

[Zhang Chen]

From: Zhang Chen <chen.zhang@intel.com>

MIGRATION_STATUS_ACTIVE is invalid here, handle it by default case.

Suggested-by: Lukas Straub <lukasstraub2@web.de>
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
---
 migration/migration.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 9059238e3d..9958b15202 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3319,11 +3319,6 @@ static void migration_iteration_finish(MigrationState *s)
          */
         s->vm_was_running = true;
         /* Fallthrough */
-    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_FAILED:
     case MIGRATION_STATUS_CANCELLED:
     case MIGRATION_STATUS_CANCELLING:
-- 
2.17.1

Re: [PATCH V3 1/3] migration/colo: Optimize COLO boot code path

[Lukas Straub]

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

On Mon,  8 Jun 2020 03:46:09 +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>
> ---

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 --]

Re: [PATCH V3 3/3] migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in migration_iteration_finish

[Lukas Straub]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On Mon,  8 Jun 2020 03:46:11 +0800
Zhang Chen <chen.zhang@intel.com > wrote:

> From: Zhang Chen <chen.zhang@intel.com>
> 
> MIGRATION_STATUS_ACTIVE is invalid here, handle it by default case.
> 
> Suggested-by: Lukas Straub <lukasstraub2@web.de>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> ---

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 --]

RE: [PATCH V3 0/3] migration/colo: Optimize COLO framework code

[Zhang, Chen]

Hi Dave,

Any comments for this series?

Thanks
Zhang Chen

> -----Original Message-----
> From: Zhang, Chen <chen.zhang@intel.com>
> Sent: Monday, June 8, 2020 3:46 AM
> To: Dr . David Alan Gilbert <dgilbert@redhat.com>; qemu-dev <qemu-
> devel@nongnu.org>
> Cc: Zhang Chen <zhangckid@gmail.com>; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Zhang, Chen <chen.zhang@intel.com>
> Subject: [PATCH V3 0/3] migration/colo: Optimize COLO framework code
> 
> From: Zhang Chen <chen.zhang@intel.com>
> 
> This series optimize some code of COLO, please review.
> 
> Zhang Chen (3):
>   migration/colo: Optimize COLO boot code path
>   migration/colo: Update checkpoint time lately
>   migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in
>     migration_iteration_finish
> 
>  migration/colo.c      |  7 ++-----
>  migration/migration.c | 12 +++++-------
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> --
> 2.17.1

Re: [PATCH V3 0/3] migration/colo: Optimize COLO framework code

[Dr. David Alan Gilbert]

* Zhang, Chen (chen.zhang@intel.com) wrote:
> Hi Dave,
> 

I think it's OK; I guess Zhanghailiang is going to do a pull for it?
One thought about the change of use of MIGRATION_STATUS_ACTIVE; I think
it's OK at the moment - but the migration states and the way they change
becomes part of the API - changing which states we use can confuse
management applications; since there's no libvirt support, I'll leave
it to Zhanghailiang to know if that's OK.

Dave


> 
> Thanks
> Zhang Chen
> 
> > -----Original Message-----
> > From: Zhang, Chen <chen.zhang@intel.com>
> > Sent: Monday, June 8, 2020 3:46 AM
> > To: Dr . David Alan Gilbert <dgilbert@redhat.com>; qemu-dev <qemu-
> > devel@nongnu.org>
> > Cc: Zhang Chen <zhangckid@gmail.com>; Zhanghailiang
> > <zhang.zhanghailiang@huawei.com>; Zhang, Chen <chen.zhang@intel.com>
> > Subject: [PATCH V3 0/3] migration/colo: Optimize COLO framework code
> > 
> > From: Zhang Chen <chen.zhang@intel.com>
> > 
> > This series optimize some code of COLO, please review.
> > 
> > Zhang Chen (3):
> >   migration/colo: Optimize COLO boot code path
> >   migration/colo: Update checkpoint time lately
> >   migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in
> >     migration_iteration_finish
> > 
> >  migration/colo.c      |  7 ++-----
> >  migration/migration.c | 12 +++++-------
> >  2 files changed, 7 insertions(+), 12 deletions(-)
> > 
> > --
> > 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

RE: [PATCH V3 0/3] migration/colo: Optimize COLO framework code

[Zhang, Chen]

> -----Original Message-----
> From: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Sent: Wednesday, June 17, 2020 8:26 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: qemu-dev <qemu-devel@nongnu.org>; Zhang Chen
> <zhangckid@gmail.com>; Zhanghailiang <zhang.zhanghailiang@huawei.com>
> Subject: Re: [PATCH V3 0/3] migration/colo: Optimize COLO framework code
> 
> * Zhang, Chen (chen.zhang@intel.com) wrote:
> > Hi Dave,
> >
> 
> I think it's OK; I guess Zhanghailiang is going to do a pull for it?
> One thought about the change of use of MIGRATION_STATUS_ACTIVE; I
> think it's OK at the moment - but the migration states and the way they
> change becomes part of the API - changing which states we use can confuse
> management applications; since there's no libvirt support, I'll leave it to
> Zhanghailiang to know if that's OK.

Hi Hailiang/Dave,

Current upstream libvirt can't support COLO, it looks no new issue for normal migration process.
For the future, new COLO migration status is good for COLO enable work on libvirt and upper layer.

Thanks
Zhang Chen

> 
> Dave
> 
> 
> >
> > Thanks
> > Zhang Chen
> >
> > > -----Original Message-----
> > > From: Zhang, Chen <chen.zhang@intel.com>
> > > Sent: Monday, June 8, 2020 3:46 AM
> > > To: Dr . David Alan Gilbert <dgilbert@redhat.com>; qemu-dev <qemu-
> > > devel@nongnu.org>
> > > Cc: Zhang Chen <zhangckid@gmail.com>; Zhanghailiang
> > > <zhang.zhanghailiang@huawei.com>; Zhang, Chen
> <chen.zhang@intel.com>
> > > Subject: [PATCH V3 0/3] migration/colo: Optimize COLO framework code
> > >
> > > From: Zhang Chen <chen.zhang@intel.com>
> > >
> > > This series optimize some code of COLO, please review.
> > >
> > > Zhang Chen (3):
> > >   migration/colo: Optimize COLO boot code path
> > >   migration/colo: Update checkpoint time lately
> > >   migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in
> > >     migration_iteration_finish
> > >
> > >  migration/colo.c      |  7 ++-----
> > >  migration/migration.c | 12 +++++-------
> > >  2 files changed, 7 insertions(+), 12 deletions(-)
> > >
> > > --
> > > 2.17.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

RE: [PATCH V3 0/3] migration/colo: Optimize COLO framework code

[Zhang, Chen]

Hi Hailiang/Dave,

No news for a while, any comments?

Thanks
Zhang Chen

> -----Original Message-----
> From: Qemu-devel <qemu-devel-
> bounces+chen.zhang=intel.com@nongnu.org> On Behalf Of Zhang, Chen
> Sent: Monday, June 22, 2020 2:53 PM
> To: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Zhanghailiang <zhang.zhanghailiang@huawei.com>; qemu-dev <qemu-
> devel@nongnu.org>; Zhang Chen <zhangckid@gmail.com>
> Subject: RE: [PATCH V3 0/3] migration/colo: Optimize COLO framework code
> 
> 
> 
> > -----Original Message-----
> > From: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Sent: Wednesday, June 17, 2020 8:26 PM
> > To: Zhang, Chen <chen.zhang@intel.com>
> > Cc: qemu-dev <qemu-devel@nongnu.org>; Zhang Chen
> > <zhangckid@gmail.com>; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>
> > Subject: Re: [PATCH V3 0/3] migration/colo: Optimize COLO framework
> > code
> >
> > * Zhang, Chen (chen.zhang@intel.com) wrote:
> > > Hi Dave,
> > >
> >
> > I think it's OK; I guess Zhanghailiang is going to do a pull for it?
> > One thought about the change of use of MIGRATION_STATUS_ACTIVE; I
> > think it's OK at the moment - but the migration states and the way
> > they change becomes part of the API - changing which states we use can
> > confuse management applications; since there's no libvirt support,
> > I'll leave it to Zhanghailiang to know if that's OK.
> 
> Hi Hailiang/Dave,
> 
> Current upstream libvirt can't support COLO, it looks no new issue for normal
> migration process.
> For the future, new COLO migration status is good for COLO enable work on
> libvirt and upper layer.
> 
> Thanks
> Zhang Chen
> 
> >
> > Dave
> >
> >
> > >
> > > Thanks
> > > Zhang Chen
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Chen <chen.zhang@intel.com>
> > > > Sent: Monday, June 8, 2020 3:46 AM
> > > > To: Dr . David Alan Gilbert <dgilbert@redhat.com>; qemu-dev <qemu-
> > > > devel@nongnu.org>
> > > > Cc: Zhang Chen <zhangckid@gmail.com>; Zhanghailiang
> > > > <zhang.zhanghailiang@huawei.com>; Zhang, Chen
> > <chen.zhang@intel.com>
> > > > Subject: [PATCH V3 0/3] migration/colo: Optimize COLO framework
> > > > code
> > > >
> > > > From: Zhang Chen <chen.zhang@intel.com>
> > > >
> > > > This series optimize some code of COLO, please review.
> > > >
> > > > Zhang Chen (3):
> > > >   migration/colo: Optimize COLO boot code path
> > > >   migration/colo: Update checkpoint time lately
> > > >   migration/migration.c: Remove MIGRATION_STATUS_ACTIVE in
> > > >     migration_iteration_finish
> > > >
> > > >  migration/colo.c      |  7 ++-----
> > > >  migration/migration.c | 12 +++++-------
> > > >  2 files changed, 7 insertions(+), 12 deletions(-)
> > > >
> > > > --
> > > > 2.17.1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>