[Qemu-devel] [PATCH 08/11] migration: use switch at the end of migration

Peter Xu posted 11 patches 7 years, 9 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 08/11] migration: use switch at the end of migration
Posted by Peter Xu 7 years, 9 months ago
It converts the old if clauses into switch, explicitly mentions the
possible migration states.  The old nested "if"s are not clear on what
we do on different states.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index acef54748b..bfcba24caa 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2296,19 +2296,23 @@ static void *migration_thread(void *opaque)
     cpu_throttle_stop();
 
     qemu_mutex_lock_iothread();
-    if (s->state == MIGRATION_STATUS_COMPLETED) {
+    switch (s->state) {
+    case MIGRATION_STATUS_COMPLETED:
         migration_calculate_complete(s);
         runstate_set(RUN_STATE_POSTMIGRATE);
-    } else {
-        if (s->state == MIGRATION_STATUS_ACTIVE) {
-            assert(migrate_colo_enabled());
-            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.
-            */
-            s->old_vm_running = true;
-        }
+        break;
+
+    case MIGRATION_STATUS_ACTIVE:
+        assert(migrate_colo_enabled());
+        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.
+         */
+        s->old_vm_running = true;
+        /* Fallthrough */
+    case MIGRATION_STATUS_FAILED:
+    case MIGRATION_STATUS_CANCELLED:
         if (s->old_vm_running) {
             vm_start();
         } else {
@@ -2316,6 +2320,12 @@ static void *migration_thread(void *opaque)
                 runstate_set(RUN_STATE_POSTMIGRATE);
             }
         }
+        break;
+
+    default:
+        /* Should not reach here, but if so, forgive the VM. */
+        error_report("%s: Unknown ending state %d", __func__, s->state);
+        break;
     }
     qemu_bh_schedule(s->cleanup_bh);
     qemu_mutex_unlock_iothread();
-- 
2.14.3


Re: [Qemu-devel] [PATCH 08/11] migration: use switch at the end of migration
Posted by Juan Quintela 7 years, 9 months ago
Peter Xu <peterx@redhat.com> wrote:
> It converts the old if clauses into switch, explicitly mentions the
> possible migration states.  The old nested "if"s are not clear on what
> we do on different states.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c | 32 +++++++++++++++++++++-----------
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index acef54748b..bfcba24caa 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2296,19 +2296,23 @@ static void *migration_thread(void *opaque)
>      cpu_throttle_stop();
>  
>      qemu_mutex_lock_iothread();
> -    if (s->state == MIGRATION_STATUS_COMPLETED) {
> +    switch (s->state) {
> +    case MIGRATION_STATUS_COMPLETED:
>          migration_calculate_complete(s);
>          runstate_set(RUN_STATE_POSTMIGRATE);
> -    } else {
> -        if (s->state == MIGRATION_STATUS_ACTIVE) {
> -            assert(migrate_colo_enabled());
> -            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.
> -            */
> -            s->old_vm_running = true;
> -        }
> +        break;
> +
> +    case MIGRATION_STATUS_ACTIVE:
> +        assert(migrate_colo_enabled());
> +        migrate_start_colo_process(s);

I think this line is wrong, you still have to proctect it with the if
(migrate_cole_enabled).

Rest of cleanup, is something that I like.

Later, Juan.


> +        /*
> +         * Fixme: we will run VM in COLO no matter its old running state.
> +         * After exited COLO, we will keep running.
> +         */
> +        s->old_vm_running = true;
> +        /* Fallthrough */

Actually, I don't like Fallthrought on C, but well, problem is C O:-)

> +    case MIGRATION_STATUS_FAILED:
> +    case MIGRATION_STATUS_CANCELLED:
>          if (s->old_vm_running) {
>              vm_start();
>          } else {
> @@ -2316,6 +2320,12 @@ static void *migration_thread(void *opaque)
>                  runstate_set(RUN_STATE_POSTMIGRATE);
>              }
>          }
> +        break;
> +
> +    default:
> +        /* Should not reach here, but if so, forgive the VM. */
> +        error_report("%s: Unknown ending state %d", __func__, s->state);
> +        break;
>      }
>      qemu_bh_schedule(s->cleanup_bh);
>      qemu_mutex_unlock_iothread();