All of lore.kernel.org
 help / color / mirror / Atom feed
* [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

* [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

* 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

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.