Re: [Qemu-devel] [PATCH 23/36] migration: Refactor and simplify error checking in migrate_fd_put_ready

[Anthony Liguori <anthony@codemonkey.ws>]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Just a general comment.  This series is fairly touch to review because you're 
repeated refactoring the same function with no other comments beyond "refactor 
and simplify".

As a reviewer, I really have no help in understanding your motiviation for 
making this changes, why it can't be done all at once, etc.

I'm not rejecting this patch, but in the future, please put a little more care 
into better rationale in commit messages to simplify reviewing.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori


> ---
>   migration.c |   21 ++++++++++-----------
>   1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index a01bf4f..432afe6 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -372,23 +372,22 @@ static void migrate_fd_put_ready(void *opaque)
>           DPRINTF("done iterating\n");
>           vm_stop(RUN_STATE_FINISH_MIGRATE);
>
> -        if ((qemu_savevm_state_complete(s->mon, s->file))<  0) {
> -            if (old_vm_running) {
> -                vm_start();
> +        if (qemu_savevm_state_complete(s->mon, s->file)<  0) {
> +            migrate_fd_error(s);
> +        } else {
> +            if (migrate_fd_cleanup(s)<  0) {
> +                migrate_fd_error(s);
> +            } else {
> +                s->state = MIG_STATE_COMPLETED;
> +                runstate_set(RUN_STATE_POSTMIGRATE);
> +                notifier_list_notify(&migration_state_notifiers, NULL);
>               }
> -            s->state = MIG_STATE_ERROR;
>           }
> -        if (migrate_fd_cleanup(s)<  0) {
> +        if (s->get_status(s) != MIG_STATE_COMPLETED) {
>               if (old_vm_running) {
>                   vm_start();
>               }
> -            s->state = MIG_STATE_ERROR;
>           }
> -        if (s->state == MIG_STATE_ACTIVE) {
> -            s->state = MIG_STATE_COMPLETED;
> -            runstate_set(RUN_STATE_POSTMIGRATE);
> -        }
> -        notifier_list_notify(&migration_state_notifiers, NULL);
>       }
>   }
>