On Fri, Nov 23, 2012 at 08:00:47PM +0100, Tim Hardeck wrote:
Thanks for the patch, Tim. Some general code review comments below. I
hope someone has time to review the VNC and WebSocket specific stuff. I
didn't check the details of buffers, whether the WebSocket spec is
correctly implemented, etc.
> QEMU does segfault if a regular VNC client connects to the Websocket
> port and then disconnects because of several unitialized lists since
> vnc_init_state wasn't run before.
> The segfault could be fixed by applying my previously sent patches
> "[PATCH 0/2] fix segfaults triggered by failed vnc handshakes".
The segfault issue should be addressed before merging it. I think the
response on that email thread was to fix the qemu-queue.h users rather
than making it okay to remove an element that isn't on a list
(especially because this relies on uninitialized elements having a NULL
value). So is the next step to fix those list users in VNC code?
> ##########################################
> +# VNC WS detection
> +if test "$vnc" = "yes" -a "$vnc_ws" != "no" ; then
> + cat > $TMPC <<EOF
> +#include <gnutls/gnutls.h>
> +int main(void) { gnutls_session_t s; gnutls_init(&s, GNUTLS_SERVER); return
> 0; }
> +EOF
> + vnc_ws_cflags=`$pkg_config --cflags gnutls 2> /dev/null`
> + vnc_ws_libs=`$pkg_config --libs gnutls 2> /dev/null`
> + if compile_prog "$vnc_ws_cflags" "$vnc_ws_libs" ; then
> + vnc_ws=yes
> + libs_softmmu="$vnc_ws_libs $libs_softmmu"
> + else
> + if test "$vnc_ws" = "yes" ; then
> + feature_not_found "vnc-ws"
> + fi
> + vnc_ws=no
> + fi
> +fi
This is really testing for GnuTLS rather than WebSockets. This probing
is duplicated from the VNC TLS option.
I suggest probing GnuTLS once and then using the result for both vnc_tls
and vnc_ws. That way we don't duplicate the GnuTLS code.
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9bb29d3..647071e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1096,6 +1096,14 @@ client is specified by the @var{display}. For reverse
> network
> connections (@var{host}:@var{d},@code{reverse}), the @var{d} argument
> is a TCP port number, not a display number.
>
> +@item websocket
> +
> +Opens an additional TCP listening port dedicated to VNC Websocket
> connections.
> +By defintion the Websocket port is 5700+@var{display}. If @var{host} is
s/defintion/definition/
> +char *vncws_extract_handshake_entry(const char *handshake,
> + size_t handshake_len, const char *name)
This function should be static.
> +void vncws_process_handshake(VncState *vs, uint8_t *line, size_t size)
> +{
> + char *protocols = vncws_extract_handshake_entry((const char *)line, size,
> + "Sec-WebSocket-Protocol: ");
> + char *version = vncws_extract_handshake_entry((const char *)line, size,
> + "Sec-WebSocket-Version: ");
> + char *key = vncws_extract_handshake_entry((const char *)line, size,
> + "Sec-WebSocket-Key: ");
> +
> + if (protocols && version && key
> + && g_strrstr(protocols, "binary") != NULL
> + && strcmp(version, WS_SUPPORTED_VERSION) == 0
> + && strlen(key) == WS_CLIENT_KEY_LEN) {
> + vncws_send_handshake_response(vs, key);
Indentation should be 4 spaces.
> + } else {
> + VNC_DEBUG("Defective Websockets header or unsupported protocol\n");
> + vnc_client_error(vs);
> + }
> +
> + g_free(protocols);
> + g_free(version);
> + g_free(key);
> +}
> +
> +void vncws_send_handshake_response(VncState *vs, const char* key)
This function should be static.
> +{
> + char combined_key[WS_CLIENT_KEY_LEN + WS_GUID_LEN + 1];
> + char response[WS_HANDSHAKE_MAX_LEN];
> + char hash[SHA1_DIGEST_LEN + 1];
Why +1 if this is a 20-byte SHA1 binary hash?
> + char *accept = NULL;
> + size_t hash_size = SHA1_DIGEST_LEN, response_size = 0;
> + gnutls_datum_t in;
> +
> + /* create combined key */
> + pstrcpy(combined_key, WS_CLIENT_KEY_LEN + 1, key);
> + pstrcat(combined_key, WS_CLIENT_KEY_LEN + WS_GUID_LEN + 1, WS_GUID);
> +
> + /* hash and encode it */
> + in.data = (void *)combined_key;
> + in.size = WS_CLIENT_KEY_LEN + WS_GUID_LEN;
> + if (gnutls_fingerprint(GNUTLS_DIG_SHA1, &in, hash, &hash_size)
> + == GNUTLS_E_SUCCESS) {
> + accept = g_base64_encode((guchar *)hash, SHA1_DIGEST_LEN);
> + }
> + if (accept == NULL) {
> + VNC_DEBUG("Hashing Websocket combined key failed\n");
> + vnc_client_error(vs);
> + return;
> + }
> +
> + /* create handshake response */
> + response_size = snprintf(response, WS_HANDSHAKE_MAX_LEN,
> + WS_HANDSHAKE, accept);
Please use sizeof(response) instead of WS_HANDSHAKE_MAX_LEN. It's safer
to use sizeof() rather than repeating the constant so that the sizes
still match up if the variable definition is changed.
> + g_free(accept);
> +
> + vnc_write(vs, response, response_size);
> + vnc_flush(vs);
> +
> + vs->encode_ws = 1;
> + vnc_init_state(vs);
> +}
> +
> +void vncws_encode_frame(Buffer *output, const void *payload,
> + const size_t payload_size)
> +{
> + size_t header_size = 0;
> + unsigned char opcode = WS_OPCODE_BINARY_FRAME;
> + char header_buf[WS_HEAD_MAX_LEN];
> + ws_header_t *header = (ws_header_t *)header_buf;
It's slightly cleaner to use:
union {
char buf[WS_HEAD_MAX_LEN];
ws_header_t ws;
} header;
That way the compiler know they are aliased and can even align buf[] so
that ws_header_t field accesses are aligned, if necessary.
> diff --git a/ui/vnc-ws.h b/ui/vnc-ws.h
> new file mode 100644
> index 0000000..0c5e2b5
> --- /dev/null
> +++ b/ui/vnc-ws.h
> @@ -0,0 +1,101 @@
> +/*
> + * QEMU VNC display driver: Websockets support
> + *
> + * Copyright (C) 2010 Joel Martin
> + * Copyright (C) 2012 Tim Hardeck
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This software is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this software; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301,
> + * USA.
> + */
> +
> +#ifndef __QEMU_VNC_WS_H
> +#define __QEMU_VNC_WS_H
> +
> +#ifndef CONFIG_VNC_TLS
> +#include <gnutls/gnutls.h>
> +#endif
Your ./configure change does not depend on CONFIG_VNC_TLS. It would be
possible to say ./configure --disable-vnc-tls --enable-vnc-ws. I think
the #ifdef here can be dropped.
> +
> +#define B64LEN(__x) (((__x + 2) / 3) * 12 / 3)
> +#define SHA1_DIGEST_LEN 20
> +
> +#define WS_ACCEPT_LEN (B64LEN(SHA1_DIGEST_LEN) + 1)
> +#define WS_CLIENT_KEY_LEN 24
> +#define WS_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
> +#define WS_GUID_LEN strlen(WS_GUID)
> +
> +#define WS_HANDSHAKE_MAX_LEN 192
> +#define WS_HANDSHAKE "HTTP/1.1 101 Switching Protocols\r\n\
> +Upgrade: websocket\r\n\
> +Connection: Upgrade\r\n\
> +Sec-WebSocket-Accept: %s\r\n\
> +Sec-WebSocket-Protocol: binary\r\n\
> +\r\n"
> +#define WS_HANDSHAKE_DELIM "\r\n"
> +#define WS_HANDSHAKE_END "\r\n\r\n"
> +#define WS_SUPPORTED_VERSION "13"
> +
> +#define WS_HEAD_MIN_LEN 2
> +#define WS_HEAD_MAX_LEN 14 /* 2 + sizeof(uint64_t) + sizeof(uint32_t) */
Perhaps best to convert the comment into code:
#define WS_HEAD_MAX_LEN (WS_HEAD_MIN_LEN + sizeof(uint64_t) + sizeof(uint32_t))
> +
> +#define WS_HTON64(n) htobe64(n)
> +#define WS_HTON16(n) htobe16(n)
> +#define WS_NTOH16(n) htobe16(n)
> +#define WS_NTOH64(n) htobe64(n)
Why wrapper macros?
> +typedef union ws_mask_s {
> + char c[4];
> + uint32_t u;
> +} ws_mask_t;
> +
> +/* XXX: The union and the structs do not need to be named.
> + * We are working around a bug present in GCC < 4.6 which prevented
> + * it from recognizing anonymous structs and unions.
> + * See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=4784
> + */
> +typedef struct __attribute__ ((__packed__)) ws_header_s {
> + unsigned char b0;
> + unsigned char b1;
> + union {
> + struct __attribute__ ((__packed__)) {
> + uint16_t l16;
> + ws_mask_t m16;
> + } s16;
> + struct __attribute__ ((__packed__)) {
> + uint64_t l64;
> + ws_mask_t m64;
> + } s64;
> + ws_mask_t m;
> + } u;
> +} ws_header_t;
QEMU naming does not allow _t. This should be WsHeader or WSHeader.
> +
> +enum {
> + WS_OPCODE_CONTINUATION = 0x0,
> + WS_OPCODE_TEXT_FRAME = 0x1,
> + WS_OPCODE_BINARY_FRAME = 0x2,
> + WS_OPCODE_CLOSE = 0x8,
> + WS_OPCODE_PING = 0x9,
> + WS_OPCODE_PONG = 0xA
> +};
> +
> +char *vncws_extract_handshake_entry(const char *header, size_t header_len,
> + const char *name);
> +void vncws_process_handshake(VncState *vs, uint8_t *line, size_t size);
> +void vncws_send_handshake_response(VncState *vs, const char* key);
> +void vncws_encode_frame(Buffer *output, const void *payload,
> + const size_t payload_size);
> +int vncws_decode_frame(Buffer *input, uint8_t **payload,
> + size_t *payload_size, size_t *frame_size);
> +
> +#endif /* __QEMU_VNC_WS_H */
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 61f120e..676ac0d 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -510,6 +510,13 @@ void buffer_append(Buffer *buffer, const void *data,
> size_t len)
> buffer->offset += len;
> }
>
> +void buffer_advance(Buffer *buf, size_t len)
> +{
> + memmove(buf->buffer, buf->buffer + len,
> + (buf->offset - len));
> + buf->offset -= len;
> +}
Please introduce this function and convert its callers in a separate
commit. That makes it easier to review, bisect, revert, etc the
commits. This change isn't WS-specific.
> +
> static void vnc_desktop_resize(VncState *vs)
> {
> DisplayState *ds = vs->ds;
> @@ -1027,6 +1034,9 @@ static void vnc_disconnect_finish(VncState *vs)
>
> buffer_free(&vs->input);
> buffer_free(&vs->output);
> +#ifdef CONFIG_VNC_WS
> + buffer_free(&vs->ws_input);
> +#endif
>
> qobject_decref(vs->info);
>
> @@ -1166,8 +1176,7 @@ static long vnc_client_write_plain(VncState *vs)
> if (!ret)
> return 0;
>
> - memmove(vs->output.buffer, vs->output.buffer + ret, (vs->output.offset -
> ret));
> - vs->output.offset -= ret;
> + buffer_advance(&vs->output, ret);
>
> if (vs->output.offset == 0) {
> qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
> @@ -1280,6 +1289,26 @@ static void vnc_jobs_bh(void *opaque)
> vnc_jobs_consume_buffer(vs);
> }
>
> +#ifdef CONFIG_VNC_WS
> +void vncws_handshake_read(void *opaque)
> +{
> + VncState *vs = opaque;
> + long ret = vnc_client_read_plain(vs);
> + if (!ret) {
> + if (vs->csock == -1) {
> + vnc_disconnect_finish(vs);
> + }
> + return;
> + }
> +
> + if (vs->input.offset > 0) {
> + qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
> + vncws_process_handshake(vs, vs->input.buffer, vs->input.offset);
> + buffer_reset(&vs->input);
> + }
> +}
> +#endif
> +
> /*
> * First function called whenever there is more data to be read from
> * the client socket. Will delegate actual work according to whether
> @@ -1289,6 +1318,7 @@ void vnc_client_read(void *opaque)
> {
> VncState *vs = opaque;
> long ret;
> + Buffer *buf;
>
> #ifdef CONFIG_VNC_SASL
> if (vs->sasl.conn && vs->sasl.runSSF)
> @@ -1302,19 +1332,49 @@ void vnc_client_read(void *opaque)
> return;
> }
>
> - while (vs->read_handler && vs->input.offset >= vs->read_handler_expect) {
> +#ifdef CONFIG_VNC_WS
> + if (vs->encode_ws) {
> + uint8_t *payload;
> + size_t payload_size, frame_size;
> +
> + /* make sure that nothing is left in the input buffer */
> + do {
> + ret = vncws_decode_frame(&vs->input, &payload,
> + &payload_size, &frame_size);
> +
> + if (ret == 0) {
> + /* not enough data to process, wait for more */
> + return;
> + } else if (ret == -1) {
> + vnc_disconnect_start(vs);
> + return;
> + } else if (ret == -2) {
> + vnc_client_error(vs);
> + return;
> + }
> +
> + buffer_reserve(&vs->ws_input, payload_size);
> + buffer_append(&vs->ws_input, payload, payload_size);
> +
> + buffer_advance(&vs->input, frame_size);
> + } while (vs->input.offset > 0);
> + buf = &vs->ws_input;
> + } else
> +#endif /* CONFIG_VNC_WS */
> + buf = &vs->input;
QEMU coding style always requires {}, even if the if/else statement body
is only 1 line.
> +
> + while (vs->read_handler && buf->offset >= vs->read_handler_expect) {
> size_t len = vs->read_handler_expect;
> int ret;
>
> - ret = vs->read_handler(vs, vs->input.buffer, len);
> + ret = vs->read_handler(vs, buf->buffer, len);
> if (vs->csock == -1) {
> vnc_disconnect_finish(vs);
> return;
> }
>
> if (!ret) {
> - memmove(vs->input.buffer, vs->input.buffer + len,
> (vs->input.offset - len));
> - vs->input.offset -= len;
> + buffer_advance(buf, len);
> } else {
> vs->read_handler_expect = ret;
> }
> @@ -1323,13 +1383,26 @@ void vnc_client_read(void *opaque)
>
> void vnc_write(VncState *vs, const void *data, size_t len)
> {
> - buffer_reserve(&vs->output, len);
> +#ifdef CONFIG_VNC_WS
> + if (vs->encode_ws) {
> + if (vs->csock != -1 && buffer_empty(&vs->output)) {
> + qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read,
> + vnc_client_write, vs);
> + }
Can this be done unconditionally outside the if statement and #ifdef?
Seems to be duplicated below into the else body.
> + vncws_encode_frame(&vs->output, data, len);
> + } else {
> +#endif /* CONFIG_VNC_WS */
> + buffer_reserve(&vs->output, len);
>
> - if (vs->csock != -1 && buffer_empty(&vs->output)) {
> - qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read,
> vnc_client_write, vs);
> - }
> + if (vs->csock != -1 && buffer_empty(&vs->output)) {
> + qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read,
> + vnc_client_write, vs);
> + }
>
> - buffer_append(&vs->output, data, len);
> + buffer_append(&vs->output, data, len);
> +#ifdef CONFIG_VNC_WS
> + }
> +#endif
This #ifdef can be avoided by:
#ifdef CONFIG_VNC_WS
if (...) {
...
} else
#endif
{
...
}
> }
>
> void vnc_write_s32(VncState *vs, int32_t value)
> @@ -2657,7 +2730,7 @@ static void vnc_remove_timer(VncDisplay *vd)
> }
> }
>
> -static void vnc_connect(VncDisplay *vd, int csock, int skipauth)
> +static void vnc_connect(VncDisplay *vd, int csock, int skipauth, int
> websocket)
bool websocket
> {
> VncState *vs = g_malloc0(sizeof(VncState));
> int i;
> @@ -2684,13 +2757,33 @@ static void vnc_connect(VncDisplay *vd, int csock,
> int skipauth)
> VNC_DEBUG("New client on socket %d\n", csock);
> dcl->idle = 0;
> socket_set_nonblock(vs->csock);
> - qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
> +#ifdef CONFIG_VNC_WS
> + if (websocket) {
> + vs->websocket = 1;
> + qemu_set_fd_handler2(vs->csock, NULL, vncws_handshake_read, NULL,
> vs);
> + } else
> +#endif
> + {
> + qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
> + }
>
> vnc_client_cache_addr(vs);
> vnc_qmp_event(vs, QEVENT_VNC_CONNECTED);
> vnc_set_share_mode(vs, VNC_SHARE_MODE_CONNECTING);
>
> vs->vd = vd;
> +
> +#ifdef CONFIG_VNC_WS
> + if (!vs->websocket) {
> + vnc_init_state(vs);
> + }
> +}
> +
> +void vnc_init_state(VncState *vs)
> +{
> + VncDisplay *vd = vs->vd;
> +#endif /* CONFIG_VNC_WS */
Using an #ifdef to split a function at compile time is too ugly IMO.
I suggest splitting the function, always have vnc_init_state().
> +
> vs->ds = vd->ds;
> vs->last_x = -1;
> vs->last_y = -1;
> @@ -2722,21 +2815,41 @@ static void vnc_connect(VncDisplay *vd, int csock,
> int skipauth)
> /* vs might be free()ed here */
> }
>
> -static void vnc_listen_read(void *opaque)
> +static void vnc_listen_read(void *opaque, int websocket)
s/int/bool/
> {
> VncDisplay *vs = opaque;
> struct sockaddr_in addr;
> socklen_t addrlen = sizeof(addr);
> + int csock;
>
> /* Catch-up */
> vga_hw_update();
> +#ifdef CONFIG_VNC_WS
> + if (websocket) {
> + csock = qemu_accept(vs->lwebsock, (struct sockaddr *)&addr,
> &addrlen);
> + } else
> +#endif
> + {
> + csock = qemu_accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
> + }
>
> - int csock = qemu_accept(vs->lsock, (struct sockaddr *)&addr, &addrlen);
> if (csock != -1) {
> - vnc_connect(vs, csock, 0);
> + vnc_connect(vs, csock, 0, websocket);
> }
> }
>
> +static void vnc_listen_regular_read(void *opaque)
> +{
> + vnc_listen_read(opaque, 0);
> +}
> +
> +#ifdef CONFIG_VNC_WS
> +static void vnc_listen_websocket_read(void *opaque)
> +{
> + vnc_listen_read(opaque, 1);
> +}
> +#endif
> +
> void vnc_display_init(DisplayState *ds)
> {
> VncDisplay *vs = g_malloc0(sizeof(*vs));
> @@ -2748,6 +2861,9 @@ void vnc_display_init(DisplayState *ds)
> vnc_display = vs;
>
> vs->lsock = -1;
> +#ifdef CONFIG_VNC_WS
> + vs->lwebsock = -1;
> +#endif
>
> vs->ds = ds;
> QTAILQ_INIT(&vs->clients);
> @@ -2789,6 +2905,17 @@ static void vnc_display_close(DisplayState *ds)
> close(vs->lsock);
> vs->lsock = -1;
> }
> +#ifdef CONFIG_VNC_WS
> + if (vs->ws_display) {
> + g_free(vs->ws_display);
> + vs->ws_display = NULL;
> + }
NULL test not necessary since g_free(NULL) is a nop.
> + if (vs->lwebsock != -1) {
> + qemu_set_fd_handler2(vs->lwebsock, NULL, NULL, NULL, NULL);
> + close(vs->lwebsock);
> + vs->lwebsock = -1;
> + }
> +#endif
> vs->auth = VNC_AUTH_INVALID;
> #ifdef CONFIG_VNC_TLS
> vs->subauth = VNC_AUTH_INVALID;
> @@ -2910,6 +3037,36 @@ void vnc_display_open(DisplayState *ds, const char
> *display, Error **errp)
> } else if (strncmp(options, "sasl", 4) == 0) {
> sasl = 1; /* Require SASL auth */
> #endif
> +#ifdef CONFIG_VNC_WS
> + } else if (strncmp(options, "websocket", 9) == 0) {
> + char *start, *end;
> + vs->websocket = 1;
> +
> + /* Check for 'websocket=<port> */
s/'websocket=<port>/'websocket=<port>'/
> + start = strchr(options, '=');
> + end = strchr(options, ',');
> + if (start && (!end || (start < end))) {
> + int len = end ? end-(start+1) : strlen(start+1);
> + if (len < 6) {
> + /* extract the host specification from display */
> + char *host = NULL, *port = NULL, *host_end = NULL;
> + port = g_strndup(start + 1, len);
> +
> + /* ipv6 host have colons */
> + end = strchr(display, ',');
> + host_end = g_strrstr_len(display, end - display, ":");
> +
> + if (host_end) {
> + host = g_strndup(display, host_end - display + 1);
> + } else {
> + host = g_strndup(":", 1);
> + }
> + vs->ws_display = g_strconcat(host, port, NULL);
> + g_free(host);
> + g_free(port);
> + }
> + }
> +#endif
> #ifdef CONFIG_VNC_TLS
> } else if (strncmp(options, "tls", 3) == 0) {
> tls = 1; /* Require TLS */
> @@ -3068,6 +3225,9 @@ void vnc_display_open(DisplayState *ds, const char
> *display, Error **errp)
> /* connect to viewer */
> int csock;
> vs->lsock = -1;
> +#ifdef CONFIG_VNC_WS
> + vs->lwebsock = -1;
> +#endif
> if (strncmp(display, "unix:", 5) == 0) {
> csock = unix_connect(display+5, errp);
> } else {
> @@ -3076,7 +3236,7 @@ void vnc_display_open(DisplayState *ds, const char
> *display, Error **errp)
> if (csock < 0) {
> goto fail;
> }
> - vnc_connect(vs, csock, 0);
> + vnc_connect(vs, csock, 0, 0);
> } else {
> /* listen for connects */
> char *dpy;
> @@ -3087,25 +3247,51 @@ void vnc_display_open(DisplayState *ds, const char
> *display, Error **errp)
> } else {
> vs->lsock = inet_listen(display, dpy, 256,
> SOCK_STREAM, 5900, errp);
> +#ifdef CONFIG_VNC_WS
> + if (vs->websocket) {
> + if (vs->ws_display) {
> + vs->lwebsock = inet_listen(vs->ws_display, NULL, 256,
> + SOCK_STREAM, 0, errp);
> + } else {
> + vs->lwebsock = inet_listen(vs->display, NULL, 256,
> + SOCK_STREAM, 5700, errp);
> + }
> + }
> +#endif
> }
> - if (vs->lsock < 0) {
> + if (vs->lsock < 0
> +#ifdef CONFIG_VNC_WS
> + || (vs->websocket && vs->lwebsock < 0)
> +#endif
> + ) {
> g_free(dpy);
> goto fail;
Will this leak the lsock file descriptor when both a regular VNC port
and a websocket port are given but inet_listen() fails on just the
websocket?
> }
> g_free(vs->display);
> vs->display = dpy;
> - qemu_set_fd_handler2(vs->lsock, NULL, vnc_listen_read, NULL, vs);
> + qemu_set_fd_handler2(vs->lsock, NULL,
> + vnc_listen_regular_read, NULL, vs);
> +#ifdef CONFIG_VNC_WS
> + if (vs->websocket) {
> + qemu_set_fd_handler2(vs->lwebsock, NULL,
> + vnc_listen_websocket_read, NULL, vs);
> + }
> +#endif
> }
> return;
>
> fail:
> g_free(vs->display);
> vs->display = NULL;
> +#ifdef CONFIG_VNC_WS
> + g_free(vs->ws_display);
> + vs->ws_display = NULL;
> +#endif
> }
>
> void vnc_display_add_client(DisplayState *ds, int csock, int skipauth)
> {
> VncDisplay *vs = ds ? (VncDisplay *)ds->opaque : vnc_display;
>
> - vnc_connect(vs, csock, skipauth);
> + vnc_connect(vs, csock, skipauth, 0);
> }
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 6141e88..2c6bfe7 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -99,6 +99,9 @@ typedef struct VncDisplay VncDisplay;
> #ifdef CONFIG_VNC_SASL
> #include "vnc-auth-sasl.h"
> #endif
> +#ifdef CONFIG_VNC_WS
> +#include "vnc-ws.h"
> +#endif
>
> struct VncRectStat
> {
> @@ -142,6 +145,11 @@ struct VncDisplay
> QEMUTimer *timer;
> int timer_interval;
> int lsock;
> +#ifdef CONFIG_VNC_WS
> + int lwebsock;
> + int websocket;
Please use bool.
> + char *ws_display;
> +#endif
> DisplayState *ds;
> kbd_layout_t *kbd_layout;
> int lock_key_sync;
> @@ -269,11 +277,18 @@ struct VncState
> #ifdef CONFIG_VNC_SASL
> VncStateSASL sasl;
> #endif
> +#ifdef CONFIG_VNC_WS
> + int encode_ws;
Please use bool.
> + int websocket;
Please use bool.
