[Qemu-devel] [PATCH v2 0/8] Support multiple listening sockets per VNC server

[Qemu-devel] [PATCH v2 0/8] Support multiple listening sockets per VNC server

[Daniel P. Berrange]

There are two core feature changes in this series

 - If a DNS name resolves to multiple distinct IP addresses,
   the VNC server is now able to listen on all of them

 - The -vnc argument syntax is extended to allow the 'vnc'
   and 'websocket' arguments to be listed multiple times.
   This allows a single VNC server to listen on multiple
   different names / addresses.

The io/ patch series that this previously depended on is now merged
in git master.

Changed in v2:

 - Fix qapi version annotation (Eric)
 - Fix misc typos in docs/commit message (Eric)
 - Replace qemu_opt_get_all() with generic QemuOpt iterator API
   to avoid needless mem allocations

Daniel P. Berrange (8):
  ui: fix regression handling bare 'websocket' option to -vnc
  ui: fix reporting of VNC auth in query-vnc-servers
  ui: refactor VncDisplay to allow multiple listening sockets
  ui: refactor code for populating SocketAddress from vnc_display_open
  ui: extract code to connect/listen from vnc_display_open
  ui: let VNC server listen on all resolved IP addresses
  util: add iterators for QemuOpts values
  ui: add ability to specify multiple VNC listen addresses

 include/qemu/option.h |   9 +
 qapi-schema.json      |  28 ++-
 qemu-options.hx       |  12 +-
 ui/vnc.c              | 656 +++++++++++++++++++++++++++++++++++---------------
 ui/vnc.h              |  10 +-
 util/qemu-option.c    |  19 ++
 6 files changed, 532 insertions(+), 202 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/8] ui: fix regression handling bare 'websocket' option to -vnc

[Daniel P. Berrange]

The -vnc argument is documented as accepting two syntaxes for
the 'websocket' option, either a bare option name, or a port
number. If using the bare option name, it is supposed to apply
the display number as an offset to base port 5700. e.g.

  -vnc localhost:3,websocket

should listen on port 5703, however, this was broken in 2.3.0 since

  commit 4db14629c38611061fc19ec6927405923de84f08
  Author: Gerd Hoffmann <kraxel@redhat.com>
  Date:   Tue Sep 16 12:33:03 2014 +0200

    vnc: switch to QemuOpts, allow multiple servers

instead qemu tries to listen on port "on" which gets looked up in
/etc/services and fails.

Fixes bug: #1455912

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 ui/vnc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 29aa9c4..c82e476 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3586,7 +3586,13 @@ void vnc_display_open(const char *id, Error **errp)
                 wsaddr->type = SOCKET_ADDRESS_KIND_INET;
                 inet = wsaddr->u.inet.data = g_new0(InetSocketAddress, 1);
                 inet->host = g_strdup(saddr->u.inet.data->host);
-                inet->port = g_strdup(websocket);
+                if (g_str_equal(websocket, "") ||
+                    g_str_equal(websocket, "on")) {
+                    inet->port = g_strdup_printf(
+                        "%d", (int)baseport + 5700);
+                } else {
+                    inet->port = g_strdup(websocket);
+                }
 
                 if (to) {
                     inet->has_to = true;
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/8] ui: fix reporting of VNC auth in query-vnc-servers

[Daniel P. Berrange]

Currently the VNC authentication info is emitted at the
top level of the query-vnc-servers data. This is wrong
because the authentication scheme differs between plain
and websockets when TLS is enabled. We should instead
report auth against the individual servers. e.g.

(QEMU) query-vnc-servers
{
    "return": [
        {
            "clients": [],
            "id": "default",
            "auth": "vencrypt",
            "vencrypt": "x509-vnc",
            "server": [
                {
                    "host": "127.0.0.1"
                    "service": "5901",
                    "websocket": false,
                    "family": "ipv4",
                    "auth": "vencrypt",
                    "vencrypt": "x509-vnc"
                },
                {
                    "host": "127.0.0.1",
                    "service": "5902",
                    "websocket": true,
                    "family": "ipv4",
                    "auth": "vnc"
                }
            ]
        }
    ]
}

This also future proofs the QMP schema so that we can
cope with multiple VNC server instances, listening on
different interfaces or ports, with different auth
setup.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qapi-schema.json | 28 ++++++++++++++++---
 ui/vnc.c         | 85 +++++++++++++++++++++++++++++++++-----------------------
 2 files changed, 74 insertions(+), 39 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index ac55f4a..846bcb8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1506,7 +1506,8 @@
 #
 # The network connection information for server
 #
-# @auth: #optional, authentication method
+# @auth: #optional, authentication method used for
+#        the plain (non-websocket) VNC server
 #
 # Since: 2.1
 ##
@@ -1597,6 +1598,25 @@
             'tls-plain', 'x509-plain',
             'tls-sasl',  'x509-sasl' ] }
 
+
+##
+# @VncServerInfo2:
+#
+# The network connection information for server
+#
+# @auth: The current authentication type used by the servers
+#
+# @vencrypt: #optional The vencrypt sub authentication type used by the
+#            servers, only specified in case auth == vencrypt.
+#
+# Since: 2.9
+##
+{ 'struct': 'VncServerInfo2',
+  'base': 'VncBasicInfo',
+  'data': { 'auth'      : 'VncPrimaryAuth',
+            '*vencrypt' : 'VncVencryptSubAuth' } }
+
+
 ##
 # @VncInfo2:
 #
@@ -1612,9 +1632,9 @@
 # @clients: A list of @VncClientInfo of all currently connected clients.
 #           The list can be empty, for obvious reasons.
 #
-# @auth: The current authentication type used by the server
+# @auth: The current authentication type used by the non-websockets servers
 #
-# @vencrypt: #optional The vencrypt sub authentication type used by the server,
+# @vencrypt: #optional The vencrypt authentication type used by the servers,
 #            only specified in case auth == vencrypt.
 #
 # @display: #optional The display device the vnc server is linked to.
@@ -1623,7 +1643,7 @@
 ##
 { 'struct': 'VncInfo2',
   'data': { 'id'        : 'str',
-            'server'    : ['VncBasicInfo'],
+            'server'    : ['VncServerInfo2'],
             'clients'   : ['VncClientInfo'],
             'auth'      : 'VncPrimaryAuth',
             '*vencrypt' : 'VncVencryptSubAuth',
diff --git a/ui/vnc.c b/ui/vnc.c
index c82e476..bd81922 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -429,12 +429,20 @@ out_error:
     return NULL;
 }
 
-static VncBasicInfoList *qmp_query_server_entry(QIOChannelSocket *ioc,
-                                                bool websocket,
-                                                VncBasicInfoList *prev)
+
+static void qmp_query_auth(int auth, int subauth,
+                           VncPrimaryAuth *qmp_auth,
+                           VncVencryptSubAuth *qmp_vencrypt,
+                           bool *qmp_has_vencrypt);
+
+static VncServerInfo2List *qmp_query_server_entry(QIOChannelSocket *ioc,
+                                                  bool websocket,
+                                                  int auth,
+                                                  int subauth,
+                                                  VncServerInfo2List *prev)
 {
-    VncBasicInfoList *list;
-    VncBasicInfo *info;
+    VncServerInfo2List *list;
+    VncServerInfo2 *info;
     Error *err = NULL;
     SocketAddress *addr;
 
@@ -444,85 +452,91 @@ static VncBasicInfoList *qmp_query_server_entry(QIOChannelSocket *ioc,
         return prev;
     }
 
-    info = g_new0(VncBasicInfo, 1);
-    vnc_init_basic_info(addr, info, &err);
+    info = g_new0(VncServerInfo2, 1);
+    vnc_init_basic_info(addr, qapi_VncServerInfo2_base(info), &err);
     qapi_free_SocketAddress(addr);
     if (err) {
-        qapi_free_VncBasicInfo(info);
+        qapi_free_VncServerInfo2(info);
         error_free(err);
         return prev;
     }
     info->websocket = websocket;
 
-    list = g_new0(VncBasicInfoList, 1);
+    qmp_query_auth(auth, subauth, &info->auth,
+                   &info->vencrypt, &info->has_vencrypt);
+
+    list = g_new0(VncServerInfo2List, 1);
     list->value = info;
     list->next = prev;
     return list;
 }
 
-static void qmp_query_auth(VncDisplay *vd, VncInfo2 *info)
+static void qmp_query_auth(int auth, int subauth,
+                           VncPrimaryAuth *qmp_auth,
+                           VncVencryptSubAuth *qmp_vencrypt,
+                           bool *qmp_has_vencrypt)
 {
-    switch (vd->auth) {
+    switch (auth) {
     case VNC_AUTH_VNC:
-        info->auth = VNC_PRIMARY_AUTH_VNC;
+        *qmp_auth = VNC_PRIMARY_AUTH_VNC;
         break;
     case VNC_AUTH_RA2:
-        info->auth = VNC_PRIMARY_AUTH_RA2;
+        *qmp_auth = VNC_PRIMARY_AUTH_RA2;
         break;
     case VNC_AUTH_RA2NE:
-        info->auth = VNC_PRIMARY_AUTH_RA2NE;
+        *qmp_auth = VNC_PRIMARY_AUTH_RA2NE;
         break;
     case VNC_AUTH_TIGHT:
-        info->auth = VNC_PRIMARY_AUTH_TIGHT;
+        *qmp_auth = VNC_PRIMARY_AUTH_TIGHT;
         break;
     case VNC_AUTH_ULTRA:
-        info->auth = VNC_PRIMARY_AUTH_ULTRA;
+        *qmp_auth = VNC_PRIMARY_AUTH_ULTRA;
         break;
     case VNC_AUTH_TLS:
-        info->auth = VNC_PRIMARY_AUTH_TLS;
+        *qmp_auth = VNC_PRIMARY_AUTH_TLS;
         break;
     case VNC_AUTH_VENCRYPT:
-        info->auth = VNC_PRIMARY_AUTH_VENCRYPT;
-        info->has_vencrypt = true;
-        switch (vd->subauth) {
+        *qmp_auth = VNC_PRIMARY_AUTH_VENCRYPT;
+        *qmp_has_vencrypt = true;
+        switch (subauth) {
         case VNC_AUTH_VENCRYPT_PLAIN:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_PLAIN;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_PLAIN;
             break;
         case VNC_AUTH_VENCRYPT_TLSNONE:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_NONE;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_NONE;
             break;
         case VNC_AUTH_VENCRYPT_TLSVNC:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_VNC;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_VNC;
             break;
         case VNC_AUTH_VENCRYPT_TLSPLAIN:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_PLAIN;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_PLAIN;
             break;
         case VNC_AUTH_VENCRYPT_X509NONE:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_NONE;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_NONE;
             break;
         case VNC_AUTH_VENCRYPT_X509VNC:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_VNC;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_VNC;
             break;
         case VNC_AUTH_VENCRYPT_X509PLAIN:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_PLAIN;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_PLAIN;
             break;
         case VNC_AUTH_VENCRYPT_TLSSASL:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_SASL;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_TLS_SASL;
             break;
         case VNC_AUTH_VENCRYPT_X509SASL:
-            info->vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_SASL;
+            *qmp_vencrypt = VNC_VENCRYPT_SUB_AUTH_X509_SASL;
             break;
         default:
-            info->has_vencrypt = false;
+            *qmp_has_vencrypt = false;
             break;
         }
         break;
     case VNC_AUTH_SASL:
-        info->auth = VNC_PRIMARY_AUTH_SASL;
+        *qmp_auth = VNC_PRIMARY_AUTH_SASL;
         break;
     case VNC_AUTH_NONE:
     default:
-        info->auth = VNC_PRIMARY_AUTH_NONE;
+        *qmp_auth = VNC_PRIMARY_AUTH_NONE;
         break;
     }
 }
@@ -538,7 +552,8 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
         info = g_new0(VncInfo2, 1);
         info->id = g_strdup(vd->id);
         info->clients = qmp_query_client_list(vd);
-        qmp_query_auth(vd, info);
+        qmp_query_auth(vd->auth, vd->subauth, &info->auth,
+                       &info->vencrypt, &info->has_vencrypt);
         if (vd->dcl.con) {
             dev = DEVICE(object_property_get_link(OBJECT(vd->dcl.con),
                                                   "device", NULL));
@@ -547,11 +562,11 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
         }
         if (vd->lsock != NULL) {
             info->server = qmp_query_server_entry(
-                vd->lsock, false, info->server);
+                vd->lsock, false, vd->auth, vd->subauth, info->server);
         }
         if (vd->lwebsock != NULL) {
             info->server = qmp_query_server_entry(
-                vd->lwebsock, true, info->server);
+                vd->lwebsock, true, vd->ws_auth, vd->ws_subauth, info->server);
         }
 
         item = g_new0(VncInfo2List, 1);
-- 
2.9.3

[Qemu-devel] [PATCH v2 3/8] ui: refactor VncDisplay to allow multiple listening sockets

[Daniel P. Berrange]

Currently there is only a single listener for plain VNC and
a single listener for websockets VNC. This means that if
getaddrinfo() returns multiple IP addresses, for a hostname,
the VNC server can only listen on one of them. This is
just bearable if listening on wildcard interface, or if
the host only has a single network interface to listen on,
but if there are multiple NICs and the VNC server needs
to listen on 2 or more specific IP addresses, it can't be
done.

This refactors the VncDisplay state so that it holds an
array of listening sockets, but still only listens on
one socket.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 ui/vnc.c | 103 +++++++++++++++++++++++++++++++++++++++++----------------------
 ui/vnc.h |  10 ++++---
 2 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index bd81922..9b9682d 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -224,8 +224,12 @@ static VncServerInfo *vnc_server_info_get(VncDisplay *vd)
     VncServerInfo *info;
     Error *err = NULL;
 
+    if (!vd->nlsock) {
+        return NULL;
+    }
+
     info = g_malloc0(sizeof(*info));
-    vnc_init_basic_info_from_server_addr(vd->lsock,
+    vnc_init_basic_info_from_server_addr(vd->lsock[0],
                                          qapi_VncServerInfo_base(info), &err);
     info->has_auth = true;
     info->auth = g_strdup(vnc_auth_name(vd));
@@ -371,7 +375,7 @@ VncInfo *qmp_query_vnc(Error **errp)
     VncDisplay *vd = vnc_display_find(NULL);
     SocketAddress *addr = NULL;
 
-    if (vd == NULL || !vd->lsock) {
+    if (vd == NULL || !vd->nlsock) {
         info->enabled = false;
     } else {
         info->enabled = true;
@@ -384,7 +388,7 @@ VncInfo *qmp_query_vnc(Error **errp)
             return info;
         }
 
-        addr = qio_channel_socket_get_local_address(vd->lsock, errp);
+        addr = qio_channel_socket_get_local_address(vd->lsock[0], errp);
         if (!addr) {
             goto out_error;
         }
@@ -547,6 +551,7 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
     VncInfo2 *info;
     VncDisplay *vd;
     DeviceState *dev;
+    size_t i;
 
     QTAILQ_FOREACH(vd, &vnc_displays, next) {
         info = g_new0(VncInfo2, 1);
@@ -560,13 +565,14 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
             info->has_display = true;
             info->display = g_strdup(dev->id);
         }
-        if (vd->lsock != NULL) {
+        for (i = 0; i < vd->nlsock; i++) {
             info->server = qmp_query_server_entry(
-                vd->lsock, false, vd->auth, vd->subauth, info->server);
+                vd->lsock[i], false, vd->auth, vd->subauth, info->server);
         }
-        if (vd->lwebsock != NULL) {
+        for (i = 0; i < vd->nlwebsock; i++) {
             info->server = qmp_query_server_entry(
-                vd->lwebsock, true, vd->ws_auth, vd->ws_subauth, info->server);
+                vd->lwebsock[i], true, vd->ws_auth,
+                vd->ws_subauth, info->server);
         }
 
         item = g_new0(VncInfo2List, 1);
@@ -3116,15 +3122,22 @@ static gboolean vnc_listen_io(QIOChannel *ioc,
     VncDisplay *vd = opaque;
     QIOChannelSocket *sioc = NULL;
     Error *err = NULL;
+    bool isWebsock = false;
+    size_t i;
+
+    for (i = 0; i < vd->nlwebsock; i++) {
+        if (ioc == QIO_CHANNEL(vd->lwebsock[i])) {
+            isWebsock = true;
+            break;
+        }
+    }
 
     sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc), &err);
     if (sioc != NULL) {
         qio_channel_set_name(QIO_CHANNEL(sioc),
-                             ioc != QIO_CHANNEL(vd->lsock) ?
-                             "vnc-ws-server" : "vnc-server");
+                             isWebsock ? "vnc-ws-server" : "vnc-server");
         qio_channel_set_delay(QIO_CHANNEL(sioc), false);
-        vnc_connect(vd, sioc, false,
-                    ioc != QIO_CHANNEL(vd->lsock));
+        vnc_connect(vd, sioc, false, isWebsock);
         object_unref(OBJECT(sioc));
     } else {
         /* client probably closed connection before we got there */
@@ -3184,24 +3197,33 @@ void vnc_display_init(const char *id)
 
 static void vnc_display_close(VncDisplay *vd)
 {
+    size_t i;
     if (!vd) {
         return;
     }
     vd->is_unix = false;
-    if (vd->lsock != NULL) {
-        if (vd->lsock_tag) {
-            g_source_remove(vd->lsock_tag);
+    for (i = 0; i < vd->nlsock; i++) {
+        if (vd->lsock_tag[i]) {
+            g_source_remove(vd->lsock_tag[i]);
         }
-        object_unref(OBJECT(vd->lsock));
-        vd->lsock = NULL;
+        object_unref(OBJECT(vd->lsock[i]));
     }
-    if (vd->lwebsock != NULL) {
-        if (vd->lwebsock_tag) {
-            g_source_remove(vd->lwebsock_tag);
+    g_free(vd->lsock);
+    g_free(vd->lsock_tag);
+    vd->lsock = NULL;
+    vd->nlsock = 0;
+
+    for (i = 0; i < vd->nlwebsock; i++) {
+        if (vd->lwebsock_tag[i]) {
+            g_source_remove(vd->lwebsock_tag[i]);
         }
-        object_unref(OBJECT(vd->lwebsock));
-        vd->lwebsock = NULL;
+        object_unref(OBJECT(vd->lwebsock[i]));
     }
+    g_free(vd->lwebsock);
+    g_free(vd->lwebsock_tag);
+    vd->lwebsock = NULL;
+    vd->nlwebsock = 0;
+
     vd->auth = VNC_AUTH_INVALID;
     vd->subauth = VNC_AUTH_INVALID;
     if (vd->tlscreds) {
@@ -3248,7 +3270,11 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
     SocketAddress *addr;
     Error *err = NULL;
 
-    addr = qio_channel_socket_get_local_address(vd->lsock, &err);
+    if (!vd->nlsock) {
+        return;
+    }
+
+    addr = qio_channel_socket_get_local_address(vd->lsock[0], &err);
     if (!addr) {
         return;
     }
@@ -3808,8 +3834,6 @@ void vnc_display_open(const char *id, Error **errp)
     if (reverse) {
         /* connect to viewer */
         QIOChannelSocket *sioc = NULL;
-        vd->lsock = NULL;
-        vd->lwebsock = NULL;
         if (ws_enabled) {
             error_setg(errp, "Cannot use websockets in reverse mode");
             goto fail;
@@ -3823,30 +3847,36 @@ void vnc_display_open(const char *id, Error **errp)
         vnc_connect(vd, sioc, false, false);
         object_unref(OBJECT(sioc));
     } else {
-        vd->lsock = qio_channel_socket_new();
-        qio_channel_set_name(QIO_CHANNEL(vd->lsock), "vnc-listen");
-        if (qio_channel_socket_listen_sync(vd->lsock, saddr, errp) < 0) {
+        vd->nlsock = 1;
+        vd->lsock = g_new0(QIOChannelSocket *, 1);
+        vd->lsock_tag = g_new0(guint, 1);
+
+        vd->lsock[0] = qio_channel_socket_new();
+        qio_channel_set_name(QIO_CHANNEL(vd->lsock[0]), "vnc-listen");
+        if (qio_channel_socket_listen_sync(vd->lsock[0], saddr, errp) < 0) {
             goto fail;
         }
         vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
 
         if (ws_enabled) {
-            vd->lwebsock = qio_channel_socket_new();
-            qio_channel_set_name(QIO_CHANNEL(vd->lwebsock), "vnc-ws-listen");
-            if (qio_channel_socket_listen_sync(vd->lwebsock,
+            vd->nlwebsock = 1;
+            vd->lwebsock = g_new0(QIOChannelSocket *, 1);
+            vd->lwebsock_tag = g_new0(guint, 1);
+
+            vd->lwebsock[0] = qio_channel_socket_new();
+            qio_channel_set_name(QIO_CHANNEL(vd->lwebsock[0]), "vnc-ws-listen");
+            if (qio_channel_socket_listen_sync(vd->lwebsock[0],
                                                wsaddr, errp) < 0) {
-                object_unref(OBJECT(vd->lsock));
-                vd->lsock = NULL;
                 goto fail;
             }
         }
 
-        vd->lsock_tag = qio_channel_add_watch(
-            QIO_CHANNEL(vd->lsock),
+        vd->lsock_tag[0] = qio_channel_add_watch(
+            QIO_CHANNEL(vd->lsock[0]),
             G_IO_IN, vnc_listen_io, vd, NULL);
         if (ws_enabled) {
-            vd->lwebsock_tag = qio_channel_add_watch(
-                QIO_CHANNEL(vd->lwebsock),
+            vd->lwebsock_tag[0] = qio_channel_add_watch(
+                QIO_CHANNEL(vd->lwebsock[0]),
                 G_IO_IN, vnc_listen_io, vd, NULL);
         }
     }
@@ -3860,6 +3890,7 @@ void vnc_display_open(const char *id, Error **errp)
     return;
 
 fail:
+    vnc_display_close(vd);
     qapi_free_SocketAddress(saddr);
     qapi_free_SocketAddress(wsaddr);
     ws_enabled = false;
diff --git a/ui/vnc.h b/ui/vnc.h
index d20b154..453695a 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -146,10 +146,12 @@ struct VncDisplay
     int num_exclusive;
     int connections_limit;
     VncSharePolicy share_policy;
-    QIOChannelSocket *lsock;
-    guint lsock_tag;
-    QIOChannelSocket *lwebsock;
-    guint lwebsock_tag;
+    size_t nlsock;
+    QIOChannelSocket **lsock;
+    guint *lsock_tag;
+    size_t nlwebsock;
+    QIOChannelSocket **lwebsock;
+    guint *lwebsock_tag;
     DisplaySurface *ds;
     DisplayChangeListener dcl;
     kbd_layout_t *kbd_layout;
-- 
2.9.3

[Qemu-devel] [PATCH v2 4/8] ui: refactor code for populating SocketAddress from vnc_display_open

[Daniel P. Berrange]

The code which interprets the CLI args to populate the SocketAddress
objects for plain & websockets VNC is quite complex already and will
need further enhancements shortly. Refactor it into separate methods
to avoid vnc_display_open getting even larger. As a side effect of
the refactoring, it is now possible to specify a listen address for
the websocket server explicitly. e.g,

  -vnc localhost:5900,websockets=0.0.0.0:8080

will listen on localhost for the plain VNC server, but expose the
websockets VNC server on the public interface. This refactoring
also removes the restriction that prevents enabling websockets
when the plain VNC server is listening on a UNIX socket.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-options.hx |  12 ++-
 ui/vnc.c        | 282 ++++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 189 insertions(+), 105 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 80df526..c1c2903 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1297,10 +1297,14 @@ is a TCP port number, not a display number.
 @item websocket
 
 Opens an additional TCP listening port dedicated to VNC Websocket connections.
-By definition the Websocket port is 5700+@var{display}. If @var{host} is
-specified connections will only be allowed from this host.
-As an alternative the Websocket port could be specified by using
-@code{websocket}=@var{port}.
+If a bare @var{websocket} option is given, the Websocket port is
+5700+@var{display}. An alternative port can be specified with the
+syntax @code{websocket}=@var{port}.
+
+If @var{host} is specified connections will only be allowed from this host.
+It is possible to control the websocket listen address independently, using
+the syntax @code{websocket}=@var{host}:@var{port}.
+
 If no TLS credentials are provided, the websocket connection runs in
 unencrypted mode. If TLS credentials are provided, the websocket connection
 requires encrypted client connections.
diff --git a/ui/vnc.c b/ui/vnc.c
index 9b9682d..f0335d8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3523,6 +3523,178 @@ vnc_display_create_creds(bool x509,
 }
 
 
+static int vnc_display_get_address(const char *addrstr,
+                                   bool websocket,
+                                   int displaynum,
+                                   int to,
+                                   bool has_ipv4,
+                                   bool has_ipv6,
+                                   bool ipv4,
+                                   bool ipv6,
+                                   SocketAddress **retaddr,
+                                   Error **errp)
+{
+    int ret = -1;
+    SocketAddress *addr = NULL;
+
+    addr = g_new0(SocketAddress, 1);
+
+    if (strncmp(addrstr, "unix:", 5) == 0) {
+        addr->type = SOCKET_ADDRESS_KIND_UNIX;
+        addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
+        addr->u.q_unix.data->path = g_strdup(addrstr + 5);
+
+        if (websocket) {
+            error_setg(errp, "UNIX sockets not supported with websock");
+            goto cleanup;
+        }
+
+        if (to) {
+            error_setg(errp, "Port range not support with UNIX socket");
+            goto cleanup;
+        }
+        ret = 0;
+    } else {
+        const char *port;
+        size_t hostlen;
+        unsigned long long baseport;
+        InetSocketAddress *inet;
+
+        port = strrchr(addrstr, ':');
+        if (!port) {
+            if (websocket) {
+                hostlen = 0;
+                port = addrstr;
+            } else {
+                error_setg(errp, "no vnc port specified");
+                goto cleanup;
+            }
+        } else {
+            hostlen = port - addrstr;
+            port++;
+            if (*port == '\0') {
+                error_setg(errp, "vnc port cannot be empty");
+                goto cleanup;
+            }
+        }
+
+        addr->type = SOCKET_ADDRESS_KIND_INET;
+        inet = addr->u.inet.data = g_new0(InetSocketAddress, 1);
+        if (addrstr[0] == '[' && addrstr[hostlen - 1] == ']') {
+            inet->host = g_strndup(addrstr + 1, hostlen - 2);
+        } else {
+            inet->host = g_strndup(addrstr, hostlen);
+        }
+        /* plain VNC port is just an offset, for websocket
+         * port is absolute */
+        if (websocket) {
+            if (g_str_equal(addrstr, "") ||
+                g_str_equal(addrstr, "on")) {
+                inet->port = g_strdup_printf(
+                    "%d", displaynum + 5700);
+                if (to) {
+                    inet->has_to = true;
+                    inet->to = to + 5700;
+                }
+            } else {
+                inet->port = g_strdup(port);
+            }
+        } else {
+            if (parse_uint_full(port, &baseport, 10) < 0) {
+                error_setg(errp, "can't convert to a number: %s", port);
+                goto cleanup;
+            }
+            if (baseport > 65535 ||
+                baseport + 5900 > 65535) {
+                error_setg(errp, "port %s out of range", port);
+                goto cleanup;
+            }
+            inet->port = g_strdup_printf(
+                "%d", (int)baseport + 5900);
+
+            if (to) {
+                inet->has_to = true;
+                inet->to = to + 5900;
+            }
+        }
+
+        inet->ipv4 = ipv4;
+        inet->has_ipv4 = has_ipv4;
+        inet->ipv6 = ipv6;
+        inet->has_ipv6 = has_ipv6;
+
+        ret = baseport;
+    }
+
+    *retaddr = addr;
+
+ cleanup:
+    if (ret < 0) {
+        qapi_free_SocketAddress(addr);
+    }
+    return ret;
+}
+
+static int vnc_display_get_addresses(QemuOpts *opts,
+                                     SocketAddress **retsaddr,
+                                     SocketAddress **retwsaddr,
+                                     Error **errp)
+{
+    SocketAddress *saddr = NULL;
+    SocketAddress *wsaddr = NULL;
+    const char *saddrstr = qemu_opt_get(opts, "vnc");
+    const char *wsaddrstr = qemu_opt_get(opts, "websocket");
+    int to = qemu_opt_get_number(opts, "to", 0);
+    bool has_ipv4 = qemu_opt_get(opts, "ipv4");
+    bool has_ipv6 = qemu_opt_get(opts, "ipv6");
+    bool ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
+    bool ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
+
+    if (!saddrstr || strcmp(saddrstr, "none") == 0) {
+        *retsaddr = NULL;
+        *retwsaddr = NULL;
+        return 0;
+    }
+
+    if (wsaddrstr &&
+        !qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA1)) {
+        error_setg(errp,
+                   "SHA1 hash support is required for websockets");
+        goto error;
+    }
+
+    int displaynum = vnc_display_get_address(saddrstr, false, 0, to,
+                                             has_ipv4, has_ipv6,
+                                             ipv4, ipv6,
+                                             &saddr, errp);
+    if (displaynum < 0) {
+        goto error;
+    }
+    if (wsaddrstr) {
+        if (vnc_display_get_address(wsaddrstr, true, displaynum, to,
+                                    has_ipv4, has_ipv6,
+                                    ipv4, ipv6,
+                                    &wsaddr, errp) < 0) {
+            goto error;
+        }
+        if (saddr->type == SOCKET_ADDRESS_KIND_INET &&
+            wsaddr->type == SOCKET_ADDRESS_KIND_INET &&
+            g_str_equal(wsaddr->u.inet.data->host, "") &&
+            !g_str_equal(saddr->u.inet.data->host, "")) {
+            g_free(wsaddr->u.inet.data->host);
+            wsaddr->u.inet.data->host = g_strdup(saddr->u.inet.data->host);
+        }
+    }
+    *retsaddr = saddr;
+    *retwsaddr = wsaddr;
+    return 0;
+
+ error:
+    qapi_free_SocketAddress(saddr);
+    qapi_free_SocketAddress(wsaddr);
+    return -1;
+}
+
 void vnc_display_open(const char *id, Error **errp)
 {
     VncDisplay *vd = vnc_display_find(id);
@@ -3532,10 +3704,7 @@ void vnc_display_open(const char *id, Error **errp)
     QemuConsole *con;
     bool password = false;
     bool reverse = false;
-    const char *vnc;
-    char *h;
     const char *credid;
-    int show_vnc_port = 0;
     bool sasl = false;
 #ifdef CONFIG_VNC_SASL
     int saslErr;
@@ -3543,7 +3712,6 @@ void vnc_display_open(const char *id, Error **errp)
     int acl = 0;
     int lock_key_sync = 1;
     int key_delay_ms;
-    bool ws_enabled = false;
 
     if (!vd) {
         error_setg(errp, "VNC display not active");
@@ -3554,102 +3722,15 @@ void vnc_display_open(const char *id, Error **errp)
     if (!opts) {
         return;
     }
-    vnc = qemu_opt_get(opts, "vnc");
-    if (!vnc || strcmp(vnc, "none") == 0) {
-        return;
-    }
-
-    h = strrchr(vnc, ':');
-    if (h) {
-        size_t hlen = h - vnc;
 
-        const char *websocket = qemu_opt_get(opts, "websocket");
-        int to = qemu_opt_get_number(opts, "to", 0);
-        bool has_ipv4 = qemu_opt_get(opts, "ipv4");
-        bool has_ipv6 = qemu_opt_get(opts, "ipv6");
-        bool ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
-        bool ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
-
-        saddr = g_new0(SocketAddress, 1);
-        if (websocket) {
-            if (!qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA1)) {
-                error_setg(errp,
-                           "SHA1 hash support is required for websockets");
-                goto fail;
-            }
-
-            wsaddr = g_new0(SocketAddress, 1);
-            ws_enabled = true;
-        }
-
-        if (strncmp(vnc, "unix:", 5) == 0) {
-            saddr->type = SOCKET_ADDRESS_KIND_UNIX;
-            saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
-            saddr->u.q_unix.data->path = g_strdup(vnc + 5);
-
-            if (ws_enabled) {
-                error_setg(errp, "UNIX sockets not supported with websock");
-                goto fail;
-            }
-        } else {
-            unsigned long long baseport;
-            InetSocketAddress *inet;
-            saddr->type = SOCKET_ADDRESS_KIND_INET;
-            inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
-            if (vnc[0] == '[' && vnc[hlen - 1] == ']') {
-                inet->host = g_strndup(vnc + 1, hlen - 2);
-            } else {
-                inet->host = g_strndup(vnc, hlen);
-            }
-            if (parse_uint_full(h + 1, &baseport, 10) < 0) {
-                error_setg(errp, "can't convert to a number: %s", h + 1);
-                goto fail;
-            }
-            if (baseport > 65535 ||
-                baseport + 5900 > 65535) {
-                error_setg(errp, "port %s out of range", h + 1);
-                goto fail;
-            }
-            inet->port = g_strdup_printf(
-                "%d", (int)baseport + 5900);
-
-            if (to) {
-                inet->has_to = true;
-                inet->to = to + 5900;
-                show_vnc_port = 1;
-            }
-            inet->ipv4 = ipv4;
-            inet->has_ipv4 = has_ipv4;
-            inet->ipv6 = ipv6;
-            inet->has_ipv6 = has_ipv6;
-
-            if (ws_enabled) {
-                wsaddr->type = SOCKET_ADDRESS_KIND_INET;
-                inet = wsaddr->u.inet.data = g_new0(InetSocketAddress, 1);
-                inet->host = g_strdup(saddr->u.inet.data->host);
-                if (g_str_equal(websocket, "") ||
-                    g_str_equal(websocket, "on")) {
-                    inet->port = g_strdup_printf(
-                        "%d", (int)baseport + 5700);
-                } else {
-                    inet->port = g_strdup(websocket);
-                }
-
-                if (to) {
-                    inet->has_to = true;
-                    inet->to = to;
-                }
-                inet->ipv4 = ipv4;
-                inet->has_ipv4 = has_ipv4;
-                inet->ipv6 = ipv6;
-                inet->has_ipv6 = has_ipv6;
-            }
-        }
-    } else {
-        error_setg(errp, "no vnc port specified");
+    if (vnc_display_get_addresses(opts, &saddr, &wsaddr, errp) < 0) {
         goto fail;
     }
 
+    if (saddr == NULL) {
+        return;
+    }
+
     password = qemu_opt_get_bool(opts, "password", false);
     if (password) {
         if (fips_get_state()) {
@@ -3834,7 +3915,7 @@ void vnc_display_open(const char *id, Error **errp)
     if (reverse) {
         /* connect to viewer */
         QIOChannelSocket *sioc = NULL;
-        if (ws_enabled) {
+        if (wsaddr) {
             error_setg(errp, "Cannot use websockets in reverse mode");
             goto fail;
         }
@@ -3858,7 +3939,7 @@ void vnc_display_open(const char *id, Error **errp)
         }
         vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
 
-        if (ws_enabled) {
+        if (wsaddr) {
             vd->nlwebsock = 1;
             vd->lwebsock = g_new0(QIOChannelSocket *, 1);
             vd->lwebsock_tag = g_new0(guint, 1);
@@ -3874,14 +3955,14 @@ void vnc_display_open(const char *id, Error **errp)
         vd->lsock_tag[0] = qio_channel_add_watch(
             QIO_CHANNEL(vd->lsock[0]),
             G_IO_IN, vnc_listen_io, vd, NULL);
-        if (ws_enabled) {
+        if (wsaddr) {
             vd->lwebsock_tag[0] = qio_channel_add_watch(
                 QIO_CHANNEL(vd->lwebsock[0]),
                 G_IO_IN, vnc_listen_io, vd, NULL);
         }
     }
 
-    if (show_vnc_port) {
+    if (qemu_opt_get(opts, "to")) {
         vnc_display_print_local_addr(vd);
     }
 
@@ -3893,7 +3974,6 @@ fail:
     vnc_display_close(vd);
     qapi_free_SocketAddress(saddr);
     qapi_free_SocketAddress(wsaddr);
-    ws_enabled = false;
 }
 
 void vnc_display_add_client(const char *id, int csock, bool skipauth)
-- 
2.9.3

[Qemu-devel] [PATCH v2 5/8] ui: extract code to connect/listen from vnc_display_open

[Daniel P. Berrange]

The code which takes a SocketAddress and connects/listens on the
network is going to get more complicated to deal with multiple
listeners. Pull it out into a separate method to avoid making the
vnc_display_open method even more complex.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 ui/vnc.c | 122 +++++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 80 insertions(+), 42 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index f0335d8..8e05c33 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3695,6 +3695,84 @@ static int vnc_display_get_addresses(QemuOpts *opts,
     return -1;
 }
 
+static int vnc_display_connect(VncDisplay *vd,
+                               SocketAddress *saddr,
+                               SocketAddress *wsaddr,
+                               Error **errp)
+{
+    /* connect to viewer */
+    QIOChannelSocket *sioc = NULL;
+    if (wsaddr) {
+        error_setg(errp, "Cannot use websockets in reverse mode");
+        return -1;
+    }
+    vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
+    sioc = qio_channel_socket_new();
+    qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse");
+    if (qio_channel_socket_connect_sync(sioc, saddr, errp) < 0) {
+        return -1;
+    }
+    vnc_connect(vd, sioc, false, false);
+    object_unref(OBJECT(sioc));
+    return 0;
+}
+
+
+static int vnc_display_listen_addr(VncDisplay *vd,
+                                   SocketAddress *addr,
+                                   const char *name,
+                                   QIOChannelSocket ***lsock,
+                                   guint **lsock_tag,
+                                   size_t *nlsock,
+                                   Error **errp)
+{
+    *nlsock = 1;
+    *lsock = g_new0(QIOChannelSocket *, 1);
+    *lsock_tag = g_new0(guint, 1);
+
+    (*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) {
+        return -1;
+    }
+
+    (*lsock_tag)[0] = qio_channel_add_watch(
+        QIO_CHANNEL((*lsock)[0]),
+        G_IO_IN, vnc_listen_io, vd, NULL);
+
+    return 0;
+}
+
+
+static int vnc_display_listen(VncDisplay *vd,
+                              SocketAddress *saddr,
+                              SocketAddress *wsaddr,
+                              Error **errp)
+{
+    vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
+
+    if (vnc_display_listen_addr(vd, saddr,
+                                "vnc-listen",
+                                &vd->lsock,
+                                &vd->lsock_tag,
+                                &vd->nlsock,
+                                errp) < 0) {
+        return -1;
+    }
+    if (wsaddr &&
+        vnc_display_listen_addr(vd, wsaddr,
+                                "vnc-ws-listen",
+                                &vd->lwebsock,
+                                &vd->lwebsock_tag,
+                                &vd->nlwebsock,
+                                errp) < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
+
 void vnc_display_open(const char *id, Error **errp)
 {
     VncDisplay *vd = vnc_display_find(id);
@@ -3913,53 +3991,13 @@ void vnc_display_open(const char *id, Error **errp)
     }
 
     if (reverse) {
-        /* connect to viewer */
-        QIOChannelSocket *sioc = NULL;
-        if (wsaddr) {
-            error_setg(errp, "Cannot use websockets in reverse mode");
-            goto fail;
-        }
-        vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
-        sioc = qio_channel_socket_new();
-        qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse");
-        if (qio_channel_socket_connect_sync(sioc, saddr, errp) < 0) {
+        if (vnc_display_connect(vd, saddr, wsaddr, errp) < 0) {
             goto fail;
         }
-        vnc_connect(vd, sioc, false, false);
-        object_unref(OBJECT(sioc));
     } else {
-        vd->nlsock = 1;
-        vd->lsock = g_new0(QIOChannelSocket *, 1);
-        vd->lsock_tag = g_new0(guint, 1);
-
-        vd->lsock[0] = qio_channel_socket_new();
-        qio_channel_set_name(QIO_CHANNEL(vd->lsock[0]), "vnc-listen");
-        if (qio_channel_socket_listen_sync(vd->lsock[0], saddr, errp) < 0) {
+        if (vnc_display_listen(vd, saddr, wsaddr, errp) < 0) {
             goto fail;
         }
-        vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
-
-        if (wsaddr) {
-            vd->nlwebsock = 1;
-            vd->lwebsock = g_new0(QIOChannelSocket *, 1);
-            vd->lwebsock_tag = g_new0(guint, 1);
-
-            vd->lwebsock[0] = qio_channel_socket_new();
-            qio_channel_set_name(QIO_CHANNEL(vd->lwebsock[0]), "vnc-ws-listen");
-            if (qio_channel_socket_listen_sync(vd->lwebsock[0],
-                                               wsaddr, errp) < 0) {
-                goto fail;
-            }
-        }
-
-        vd->lsock_tag[0] = qio_channel_add_watch(
-            QIO_CHANNEL(vd->lsock[0]),
-            G_IO_IN, vnc_listen_io, vd, NULL);
-        if (wsaddr) {
-            vd->lwebsock_tag[0] = qio_channel_add_watch(
-                QIO_CHANNEL(vd->lwebsock[0]),
-                G_IO_IN, vnc_listen_io, vd, NULL);
-        }
     }
 
     if (qemu_opt_get(opts, "to")) {
-- 
2.9.3

[Qemu-devel] [PATCH v2 6/8] ui: let VNC server listen on all resolved IP addresses

[Daniel P. Berrange]

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 resolved IP addresses.
The server must successfully listen on at least one of the
resolved IP addresses, otherwise an error will be reported.

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

diff --git a/ui/vnc.c b/ui/vnc.c
index 8e05c33..82759c6 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
@@ -3726,19 +3727,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;
 }
-- 
2.9.3

[Qemu-devel] [PATCH v2 7/8] util: add iterators for QemuOpts values

[Daniel P. Berrange]

To iterate over all QemuOpts currently requires using a callback
function which is inconvenient for control flow. Add support for
using iterator functions more directly

  QemuOptsIter iter;
  QemuOpt *opt;

  qemu_opts_iter_init(&iter, opts, "repeated-key");
  while ((opt = qemu_opts_iter_next(&iter)) != NULL) {
      ....do something...
  }

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/option.h |  9 +++++++++
 util/qemu-option.c    | 19 +++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 1f9e3f9..e786df0 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -100,6 +100,15 @@ typedef int (*qemu_opt_loopfunc)(void *opaque,
 int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
                      Error **errp);
 
+typedef struct {
+    QemuOpts *opts;
+    QemuOpt *opt;
+    const char *name;
+} QemuOptsIter;
+
+void qemu_opt_iter_init(QemuOptsIter *iter, QemuOpts *opts, const char *name);
+const char *qemu_opt_iter_next(QemuOptsIter *iter);
+
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
                            int fail_if_exists, Error **errp);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 3467dc2..d611946 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -332,6 +332,25 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name)
     return opt ? opt->str : NULL;
 }
 
+void qemu_opt_iter_init(QemuOptsIter *iter, QemuOpts *opts, const char *name)
+{
+    iter->opts = opts;
+    iter->opt = QTAILQ_FIRST(&opts->head);
+    iter->name = name;
+}
+
+const char *qemu_opt_iter_next(QemuOptsIter *iter)
+{
+    QemuOpt *ret = iter->opt;
+    if (iter->name) {
+        while (ret && !g_str_equal(iter->name, ret->name)) {
+            ret = QTAILQ_NEXT(ret, next);
+        }
+    }
+    iter->opt = ret ? QTAILQ_NEXT(ret, next) : NULL;
+    return ret ? ret->str : NULL;
+}
+
 /* Get a known option (or its default) and remove it from the list
  * all in one action. Return a malloced string of the option value.
  * Result must be freed by caller with g_free().
-- 
2.9.3

[Qemu-devel] [PATCH v2 8/8] ui: add ability to specify multiple VNC listen addresses

[Daniel P. Berrange]

This change allows the listen address and websocket address
options for -vnc to be repeated. This causes the VNC server
to listen on multiple addresses. e.g.

 $ $QEMU -vnc vnc=localhost:1,vnc=unix:/tmp/vnc,\
              websocket=127.0.0.1:8080,websocket=[::]:8081

results in listening on

127.0.0.1:5901, 127.0.0.1:8080, ::1:5901, :::8081 & /tmp/vnc

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

diff --git a/ui/vnc.c b/ui/vnc.c
index 82759c6..2b4d9e6 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3591,6 +3591,10 @@ static int vnc_display_get_address(const char *addrstr,
         if (websocket) {
             if (g_str_equal(addrstr, "") ||
                 g_str_equal(addrstr, "on")) {
+                if (displaynum == -1) {
+                    error_setg(errp, "explicit websocket port is required");
+                    goto cleanup;
+                }
                 inet->port = g_strdup_printf(
                     "%d", displaynum + 5700);
                 if (to) {
@@ -3637,80 +3641,133 @@ static int vnc_display_get_address(const char *addrstr,
 }
 
 static int vnc_display_get_addresses(QemuOpts *opts,
-                                     SocketAddress **retsaddr,
-                                     SocketAddress **retwsaddr,
+                                     SocketAddress ***retsaddr,
+                                     size_t *retnsaddr,
+                                     SocketAddress ***retwsaddr,
+                                     size_t *retnwsaddr,
                                      Error **errp)
 {
     SocketAddress *saddr = NULL;
     SocketAddress *wsaddr = NULL;
-    const char *saddrstr = qemu_opt_get(opts, "vnc");
-    const char *wsaddrstr = qemu_opt_get(opts, "websocket");
+    QemuOptsIter addriter;
+    const char *addr;
     int to = qemu_opt_get_number(opts, "to", 0);
     bool has_ipv4 = qemu_opt_get(opts, "ipv4");
     bool has_ipv6 = qemu_opt_get(opts, "ipv6");
     bool ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
     bool ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
+    size_t i;
+    int displaynum = -1;
+    int ret = -1;
 
-    if (!saddrstr || strcmp(saddrstr, "none") == 0) {
-        *retsaddr = NULL;
-        *retwsaddr = NULL;
-        return 0;
-    }
+    *retsaddr = NULL;
+    *retnsaddr = 0;
+    *retwsaddr = NULL;
+    *retnwsaddr = 0;
 
-    if (wsaddrstr &&
+    addr = qemu_opt_get(opts, "vnc");
+    if (addr == NULL || g_str_equal(addr, "none")) {
+        ret = 0;
+        goto cleanup;
+    }
+    if (qemu_opt_get(opts, "websocket") &&
         !qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA1)) {
         error_setg(errp,
                    "SHA1 hash support is required for websockets");
-        goto error;
+        goto cleanup;
+    }
+
+    qemu_opt_iter_init(&addriter, opts, "vnc");
+    while ((addr = qemu_opt_iter_next(&addriter)) != NULL) {
+        int rv;
+        rv = vnc_display_get_address(addr, false, 0, to,
+                                     has_ipv4, has_ipv6,
+                                     ipv4, ipv6,
+                                     &saddr, errp);
+        if (rv < 0) {
+            goto cleanup;
+        }
+        /* Historical compat - first listen address can be used
+         * to set the default websocket port
+         */
+        if (displaynum == -1) {
+            displaynum = rv;
+        }
+        *retsaddr = g_renew(SocketAddress *, *retsaddr, *retnsaddr + 1);
+        (*retsaddr)[(*retnsaddr)++] = saddr;
     }
 
-    int displaynum = vnc_display_get_address(saddrstr, false, 0, to,
-                                             has_ipv4, has_ipv6,
-                                             ipv4, ipv6,
-                                             &saddr, errp);
-    if (displaynum < 0) {
-        goto error;
+    /* If we had multiple primary displays, we don't do defaults
+     * for websocket, and require explicit config instead. */
+    if (*retnsaddr > 1) {
+        displaynum = -1;
     }
-    if (wsaddrstr) {
-        if (vnc_display_get_address(wsaddrstr, true, displaynum, to,
+
+    qemu_opt_iter_init(&addriter, opts, "websocket");
+    while ((addr = qemu_opt_iter_next(&addriter)) != NULL) {
+        if (vnc_display_get_address(addr, true, displaynum, to,
                                     has_ipv4, has_ipv6,
                                     ipv4, ipv6,
                                     &wsaddr, errp) < 0) {
-            goto error;
+            goto cleanup;
         }
-        if (saddr->type == SOCKET_ADDRESS_KIND_INET &&
+
+        /* Historical compat - if only a single listen address was
+         * provided, then this is used to set the default listen
+         * address for websocket too
+         */
+        if (*retnsaddr == 1 &&
+            (*retsaddr)[0]->type == SOCKET_ADDRESS_KIND_INET &&
             wsaddr->type == SOCKET_ADDRESS_KIND_INET &&
             g_str_equal(wsaddr->u.inet.data->host, "") &&
-            !g_str_equal(saddr->u.inet.data->host, "")) {
+            !g_str_equal((*retsaddr)[0]->u.inet.data->host, "")) {
             g_free(wsaddr->u.inet.data->host);
-            wsaddr->u.inet.data->host = g_strdup(saddr->u.inet.data->host);
+            wsaddr->u.inet.data->host =
+                g_strdup((*retsaddr)[0]->u.inet.data->host);
         }
+
+        *retwsaddr = g_renew(SocketAddress *, *retwsaddr, *retnwsaddr + 1);
+        (*retwsaddr)[(*retnwsaddr)++] = wsaddr;
     }
-    *retsaddr = saddr;
-    *retwsaddr = wsaddr;
-    return 0;
 
- error:
-    qapi_free_SocketAddress(saddr);
-    qapi_free_SocketAddress(wsaddr);
-    return -1;
+    ret = 0;
+ cleanup:
+    if (ret < 0) {
+        for (i = 0; i < *retnsaddr; i++) {
+            qapi_free_SocketAddress((*retsaddr)[i]);
+        }
+        g_free(*retsaddr);
+        for (i = 0; i < *retnwsaddr; i++) {
+            qapi_free_SocketAddress((*retwsaddr)[i]);
+        }
+        g_free(*retwsaddr);
+        *retsaddr = *retwsaddr = NULL;
+        *retnsaddr = *retnwsaddr = 0;
+    }
+    return ret;
 }
 
 static int vnc_display_connect(VncDisplay *vd,
-                               SocketAddress *saddr,
-                               SocketAddress *wsaddr,
+                               SocketAddress **saddr,
+                               size_t nsaddr,
+                               SocketAddress **wsaddr,
+                               size_t nwsaddr,
                                Error **errp)
 {
     /* connect to viewer */
     QIOChannelSocket *sioc = NULL;
-    if (wsaddr) {
+    if (nwsaddr != 0) {
         error_setg(errp, "Cannot use websockets in reverse mode");
         return -1;
     }
-    vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
+    if (nsaddr != 1) {
+        error_setg(errp, "Expected a single address in reverse mode");
+        return -1;
+    }
+    vd->is_unix = saddr[0]->type == SOCKET_ADDRESS_KIND_UNIX;
     sioc = qio_channel_socket_new();
     qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse");
-    if (qio_channel_socket_connect_sync(sioc, saddr, errp) < 0) {
+    if (qio_channel_socket_connect_sync(sioc, saddr[0], errp) < 0) {
         return -1;
     }
     vnc_connect(vd, sioc, false, false);
@@ -3731,6 +3788,7 @@ static int vnc_display_listen_addr(VncDisplay *vd,
     SocketAddress **rawaddrs = NULL;
     size_t nrawaddrs = 0;
     Error *listenerr = NULL;
+    bool listening = false;
     size_t i;
 
     if (qio_dns_resolver_lookup_sync(resolver, addr, &nrawaddrs,
@@ -3746,6 +3804,7 @@ static int vnc_display_listen_addr(VncDisplay *vd,
                 sioc, rawaddrs[i], listenerr == NULL ? &listenerr : NULL) < 0) {
             continue;
         }
+        listening = true;
         (*nlsock)++;
         *lsock = g_renew(QIOChannelSocket *, *lsock, *nlsock);
         *lsock_tag = g_renew(guint, *lsock_tag, *nlsock);
@@ -3760,7 +3819,7 @@ static int vnc_display_listen_addr(VncDisplay *vd,
     g_free(rawaddrs);
 
     if (listenerr) {
-        if (*nlsock == 0) {
+        if (!listening) {
             error_propagate(errp, listenerr);
             return -1;
         } else {
@@ -3779,28 +3838,33 @@ static int vnc_display_listen_addr(VncDisplay *vd,
 
 
 static int vnc_display_listen(VncDisplay *vd,
-                              SocketAddress *saddr,
-                              SocketAddress *wsaddr,
+                              SocketAddress **saddr,
+                              size_t nsaddr,
+                              SocketAddress **wsaddr,
+                              size_t nwsaddr,
                               Error **errp)
 {
-    vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
-
-    if (vnc_display_listen_addr(vd, saddr,
-                                "vnc-listen",
-                                &vd->lsock,
-                                &vd->lsock_tag,
-                                &vd->nlsock,
-                                errp) < 0) {
-        return -1;
+    size_t i;
+
+    for (i = 0; i < nsaddr; i++) {
+        if (vnc_display_listen_addr(vd, saddr[i],
+                                    "vnc-listen",
+                                    &vd->lsock,
+                                    &vd->lsock_tag,
+                                    &vd->nlsock,
+                                    errp) < 0) {
+            return -1;
+        }
     }
-    if (wsaddr &&
-        vnc_display_listen_addr(vd, wsaddr,
-                                "vnc-ws-listen",
-                                &vd->lwebsock,
-                                &vd->lwebsock_tag,
-                                &vd->nlwebsock,
-                                errp) < 0) {
-        return -1;
+    for (i = 0; i < nwsaddr; i++) {
+        if (vnc_display_listen_addr(vd, wsaddr[i],
+                                    "vnc-ws-listen",
+                                    &vd->lwebsock,
+                                    &vd->lwebsock_tag,
+                                    &vd->nlwebsock,
+                                    errp) < 0) {
+            return -1;
+        }
     }
 
     return 0;
@@ -3811,7 +3875,8 @@ void vnc_display_open(const char *id, Error **errp)
 {
     VncDisplay *vd = vnc_display_find(id);
     QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
-    SocketAddress *saddr = NULL, *wsaddr = NULL;
+    SocketAddress **saddr = NULL, **wsaddr = NULL;
+    size_t nsaddr, nwsaddr;
     const char *share, *device_id;
     QemuConsole *con;
     bool password = false;
@@ -3824,6 +3889,7 @@ void vnc_display_open(const char *id, Error **errp)
     int acl = 0;
     int lock_key_sync = 1;
     int key_delay_ms;
+    size_t i;
 
     if (!vd) {
         error_setg(errp, "VNC display not active");
@@ -3835,7 +3901,9 @@ void vnc_display_open(const char *id, Error **errp)
         return;
     }
 
-    if (vnc_display_get_addresses(opts, &saddr, &wsaddr, errp) < 0) {
+    if (vnc_display_get_addresses(opts, &saddr, &nsaddr,
+                                  &wsaddr, &nwsaddr, errp) < 0) {
+        g_printerr("failing\n");
         goto fail;
     }
 
@@ -4025,11 +4093,11 @@ void vnc_display_open(const char *id, Error **errp)
     }
 
     if (reverse) {
-        if (vnc_display_connect(vd, saddr, wsaddr, errp) < 0) {
+        if (vnc_display_connect(vd, saddr, nsaddr, wsaddr, nwsaddr, errp) < 0) {
             goto fail;
         }
     } else {
-        if (vnc_display_listen(vd, saddr, wsaddr, errp) < 0) {
+        if (vnc_display_listen(vd, saddr, nsaddr, wsaddr, nwsaddr, errp) < 0) {
             goto fail;
         }
     }
@@ -4038,14 +4106,18 @@ void vnc_display_open(const char *id, Error **errp)
         vnc_display_print_local_addr(vd);
     }
 
-    qapi_free_SocketAddress(saddr);
-    qapi_free_SocketAddress(wsaddr);
+ cleanup:
+    for (i = 0; i < nsaddr; i++) {
+        qapi_free_SocketAddress(saddr[i]);
+    }
+    for (i = 0; i < nwsaddr; i++) {
+        qapi_free_SocketAddress(wsaddr[i]);
+    }
     return;
 
 fail:
     vnc_display_close(vd);
-    qapi_free_SocketAddress(saddr);
-    qapi_free_SocketAddress(wsaddr);
+    goto cleanup;
 }
 
 void vnc_display_add_client(const char *id, int csock, bool skipauth)
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 7/8] util: add iterators for QemuOpts values

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

On 01/24/2017 03:53 AM, Daniel P. Berrange wrote:
> To iterate over all QemuOpts currently requires using a callback
> function which is inconvenient for control flow. Add support for
> using iterator functions more directly
> 
>   QemuOptsIter iter;
>   QemuOpt *opt;
> 
>   qemu_opts_iter_init(&iter, opts, "repeated-key");
>   while ((opt = qemu_opts_iter_next(&iter)) != NULL) {
>       ....do something...
>   }
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  include/qemu/option.h |  9 +++++++++
>  util/qemu-option.c    | 19 +++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 8/8] ui: add ability to specify multiple VNC listen addresses

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

On 01/24/2017 03:53 AM, Daniel P. Berrange wrote:
> This change allows the listen address and websocket address
> options for -vnc to be repeated. This causes the VNC server
> to listen on multiple addresses. e.g.
> 
>  $ $QEMU -vnc vnc=localhost:1,vnc=unix:/tmp/vnc,\
>               websocket=127.0.0.1:8080,websocket=[::]:8081
> 
> results in listening on
> 
> 127.0.0.1:5901, 127.0.0.1:8080, ::1:5901, :::8081 & /tmp/vnc
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  ui/vnc.c | 198 +++++++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 135 insertions(+), 63 deletions(-)
> 

> @@ -3835,7 +3901,9 @@ void vnc_display_open(const char *id, Error **errp)
>          return;
>      }
>  
> -    if (vnc_display_get_addresses(opts, &saddr, &wsaddr, errp) < 0) {
> +    if (vnc_display_get_addresses(opts, &saddr, &nsaddr,
> +                                  &wsaddr, &nwsaddr, errp) < 0) {
> +        g_printerr("failing\n");
>          goto fail;

Leftover debugging?

Otherwise looks okay, so with that line gone,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]