Re: [Qemu-devel] [PATCH 09/18] migration (incoming): add error propagation for fd and exec protocols

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed,  3 Oct 2012 16:36:56 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> And remove the superfluous integer return value.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  migration-exec.c |  8 +++-----
>  migration-fd.c   |  8 +++-----
>  migration-tcp.c  |  7 ++-----
>  migration-unix.c |  5 ++---
>  migration.c      | 15 ++++++---------
>  migration.h      | 10 +++++-----
>  vl.c             | 16 ++++++----------
>  7 file modificati, 27 inserzioni(+), 42 rimozioni(-)
> 
> diff --git a/migration-exec.c b/migration-exec.c
> index f3baf85..232be6b 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -100,19 +100,17 @@ static void exec_accept_incoming_migration(void *opaque)
>      qemu_fclose(f);
>  }
>  
> -int exec_start_incoming_migration(const char *command)
> +void exec_start_incoming_migration(const char *command, Error **errp)
>  {
>      QEMUFile *f;
>  
>      DPRINTF("Attempting to start an incoming migration\n");
>      f = qemu_popen_cmd(command, "r");
>      if(f == NULL) {
> -        DPRINTF("Unable to apply qemu wrapper to popen file\n");
> -        return -errno;
> +        error_setg_errno(errp, errno, "failed to popen the migration source");
> +        return;
>      }
>  
>      qemu_set_fd_handler2(qemu_stdio_fd(f), NULL,
>  			 exec_accept_incoming_migration, NULL, f);
> -
> -    return 0;
>  }
> diff --git a/migration-fd.c b/migration-fd.c
> index 938a109..24eedd5 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -105,7 +105,7 @@ static void fd_accept_incoming_migration(void *opaque)
>      qemu_fclose(f);
>  }
>  
> -int fd_start_incoming_migration(const char *infd)
> +void fd_start_incoming_migration(const char *infd, Error **errp)
>  {
>      int fd;
>      QEMUFile *f;
> @@ -115,11 +115,9 @@ int fd_start_incoming_migration(const char *infd)
>      fd = strtol(infd, NULL, 0);
>      f = qemu_fdopen(fd, "rb");
>      if(f == NULL) {
> -        DPRINTF("Unable to apply qemu wrapper to file descriptor\n");
> -        return -errno;
> +        error_setg_errno(errp, errno, "failed to open the source descriptor");
> +        return;
>      }
>  
>      qemu_set_fd_handler2(fd, NULL, fd_accept_incoming_migration, NULL, f);
> -
> -    return 0;
>  }
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 5e54e68..46f6ac5 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -111,18 +111,15 @@ out2:
>      close(s);
>  }
>  
> -int tcp_start_incoming_migration(const char *host_port, Error **errp)
> +void tcp_start_incoming_migration(const char *host_port, Error **errp)
>  {
>      int s;
>  
>      s = inet_listen(host_port, NULL, 256, SOCK_STREAM, 0, errp);
> -
>      if (s < 0) {
> -        return -1;
> +        return;
>      }
>  
>      qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
>                           (void *)(intptr_t)s);
> -
> -    return 0;
>  }
> diff --git a/migration-unix.c b/migration-unix.c
> index 34a413a..ed3db3a 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -111,16 +111,15 @@ out2:
>      close(s);
>  }
>  
> -int unix_start_incoming_migration(const char *path, Error **errp)
> +void unix_start_incoming_migration(const char *path, Error **errp)
>  {
>      int s;
>  
>      s = unix_listen(path, NULL, 0, errp);
>      if (s < 0) {
> -        return -1;
> +        return;
>      }
>  
>      qemu_set_fd_handler2(s, NULL, unix_accept_incoming_migration, NULL,
>                           (void *)(intptr_t)s);
> -    return 0;
>  }
> diff --git a/migration.c b/migration.c
> index f7f0138..2802918 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -64,26 +64,23 @@ static MigrationState *migrate_get_current(void)
>      return &current_migration;
>  }
>  
> -int qemu_start_incoming_migration(const char *uri, Error **errp)
> +void qemu_start_incoming_migration(const char *uri, Error **errp)
>  {
>      const char *p;
> -    int ret;
>  
>      if (strstart(uri, "tcp:", &p))
> -        ret = tcp_start_incoming_migration(p, errp);
> +        tcp_start_incoming_migration(p, errp);
>  #if !defined(WIN32)
>      else if (strstart(uri, "exec:", &p))
> -        ret =  exec_start_incoming_migration(p);
> +        exec_start_incoming_migration(p, errp);
>      else if (strstart(uri, "unix:", &p))
> -        ret = unix_start_incoming_migration(p, errp);
> +        unix_start_incoming_migration(p, errp);
>      else if (strstart(uri, "fd:", &p))
> -        ret = fd_start_incoming_migration(p);
> +        fd_start_incoming_migration(p, errp);
>  #endif
>      else {
> -        fprintf(stderr, "unknown migration protocol: %s\n", uri);
> -        ret = -EPROTONOSUPPORT;
> +        error_setg(errp, "unknown migration protocol: %s\n", uri);
>      }
> -    return ret;
>  }
>  
>  void process_incoming_migration(QEMUFile *f)
> diff --git a/migration.h b/migration.h
> index 275d2d8..aa44e1b 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -46,7 +46,7 @@ struct MigrationState
>  
>  void process_incoming_migration(QEMUFile *f);
>  
> -int qemu_start_incoming_migration(const char *uri, Error **errp);
> +void qemu_start_incoming_migration(const char *uri, Error **errp);
>  
>  uint64_t migrate_max_downtime(void);
>  
> @@ -54,19 +54,19 @@ void do_info_migrate_print(Monitor *mon, const QObject *data);
>  
>  void do_info_migrate(Monitor *mon, QObject **ret_data);
>  
> -int exec_start_incoming_migration(const char *host_port);
> +void exec_start_incoming_migration(const char *host_port, Error **errp);
>  
>  void exec_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
>  
> -int tcp_start_incoming_migration(const char *host_port, Error **errp);
> +void tcp_start_incoming_migration(const char *host_port, Error **errp);
>  
>  void tcp_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);
>  
> -int unix_start_incoming_migration(const char *path, Error **errp);
> +void unix_start_incoming_migration(const char *path, Error **errp);
>  
>  void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp);
>  
> -int fd_start_incoming_migration(const char *path);
> +void fd_start_incoming_migration(const char *path, Error **errp);
>  
>  void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp);
>  
> diff --git a/vl.c b/vl.c
> index 8d305ca..53917c9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3747,16 +3747,12 @@ int main(int argc, char **argv, char **envp)
>      }
>  
>      if (incoming) {
> -        Error *errp = NULL;
> -        int ret = qemu_start_incoming_migration(incoming, &errp);
> -        if (ret < 0) {
> -            if (error_is_set(&errp)) {
> -                fprintf(stderr, "Migrate: %s\n", error_get_pretty(errp));
> -                error_free(errp);
> -            }
> -            fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n",
> -                    incoming, ret);
> -            exit(ret);
> +        Error *local_err = NULL;
> +        qemu_start_incoming_migration(incoming, &local_err);
> +        if (local_err) {
> +            fprintf(stderr, "-incoming %s: %s\n", incoming, error_get_pretty(local_err));
> +            error_free(local_err);
> +            exit(1);
>          }
>      } else if (autostart) {
>          vm_start();