Skip to contentSkip to conversation

migration: convert socket server to QIONetListener

PATCH In review Daniel P. Berrangé 3 participants 7 messages Mar 12, 2018 – Mar 23, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 migration: convert socket server to QIONetListener

From: "Daniel P. Berrange" <[email protected]>

Instead of creating a QIOChannelSocket directly for the migration server socket, use a QIONetListener. This provides the ability to listen on multiple sockets at the same time, so enables full support for IPv4/IPv6 dual stack.

For example, '$QEMU -incoming tcp::9000' now correctly listens
on both 0.0.0.0 and :: at the same time, instead of only on 0.0.0.0.

---
 migration/socket.c | 48 ++++++++++++++++--------------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)
migration/socket.c +16 -32
index 8a93fb1af5..122d8ccfbe 100644
@@ -24,6 +24,7 @@
 #include "migration.h"
 #include "qemu-file.h"
 #include "io/channel-socket.h"
+#include "io/net-listener.h"
 #include "trace.h"
 
 
@@ -129,34 +130,20 @@ void unix_start_outgoing_migration(MigrationState *s,
 }
 
 
-static gboolean socket_accept_incoming_migration(QIOChannel *ioc,
-                                                 GIOCondition condition,
-                                                 gpointer opaque)
+static void socket_accept_incoming_migration(QIONetListener *listener,
+                                             QIOChannelSocket *cioc,
+                                             gpointer opaque)
 {
-    QIOChannelSocket *sioc;
-    Error *err = NULL;
-
-    sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
-                                     &err);
-    if (!sioc) {
-        error_report("could not accept migration connection (%s)",
-                     error_get_pretty(err));
-        goto out;
-    }
-
     trace_migration_socket_incoming_accepted();
 
-    qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-incoming");
-    migration_channel_process_incoming(QIO_CHANNEL(sioc));
-    object_unref(OBJECT(sioc));
+    qio_channel_set_name(QIO_CHANNEL(cioc), "migration-socket-incoming");
+    migration_channel_process_incoming(QIO_CHANNEL(cioc));
 
-out:
     if (migration_has_all_channels()) {
         /* Close listening socket as its no longer needed */
-        qio_channel_close(ioc, NULL);
-        return G_SOURCE_REMOVE;
-    } else {
-        return G_SOURCE_CONTINUE;
+        qio_net_listener_disconnect(listener);
+
+        object_unref(OBJECT(listener));
     }
 }
 
@@ -164,21 +151,18 @@ out:
 static void socket_start_incoming_migration(SocketAddress *saddr,
                                             Error **errp)
 {
-    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    QIONetListener *listener = qio_net_listener_new();
 
-    qio_channel_set_name(QIO_CHANNEL(listen_ioc),
-                         "migration-socket-listener");
+    qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
+    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+        object_unref(OBJECT(listener));
         return;
     }
 
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          socket_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+    qio_net_listener_set_client_func(listener,
+                                     socket_accept_incoming_migration,
+                                     NULL, NULL);
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
Signature · 1 line
2.14.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 11 lines · * Daniel P. Berrangé ([email protected]) wrote:

From: "Daniel P. Berrange" <[email protected]>

Instead of creating a QIOChannelSocket directly for the migration server socket, use a QIONetListener. This provides the ability to listen on multiple sockets at the same time, so enables full support for IPv4/IPv6 dual stack.

For example, '$QEMU -incoming tcp::9000' now correctly listens
on both 0.0.0.0 and :: at the same time, instead of only on 0.0.0.0.

Quoted 91 lines
---
 migration/socket.c | 48 ++++++++++++++++--------------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)
migration/socket.c +16 -32
index 8a93fb1af5..122d8ccfbe 100644
@@ -24,6 +24,7 @@
 #include "migration.h"
 #include "qemu-file.h"
 #include "io/channel-socket.h"
+#include "io/net-listener.h"
 #include "trace.h"
 
 
@@ -129,34 +130,20 @@ void unix_start_outgoing_migration(MigrationState *s,
 }
 
 
-static gboolean socket_accept_incoming_migration(QIOChannel *ioc,
-                                                 GIOCondition condition,
-                                                 gpointer opaque)
+static void socket_accept_incoming_migration(QIONetListener *listener,
+                                             QIOChannelSocket *cioc,
+                                             gpointer opaque)
 {
-    QIOChannelSocket *sioc;
-    Error *err = NULL;
-
-    sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
-                                     &err);
-    if (!sioc) {
-        error_report("could not accept migration connection (%s)",
-                     error_get_pretty(err));
-        goto out;
-    }
-
     trace_migration_socket_incoming_accepted();
 
-    qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-incoming");
-    migration_channel_process_incoming(QIO_CHANNEL(sioc));
-    object_unref(OBJECT(sioc));
+    qio_channel_set_name(QIO_CHANNEL(cioc), "migration-socket-incoming");
+    migration_channel_process_incoming(QIO_CHANNEL(cioc));
 
-out:
     if (migration_has_all_channels()) {
         /* Close listening socket as its no longer needed */
-        qio_channel_close(ioc, NULL);
-        return G_SOURCE_REMOVE;
-    } else {
-        return G_SOURCE_CONTINUE;
+        qio_net_listener_disconnect(listener);
+
+        object_unref(OBJECT(listener));
     }
 }
 
@@ -164,21 +151,18 @@ out:
 static void socket_start_incoming_migration(SocketAddress *saddr,
                                             Error **errp)
 {
-    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    QIONetListener *listener = qio_net_listener_new();
 
-    qio_channel_set_name(QIO_CHANNEL(listen_ioc),
-                         "migration-socket-listener");
+    qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
+    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+        object_unref(OBJECT(listener));
         return;
     }
 
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          socket_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+    qio_net_listener_set_client_func(listener,
+                                     socket_accept_incoming_migration,
+                                     NULL, NULL);
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
Signature · 1 line
2.14.3

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#2 lore raw
Quoted 27 lines · Dr. David Alan Gilbert
Quoted 27 lines

static void socket_start_incoming_migration(SocketAddress *saddr,
Error **errp)
{

migration/socket.c +7 -10
-    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    QIONetListener *listener = qio_net_listener_new();
 
-    qio_channel_set_name(QIO_CHANNEL(listen_ioc),
-                         "migration-socket-listener");
+    qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
+    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+        object_unref(OBJECT(listener));
         return;
     }
 
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          socket_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+    qio_net_listener_set_client_func(listener,
+                                     socket_accept_incoming_migration,
+                                     NULL, NULL);
 }
Quoted 7 lines
Quoted 4 lines

void tcp_start_incoming_migration(const char *host_port, Error **errp)

Signature · 1 line
2.14.3

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Could this get in 2.12 as a bugfix, even after soft freeze? It's the last user of qio_channel_listen_sync that blocks full IPv4/IPv6 support.
All other users are for vsock/AF_UNIX.

Paolo

#3 lore raw
Quoted 32 lines · Paolo Bonzini
Quoted 31 lines · Dr. David Alan Gilbert
Quoted 31 lines

static void socket_start_incoming_migration(SocketAddress *saddr,
Error **errp)
{

migration/socket.c +7 -10
-    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    QIONetListener *listener = qio_net_listener_new();
 
-    qio_channel_set_name(QIO_CHANNEL(listen_ioc),
-                         "migration-socket-listener");
+    qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
+    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+        object_unref(OBJECT(listener));
         return;
     }
 
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          socket_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+    qio_net_listener_set_client_func(listener,
+                                     socket_accept_incoming_migration,
+                                     NULL, NULL);
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
Signature · 1 line
2.14.3
Quoted 7 lines
Quoted 3 lines

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Could this get in 2.12 as a bugfix, even after soft freeze? It's the last user of qio_channel_listen_sync that blocks full IPv4/IPv6 support.
All other users are for vsock/AF_UNIX.

I had been thinking of all this conversion as feature, but I guess if we squint our eyes a bit we could claim it is just a bugfix on the grounds that it is fixing broken IPv4/6 dual stack support.

Regards,
Daniel

Signature · 3 lines
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
#4 lore raw
Quoted 37 lines · Daniel P. Berrangé
Quoted 36 lines · Paolo Bonzini
Quoted 34 lines · Dr. David Alan Gilbert
Quoted 31 lines

static void socket_start_incoming_migration(SocketAddress *saddr,
Error **errp)
{

migration/socket.c +7 -10
-    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    QIONetListener *listener = qio_net_listener_new();
 
-    qio_channel_set_name(QIO_CHANNEL(listen_ioc),
-                         "migration-socket-listener");
+    qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
+    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+        object_unref(OBJECT(listener));
         return;
     }
 
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          socket_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+    qio_net_listener_set_client_func(listener,
+                                     socket_accept_incoming_migration,
+                                     NULL, NULL);
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
Signature · 1 line
2.14.3

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Quoted 7 lines
Quoted 3 lines

Could this get in 2.12 as a bugfix, even after soft freeze? It's the last user of qio_channel_listen_sync that blocks full IPv4/IPv6 support.
All other users are for vsock/AF_UNIX.

I had been thinking of all this conversion as feature, but I guess if we squint our eyes a bit we could claim it is just a bugfix on the grounds that it is fixing broken IPv4/6 dual stack support.

It is certainly a feature, but when only one listener remains broken then it becomes more of a bugfix.

Paolo

#5 lore raw
Quoted 42 lines · * Paolo Bonzini ([email protected]) wrote:
Quoted 41 lines · Daniel P. Berrangé
Quoted 39 lines · Paolo Bonzini
Quoted 27 lines · Dr. David Alan Gilbert
Quoted 27 lines

static void socket_start_incoming_migration(SocketAddress *saddr,
Error **errp)
{

migration/socket.c +7 -10
-    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    QIONetListener *listener = qio_net_listener_new();
 
-    qio_channel_set_name(QIO_CHANNEL(listen_ioc),
-                         "migration-socket-listener");
+    qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
+    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+        object_unref(OBJECT(listener));
         return;
     }
 
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          socket_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+    qio_net_listener_set_client_func(listener,
+                                     socket_accept_incoming_migration,
+                                     NULL, NULL);
 }
Quoted 7 lines
Quoted 4 lines

void tcp_start_incoming_migration(const char *host_port, Error **errp)

Signature · 1 line
2.14.3

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

Could this get in 2.12 as a bugfix, even after soft freeze? It's the last user of qio_channel_listen_sync that blocks full IPv4/IPv6 support.
All other users are for vsock/AF_UNIX.

Quoted 6 lines
Quoted 3 lines

I had been thinking of all this conversion as feature, but I guess if we squint our eyes a bit we could claim it is just a bugfix on the grounds that it is fixing broken IPv4/6 dual stack support.

It is certainly a feature, but when only one listener remains broken then it becomes more of a bugfix.

Sounds reasonable to me.

Dave

Quoted 1 line

Paolo

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK

#6 lore raw
In reply to Daniel P. Berrangé
Quoted 11 lines · * Daniel P. Berrangé ([email protected]) wrote:

From: "Daniel P. Berrange" <[email protected]>

Instead of creating a QIOChannelSocket directly for the migration server socket, use a QIONetListener. This provides the ability to listen on multiple sockets at the same time, so enables full support for IPv4/IPv6 dual stack.

For example, '$QEMU -incoming tcp::9000' now correctly listens
on both 0.0.0.0 and :: at the same time, instead of only on 0.0.0.0.

<squints really hard>
Queued for 2.12

Quoted 92 lines
---
 migration/socket.c | 48 ++++++++++++++++--------------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)
migration/socket.c +16 -32
index 8a93fb1af5..122d8ccfbe 100644
@@ -24,6 +24,7 @@
 #include "migration.h"
 #include "qemu-file.h"
 #include "io/channel-socket.h"
+#include "io/net-listener.h"
 #include "trace.h"
 
 
@@ -129,34 +130,20 @@ void unix_start_outgoing_migration(MigrationState *s,
 }
 
 
-static gboolean socket_accept_incoming_migration(QIOChannel *ioc,
-                                                 GIOCondition condition,
-                                                 gpointer opaque)
+static void socket_accept_incoming_migration(QIONetListener *listener,
+                                             QIOChannelSocket *cioc,
+                                             gpointer opaque)
 {
-    QIOChannelSocket *sioc;
-    Error *err = NULL;
-
-    sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
-                                     &err);
-    if (!sioc) {
-        error_report("could not accept migration connection (%s)",
-                     error_get_pretty(err));
-        goto out;
-    }
-
     trace_migration_socket_incoming_accepted();
 
-    qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-incoming");
-    migration_channel_process_incoming(QIO_CHANNEL(sioc));
-    object_unref(OBJECT(sioc));
+    qio_channel_set_name(QIO_CHANNEL(cioc), "migration-socket-incoming");
+    migration_channel_process_incoming(QIO_CHANNEL(cioc));
 
-out:
     if (migration_has_all_channels()) {
         /* Close listening socket as its no longer needed */
-        qio_channel_close(ioc, NULL);
-        return G_SOURCE_REMOVE;
-    } else {
-        return G_SOURCE_CONTINUE;
+        qio_net_listener_disconnect(listener);
+
+        object_unref(OBJECT(listener));
     }
 }
 
@@ -164,21 +151,18 @@ out:
 static void socket_start_incoming_migration(SocketAddress *saddr,
                                             Error **errp)
 {
-    QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+    QIONetListener *listener = qio_net_listener_new();
 
-    qio_channel_set_name(QIO_CHANNEL(listen_ioc),
-                         "migration-socket-listener");
+    qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-        object_unref(OBJECT(listen_ioc));
+    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+        object_unref(OBJECT(listener));
         return;
     }
 
-    qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-                          G_IO_IN,
-                          socket_accept_incoming_migration,
-                          listen_ioc,
-                          (GDestroyNotify)object_unref);
+    qio_net_listener_set_client_func(listener,
+                                     socket_accept_incoming_migration,
+                                     NULL, NULL);
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
Signature · 1 line
2.14.3

--
Dr. David Alan Gilbert / [email protected] / Manchester, UK