Re: [Qemu-devel] [PATCH v3 14/27] migration: convert fd socket protocol to use QIOChannel

[Dr. David Alan Gilbert]

* Daniel P. Berrange (address@hidden) wrote:
> Convert the fd socket migration protocol driver to use
> QIOChannel and QEMUFileChannel, instead of plain sockets
> APIs. It can be unconditionally built because the
> QIOChannel APIs it uses will take care to report suitable
> error messages if needed.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

(Dependent on the qio_channel_new_fd patch that doesn't seem
to be there yet).

Dave
> ---
>  migration/Makefile.objs |  4 +--
>  migration/fd.c          | 76 
> +++++++++++++++++++------------------------------
>  migration/migration.c   |  4 ---
>  trace-events            |  4 +++
>  4 files changed, 35 insertions(+), 53 deletions(-)
> 
> diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> index 59e8c09..0987bb6 100644
> --- a/migration/Makefile.objs
> +++ b/migration/Makefile.objs
> @@ -1,11 +1,11 @@
> -common-obj-y += migration.o socket.o
> +common-obj-y += migration.o socket.o fd.o
>  common-obj-y += vmstate.o
>  common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o 
> qemu-file-stdio.o
>  common-obj-y += qemu-file-channel.o
>  common-obj-y += xbzrle.o postcopy-ram.o
>  
>  common-obj-$(CONFIG_RDMA) += rdma.o
> -common-obj-$(CONFIG_POSIX) += exec.o fd.o
> +common-obj-$(CONFIG_POSIX) += exec.o
>  
>  common-obj-y += block.o
>  
> diff --git a/migration/fd.c b/migration/fd.c
> index 085dd7c..1a7fd43 100644
> --- a/migration/fd.c
> +++ b/migration/fd.c
> @@ -1,10 +1,11 @@
>  /*
>   * QEMU live migration via generic fd
>   *
> - * Copyright Red Hat, Inc. 2009
> + * Copyright Red Hat, Inc. 2009-2016
>   *
>   * Authors:
>   *  Chris Lalancette <address@hidden>
> + *  Daniel P. Berrange <address@hidden>
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2.  See
>   * the COPYING file in the top-level directory.
> @@ -14,76 +15,57 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu-common.h"
> -#include "qemu/main-loop.h"
> -#include "qemu/sockets.h"
>  #include "migration/migration.h"
>  #include "monitor/monitor.h"
> -#include "migration/qemu-file.h"
> -#include "block/block.h"
> +#include "io/channel-util.h"
> +#include "trace.h"
>  
> -//#define DEBUG_MIGRATION_FD
> -
> -#ifdef DEBUG_MIGRATION_FD
> -#define DPRINTF(fmt, ...) \
> -    do { printf("migration-fd: " fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> -#endif
> -
> -static bool fd_is_socket(int fd)
> -{
> -    struct stat stat;
> -    int ret = fstat(fd, &stat);
> -    if (ret == -1) {
> -        /* When in doubt say no */
> -        return false;
> -    }
> -    return S_ISSOCK(stat.st_mode);
> -}
>  
>  void fd_start_outgoing_migration(MigrationState *s, const char *fdname, 
> Error **errp)
>  {
> +    QIOChannel *ioc;
>      int fd = monitor_get_fd(cur_mon, fdname, errp);
>      if (fd == -1) {
>          return;
>      }
>  
> -    if (fd_is_socket(fd)) {
> -        s->to_dst_file = qemu_fopen_socket(fd, "wb");
> -    } else {
> -        s->to_dst_file = qemu_fdopen(fd, "wb");
> +    trace_migration_fd_outgoing(fd);
> +    ioc = qio_channel_new_fd(fd, errp);
> +    if (!ioc) {
> +        close(fd);
> +        return;
>      }
>  
> -    migrate_fd_connect(s);
> +    migration_set_outgoing_channel(s, ioc);
> +    object_unref(OBJECT(ioc));
>  }
>  
> -static void fd_accept_incoming_migration(void *opaque)
> +static gboolean fd_accept_incoming_migration(QIOChannel *ioc,
> +                                             GIOCondition condition,
> +                                             gpointer opaque)
>  {
> -    QEMUFile *f = opaque;
> -
> -    qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
> -    process_incoming_migration(f);
> +    migration_set_incoming_channel(migrate_get_current(), ioc);
> +    object_unref(OBJECT(ioc));
> +    return FALSE; /* unregister */
>  }
>  
>  void fd_start_incoming_migration(const char *infd, Error **errp)
>  {
> +    QIOChannel *ioc;
>      int fd;
> -    QEMUFile *f;
> -
> -    DPRINTF("Attempting to start an incoming migration via fd\n");
>  
>      fd = strtol(infd, NULL, 0);
> -    if (fd_is_socket(fd)) {
> -        f = qemu_fopen_socket(fd, "rb");
> -    } else {
> -        f = qemu_fdopen(fd, "rb");
> -    }
> -    if(f == NULL) {
> -        error_setg_errno(errp, errno, "failed to open the source 
> descriptor");
> +    trace_migration_fd_incoming(fd);
> +
> +    ioc = qio_channel_new_fd(fd, errp);
> +    if (!ioc) {
> +        close(fd);
>          return;
>      }
>  
> -    qemu_set_fd_handler(fd, fd_accept_incoming_migration, NULL, f);
> +    qio_channel_add_watch(ioc,
> +                          G_IO_IN,
> +                          fd_accept_incoming_migration,
> +                          NULL,
> +                          NULL);
>  }
> diff --git a/migration/migration.c b/migration/migration.c
> index da7e027..bbbd381 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -317,10 +317,8 @@ void qemu_start_incoming_migration(const char *uri, 
> Error **errp)
>  #endif
>      } else if (strstart(uri, "unix:", &p)) {
>          unix_start_incoming_migration(p, errp);
> -#if !defined(WIN32)
>      } else if (strstart(uri, "fd:", &p)) {
>          fd_start_incoming_migration(p, errp);
> -#endif
>      } else {
>          error_setg(errp, "unknown migration protocol: %s", uri);
>      }
> @@ -1055,10 +1053,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
> blk,
>  #endif
>      } else if (strstart(uri, "unix:", &p)) {
>          unix_start_outgoing_migration(s, p, &local_err);
> -#if !defined(WIN32)
>      } else if (strstart(uri, "fd:", &p)) {
>          fd_start_outgoing_migration(s, p, &local_err);
> -#endif
>      } else {
>          error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
>                     "a valid migration protocol");
> diff --git a/trace-events b/trace-events
> index 6608cf0..90afcff 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1600,6 +1600,10 @@ postcopy_ram_incoming_cleanup_entry(void) ""
>  postcopy_ram_incoming_cleanup_exit(void) ""
>  postcopy_ram_incoming_cleanup_join(void) ""
>  
> +# migration/fd.c
> +migration_fd_outgoing(int fd) "fd=%d"
> +migration_fd_incoming(int fd) "fd=%d"
> +
>  # migration/socket.c
>  migration_socket_incoming_accepted(void) ""
>  migration_socket_outgoing_connected(void) ""
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK