Re: [Qemu-devel] [PATCH 21/36] migration: move migrate_new to do_migrate

[Anthony Liguori <anthony@codemonkey.ws>]

On 10/11/2011 05:00 AM, Juan Quintela wrote:
> Once there, remove all parameters that don't need to be passed to
> *start_outgoing_migration() functions
>
> Signed-off-by: Juan Quintela<quintela@redhat.com>

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

Regards,

Anthony Liguori

> ---
>   migration-exec.c |   19 +++++--------------
>   migration-fd.c   |   22 ++++++----------------
>   migration-tcp.c  |   22 +++++++---------------
>   migration-unix.c |   20 +++++---------------
>   migration.c      |   32 +++++++++++++++++++-------------
>   migration.h      |   31 ++++---------------------------
>   6 files changed, 46 insertions(+), 100 deletions(-)
>
> diff --git a/migration-exec.c b/migration-exec.c
> index d0119c6..b7b1055 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -61,22 +61,14 @@ static int exec_close(MigrationState *s)
>       return ret;
>   }
>
> -MigrationState *exec_start_outgoing_migration(Monitor *mon,
> -                                              const char *command,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc)
> +int exec_start_outgoing_migration(MigrationState *s, const char *command)
>   {
> -    MigrationState *s;
>       FILE *f;
>
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
>       f = popen(command, "w");
>       if (f == NULL) {
>           DPRINTF("Unable to popen exec target\n");
> -        goto err_after_alloc;
> +        goto err_after_popen;
>       }
>
>       s->fd = fileno(f);
> @@ -94,13 +86,12 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
>       s->write = file_write;
>
>       migrate_fd_connect(s);
> -    return s;
> +    return 0;
>
>   err_after_open:
>       pclose(f);
> -err_after_alloc:
> -    g_free(s);
> -    return NULL;
> +err_after_popen:
> +    return -1;
>   }
>
>   static void exec_accept_incoming_migration(void *opaque)
> diff --git a/migration-fd.c b/migration-fd.c
> index 9d3ca42..d0aec89 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -50,21 +50,12 @@ static int fd_close(MigrationState *s)
>       return 0;
>   }
>
> -MigrationState *fd_start_outgoing_migration(Monitor *mon,
> -					    const char *fdname,
> -					    int64_t bandwidth_limit,
> -					    int detach,
> -					    int blk,
> -					    int inc)
> +int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
>   {
> -    MigrationState *s;
> -
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
> -    s->fd = monitor_get_fd(mon, fdname);
> +    s->fd = monitor_get_fd(s->mon, fdname);
>       if (s->fd == -1) {
>           DPRINTF("fd_migration: invalid file descriptor identifier\n");
> -        goto err_after_alloc;
> +        goto err_after_get_fd;
>       }
>
>       if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
> @@ -77,13 +68,12 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
>       s->close = fd_close;
>
>       migrate_fd_connect(s);
> -    return s;
> +    return 0;
>
>   err_after_open:
>       close(s->fd);
> -err_after_alloc:
> -    g_free(s);
> -    return NULL;
> +err_after_get_fd:
> +    return -1;
>   }
>
>   static void fd_accept_incoming_migration(void *opaque)
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 999d4c9..f6b2288 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -75,30 +75,22 @@ static void tcp_wait_for_connect(void *opaque)
>       }
>   }
>
> -MigrationState *tcp_start_outgoing_migration(Monitor *mon,
> -                                             const char *host_port,
> -                                             int64_t bandwidth_limit,
> -                                             int detach,
> -					     int blk,
> -					     int inc)
> +int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
>   {
>       struct sockaddr_in addr;
> -    MigrationState *s;
>       int ret;
>
> -    if (parse_host_port(&addr, host_port)<  0)
> -        return NULL;
> -
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
> +    ret = parse_host_port(&addr, host_port);
> +    if (ret<  0) {
> +        return ret;
> +    }
>       s->get_error = socket_errno;
>       s->write = socket_write;
>       s->close = tcp_close;
>
>       s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
>       if (s->fd == -1) {
> -        g_free(s);
> -        return NULL;
> +        return -1;
>       }
>
>       socket_set_nonblock(s->fd);
> @@ -118,7 +110,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
>       } else if (ret>= 0)
>           migrate_fd_connect(s);
>
> -    return s;
> +    return 0;
>   }
>
>   static void tcp_accept_incoming_migration(void *opaque)
> diff --git a/migration-unix.c b/migration-unix.c
> index bee71d9..bd8d40f 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -74,22 +74,13 @@ static void unix_wait_for_connect(void *opaque)
>       }
>   }
>
> -MigrationState *unix_start_outgoing_migration(Monitor *mon,
> -                                              const char *path,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc)
> +int unix_start_outgoing_migration(MigrationState *s, const char *path)
>   {
> -    MigrationState *s;
>       struct sockaddr_un addr;
>       int ret;
>
>       addr.sun_family = AF_UNIX;
>       snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", path);
> -
> -    s = migrate_new(mon, bandwidth_limit, detach, blk, inc);
> -
>       s->get_error = unix_errno;
>       s->write = unix_write;
>       s->close = unix_close;
> @@ -97,7 +88,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
>       s->fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
>       if (s->fd<  0) {
>           DPRINTF("Unable to open socket");
> -        goto err_after_alloc;
> +        goto err_after_socket;
>       }
>
>       socket_set_nonblock(s->fd);
> @@ -119,14 +110,13 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
>       if (ret>= 0)
>           migrate_fd_connect(s);
>
> -    return s;
> +    return 0;
>
>   err_after_open:
>       close(s->fd);
>
> -err_after_alloc:
> -    g_free(s);
> -    return NULL;
> +err_after_socket:
> +    return -1;
>   }
>
>   static void unix_accept_incoming_migration(void *opaque)
> diff --git a/migration.c b/migration.c
> index 5a3e3ad..e93f3f7 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -77,6 +77,9 @@ void process_incoming_migration(QEMUFile *f)
>       }
>   }
>
> +static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> +                                   int detach, int blk, int inc);
> +
>   int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
>       MigrationState *s = NULL;
> @@ -85,6 +88,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>       int blk = qdict_get_try_bool(qdict, "blk", 0);
>       int inc = qdict_get_try_bool(qdict, "inc", 0);
>       const char *uri = qdict_get_str(qdict, "uri");
> +    int ret;
>
>       if (current_migration&&
>           current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
> @@ -96,28 +100,27 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           return -1;
>       }
>
> +    s = migrate_new(mon, max_throttle, detach, blk, inc);
> +
>       if (strstart(uri, "tcp:",&p)) {
> -        s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                         blk, inc);
> +        ret = tcp_start_outgoing_migration(s, p);
>   #if !defined(WIN32)
>       } else if (strstart(uri, "exec:",&p)) {
> -        s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                          blk, inc);
> +        ret = exec_start_outgoing_migration(s, p);
>       } else if (strstart(uri, "unix:",&p)) {
> -        s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                          blk, inc);
> +        ret = unix_start_outgoing_migration(s, p);
>       } else if (strstart(uri, "fd:",&p)) {
> -        s = fd_start_outgoing_migration(mon, p, max_throttle, detach,
> -                                        blk, inc);
> +        ret = fd_start_outgoing_migration(s, p);
>   #endif
>       } else {
>           monitor_printf(mon, "unknown migration protocol: %s\n", uri);
> -        return -1;
> +        ret  = -EINVAL;
> +        goto free_migrate_state;
>       }
>
> -    if (s == NULL) {
> +    if (ret<  0) {
>           monitor_printf(mon, "migration failed\n");
> -        return -1;
> +        goto free_migrate_state;
>       }
>
>       if (current_migration) {
> @@ -127,6 +130,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>       current_migration = s;
>       notifier_list_notify(&migration_state_notifiers, NULL);
>       return 0;
> +free_migrate_state:
> +    g_free(s);
> +    return -1;
>   }
>
>   int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
> @@ -489,8 +495,8 @@ void migrate_fd_connect(MigrationState *s)
>       migrate_fd_put_ready(s);
>   }
>
> -MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> -                                     int detach, int blk, int inc)
> +static MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> +                                   int detach, int blk, int inc)
>   {
>       MigrationState *s = g_malloc0(sizeof(*s));
>
> diff --git a/migration.h b/migration.h
> index 892b636..14c3ebc 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -64,47 +64,24 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);
>
>   int exec_start_incoming_migration(const char *host_port);
>
> -MigrationState *exec_start_outgoing_migration(Monitor *mon,
> -                                              const char *host_port,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc);
> +int exec_start_outgoing_migration(MigrationState *s, const char *host_port);
>
>   int tcp_start_incoming_migration(const char *host_port);
>
> -MigrationState *tcp_start_outgoing_migration(Monitor *mon,
> -                                             const char *host_port,
> -					     int64_t bandwidth_limit,
> -					     int detach,
> -					     int blk,
> -					     int inc);
> +int tcp_start_outgoing_migration(MigrationState *s, const char *host_port);
>
>   int unix_start_incoming_migration(const char *path);
>
> -MigrationState *unix_start_outgoing_migration(Monitor *mon,
> -                                              const char *path,
> -					      int64_t bandwidth_limit,
> -					      int detach,
> -					      int blk,
> -					      int inc);
> +int unix_start_outgoing_migration(MigrationState *s, const char *path);
>
>   int fd_start_incoming_migration(const char *path);
>
> -MigrationState *fd_start_outgoing_migration(Monitor *mon,
> -					    const char *fdname,
> -					    int64_t bandwidth_limit,
> -					    int detach,
> -					    int blk,
> -					    int inc);
> +int fd_start_outgoing_migration(MigrationState *s, const char *fdname);
>
>   void migrate_fd_error(MigrationState *s);
>
>   void migrate_fd_connect(MigrationState *s);
>
> -MigrationState *migrate_new(Monitor *mon, int64_t bandwidth_limit,
> -                            int detach, int blk, int inc);
> -
>   void add_migration_state_change_notifier(Notifier *notify);
>   void remove_migration_state_change_notifier(Notifier *notify);
>   int get_migration_state(void);