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

[Eric Blake <eblake@redhat.com>]

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

On 01/05/2017 10:06 AM, Daniel P. Berrange wrote:
> Currently there is only a single listener for plain VNC and
> a single listener for websockets VNC. This means that is

s/is/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.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  ui/vnc.c | 103 +++++++++++++++++++++++++++++++++++++++++----------------------
>  ui/vnc.h |  10 ++++---
>  2 files changed, 73 insertions(+), 40 deletions(-)

Are you using git's order directives to put .h files first?


> @@ -3819,30 +3843,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);

Is guint really the best type here...

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

...and here?

> +    size_t nlwebsock;
> +    QIOChannelSocket **lwebsock;
> +    guint *lwebsock_tag;
>      DisplaySurface *ds;
>      DisplayChangeListener dcl;
>      kbd_layout_t *kbd_layout;
> 

Whether or not you tweak the types to something a little more direct (I
thing guint is more hassle than it's worth when not directly passing the
field to glib functions),
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 --]