Allow UNIX socket for VNC websocket

[PATCH] Allow UNIX socket for VNC websocket

Posted by Sergii Zasenko 2 years, 6 months ago

Signed-off-by: Sergii Zasenko <sergii@zasenko.name>
---
 ui/vnc.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 92964dc..dea1414 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char *addrstr,
         addr->type = SOCKET_ADDRESS_TYPE_UNIX;
         addr->u.q_unix.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;
-- 
2.39.2

Re: [PATCH] Allow UNIX socket for VNC websocket

Posted by Daniel P. Berrangé 2 years, 6 months ago

On Mon, Jul 24, 2023 at 12:03:56AM +0300, Sergii Zasenko wrote:
> Signed-off-by: Sergii Zasenko <sergii@zasenko.name>
> ---
>  ui/vnc.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 92964dc..dea1414 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char *addrstr,
>          addr->type = SOCKET_ADDRESS_TYPE_UNIX;
>          addr->u.q_unix.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;

Missing docs update to qemu-options.hx to describe the permitted
syntax for the new feature.


With 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] Allow UNIX socket option for VNC websocket

Posted by Sergii Zasenko 2 years, 6 months ago

- Remove unix socket option limitation for VNC websocket
- Reflect websocket option changes in documentation

Signed-off-by: Sergii Zasenko <sergii@zasenko.name>
---
 qemu-options.hx | 4 ++++
 ui/vnc.c        | 5 -----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 29b98c3..8cc910d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2451,6 +2451,10 @@ SRST
         host. It is possible to control the websocket listen address
         independently, using the syntax ``websocket``\ =host:port.
 
+        Websocket could be allowed over UNIX domain socket, using the syntax
+        ``websocket``\ =unix:path, where path is the location of a unix socket
+        to listen for connections on.
+
         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 92964dc..dea1414 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char *addrstr,
         addr->type = SOCKET_ADDRESS_TYPE_UNIX;
         addr->u.q_unix.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;
-- 
2.39.2

Re: [PATCH] Allow UNIX socket option for VNC websocket

Posted by Marc-André Lureau 2 years, 6 months ago

On Mon, Jul 24, 2023 at 2:04 PM Sergii Zasenko <sergii@zasenko.name> wrote:

> - Remove unix socket option limitation for VNC websocket
> - Reflect websocket option changes in documentation
>
> Signed-off-by: Sergii Zasenko <sergii@zasenko.name>
>

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


> ---
>  qemu-options.hx | 4 ++++
>  ui/vnc.c        | 5 -----
>  2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 29b98c3..8cc910d 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2451,6 +2451,10 @@ SRST
>          host. It is possible to control the websocket listen address
>          independently, using the syntax ``websocket``\ =host:port.
>
> +        Websocket could be allowed over UNIX domain socket, using the
> syntax
> +        ``websocket``\ =unix:path, where path is the location of a unix
> socket
> +        to listen for connections on.
> +
>          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 92964dc..dea1414 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char
> *addrstr,
>          addr->type = SOCKET_ADDRESS_TYPE_UNIX;
>          addr->u.q_unix.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;
> --
> 2.39.2
>
>
>

-- 
Marc-André Lureau

Re: [PATCH] Allow UNIX socket for VNC websocket

Posted by Marc-André Lureau 2 years, 6 months ago

Hi

Daniel, in commit 275e0d616b ("ui: refactor code for populating
SocketAddress from vnc_display_open"), you said "This refactoring also
removes the restriction that prevents enabling websockets when the plain
VNC server is listening on a UNIX socket.". But you didn't remove the
condition. I suppose it was a left-over?

On Mon, Jul 24, 2023 at 1:59 AM Sergii Zasenko <sergii@zasenko.name> wrote:

> Signed-off-by: Sergii Zasenko <sergii@zasenko.name>
>

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

> ---
>  ui/vnc.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 92964dc..dea1414 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3715,11 +3715,6 @@ static int vnc_display_get_address(const char
> *addrstr,
>          addr->type = SOCKET_ADDRESS_TYPE_UNIX;
>          addr->u.q_unix.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;
> --
> 2.39.2
>
>
>

-- 
Marc-André Lureau

Re: [PATCH] Allow UNIX socket for VNC websocket

Posted by Daniel P. Berrangé 2 years, 6 months ago

On Mon, Jul 24, 2023 at 12:08:26PM +0400, Marc-André Lureau wrote:
> Hi
> 
> Daniel, in commit 275e0d616b ("ui: refactor code for populating
> SocketAddress from vnc_display_open"), you said "This refactoring also
> removes the restriction that prevents enabling websockets when the plain
> VNC server is listening on a UNIX socket.". But you didn't remove the
> condition. I suppose it was a left-over?

That commit was allowing the regular VNC port to be put on UNIX
socket, at the same time as WS was enabled, it wasn't trying to
put WS on a UNIX socket. Still I seee no reason to keep the
limitation.


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