diff mbox

[6/8] ui: let VNC server listen on all resolved IP addresses

Message ID 20170105160701.22118-7-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Jan. 5, 2017, 4:06 p.m. UTC
Remove the limitation that the VNC server can only listen on
a single resolved IP address. This uses the new DNS resolver
API to resolve a SocketAddress struct into an array of
SocketAddress structs containing raw IP addresses. The VNC
server will then attempt to listen on all resolve IP addresses.
An error will be returned if it was not possible to listen on
all of the IP addresses.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 ui/vnc.c | 52 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 9 deletions(-)

Comments

Eric Blake Jan. 6, 2017, 4:14 p.m. UTC | #1
On 01/05/2017 10:06 AM, Daniel P. Berrange wrote:
> Remove the limitation that the VNC server can only listen on
> a single resolved IP address. This uses the new DNS resolver
> API to resolve a SocketAddress struct into an array of
> SocketAddress structs containing raw IP addresses. The VNC
> server will then attempt to listen on all resolve IP addresses.

s/resolve/resolved/

> An error will be returned if it was not possible to listen on
> all of the IP addresses.

s/all/any/ ?  That is, we fail only if we don't get at least one working
listener, or do we fail if even one listener fails while the other work?

> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  ui/vnc.c | 52 +++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 9 deletions(-)
> 

> -    (*lsock_tag)[0] = qio_channel_add_watch(
> -        QIO_CHANNEL((*lsock)[0]),
> -        G_IO_IN, vnc_listen_io, vd, NULL);
> +    for (i = 0; i < nrawaddrs; i++) {
> +        QIOChannelSocket *sioc = qio_channel_socket_new();
> +
> +        qio_channel_set_name(QIO_CHANNEL(sioc), name);
> +        if (qio_channel_socket_listen_sync(
> +                sioc, rawaddrs[i], listenerr == NULL ? &listenerr : NULL) < 0) {
> +            continue;
> +        }
> +        (*nlsock)++;
> +        *lsock = g_renew(QIOChannelSocket *, *lsock, *nlsock);
> +        *lsock_tag = g_renew(guint, *lsock_tag, *nlsock);
> +
> +        (*lsock)[*nlsock - 1] = sioc;
> +        (*lsock_tag)[*nlsock - 1] = 0;
> +    }
> +
> +    for (i = 0; i < nrawaddrs; i++) {
> +        qapi_free_SocketAddress(rawaddrs[i]);
> +    }
> +    g_free(rawaddrs);
> +
> +    if (listenerr) {
> +        if (*nlsock == 0) {
> +            error_propagate(errp, listenerr);
> +            return -1;
> +        } else {
> +            error_free(listenerr);
> +        }
> +    }

Answering my question - you fail only if ALL resolved addresses fail. If
at least one succeeds, the overall function succeeds.

With the commit message improved,
Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/ui/vnc.c b/ui/vnc.c
index 94135c8..d21ede9 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -45,6 +45,7 @@ 
 #include "crypto/tlscredsx509.h"
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
+#include "io/dns-resolver.h"
 
 #define VNC_REFRESH_INTERVAL_BASE GUI_REFRESH_INTERVAL_DEFAULT
 #define VNC_REFRESH_INTERVAL_INC  50
@@ -3722,19 +3723,52 @@  static int vnc_display_listen_addr(VncDisplay *vd,
                                    size_t *nlsock,
                                    Error **errp)
 {
-    *nlsock = 1;
-    *lsock = g_new0(QIOChannelSocket *, 1);
-    *lsock_tag = g_new0(guint, 1);
+    QIODNSResolver *resolver = qio_dns_resolver_get_instance();
+    SocketAddress **rawaddrs = NULL;
+    size_t nrawaddrs = 0;
+    Error *listenerr = NULL;
+    size_t i;
 
-    (*lsock)[0] = qio_channel_socket_new();
-    qio_channel_set_name(QIO_CHANNEL((*lsock)[0]), name);
-    if (qio_channel_socket_listen_sync((*lsock)[0], addr, errp) < 0) {
+    if (qio_dns_resolver_lookup_sync(resolver, addr, &nrawaddrs,
+                                     &rawaddrs, errp) < 0) {
         return -1;
     }
 
-    (*lsock_tag)[0] = qio_channel_add_watch(
-        QIO_CHANNEL((*lsock)[0]),
-        G_IO_IN, vnc_listen_io, vd, NULL);
+    for (i = 0; i < nrawaddrs; i++) {
+        QIOChannelSocket *sioc = qio_channel_socket_new();
+
+        qio_channel_set_name(QIO_CHANNEL(sioc), name);
+        if (qio_channel_socket_listen_sync(
+                sioc, rawaddrs[i], listenerr == NULL ? &listenerr : NULL) < 0) {
+            continue;
+        }
+        (*nlsock)++;
+        *lsock = g_renew(QIOChannelSocket *, *lsock, *nlsock);
+        *lsock_tag = g_renew(guint, *lsock_tag, *nlsock);
+
+        (*lsock)[*nlsock - 1] = sioc;
+        (*lsock_tag)[*nlsock - 1] = 0;
+    }
+
+    for (i = 0; i < nrawaddrs; i++) {
+        qapi_free_SocketAddress(rawaddrs[i]);
+    }
+    g_free(rawaddrs);
+
+    if (listenerr) {
+        if (*nlsock == 0) {
+            error_propagate(errp, listenerr);
+            return -1;
+        } else {
+            error_free(listenerr);
+        }
+    }
+
+    for (i = 0; i < *nlsock; i++) {
+        (*lsock_tag)[i] = qio_channel_add_watch(
+            QIO_CHANNEL((*lsock)[i]),
+            G_IO_IN, vnc_listen_io, vd, NULL);
+    }
 
     return 0;
 }