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

[Juan Quintela <quintela@redhat.com>]

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();