* [PATCH v4 1/3] ui/vnc: refactor arrays of addresses to SocketAddressList
2022-02-10 14:14 [PATCH v4 0/3] qapi/ui: change vnc addresses Vladimir Sementsov-Ogievskiy
@ 2022-02-10 14:14 ` Vladimir Sementsov-Ogievskiy
2022-02-10 14:14 ` [PATCH v4 2/3] qapi/ui: display-update: add possibility to change listen address Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-10 14:14 UTC (permalink / raw)
To: qemu-devel
Cc: bleal, wainersm, f4bug, crosa, eblake, armbru, kraxel, vsementsov,
berrange, marcandre.lureau
Let's use SocketAddressList instead of dynamic arrays.
Benefits:
- Automatic cleanup: don't need specific freeing function and drop
some gotos.
- Less indirection: no triple asterix anymore!
- Prepare for the following commit, which will reuse new interface of
vnc_display_listen().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
ui/vnc.c | 129 ++++++++++++++++++++++---------------------------------
1 file changed, 51 insertions(+), 78 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ccd33dedc..fa0fb736d3 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3812,30 +3812,19 @@ static int vnc_display_get_address(const char *addrstr,
return ret;
}
-static void vnc_free_addresses(SocketAddress ***retsaddr,
- size_t *retnsaddr)
-{
- size_t i;
-
- for (i = 0; i < *retnsaddr; i++) {
- qapi_free_SocketAddress((*retsaddr)[i]);
- }
- g_free(*retsaddr);
-
- *retsaddr = NULL;
- *retnsaddr = 0;
-}
-
static int vnc_display_get_addresses(QemuOpts *opts,
bool reverse,
- SocketAddress ***retsaddr,
- size_t *retnsaddr,
- SocketAddress ***retwsaddr,
- size_t *retnwsaddr,
+ SocketAddressList **saddr_list_ret,
+ SocketAddressList **wsaddr_list_ret,
Error **errp)
{
SocketAddress *saddr = NULL;
SocketAddress *wsaddr = NULL;
+ g_autoptr(SocketAddressList) saddr_list = NULL;
+ SocketAddressList **saddr_tail = &saddr_list;
+ SocketAddress *single_saddr = NULL;
+ g_autoptr(SocketAddressList) wsaddr_list = NULL;
+ SocketAddressList **wsaddr_tail = &wsaddr_list;
QemuOptsIter addriter;
const char *addr;
int to = qemu_opt_get_number(opts, "to", 0);
@@ -3844,23 +3833,16 @@ static int vnc_display_get_addresses(QemuOpts *opts,
bool ipv4 = qemu_opt_get_bool(opts, "ipv4", false);
bool ipv6 = qemu_opt_get_bool(opts, "ipv6", false);
int displaynum = -1;
- int ret = -1;
-
- *retsaddr = NULL;
- *retnsaddr = 0;
- *retwsaddr = NULL;
- *retnwsaddr = 0;
addr = qemu_opt_get(opts, "vnc");
if (addr == NULL || g_str_equal(addr, "none")) {
- ret = 0;
- goto cleanup;
+ return 0;
}
if (qemu_opt_get(opts, "websocket") &&
!qcrypto_hash_supports(QCRYPTO_HASH_ALG_SHA1)) {
error_setg(errp,
"SHA1 hash support is required for websockets");
- goto cleanup;
+ return -1;
}
qemu_opt_iter_init(&addriter, opts, "vnc");
@@ -3871,7 +3853,7 @@ static int vnc_display_get_addresses(QemuOpts *opts,
ipv4, ipv6,
&saddr, errp);
if (rv < 0) {
- goto cleanup;
+ return -1;
}
/* Historical compat - first listen address can be used
* to set the default websocket port
@@ -3879,13 +3861,16 @@ static int vnc_display_get_addresses(QemuOpts *opts,
if (displaynum == -1) {
displaynum = rv;
}
- *retsaddr = g_renew(SocketAddress *, *retsaddr, *retnsaddr + 1);
- (*retsaddr)[(*retnsaddr)++] = saddr;
+ QAPI_LIST_APPEND(saddr_tail, saddr);
}
- /* If we had multiple primary displays, we don't do defaults
- * for websocket, and require explicit config instead. */
- if (*retnsaddr > 1) {
+ if (saddr_list && !saddr_list->next) {
+ single_saddr = saddr_list->value;
+ } else {
+ /*
+ * If we had multiple primary displays, we don't do defaults
+ * for websocket, and require explicit config instead.
+ */
displaynum = -1;
}
@@ -3895,57 +3880,50 @@ static int vnc_display_get_addresses(QemuOpts *opts,
has_ipv4, has_ipv6,
ipv4, ipv6,
&wsaddr, errp) < 0) {
- goto cleanup;
+ return -1;
}
/* 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_TYPE_INET &&
+ if (single_saddr &&
+ single_saddr->type == SOCKET_ADDRESS_TYPE_INET &&
wsaddr->type == SOCKET_ADDRESS_TYPE_INET &&
g_str_equal(wsaddr->u.inet.host, "") &&
- !g_str_equal((*retsaddr)[0]->u.inet.host, "")) {
+ !g_str_equal(single_saddr->u.inet.host, "")) {
g_free(wsaddr->u.inet.host);
- wsaddr->u.inet.host = g_strdup((*retsaddr)[0]->u.inet.host);
+ wsaddr->u.inet.host = g_strdup(single_saddr->u.inet.host);
}
- *retwsaddr = g_renew(SocketAddress *, *retwsaddr, *retnwsaddr + 1);
- (*retwsaddr)[(*retnwsaddr)++] = wsaddr;
+ QAPI_LIST_APPEND(wsaddr_tail, wsaddr);
}
- ret = 0;
- cleanup:
- if (ret < 0) {
- vnc_free_addresses(retsaddr, retnsaddr);
- vnc_free_addresses(retwsaddr, retnwsaddr);
- }
- return ret;
+ *saddr_list_ret = g_steal_pointer(&saddr_list);
+ *wsaddr_list_ret = g_steal_pointer(&wsaddr_list);
+ return 0;
}
static int vnc_display_connect(VncDisplay *vd,
- SocketAddress **saddr,
- size_t nsaddr,
- SocketAddress **wsaddr,
- size_t nwsaddr,
+ SocketAddressList *saddr_list,
+ SocketAddressList *wsaddr_list,
Error **errp)
{
/* connect to viewer */
QIOChannelSocket *sioc = NULL;
- if (nwsaddr != 0) {
+ if (wsaddr_list) {
error_setg(errp, "Cannot use websockets in reverse mode");
return -1;
}
- if (nsaddr != 1) {
+ if (!saddr_list || saddr_list->next) {
error_setg(errp, "Expected a single address in reverse mode");
return -1;
}
/* TODO SOCKET_ADDRESS_TYPE_FD when fd has AF_UNIX */
- vd->is_unix = saddr[0]->type == SOCKET_ADDRESS_TYPE_UNIX;
+ vd->is_unix = saddr_list->value->type == SOCKET_ADDRESS_TYPE_UNIX;
sioc = qio_channel_socket_new();
qio_channel_set_name(QIO_CHANNEL(sioc), "vnc-reverse");
- if (qio_channel_socket_connect_sync(sioc, saddr[0], errp) < 0) {
+ if (qio_channel_socket_connect_sync(sioc, saddr_list->value, errp) < 0) {
object_unref(OBJECT(sioc));
return -1;
}
@@ -3956,20 +3934,18 @@ static int vnc_display_connect(VncDisplay *vd,
static int vnc_display_listen(VncDisplay *vd,
- SocketAddress **saddr,
- size_t nsaddr,
- SocketAddress **wsaddr,
- size_t nwsaddr,
+ SocketAddressList *saddr_list,
+ SocketAddressList *wsaddr_list,
Error **errp)
{
- size_t i;
+ SocketAddressList *el;
- if (nsaddr) {
+ if (saddr_list) {
vd->listener = qio_net_listener_new();
qio_net_listener_set_name(vd->listener, "vnc-listen");
- for (i = 0; i < nsaddr; i++) {
+ for (el = saddr_list; el; el = el->next) {
if (qio_net_listener_open_sync(vd->listener,
- saddr[i], 1,
+ el->value, 1,
errp) < 0) {
return -1;
}
@@ -3979,12 +3955,12 @@ static int vnc_display_listen(VncDisplay *vd,
vnc_listen_io, vd, NULL);
}
- if (nwsaddr) {
+ if (wsaddr_list) {
vd->wslistener = qio_net_listener_new();
qio_net_listener_set_name(vd->wslistener, "vnc-ws-listen");
- for (i = 0; i < nwsaddr; i++) {
+ for (el = wsaddr_list; el; el = el->next) {
if (qio_net_listener_open_sync(vd->wslistener,
- wsaddr[i], 1,
+ el->value, 1,
errp) < 0) {
return -1;
}
@@ -4002,8 +3978,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;
- size_t nsaddr, nwsaddr;
+ g_autoptr(SocketAddressList) saddr_list = NULL;
+ g_autoptr(SocketAddressList) wsaddr_list = NULL;
const char *share, *device_id;
QemuConsole *con;
bool password = false;
@@ -4028,8 +4004,8 @@ void vnc_display_open(const char *id, Error **errp)
}
reverse = qemu_opt_get_bool(opts, "reverse", false);
- if (vnc_display_get_addresses(opts, reverse, &saddr, &nsaddr,
- &wsaddr, &nwsaddr, errp) < 0) {
+ if (vnc_display_get_addresses(opts, reverse, &saddr_list, &wsaddr_list,
+ errp) < 0) {
goto fail;
}
@@ -4211,16 +4187,16 @@ void vnc_display_open(const char *id, Error **errp)
}
qkbd_state_set_delay(vd->kbd, key_delay_ms);
- if (saddr == NULL) {
- goto cleanup;
+ if (saddr_list == NULL) {
+ return;
}
if (reverse) {
- if (vnc_display_connect(vd, saddr, nsaddr, wsaddr, nwsaddr, errp) < 0) {
+ if (vnc_display_connect(vd, saddr_list, wsaddr_list, errp) < 0) {
goto fail;
}
} else {
- if (vnc_display_listen(vd, saddr, nsaddr, wsaddr, nwsaddr, errp) < 0) {
+ if (vnc_display_listen(vd, saddr_list, wsaddr_list, errp) < 0) {
goto fail;
}
}
@@ -4229,14 +4205,11 @@ void vnc_display_open(const char *id, Error **errp)
vnc_display_print_local_addr(vd);
}
- cleanup:
- vnc_free_addresses(&saddr, &nsaddr);
- vnc_free_addresses(&wsaddr, &nwsaddr);
+ /* Success */
return;
fail:
vnc_display_close(vd);
- goto cleanup;
}
void vnc_display_add_client(const char *id, int csock, bool skipauth)
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v4 2/3] qapi/ui: display-update: add possibility to change listen address
2022-02-10 14:14 [PATCH v4 0/3] qapi/ui: change vnc addresses Vladimir Sementsov-Ogievskiy
2022-02-10 14:14 ` [PATCH v4 1/3] ui/vnc: refactor arrays of addresses to SocketAddressList Vladimir Sementsov-Ogievskiy
@ 2022-02-10 14:14 ` Vladimir Sementsov-Ogievskiy
2022-02-10 14:32 ` Daniel P. Berrangé
2022-02-10 14:14 ` [PATCH v4 3/3] avocado/vnc: add test_change_listen Vladimir Sementsov-Ogievskiy
2022-02-10 15:46 ` [PATCH v4 0/3] qapi/ui: change vnc addresses Markus Armbruster
3 siblings, 1 reply; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-10 14:14 UTC (permalink / raw)
To: qemu-devel
Cc: bleal, wainersm, f4bug, crosa, eblake, armbru, kraxel, vsementsov,
berrange, marcandre.lureau
Add possibility to change addresses where VNC server listens for new
connections. Prior to 6.0 this functionality was available through
'change' qmp command which was deleted.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
docs/about/removed-features.rst | 3 +-
qapi/ui.json | 65 +++++++++++++++++++++++++++++++++
include/ui/console.h | 1 +
monitor/qmp-cmds.c | 15 ++++++++
ui/vnc.c | 23 ++++++++++++
5 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 4c4da20d0f..63aba60873 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -355,7 +355,8 @@ documentation of ``query-hotpluggable-cpus`` for additional details.
``change`` (removed in 6.0)
'''''''''''''''''''''''''''
-Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.
+Use ``blockdev-change-medium`` or ``change-vnc-password`` or
+``display-update`` instead.
``query-events`` (removed in 6.0)
'''''''''''''''''''''''''''''''''
diff --git a/qapi/ui.json b/qapi/ui.json
index 9354f4c467..a0851eeefa 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1334,3 +1334,68 @@
{ 'command': 'display-reload',
'data': 'DisplayReloadOptions',
'boxed' : true }
+
+##
+# @DisplayUpdateType:
+#
+# Available DisplayUpdate types.
+#
+# @vnc: VNC display
+#
+# Since: 7.0
+#
+##
+{ 'enum': 'DisplayUpdateType',
+ 'data': ['vnc'] }
+
+##
+# @DisplayUpdateOptionsVNC:
+#
+# Specify the VNC reload options.
+#
+# @addresses: If specified, change set of addresses
+# to listen for connections. Addresses configured
+# for websockets are not touched.
+#
+# Since: 7.0
+#
+##
+{ 'struct': 'DisplayUpdateOptionsVNC',
+ 'data': { '*addresses': ['SocketAddress'] } }
+
+##
+# @DisplayUpdateOptions:
+#
+# Options of the display configuration reload.
+#
+# @type: Specify the display type.
+#
+# Since: 7.0
+#
+##
+{ 'union': 'DisplayUpdateOptions',
+ 'base': {'type': 'DisplayUpdateType'},
+ 'discriminator': 'type',
+ 'data': { 'vnc': 'DisplayUpdateOptionsVNC' } }
+
+##
+# @display-update:
+#
+# Update display configuration.
+#
+# Returns: Nothing on success.
+#
+# Since: 7.0
+#
+# Example:
+#
+# -> { "execute": "display-reload",
+# "arguments": { "type": "vnc", "addresses":
+# [ { "type": "inet", "host": "0.0.0.0",
+# "port": "5901" } ] } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'display-update',
+ 'data': 'DisplayUpdateOptions',
+ 'boxed' : true }
diff --git a/include/ui/console.h b/include/ui/console.h
index f590819880..45f2ed4e2c 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -513,6 +513,7 @@ int vnc_display_pw_expire(const char *id, time_t expires);
void vnc_parse(const char *str);
int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
bool vnc_display_reload_certs(const char *id, Error **errp);
+bool vnc_display_update(DisplayUpdateOptionsVNC *arg, Error **errp);
/* input.c */
int index_from_key(const char *key, size_t key_length);
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index db4d186448..246c8c1202 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -368,6 +368,21 @@ void qmp_display_reload(DisplayReloadOptions *arg, Error **errp)
}
}
+void qmp_display_update(DisplayUpdateOptions *arg, Error **errp)
+{
+ switch (arg->type) {
+ case DISPLAY_UPDATE_TYPE_VNC:
+#ifdef CONFIG_VNC
+ vnc_display_update(&arg->u.vnc, errp);
+#else
+ error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'");
+#endif
+ break;
+ default:
+ abort();
+ }
+}
+
static int qmp_x_query_rdma_foreach(Object *obj, void *opaque)
{
RdmaProvider *rdma;
diff --git a/ui/vnc.c b/ui/vnc.c
index fa0fb736d3..c57146d9f8 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3973,6 +3973,29 @@ static int vnc_display_listen(VncDisplay *vd,
return 0;
}
+bool vnc_display_update(DisplayUpdateOptionsVNC *arg, Error **errp)
+{
+ VncDisplay *vd = vnc_display_find(NULL);
+
+ if (!vd) {
+ error_setg(errp, "Can not find vnc display");
+ return false;
+ }
+
+ if (arg->has_addresses) {
+ if (vd->listener) {
+ qio_net_listener_disconnect(vd->listener);
+ object_unref(OBJECT(vd->listener));
+ vd->listener = NULL;
+ }
+
+ if (vnc_display_listen(vd, arg->addresses, NULL, errp) < 0) {
+ return false;
+ }
+ }
+
+ return true;
+}
void vnc_display_open(const char *id, Error **errp)
{
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v4 3/3] avocado/vnc: add test_change_listen
2022-02-10 14:14 [PATCH v4 0/3] qapi/ui: change vnc addresses Vladimir Sementsov-Ogievskiy
2022-02-10 14:14 ` [PATCH v4 1/3] ui/vnc: refactor arrays of addresses to SocketAddressList Vladimir Sementsov-Ogievskiy
2022-02-10 14:14 ` [PATCH v4 2/3] qapi/ui: display-update: add possibility to change listen address Vladimir Sementsov-Ogievskiy
@ 2022-02-10 14:14 ` Vladimir Sementsov-Ogievskiy
2022-02-10 14:37 ` Daniel P. Berrangé
2022-02-10 15:46 ` [PATCH v4 0/3] qapi/ui: change vnc addresses Markus Armbruster
3 siblings, 1 reply; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2022-02-10 14:14 UTC (permalink / raw)
To: qemu-devel
Cc: bleal, wainersm, f4bug, crosa, eblake, armbru, kraxel, vsementsov,
berrange, marcandre.lureau
Add simple test-case for new display-update qmp command.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/avocado/vnc.py | 63 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/tests/avocado/vnc.py b/tests/avocado/vnc.py
index 096432988f..187fd3febc 100644
--- a/tests/avocado/vnc.py
+++ b/tests/avocado/vnc.py
@@ -8,9 +8,48 @@
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
+import socket
+from typing import List
+
from avocado_qemu import QemuSystemTest
+VNC_ADDR = '127.0.0.1'
+VNC_PORT_START = 32768
+VNC_PORT_END = VNC_PORT_START + 1024
+
+
+def check_bind(port: int) -> bool:
+ with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
+ try:
+ sock.bind((VNC_ADDR, port))
+ except OSError:
+ return False
+
+ return True
+
+
+def check_connect(port: int) -> bool:
+ with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
+ try:
+ sock.connect((VNC_ADDR, port))
+ except ConnectionRefusedError:
+ return False
+
+ return True
+
+
+def find_free_ports(count: int) -> List[int]:
+ result = []
+ for port in range(VNC_PORT_START, VNC_PORT_END):
+ if check_bind(port):
+ result.append(port)
+ if len(result) >= count:
+ break
+ assert len(result) == count
+ return result
+
+
class Vnc(QemuSystemTest):
"""
:avocado: tags=vnc,quick
@@ -51,3 +90,27 @@ def test_change_password(self):
set_password_response = self.vm.qmp('change-vnc-password',
password='new_password')
self.assertEqual(set_password_response['return'], {})
+
+ def test_change_listen(self):
+ a, b, c = find_free_ports(3)
+ self.assertFalse(check_connect(a))
+ self.assertFalse(check_connect(b))
+ self.assertFalse(check_connect(c))
+
+ self.vm.add_args('-nodefaults', '-S', '-vnc', f'{VNC_ADDR}:{a - 5900}')
+ self.vm.launch()
+ self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], str(a))
+ self.assertTrue(check_connect(a))
+ self.assertFalse(check_connect(b))
+ self.assertFalse(check_connect(c))
+
+ res = self.vm.qmp('display-update', type='vnc',
+ addresses=[{'type': 'inet', 'host': VNC_ADDR,
+ 'port': str(b)},
+ {'type': 'inet', 'host': VNC_ADDR,
+ 'port': str(c)}])
+ self.assertEqual(res['return'], {})
+ self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], str(b))
+ self.assertFalse(check_connect(a))
+ self.assertTrue(check_connect(b))
+ self.assertTrue(check_connect(c))
--
2.31.1
^ permalink raw reply related [flat|nested] 7+ messages in thread