Re: [Qemu-devel] [PATCH 1/5] Exit if incoming migration fails

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 25 May 2010 16:21:01 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c |   16 ++++++++++------
>  migration.h |    2 +-
>  vl.c        |    7 ++++++-
>  3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 05f6cc5..9c1d4b6 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -36,22 +36,26 @@ static uint32_t max_throttle = (32 << 20);
> 
>  static MigrationState *current_migration;
> 
> -void qemu_start_incoming_migration(const char *uri)
> +int qemu_start_incoming_migration(const char *uri)
>  {
>      const char *p;
> +    int ret;
> 
>      if (strstart(uri, "tcp:", &p))
> -        tcp_start_incoming_migration(p);
> +        ret = tcp_start_incoming_migration(p);
>  #if !defined(WIN32)
>      else if (strstart(uri, "exec:", &p))
> -        exec_start_incoming_migration(p);
> +        ret =  exec_start_incoming_migration(p);
>      else if (strstart(uri, "unix:", &p))
> -        unix_start_incoming_migration(p);
> +        ret = unix_start_incoming_migration(p);
>      else if (strstart(uri, "fd:", &p))
> -        fd_start_incoming_migration(p);
> +        ret = fd_start_incoming_migration(p);
>  #endif
> -    else
> +    else {
>          fprintf(stderr, "unknown migration protocol: %s\n", uri);
> +        ret = -EPROTONOSUPPORT;
> +    }
> +    return ret;
>  }
> 
>  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/migration.h b/migration.h
> index 385423f..dd423a1 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -50,7 +50,7 @@ struct FdMigrationState
>      void *opaque;
>  };
> 
> -void qemu_start_incoming_migration(const char *uri);
> +int qemu_start_incoming_migration(const char *uri);
> 
>  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
> 
> diff --git a/vl.c b/vl.c
> index 328395e..d13440d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3823,7 +3823,12 @@ int main(int argc, char **argv, char **envp)
>      }
> 
>      if (incoming) {
> -        qemu_start_incoming_migration(incoming);
> +        int ret = qemu_start_incoming_migration(incoming);
> +        if (ret < 0) {
> +            fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n",
> +                    incoming, ret);
> +            exit(ret);

 While I agree on the change, I have two comments:

1. By taking a look at the code I have the impression that most of the
   fun failures will happen on the handler passed to qemu_set_fd_handler2(),
   do you agree? Any plan to address that?

1. Is exit()ing the best thing to be done? I understand it's the easiest
   and maybe better than nothing, but wouldn't it be better to enter in
   paused-forever state so that clients can query and decide what to do?

> +        }
>      } else if (autostart) {
>          vm_start();
>      }