[PATCH 0/2] qapi/ui: add change-vnc-listen

[PATCH 0/2] qapi/ui: add change-vnc-listen

[Vladimir Sementsov-Ogievskiy]

Hi all!

Recently our customer requested a possibility to change VNC listen port
dynamically.

Happily in Rhel7-based Qemu we already have this possibility: through
deprecated "change" qmp command.

But since 6.0 "change" qmp command was removed, with recommendation to
use change-vnc-password or blockdev-change-medium instead. Of course,
neither of them allow change VNC listen port.

So, let's reimplement the possibility.

Note: now, reconnecting may trigger existing deadlock, as I described
in my message "Re: [PULL 09/11] ui/vnc: clipboard support":
 <973ddebe-14a9-4ba7-c389-7a97d6017237@virtuozzo.com>

Simple hack helps, but I'm not sure it's safe itself:

    diff --git a/ui/vnc.c b/ui/vnc.c
    index 69bbf3b6f6..8c6b378e2e 100644
    --- a/ui/vnc.c
    +++ b/ui/vnc.c
    @@ -1354,12 +1354,12 @@ void vnc_disconnect_finish(VncState *vs)
             /* last client gone */
             vnc_update_server_surface(vs->vd);
         }
    +    vnc_unlock_output(vs);
    +
         if (vs->cbpeer.update.notify) {
             qemu_clipboard_peer_unregister(&vs->cbpeer);
         }
     
    -    vnc_unlock_output(vs);
    -
         qemu_mutex_destroy(&vs->output_mutex);
         if (vs->bh != NULL) {
             qemu_bh_delete(vs->bh);


Vladimir Sementsov-Ogievskiy (2):
  ui/vnc: refactor arrays of addresses to SocketAddressList
  qapi/ui: introduce change-vnc-listen

 docs/about/removed-features.rst |   3 +-
 qapi/ui.json                    |  12 +++
 ui/vnc.c                        | 155 ++++++++++++++++----------------
 3 files changed, 91 insertions(+), 79 deletions(-)

-- 
2.31.1

[PATCH 1/2] ui/vnc: refactor arrays of addresses to SocketAddressList

[Vladimir Sementsov-Ogievskiy]

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>
---
 ui/vnc.c | 129 ++++++++++++++++++++++---------------------------------
 1 file changed, 51 insertions(+), 78 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index af02522e84..c9e26c70df 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

Re: [PATCH 1/2] ui/vnc: refactor arrays of addresses to SocketAddressList

[Marc-André Lureau]

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

Hi

On Mon, Dec 20, 2021 at 10:21 PM Vladimir Sementsov-Ogievskiy <
vsementsov@virtuozzo.com> wrote:

> 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>
>

Nice
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  ui/vnc.c | 129 ++++++++++++++++++++++---------------------------------
>  1 file changed, 51 insertions(+), 78 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index af02522e84..c9e26c70df 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
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 14642 bytes --]

[PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Vladimir Sementsov-Ogievskiy]

Add command that can 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                    | 12 ++++++++++++
 ui/vnc.c                        | 26 ++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index d42c3341de..20e6901a82 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -348,7 +348,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
+``change-vnc-listen`` instead.
 
 ``query-events`` (removed in 6.0)
 '''''''''''''''''''''''''''''''''
diff --git a/qapi/ui.json b/qapi/ui.json
index d7567ac866..14e6fe0b4c 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1304,3 +1304,15 @@
 { 'command': 'display-reload',
   'data': 'DisplayReloadOptions',
   'boxed' : true }
+
+##
+# @change-vnc-listen:
+#
+# Change set of addresses to listen for connections.
+#
+# Since: 7.0
+#
+##
+{ 'command': 'change-vnc-listen',
+  'data': { 'id': 'str', 'addresses': ['SocketAddress'],
+            '*websockets': ['SocketAddress'] } }
diff --git a/ui/vnc.c b/ui/vnc.c
index c9e26c70df..69bbf3b6f6 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4212,6 +4212,32 @@ fail:
     vnc_display_close(vd);
 }
 
+void qmp_change_vnc_listen(const char *id, SocketAddressList *addresses,
+                           bool has_websockets, SocketAddressList *websockets,
+                           Error **errp)
+{
+    VncDisplay *vd = vnc_display_find(id);
+
+    if (!vd) {
+        error_setg(errp, "VNC display '%s' not active", id);
+        return;
+    }
+
+    if (vd->listener) {
+        qio_net_listener_disconnect(vd->listener);
+        object_unref(OBJECT(vd->listener));
+    }
+    vd->listener = NULL;
+
+    if (vd->wslistener) {
+        qio_net_listener_disconnect(vd->wslistener);
+        object_unref(OBJECT(vd->wslistener));
+    }
+    vd->wslistener = NULL;
+
+    vnc_display_listen(vd, addresses, websockets, errp);
+}
+
 void vnc_display_add_client(const char *id, int csock, bool skipauth)
 {
     VncDisplay *vd = vnc_display_find(id);
-- 
2.31.1

Re: [PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Marc-André Lureau]

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

Hi

On Mon, Dec 20, 2021 at 10:24 PM Vladimir Sementsov-Ogievskiy <
vsementsov@virtuozzo.com> wrote:

> Add command that can 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>
>

Looks good to me,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Could you write an avocado test for it? (tests/avocado/vnc.py)

---
>  docs/about/removed-features.rst |  3 ++-
>  qapi/ui.json                    | 12 ++++++++++++
>  ui/vnc.c                        | 26 ++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/docs/about/removed-features.rst
> b/docs/about/removed-features.rst
> index d42c3341de..20e6901a82 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -348,7 +348,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
> +``change-vnc-listen`` instead.
>
>  ``query-events`` (removed in 6.0)
>  '''''''''''''''''''''''''''''''''
> diff --git a/qapi/ui.json b/qapi/ui.json
> index d7567ac866..14e6fe0b4c 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1304,3 +1304,15 @@
>  { 'command': 'display-reload',
>    'data': 'DisplayReloadOptions',
>    'boxed' : true }
> +
> +##
> +# @change-vnc-listen:
> +#
> +# Change set of addresses to listen for connections.
> +#
> +# Since: 7.0
> +#
> +##
> +{ 'command': 'change-vnc-listen',
> +  'data': { 'id': 'str', 'addresses': ['SocketAddress'],
> +            '*websockets': ['SocketAddress'] } }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index c9e26c70df..69bbf3b6f6 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -4212,6 +4212,32 @@ fail:
>      vnc_display_close(vd);
>  }
>
> +void qmp_change_vnc_listen(const char *id, SocketAddressList *addresses,
> +                           bool has_websockets, SocketAddressList
> *websockets,
> +                           Error **errp)
> +{
> +    VncDisplay *vd = vnc_display_find(id);
> +
> +    if (!vd) {
> +        error_setg(errp, "VNC display '%s' not active", id);
> +        return;
> +    }
> +
> +    if (vd->listener) {
> +        qio_net_listener_disconnect(vd->listener);
> +        object_unref(OBJECT(vd->listener));
> +    }
> +    vd->listener = NULL;
> +
> +    if (vd->wslistener) {
> +        qio_net_listener_disconnect(vd->wslistener);
> +        object_unref(OBJECT(vd->wslistener));
> +    }
> +    vd->wslistener = NULL;
> +
> +    vnc_display_listen(vd, addresses, websockets, errp);
> +}
> +
>  void vnc_display_add_client(const char *id, int csock, bool skipauth)
>  {
>      VncDisplay *vd = vnc_display_find(id);
> --
> 2.31.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 4463 bytes --]

Re: [PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Vladimir Sementsov-Ogievskiy]

21.12.2021 11:13, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Dec 20, 2021 at 10:24 PM Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com <mailto:vsementsov@virtuozzo.com>> wrote:
> 
>     Add command that can 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 <mailto:vsementsov@virtuozzo.com>>
> 
> 
> Looks good to me,
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com <mailto:marcandre.lureau@redhat.com>>
> 
> Could you write an avocado test for it? (tests/avocado/vnc.py)

Thanks a lot for reviewing! I will.


-- 
Best regards,
Vladimir

Re: [PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> Add command that can 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                    | 12 ++++++++++++
>  ui/vnc.c                        | 26 ++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index d42c3341de..20e6901a82 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -348,7 +348,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
> +``change-vnc-listen`` instead.
>  
>  ``query-events`` (removed in 6.0)
>  '''''''''''''''''''''''''''''''''
> diff --git a/qapi/ui.json b/qapi/ui.json
> index d7567ac866..14e6fe0b4c 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1304,3 +1304,15 @@
>  { 'command': 'display-reload',
>    'data': 'DisplayReloadOptions',
>    'boxed' : true }
> +
> +##
> +# @change-vnc-listen:
> +#
> +# Change set of addresses to listen for connections.

Please document the arguments:

   # @id: lorem ipsum
   #
   # @address: dolor sit amet
   #
   # @websockets: consectetur adipisici elit

> +#
> +# Since: 7.0
> +#
> +##
> +{ 'command': 'change-vnc-listen',
> +  'data': { 'id': 'str', 'addresses': ['SocketAddress'],
> +            '*websockets': ['SocketAddress'] } }

Lacks 'if': 'CONFIG_VNC'.

We already have change-vnc-password.  You add change-vnc-listen.  Is
there anything else we might want to change?

Aside: what's the difference between change-vnc-password and
set_password?

[...]

Re: [PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Vladimir Sementsov-Ogievskiy]

21.12.2021 17:15, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> 
>> Add command that can 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                    | 12 ++++++++++++
>>   ui/vnc.c                        | 26 ++++++++++++++++++++++++++
>>   3 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
>> index d42c3341de..20e6901a82 100644
>> --- a/docs/about/removed-features.rst
>> +++ b/docs/about/removed-features.rst
>> @@ -348,7 +348,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
>> +``change-vnc-listen`` instead.
>>   
>>   ``query-events`` (removed in 6.0)
>>   '''''''''''''''''''''''''''''''''
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index d7567ac866..14e6fe0b4c 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1304,3 +1304,15 @@
>>   { 'command': 'display-reload',
>>     'data': 'DisplayReloadOptions',
>>     'boxed' : true }
>> +
>> +##
>> +# @change-vnc-listen:
>> +#
>> +# Change set of addresses to listen for connections.
> 
> Please document the arguments:
> 
>     # @id: lorem ipsum
>     #
>     # @address: dolor sit amet
>     #
>     # @websockets: consectetur adipisici elit

Oops :)

# @id: vnc display identifier
#
# @addresses: list of addresses for listen at
#
# @websockets: list of addresses to listen with websockets

> 
>> +#
>> +# Since: 7.0
>> +#
>> +##
>> +{ 'command': 'change-vnc-listen',
>> +  'data': { 'id': 'str', 'addresses': ['SocketAddress'],
>> +            '*websockets': ['SocketAddress'] } }
> 
> Lacks 'if': 'CONFIG_VNC'.

Oops, yes.

> 
> We already have change-vnc-password.  You add change-vnc-listen.  Is
> there anything else we might want to change?

I don't know. I have a request to change only the port of connection.

But creating a special command to change only the port is too specific.

On the other hand, creating command that will allow to change many other vnc parameters means deeper refactoring the vnc code, it's too much for me.

Old removed "change" command allowed to change many vnc arguments as I understand, but they we parsed from one string argument, which is bad for QMP.

So, I decided that the golden mean is make an interface to change the addresses to listen.

Actually, I don't need "websockets", and even don't know how to test them, so we can drop this parameter for now, it's simple to add it later on demand. Or we can keep it as is.

> 
> Aside: what's the difference between change-vnc-password and
> set_password?

Looking at code, the difference is that set_password can also change password on spice, and has some additional logic  with "connected" argument.

> 
> [...]
> 


-- 
Best regards,
Vladimir

Re: [PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Daniel P. Berrangé]

On Mon, Dec 20, 2021 at 04:44:18PM +0100, Vladimir Sementsov-Ogievskiy wrote:
> Add command that can 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                    | 12 ++++++++++++
>  ui/vnc.c                        | 26 ++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index d42c3341de..20e6901a82 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -348,7 +348,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
> +``change-vnc-listen`` instead.
>  
>  ``query-events`` (removed in 6.0)
>  '''''''''''''''''''''''''''''''''
> diff --git a/qapi/ui.json b/qapi/ui.json
> index d7567ac866..14e6fe0b4c 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1304,3 +1304,15 @@
>  { 'command': 'display-reload',
>    'data': 'DisplayReloadOptions',
>    'boxed' : true }
> +
> +##
> +# @change-vnc-listen:
> +#
> +# Change set of addresses to listen for connections.
> +#
> +# Since: 7.0
> +#
> +##
> +{ 'command': 'change-vnc-listen',
> +  'data': { 'id': 'str', 'addresses': ['SocketAddress'],
> +            '*websockets': ['SocketAddress'] } }

We already have a general purpose command above 'display-reload'
for doing live changes to the display backends.

THis should instead be

{ 'struct': 'DisplayReloadOptionsVNC',
  'data': { '*tls-certs': 'bool',
            '*addresses': ['SocketAddress'],
	    '*websockets': ['SocketAddress'] } }

if 'addresses' is non-null then the listener can be updated.


Regards,
Daniel
-- 
|: 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 :|

Re: [PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Dec 20, 2021 at 04:44:18PM +0100, Vladimir Sementsov-Ogievskiy wrote:
>> Add command that can 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                    | 12 ++++++++++++
>>  ui/vnc.c                        | 26 ++++++++++++++++++++++++++
>>  3 files changed, 40 insertions(+), 1 deletion(-)
>> 
>> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
>> index d42c3341de..20e6901a82 100644
>> --- a/docs/about/removed-features.rst
>> +++ b/docs/about/removed-features.rst
>> @@ -348,7 +348,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
>> +``change-vnc-listen`` instead.
>>  
>>  ``query-events`` (removed in 6.0)
>>  '''''''''''''''''''''''''''''''''
>> diff --git a/qapi/ui.json b/qapi/ui.json
>> index d7567ac866..14e6fe0b4c 100644
>> --- a/qapi/ui.json
>> +++ b/qapi/ui.json
>> @@ -1304,3 +1304,15 @@
>>  { 'command': 'display-reload',
>>    'data': 'DisplayReloadOptions',
>>    'boxed' : true }
>> +
>> +##
>> +# @change-vnc-listen:
>> +#
>> +# Change set of addresses to listen for connections.
>> +#
>> +# Since: 7.0
>> +#
>> +##
>> +{ 'command': 'change-vnc-listen',
>> +  'data': { 'id': 'str', 'addresses': ['SocketAddress'],
>> +            '*websockets': ['SocketAddress'] } }
>
> We already have a general purpose command above 'display-reload'
> for doing live changes to the display backends.
>
> THis should instead be
>
> { 'struct': 'DisplayReloadOptionsVNC',
>   'data': { '*tls-certs': 'bool',
>             '*addresses': ['SocketAddress'],
> 	    '*websockets': ['SocketAddress'] } }
>
> if 'addresses' is non-null then the listener can be updated.

Good point.  Gerd, what do you think?

Re: [PATCH 2/2] qapi/ui: introduce change-vnc-listen

[Daniel P. Berrangé]

On Thu, Jan 13, 2022 at 05:27:08PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Mon, Dec 20, 2021 at 04:44:18PM +0100, Vladimir Sementsov-Ogievskiy wrote:
> >> Add command that can 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                    | 12 ++++++++++++
> >>  ui/vnc.c                        | 26 ++++++++++++++++++++++++++
> >>  3 files changed, 40 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> >> index d42c3341de..20e6901a82 100644
> >> --- a/docs/about/removed-features.rst
> >> +++ b/docs/about/removed-features.rst
> >> @@ -348,7 +348,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
> >> +``change-vnc-listen`` instead.
> >>  
> >>  ``query-events`` (removed in 6.0)
> >>  '''''''''''''''''''''''''''''''''
> >> diff --git a/qapi/ui.json b/qapi/ui.json
> >> index d7567ac866..14e6fe0b4c 100644
> >> --- a/qapi/ui.json
> >> +++ b/qapi/ui.json
> >> @@ -1304,3 +1304,15 @@
> >>  { 'command': 'display-reload',
> >>    'data': 'DisplayReloadOptions',
> >>    'boxed' : true }
> >> +
> >> +##
> >> +# @change-vnc-listen:
> >> +#
> >> +# Change set of addresses to listen for connections.
> >> +#
> >> +# Since: 7.0
> >> +#
> >> +##
> >> +{ 'command': 'change-vnc-listen',
> >> +  'data': { 'id': 'str', 'addresses': ['SocketAddress'],
> >> +            '*websockets': ['SocketAddress'] } }
> >
> > We already have a general purpose command above 'display-reload'
> > for doing live changes to the display backends.
> >
> > THis should instead be
> >
> > { 'struct': 'DisplayReloadOptionsVNC',
> >   'data': { '*tls-certs': 'bool',
> >             '*addresses': ['SocketAddress'],
> > 	    '*websockets': ['SocketAddress'] } }
> >
> > if 'addresses' is non-null then the listener can be updated.
> 
> Good point.  Gerd, what do you think?

I guess you could argue that 'display-reload' is more about reloading
state, rather than changing configuration.  If we want to make such a
distinction, then we could have a very similar 'display-update' command
for configuration changes.

Regards,
Daniel
-- 
|: 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 :|

[PATCH 3/2] avocado/vnc: add test_change_listen

[Vladimir Sementsov-Ogievskiy]

Add simple test-case for new change-vnc-listen qmp command.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/avocado/vnc.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tests/avocado/vnc.py b/tests/avocado/vnc.py
index 096432988f..f05ee1e00a 100644
--- a/tests/avocado/vnc.py
+++ b/tests/avocado/vnc.py
@@ -51,3 +51,13 @@ 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):
+        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0')
+        self.vm.launch()
+        self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], '5900')
+        res = self.vm.qmp('change-vnc-listen', id='default',
+                          addresses=[{'type': 'inet', 'host': '0.0.0.0',
+                                      'port': '5901'}])
+        self.assertEqual(res['return'], {})
+        self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], '5901')
-- 
2.31.1

Re: [PATCH 3/2] avocado/vnc: add test_change_listen

[Marc-André Lureau]

On Tue, Dec 21, 2021 at 5:38 PM Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> Add simple test-case for new change-vnc-listen qmp command.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

thanks

> ---
>  tests/avocado/vnc.py | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/tests/avocado/vnc.py b/tests/avocado/vnc.py
> index 096432988f..f05ee1e00a 100644
> --- a/tests/avocado/vnc.py
> +++ b/tests/avocado/vnc.py
> @@ -51,3 +51,13 @@ 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):
> +        self.vm.add_args('-nodefaults', '-S', '-vnc', ':0')
> +        self.vm.launch()
> +        self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], '5900')
> +        res = self.vm.qmp('change-vnc-listen', id='default',
> +                          addresses=[{'type': 'inet', 'host': '0.0.0.0',
> +                                      'port': '5901'}])
> +        self.assertEqual(res['return'], {})
> +        self.assertEqual(self.vm.qmp('query-vnc')['return']['service'], '5901')
> --
> 2.31.1
>