[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 09/22] migration: convert unix socket protoco
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v1 09/22] migration: convert unix socket protocol to use QIOChannel |
Date: |
Wed, 3 Feb 2016 11:25:58 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, Feb 02, 2016 at 06:02:13PM +0000, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (address@hidden) wrote:
> > Convert the unix socket migration protocol driver to use
> > QIOChannel and QEMUFileChannel, instead of plain sockets
> > APIs. It can be unconditionally built, since the socket
> > impl of QIOChannel will report a suitable error on platforms
> > where UNIX sockets are unavailable.
> >
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> > migration/Makefile.objs | 4 +-
> > migration/migration.c | 4 ++
> > migration/unix.c | 103
> > +++++++++++++++++++++++++++++++-----------------
> > 3 files changed, 72 insertions(+), 39 deletions(-)
> >
> > diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> > index b357e2f..a5f8a03 100644
> > --- a/migration/Makefile.objs
> > +++ b/migration/Makefile.objs
> > @@ -1,11 +1,11 @@
> > -common-obj-y += migration.o tcp.o
> > +common-obj-y += migration.o tcp.o unix.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 unix.o fd.o
> > +common-obj-$(CONFIG_POSIX) += exec.o fd.o
> >
> > common-obj-y += block.o
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index e921b20..1c5f12e 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -312,8 +312,10 @@ void qemu_start_incoming_migration(const char *uri,
> > Error **errp)
> > #if !defined(WIN32)
> > } else if (strstart(uri, "exec:", &p)) {
> > exec_start_incoming_migration(p, 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
> > @@ -1017,8 +1019,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> > blk,
> > #if !defined(WIN32)
> > } else if (strstart(uri, "exec:", &p)) {
> > exec_start_outgoing_migration(s, p, &local_err);
> > +#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
> > diff --git a/migration/unix.c b/migration/unix.c
> > index b591813..4674640 100644
> > --- a/migration/unix.c
> > +++ b/migration/unix.c
> > @@ -1,10 +1,11 @@
> > /*
> > * QEMU live migration via Unix Domain Sockets
> > *
> > - * Copyright Red Hat, Inc. 2009
> > + * Copyright Red Hat, Inc. 2009-2015
>
> year++ ?
>
> > *
> > * 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.
> > @@ -17,11 +18,9 @@
> >
> > #include "qemu-common.h"
> > #include "qemu/error-report.h"
> > -#include "qemu/sockets.h"
> > -#include "qemu/main-loop.h"
> > #include "migration/migration.h"
> > #include "migration/qemu-file.h"
> > -#include "block/block.h"
> > +#include "io/channel-socket.h"
> >
> > //#define DEBUG_MIGRATION_UNIX
> >
> > @@ -33,71 +32,101 @@
> > do { } while (0)
> > #endif
> >
> > -static void unix_wait_for_connect(int fd, Error *err, void *opaque)
> > +
> > +static SocketAddress *unix_build_address(const char *path)
> > +{
> > + SocketAddress *saddr;
> > +
> > + saddr = g_new0(SocketAddress, 1);
> > + saddr->type = SOCKET_ADDRESS_KIND_UNIX;
> > + saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
> > + saddr->u.q_unix->path = g_strdup(path);
> > +
> > + return saddr;
> > +}
> > +
> > +
> > +static void unix_outgoing_migration(Object *src,
> > + Error *err,
> > + gpointer opaque)
> > {
> > MigrationState *s = opaque;
> > + QIOChannel *sioc = QIO_CHANNEL(src);
> >
> > - if (fd < 0) {
> > + if (err) {
> > DPRINTF("migrate connect error: %s\n", error_get_pretty(err));
> > s->file = NULL;
> > migrate_fd_error(s);
> > } else {
> > DPRINTF("migrate connect success\n");
> > - s->file = qemu_fopen_socket(fd, "wb");
> > + s->file = qemu_fopen_channel_output(sioc);
> > migrate_fd_connect(s);
> > }
> > + object_unref(src);
> > }
> >
> > +
> > void unix_start_outgoing_migration(MigrationState *s, const char *path,
> > Error **errp)
> > {
> > - unix_nonblocking_connect(path, unix_wait_for_connect, s, errp);
> > + SocketAddress *saddr = unix_build_address(path);
> > + QIOChannelSocket *sioc;
> > + sioc = qio_channel_socket_new();
> > + qio_channel_socket_connect_async(sioc,
> > + saddr,
> > + unix_outgoing_migration,
> > + s,
> > + NULL);
> > + qapi_free_SocketAddress(saddr);
>
> It took me a bit of digging to see that qio_channel_socket_connect_async
> takes a copy of the address; it's not mentioned in channel-socket.h
>
> > }
> >
> > -static void unix_accept_incoming_migration(void *opaque)
> > +
> > +static gboolean unix_accept_incoming_migration(QIOChannel *ioc,
> > + GIOCondition condition,
> > + gpointer opaque)
> > {
> > - struct sockaddr_un addr;
> > - socklen_t addrlen = sizeof(addr);
> > - int s = (intptr_t)opaque;
> > QEMUFile *f;
> > - int c, err;
> > -
> > - do {
> > - c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
> > - err = errno;
> > - } while (c < 0 && err == EINTR);
> > - qemu_set_fd_handler(s, NULL, NULL, NULL);
> > - close(s);
> > + QIOChannelSocket *cioc;
>
> (not important but...) why is that a cioc not a sioc?
IIRC an earlier version of this patch had already used sioc for another
variable. I'll change this now though.
>
> > + Error *err = NULL;
> >
> > - DPRINTF("accepted migration\n");
> > -
> > - if (c < 0) {
> > + cioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
> > + &err);
> > + if (!cioc) {
> > error_report("could not accept migration connection (%s)",
> > - strerror(err));
> > - return;
> > - }
> > -
> > - f = qemu_fopen_socket(c, "rb");
> > - if (f == NULL) {
> > - error_report("could not qemu_fopen socket");
> > + error_get_pretty(err));
> > goto out;
> > }
> >
> > + DPRINTF("accepted migration\n");
>
> Feel free to replace any DPRINTF's by trace_ entries.
Rather than change all my patches in this regard, I'll create an
add-on patch to introduce some useful trace_ entries across all
the migration backends in one go.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|