* [Qemu-devel] [PATCH 1/9] ui: remove misleading comment from vnc_init_state
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 2/9] ui: remove 'enabled' and 'ws_enabled' fields from VncState Daniel P. Berrange
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
The last line in vnc_init_state() says
/* vs might be free()ed here */
This was added in
commit 198a0039c5fca224a77e9761e2350dd9cc102ad0
Author: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue Jun 16 14:19:48 2009 +0200
vnc: rework VncState release workflow.
because the preceeding 'vnc_update_client()' could indeed
release the VncState instance.
The call to vnc_update_client() was removed not long after
though in
commit 1fc624122fb923c7fc4c1f426541d953e7df13c9
Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Date: Mon Aug 3 10:54:32 2009 +0100
single vnc server surface
and so the comment has been wrong ever since
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 76a3273..5faf79b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3092,8 +3092,6 @@ void vnc_init_state(VncState *vs)
vs->mouse_mode_notifier.notify = check_pointer_type_change;
qemu_add_mouse_mode_change_notifier(&vs->mouse_mode_notifier);
-
- /* vs might be free()ed here */
}
static gboolean vnc_listen_io(QIOChannel *ioc,
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 2/9] ui: remove 'enabled' and 'ws_enabled' fields from VncState
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 1/9] ui: remove misleading comment from vnc_init_state Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 3/9] ui: remove 'ws_tls' field " Daniel P. Berrange
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
The 'ws_enabled' field is never used outside of the
vnc_display_open method, so can be a local variable.
The 'enabled' field is easily replaced by a check
for whether 'lsock' is non-NULL.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 23 ++++++++++-------------
ui/vnc.h | 2 --
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 5faf79b..45a23d3 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -371,7 +371,7 @@ VncInfo *qmp_query_vnc(Error **errp)
VncDisplay *vd = vnc_display_find(NULL);
SocketAddress *addr = NULL;
- if (vd == NULL || !vd->enabled) {
+ if (vd == NULL || !vd->lsock) {
info->enabled = false;
} else {
info->enabled = true;
@@ -3169,7 +3169,6 @@ static void vnc_display_close(VncDisplay *vs)
{
if (!vs)
return;
- vs->enabled = false;
vs->is_unix = false;
if (vs->lsock != NULL) {
if (vs->lsock_tag) {
@@ -3178,7 +3177,6 @@ static void vnc_display_close(VncDisplay *vs)
object_unref(OBJECT(vs->lsock));
vs->lsock = NULL;
}
- vs->ws_enabled = false;
if (vs->lwebsock != NULL) {
if (vs->lwebsock_tag) {
g_source_remove(vs->lwebsock_tag);
@@ -3538,6 +3536,7 @@ void vnc_display_open(const char *id, Error **errp)
int acl = 0;
int lock_key_sync = 1;
int key_delay_ms;
+ bool ws_enabled = false;
if (!vs) {
error_setg(errp, "VNC display not active");
@@ -3573,7 +3572,7 @@ void vnc_display_open(const char *id, Error **errp)
}
wsaddr = g_new0(SocketAddress, 1);
- vs->ws_enabled = true;
+ ws_enabled = true;
}
if (strncmp(vnc, "unix:", 5) == 0) {
@@ -3581,7 +3580,7 @@ void vnc_display_open(const char *id, Error **errp)
saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
saddr->u.q_unix.data->path = g_strdup(vnc + 5);
- if (vs->ws_enabled) {
+ if (ws_enabled) {
error_setg(errp, "UNIX sockets not supported with websock");
goto fail;
}
@@ -3617,7 +3616,7 @@ void vnc_display_open(const char *id, Error **errp)
inet->ipv6 = ipv6;
inet->has_ipv6 = has_ipv6;
- if (vs->ws_enabled) {
+ if (ws_enabled) {
wsaddr->type = SOCKET_ADDRESS_KIND_INET;
inet = wsaddr->u.inet.data = g_new0(InetSocketAddress, 1);
inet->host = g_strdup(saddr->u.inet.data->host);
@@ -3777,7 +3776,7 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif
- if (vnc_display_setup_auth(vs, password, sasl, vs->ws_enabled, errp) < 0) {
+ if (vnc_display_setup_auth(vs, password, sasl, ws_enabled, errp) < 0) {
goto fail;
}
@@ -3816,7 +3815,7 @@ void vnc_display_open(const char *id, Error **errp)
QIOChannelSocket *sioc = NULL;
vs->lsock = NULL;
vs->lwebsock = NULL;
- if (vs->ws_enabled) {
+ if (ws_enabled) {
error_setg(errp, "Cannot use websockets in reverse mode");
goto fail;
}
@@ -3833,9 +3832,8 @@ void vnc_display_open(const char *id, Error **errp)
goto fail;
}
vs->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
- vs->enabled = true;
- if (vs->ws_enabled) {
+ if (ws_enabled) {
vs->lwebsock = qio_channel_socket_new();
if (qio_channel_socket_listen_sync(vs->lwebsock,
wsaddr, errp) < 0) {
@@ -3848,7 +3846,7 @@ void vnc_display_open(const char *id, Error **errp)
vs->lsock_tag = qio_channel_add_watch(
QIO_CHANNEL(vs->lsock),
G_IO_IN, vnc_listen_io, vs, NULL);
- if (vs->ws_enabled) {
+ if (ws_enabled) {
vs->lwebsock_tag = qio_channel_add_watch(
QIO_CHANNEL(vs->lwebsock),
G_IO_IN, vnc_listen_io, vs, NULL);
@@ -3866,8 +3864,7 @@ void vnc_display_open(const char *id, Error **errp)
fail:
qapi_free_SocketAddress(saddr);
qapi_free_SocketAddress(wsaddr);
- vs->enabled = false;
- vs->ws_enabled = false;
+ ws_enabled = false;
}
void vnc_display_add_client(const char *id, int csock, bool skipauth)
diff --git a/ui/vnc.h b/ui/vnc.h
index ab5f244..a0519cc 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -150,7 +150,6 @@ struct VncDisplay
guint lsock_tag;
QIOChannelSocket *lwebsock;
guint lwebsock_tag;
- bool ws_enabled;
DisplaySurface *ds;
DisplayChangeListener dcl;
kbd_layout_t *kbd_layout;
@@ -167,7 +166,6 @@ struct VncDisplay
const char *id;
QTAILQ_ENTRY(VncDisplay) next;
- bool enabled;
bool is_unix;
char *password;
time_t expires;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 3/9] ui: remove 'ws_tls' field from VncState
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 1/9] ui: remove misleading comment from vnc_init_state Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 2/9] ui: remove 'enabled' and 'ws_enabled' fields from VncState Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 4/9] ui: rename misleading 'VncDisplay' variables Daniel P. Berrange
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
The 'ws_tls' field in VncState is only ever representing
the result of 'tlscreds != NULL' and is thus pointless.
Replace use of 'ws_tls' with a direct check against
'tlscreds'
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 11 +----------
ui/vnc.h | 1 -
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 45a23d3..83a608b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3029,7 +3029,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
qio_channel_set_blocking(vs->ioc, false, NULL);
if (websocket) {
vs->websocket = 1;
- if (vd->ws_tls) {
+ if (vd->tlscreds) {
vs->ioc_tag = qio_channel_add_watch(
vs->ioc, G_IO_IN, vncws_tls_handshake_io, vs, NULL);
} else {
@@ -3379,9 +3379,6 @@ vnc_display_setup_auth(VncDisplay *vs,
if (password) {
if (vs->tlscreds) {
vs->auth = VNC_AUTH_VENCRYPT;
- if (websocket) {
- vs->ws_tls = true;
- }
if (object_dynamic_cast(OBJECT(vs->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_X509)) {
VNC_DEBUG("Initializing VNC server with x509 password auth\n");
@@ -3409,9 +3406,6 @@ vnc_display_setup_auth(VncDisplay *vs,
} else if (sasl) {
if (vs->tlscreds) {
vs->auth = VNC_AUTH_VENCRYPT;
- if (websocket) {
- vs->ws_tls = true;
- }
if (object_dynamic_cast(OBJECT(vs->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_X509)) {
VNC_DEBUG("Initializing VNC server with x509 SASL auth\n");
@@ -3439,9 +3433,6 @@ vnc_display_setup_auth(VncDisplay *vs,
} else {
if (vs->tlscreds) {
vs->auth = VNC_AUTH_VENCRYPT;
- if (websocket) {
- vs->ws_tls = true;
- }
if (object_dynamic_cast(OBJECT(vs->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_X509)) {
VNC_DEBUG("Initializing VNC server with x509 no auth\n");
diff --git a/ui/vnc.h b/ui/vnc.h
index a0519cc..223af38 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -172,7 +172,6 @@ struct VncDisplay
int auth;
int subauth; /* Used by VeNCrypt */
int ws_auth; /* Used by websockets */
- bool ws_tls; /* Used by websockets */
bool lossy;
bool non_adaptive;
QCryptoTLSCreds *tlscreds;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 4/9] ui: rename misleading 'VncDisplay' variables
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
` (2 preceding siblings ...)
2016-09-29 15:45 ` [Qemu-devel] [PATCH 3/9] ui: remove 'ws_tls' field " Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 5/9] ui: refactor method for setting up VncDisplay auth types Daniel P. Berrange
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
Normally code declares 'VncDisplay *vd' or 'VncState *vs'
but there are a bunch of places which misleadingly declare
'VncDisplay *vs'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 274 ++++++++++++++++++++++++++++++++-------------------------------
1 file changed, 138 insertions(+), 136 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 83a608b..1104697 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3098,17 +3098,17 @@ static gboolean vnc_listen_io(QIOChannel *ioc,
GIOCondition condition,
void *opaque)
{
- VncDisplay *vs = opaque;
+ VncDisplay *vd = opaque;
QIOChannelSocket *sioc = NULL;
Error *err = NULL;
/* Catch-up */
- graphic_hw_update(vs->dcl.con);
+ graphic_hw_update(vd->dcl.con);
sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc), &err);
if (sioc != NULL) {
qio_channel_set_delay(QIO_CHANNEL(sioc), false);
- vnc_connect(vs, sioc, false,
- ioc != QIO_CHANNEL(vs->lsock));
+ vnc_connect(vd, sioc, false,
+ ioc != QIO_CHANNEL(vd->lsock));
object_unref(OBJECT(sioc));
} else {
/* client probably closed connection before we got there */
@@ -3131,106 +3131,108 @@ static const DisplayChangeListenerOps dcl_ops = {
void vnc_display_init(const char *id)
{
- VncDisplay *vs;
+ VncDisplay *vd;
if (vnc_display_find(id) != NULL) {
return;
}
- vs = g_malloc0(sizeof(*vs));
+ vd = g_malloc0(sizeof(*vd));
- vs->id = strdup(id);
- QTAILQ_INSERT_TAIL(&vnc_displays, vs, next);
+ vd->id = strdup(id);
+ QTAILQ_INSERT_TAIL(&vnc_displays, vd, next);
- QTAILQ_INIT(&vs->clients);
- vs->expires = TIME_MAX;
+ QTAILQ_INIT(&vd->clients);
+ vd->expires = TIME_MAX;
if (keyboard_layout) {
trace_vnc_key_map_init(keyboard_layout);
- vs->kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout);
+ vd->kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout);
} else {
- vs->kbd_layout = init_keyboard_layout(name2keysym, "en-us");
+ vd->kbd_layout = init_keyboard_layout(name2keysym, "en-us");
}
- if (!vs->kbd_layout)
+ if (!vd->kbd_layout) {
exit(1);
+ }
- vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
- vs->connections_limit = 32;
+ vd->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
+ vd->connections_limit = 32;
- qemu_mutex_init(&vs->mutex);
+ qemu_mutex_init(&vd->mutex);
vnc_start_worker_thread();
- vs->dcl.ops = &dcl_ops;
- register_displaychangelistener(&vs->dcl);
+ vd->dcl.ops = &dcl_ops;
+ register_displaychangelistener(&vd->dcl);
}
-static void vnc_display_close(VncDisplay *vs)
+static void vnc_display_close(VncDisplay *vd)
{
- if (!vs)
+ if (!vd) {
return;
- vs->is_unix = false;
- if (vs->lsock != NULL) {
- if (vs->lsock_tag) {
- g_source_remove(vs->lsock_tag);
+ }
+ vd->is_unix = false;
+ if (vd->lsock != NULL) {
+ if (vd->lsock_tag) {
+ g_source_remove(vd->lsock_tag);
}
- object_unref(OBJECT(vs->lsock));
- vs->lsock = NULL;
+ object_unref(OBJECT(vd->lsock));
+ vd->lsock = NULL;
}
- if (vs->lwebsock != NULL) {
- if (vs->lwebsock_tag) {
- g_source_remove(vs->lwebsock_tag);
+ if (vd->lwebsock != NULL) {
+ if (vd->lwebsock_tag) {
+ g_source_remove(vd->lwebsock_tag);
}
- object_unref(OBJECT(vs->lwebsock));
- vs->lwebsock = NULL;
+ object_unref(OBJECT(vd->lwebsock));
+ vd->lwebsock = NULL;
}
- vs->auth = VNC_AUTH_INVALID;
- vs->subauth = VNC_AUTH_INVALID;
- if (vs->tlscreds) {
- object_unparent(OBJECT(vs->tlscreds));
- vs->tlscreds = NULL;
+ vd->auth = VNC_AUTH_INVALID;
+ vd->subauth = VNC_AUTH_INVALID;
+ if (vd->tlscreds) {
+ object_unparent(OBJECT(vd->tlscreds));
+ vd->tlscreds = NULL;
}
- g_free(vs->tlsaclname);
- vs->tlsaclname = NULL;
+ g_free(vd->tlsaclname);
+ vd->tlsaclname = NULL;
}
int vnc_display_password(const char *id, const char *password)
{
- VncDisplay *vs = vnc_display_find(id);
+ VncDisplay *vd = vnc_display_find(id);
- if (!vs) {
+ if (!vd) {
return -EINVAL;
}
- if (vs->auth == VNC_AUTH_NONE) {
+ if (vd->auth == VNC_AUTH_NONE) {
error_printf_unless_qmp("If you want use passwords please enable "
"password auth using '-vnc ${dpy},password'.\n");
return -EINVAL;
}
- g_free(vs->password);
- vs->password = g_strdup(password);
+ g_free(vd->password);
+ vd->password = g_strdup(password);
return 0;
}
int vnc_display_pw_expire(const char *id, time_t expires)
{
- VncDisplay *vs = vnc_display_find(id);
+ VncDisplay *vd = vnc_display_find(id);
- if (!vs) {
+ if (!vd) {
return -EINVAL;
}
- vs->expires = expires;
+ vd->expires = expires;
return 0;
}
-static void vnc_display_print_local_addr(VncDisplay *vs)
+static void vnc_display_print_local_addr(VncDisplay *vd)
{
SocketAddress *addr;
Error *err = NULL;
- addr = qio_channel_socket_get_local_address(vs->lsock, &err);
+ addr = qio_channel_socket_get_local_address(vd->lsock, &err);
if (!addr) {
return;
}
@@ -3323,7 +3325,7 @@ static QemuOptsList qemu_vnc_opts = {
static int
-vnc_display_setup_auth(VncDisplay *vs,
+vnc_display_setup_auth(VncDisplay *vd,
bool password,
bool sasl,
bool websocket,
@@ -3377,85 +3379,85 @@ vnc_display_setup_auth(VncDisplay *vs,
* result has the same security characteristics.
*/
if (password) {
- if (vs->tlscreds) {
- vs->auth = VNC_AUTH_VENCRYPT;
- if (object_dynamic_cast(OBJECT(vs->tlscreds),
+ if (vd->tlscreds) {
+ vd->auth = VNC_AUTH_VENCRYPT;
+ if (object_dynamic_cast(OBJECT(vd->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_X509)) {
VNC_DEBUG("Initializing VNC server with x509 password auth\n");
- vs->subauth = VNC_AUTH_VENCRYPT_X509VNC;
- } else if (object_dynamic_cast(OBJECT(vs->tlscreds),
+ vd->subauth = VNC_AUTH_VENCRYPT_X509VNC;
+ } else if (object_dynamic_cast(OBJECT(vd->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_ANON)) {
VNC_DEBUG("Initializing VNC server with TLS password auth\n");
- vs->subauth = VNC_AUTH_VENCRYPT_TLSVNC;
+ vd->subauth = VNC_AUTH_VENCRYPT_TLSVNC;
} else {
error_setg(errp,
"Unsupported TLS cred type %s",
- object_get_typename(OBJECT(vs->tlscreds)));
+ object_get_typename(OBJECT(vd->tlscreds)));
return -1;
}
} else {
VNC_DEBUG("Initializing VNC server with password auth\n");
- vs->auth = VNC_AUTH_VNC;
- vs->subauth = VNC_AUTH_INVALID;
+ vd->auth = VNC_AUTH_VNC;
+ vd->subauth = VNC_AUTH_INVALID;
}
if (websocket) {
- vs->ws_auth = VNC_AUTH_VNC;
+ vd->ws_auth = VNC_AUTH_VNC;
} else {
- vs->ws_auth = VNC_AUTH_INVALID;
+ vd->ws_auth = VNC_AUTH_INVALID;
}
} else if (sasl) {
- if (vs->tlscreds) {
- vs->auth = VNC_AUTH_VENCRYPT;
- if (object_dynamic_cast(OBJECT(vs->tlscreds),
+ if (vd->tlscreds) {
+ vd->auth = VNC_AUTH_VENCRYPT;
+ if (object_dynamic_cast(OBJECT(vd->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_X509)) {
VNC_DEBUG("Initializing VNC server with x509 SASL auth\n");
- vs->subauth = VNC_AUTH_VENCRYPT_X509SASL;
- } else if (object_dynamic_cast(OBJECT(vs->tlscreds),
+ vd->subauth = VNC_AUTH_VENCRYPT_X509SASL;
+ } else if (object_dynamic_cast(OBJECT(vd->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_ANON)) {
VNC_DEBUG("Initializing VNC server with TLS SASL auth\n");
- vs->subauth = VNC_AUTH_VENCRYPT_TLSSASL;
+ vd->subauth = VNC_AUTH_VENCRYPT_TLSSASL;
} else {
error_setg(errp,
"Unsupported TLS cred type %s",
- object_get_typename(OBJECT(vs->tlscreds)));
+ object_get_typename(OBJECT(vd->tlscreds)));
return -1;
}
} else {
VNC_DEBUG("Initializing VNC server with SASL auth\n");
- vs->auth = VNC_AUTH_SASL;
- vs->subauth = VNC_AUTH_INVALID;
+ vd->auth = VNC_AUTH_SASL;
+ vd->subauth = VNC_AUTH_INVALID;
}
if (websocket) {
- vs->ws_auth = VNC_AUTH_SASL;
+ vd->ws_auth = VNC_AUTH_SASL;
} else {
- vs->ws_auth = VNC_AUTH_INVALID;
+ vd->ws_auth = VNC_AUTH_INVALID;
}
} else {
- if (vs->tlscreds) {
- vs->auth = VNC_AUTH_VENCRYPT;
- if (object_dynamic_cast(OBJECT(vs->tlscreds),
+ if (vd->tlscreds) {
+ vd->auth = VNC_AUTH_VENCRYPT;
+ if (object_dynamic_cast(OBJECT(vd->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_X509)) {
VNC_DEBUG("Initializing VNC server with x509 no auth\n");
- vs->subauth = VNC_AUTH_VENCRYPT_X509NONE;
- } else if (object_dynamic_cast(OBJECT(vs->tlscreds),
+ vd->subauth = VNC_AUTH_VENCRYPT_X509NONE;
+ } else if (object_dynamic_cast(OBJECT(vd->tlscreds),
TYPE_QCRYPTO_TLS_CREDS_ANON)) {
VNC_DEBUG("Initializing VNC server with TLS no auth\n");
- vs->subauth = VNC_AUTH_VENCRYPT_TLSNONE;
+ vd->subauth = VNC_AUTH_VENCRYPT_TLSNONE;
} else {
error_setg(errp,
"Unsupported TLS cred type %s",
- object_get_typename(OBJECT(vs->tlscreds)));
+ object_get_typename(OBJECT(vd->tlscreds)));
return -1;
}
} else {
VNC_DEBUG("Initializing VNC server with no auth\n");
- vs->auth = VNC_AUTH_NONE;
- vs->subauth = VNC_AUTH_INVALID;
+ vd->auth = VNC_AUTH_NONE;
+ vd->subauth = VNC_AUTH_INVALID;
}
if (websocket) {
- vs->ws_auth = VNC_AUTH_NONE;
+ vd->ws_auth = VNC_AUTH_NONE;
} else {
- vs->ws_auth = VNC_AUTH_INVALID;
+ vd->ws_auth = VNC_AUTH_INVALID;
}
}
return 0;
@@ -3509,7 +3511,7 @@ vnc_display_create_creds(bool x509,
void vnc_display_open(const char *id, Error **errp)
{
- VncDisplay *vs = vnc_display_find(id);
+ VncDisplay *vd = vnc_display_find(id);
QemuOpts *opts = qemu_opts_find(&qemu_vnc_opts, id);
SocketAddress *saddr = NULL, *wsaddr = NULL;
const char *share, *device_id;
@@ -3529,11 +3531,11 @@ void vnc_display_open(const char *id, Error **errp)
int key_delay_ms;
bool ws_enabled = false;
- if (!vs) {
+ if (!vd) {
error_setg(errp, "VNC display not active");
return;
}
- vnc_display_close(vs);
+ vnc_display_close(vd);
if (!opts) {
return;
@@ -3674,17 +3676,17 @@ void vnc_display_open(const char *id, Error **errp)
credid);
goto fail;
}
- vs->tlscreds = (QCryptoTLSCreds *)
+ vd->tlscreds = (QCryptoTLSCreds *)
object_dynamic_cast(creds,
TYPE_QCRYPTO_TLS_CREDS);
- if (!vs->tlscreds) {
+ if (!vd->tlscreds) {
error_setg(errp, "Object with id '%s' is not TLS credentials",
credid);
goto fail;
}
- object_ref(OBJECT(vs->tlscreds));
+ object_ref(OBJECT(vd->tlscreds));
- if (vs->tlscreds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
+ if (vd->tlscreds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
error_setg(errp,
"Expecting TLS credentials with a server endpoint");
goto fail;
@@ -3705,12 +3707,12 @@ void vnc_display_open(const char *id, Error **errp)
x509verify = true;
}
}
- vs->tlscreds = vnc_display_create_creds(x509,
+ vd->tlscreds = vnc_display_create_creds(x509,
x509verify,
path,
- vs->id,
+ vd->id,
errp);
- if (!vs->tlscreds) {
+ if (!vd->tlscreds) {
goto fail;
}
}
@@ -3720,54 +3722,54 @@ void vnc_display_open(const char *id, Error **errp)
share = qemu_opt_get(opts, "share");
if (share) {
if (strcmp(share, "ignore") == 0) {
- vs->share_policy = VNC_SHARE_POLICY_IGNORE;
+ vd->share_policy = VNC_SHARE_POLICY_IGNORE;
} else if (strcmp(share, "allow-exclusive") == 0) {
- vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
+ vd->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
} else if (strcmp(share, "force-shared") == 0) {
- vs->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
+ vd->share_policy = VNC_SHARE_POLICY_FORCE_SHARED;
} else {
error_setg(errp, "unknown vnc share= option");
goto fail;
}
} else {
- vs->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
+ vd->share_policy = VNC_SHARE_POLICY_ALLOW_EXCLUSIVE;
}
- vs->connections_limit = qemu_opt_get_number(opts, "connections", 32);
+ vd->connections_limit = qemu_opt_get_number(opts, "connections", 32);
#ifdef CONFIG_VNC_JPEG
- vs->lossy = qemu_opt_get_bool(opts, "lossy", false);
+ vd->lossy = qemu_opt_get_bool(opts, "lossy", false);
#endif
- vs->non_adaptive = qemu_opt_get_bool(opts, "non-adaptive", false);
+ vd->non_adaptive = qemu_opt_get_bool(opts, "non-adaptive", false);
/* adaptive updates are only used with tight encoding and
* if lossy updates are enabled so we can disable all the
* calculations otherwise */
- if (!vs->lossy) {
- vs->non_adaptive = true;
+ if (!vd->lossy) {
+ vd->non_adaptive = true;
}
if (acl) {
- if (strcmp(vs->id, "default") == 0) {
- vs->tlsaclname = g_strdup("vnc.x509dname");
+ if (strcmp(vd->id, "default") == 0) {
+ vd->tlsaclname = g_strdup("vnc.x509dname");
} else {
- vs->tlsaclname = g_strdup_printf("vnc.%s.x509dname", vs->id);
+ vd->tlsaclname = g_strdup_printf("vnc.%s.x509dname", vd->id);
}
- qemu_acl_init(vs->tlsaclname);
+ qemu_acl_init(vd->tlsaclname);
}
#ifdef CONFIG_VNC_SASL
if (acl && sasl) {
char *aclname;
- if (strcmp(vs->id, "default") == 0) {
+ if (strcmp(vd->id, "default") == 0) {
aclname = g_strdup("vnc.username");
} else {
- aclname = g_strdup_printf("vnc.%s.username", vs->id);
+ aclname = g_strdup_printf("vnc.%s.username", vd->id);
}
- vs->sasl.acl = qemu_acl_init(aclname);
+ vd->sasl.acl = qemu_acl_init(aclname);
g_free(aclname);
}
#endif
- if (vnc_display_setup_auth(vs, password, sasl, ws_enabled, errp) < 0) {
+ if (vnc_display_setup_auth(vd, password, sasl, ws_enabled, errp) < 0) {
goto fail;
}
@@ -3778,8 +3780,8 @@ void vnc_display_open(const char *id, Error **errp)
goto fail;
}
#endif
- vs->lock_key_sync = lock_key_sync;
- vs->key_delay_ms = key_delay_ms;
+ vd->lock_key_sync = lock_key_sync;
+ vd->key_delay_ms = key_delay_ms;
device_id = qemu_opt_get(opts, "display");
if (device_id) {
@@ -3795,57 +3797,57 @@ void vnc_display_open(const char *id, Error **errp)
con = NULL;
}
- if (con != vs->dcl.con) {
- unregister_displaychangelistener(&vs->dcl);
- vs->dcl.con = con;
- register_displaychangelistener(&vs->dcl);
+ if (con != vd->dcl.con) {
+ unregister_displaychangelistener(&vd->dcl);
+ vd->dcl.con = con;
+ register_displaychangelistener(&vd->dcl);
}
if (reverse) {
/* connect to viewer */
QIOChannelSocket *sioc = NULL;
- vs->lsock = NULL;
- vs->lwebsock = NULL;
+ vd->lsock = NULL;
+ vd->lwebsock = NULL;
if (ws_enabled) {
error_setg(errp, "Cannot use websockets in reverse mode");
goto fail;
}
- vs->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
+ vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
sioc = qio_channel_socket_new();
if (qio_channel_socket_connect_sync(sioc, saddr, errp) < 0) {
goto fail;
}
- vnc_connect(vs, sioc, false, false);
+ vnc_connect(vd, sioc, false, false);
object_unref(OBJECT(sioc));
} else {
- vs->lsock = qio_channel_socket_new();
- if (qio_channel_socket_listen_sync(vs->lsock, saddr, errp) < 0) {
+ vd->lsock = qio_channel_socket_new();
+ if (qio_channel_socket_listen_sync(vd->lsock, saddr, errp) < 0) {
goto fail;
}
- vs->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
+ vd->is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
if (ws_enabled) {
- vs->lwebsock = qio_channel_socket_new();
- if (qio_channel_socket_listen_sync(vs->lwebsock,
+ vd->lwebsock = qio_channel_socket_new();
+ if (qio_channel_socket_listen_sync(vd->lwebsock,
wsaddr, errp) < 0) {
- object_unref(OBJECT(vs->lsock));
- vs->lsock = NULL;
+ object_unref(OBJECT(vd->lsock));
+ vd->lsock = NULL;
goto fail;
}
}
- vs->lsock_tag = qio_channel_add_watch(
- QIO_CHANNEL(vs->lsock),
- G_IO_IN, vnc_listen_io, vs, NULL);
+ vd->lsock_tag = qio_channel_add_watch(
+ QIO_CHANNEL(vd->lsock),
+ G_IO_IN, vnc_listen_io, vd, NULL);
if (ws_enabled) {
- vs->lwebsock_tag = qio_channel_add_watch(
- QIO_CHANNEL(vs->lwebsock),
- G_IO_IN, vnc_listen_io, vs, NULL);
+ vd->lwebsock_tag = qio_channel_add_watch(
+ QIO_CHANNEL(vd->lwebsock),
+ G_IO_IN, vnc_listen_io, vd, NULL);
}
}
if (show_vnc_port) {
- vnc_display_print_local_addr(vs);
+ vnc_display_print_local_addr(vd);
}
qapi_free_SocketAddress(saddr);
@@ -3860,16 +3862,16 @@ fail:
void vnc_display_add_client(const char *id, int csock, bool skipauth)
{
- VncDisplay *vs = vnc_display_find(id);
+ VncDisplay *vd = vnc_display_find(id);
QIOChannelSocket *sioc;
- if (!vs) {
+ if (!vd) {
return;
}
sioc = qio_channel_socket_new_fd(csock, NULL);
if (sioc) {
- vnc_connect(vs, sioc, skipauth, false);
+ vnc_connect(vd, sioc, skipauth, false);
object_unref(OBJECT(sioc));
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 5/9] ui: refactor method for setting up VncDisplay auth types
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
` (3 preceding siblings ...)
2016-09-29 15:45 ` [Qemu-devel] [PATCH 4/9] ui: rename misleading 'VncDisplay' variables Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 16:10 ` Eric Blake
2016-09-29 15:45 ` [Qemu-devel] [PATCH 6/9] ui: remove bogus call to graphic_hw_update() in vnc_listen_io Daniel P. Berrange
` (4 subsequent siblings)
9 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
There is alot of repeated code in the auth type setup method,
particularly around checking TLS credential types. Refactor
it to reduce duplication and instead of having one method
do both plain and websockets at once, call it separately
for each.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 124 +++++++++++++++++++++++++++------------------------------------
ui/vnc.h | 1 +
2 files changed, 53 insertions(+), 72 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 1104697..2f3ebdc 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3325,7 +3325,9 @@ static QemuOptsList qemu_vnc_opts = {
static int
-vnc_display_setup_auth(VncDisplay *vd,
+vnc_display_setup_auth(int *auth,
+ int *subauth,
+ QCryptoTLSCreds *tlscreds,
bool password,
bool sasl,
bool websocket,
@@ -3378,86 +3380,56 @@ vnc_display_setup_auth(VncDisplay *vd,
* VNC auth mechs for plain VNC vs websockets VNC, the end
* result has the same security characteristics.
*/
- if (password) {
- if (vd->tlscreds) {
- vd->auth = VNC_AUTH_VENCRYPT;
- if (object_dynamic_cast(OBJECT(vd->tlscreds),
- TYPE_QCRYPTO_TLS_CREDS_X509)) {
- VNC_DEBUG("Initializing VNC server with x509 password auth\n");
- vd->subauth = VNC_AUTH_VENCRYPT_X509VNC;
- } else if (object_dynamic_cast(OBJECT(vd->tlscreds),
- TYPE_QCRYPTO_TLS_CREDS_ANON)) {
- VNC_DEBUG("Initializing VNC server with TLS password auth\n");
- vd->subauth = VNC_AUTH_VENCRYPT_TLSVNC;
- } else {
- error_setg(errp,
- "Unsupported TLS cred type %s",
- object_get_typename(OBJECT(vd->tlscreds)));
- return -1;
- }
- } else {
+ if (websocket || !tlscreds) {
+ if (password) {
VNC_DEBUG("Initializing VNC server with password auth\n");
- vd->auth = VNC_AUTH_VNC;
- vd->subauth = VNC_AUTH_INVALID;
- }
- if (websocket) {
- vd->ws_auth = VNC_AUTH_VNC;
+ *auth = VNC_AUTH_VNC;
+ } else if (sasl) {
+ VNC_DEBUG("Initializing VNC server with SASL auth\n");
+ *auth = VNC_AUTH_SASL;
} else {
- vd->ws_auth = VNC_AUTH_INVALID;
+ VNC_DEBUG("Initializing VNC server with no auth\n");
+ *auth = VNC_AUTH_NONE;
}
- } else if (sasl) {
- if (vd->tlscreds) {
- vd->auth = VNC_AUTH_VENCRYPT;
- if (object_dynamic_cast(OBJECT(vd->tlscreds),
- TYPE_QCRYPTO_TLS_CREDS_X509)) {
+ *subauth = VNC_AUTH_INVALID;
+ } else {
+ bool is_x509 = object_dynamic_cast(OBJECT(tlscreds),
+ TYPE_QCRYPTO_TLS_CREDS_X509) != NULL;
+ bool is_anon = object_dynamic_cast(OBJECT(tlscreds),
+ TYPE_QCRYPTO_TLS_CREDS_ANON) != NULL;
+
+ if (!is_x509 && !is_anon) {
+ error_setg(errp,
+ "Unsupported TLS cred type %s",
+ object_get_typename(OBJECT(tlscreds)));
+ return -1;
+ }
+ *auth = VNC_AUTH_VENCRYPT;
+ if (password) {
+ if (is_x509) {
+ VNC_DEBUG("Initializing VNC server with x509 password auth\n");
+ *subauth = VNC_AUTH_VENCRYPT_X509VNC;
+ } else {
+ VNC_DEBUG("Initializing VNC server with TLS password auth\n");
+ *subauth = VNC_AUTH_VENCRYPT_TLSVNC;
+ }
+
+ } else if (sasl) {
+ if (is_x509) {
VNC_DEBUG("Initializing VNC server with x509 SASL auth\n");
- vd->subauth = VNC_AUTH_VENCRYPT_X509SASL;
- } else if (object_dynamic_cast(OBJECT(vd->tlscreds),
- TYPE_QCRYPTO_TLS_CREDS_ANON)) {
- VNC_DEBUG("Initializing VNC server with TLS SASL auth\n");
- vd->subauth = VNC_AUTH_VENCRYPT_TLSSASL;
+ *subauth = VNC_AUTH_VENCRYPT_X509SASL;
} else {
- error_setg(errp,
- "Unsupported TLS cred type %s",
- object_get_typename(OBJECT(vd->tlscreds)));
- return -1;
+ VNC_DEBUG("Initializing VNC server with TLS SASL auth\n");
+ *subauth = VNC_AUTH_VENCRYPT_TLSSASL;
}
} else {
- VNC_DEBUG("Initializing VNC server with SASL auth\n");
- vd->auth = VNC_AUTH_SASL;
- vd->subauth = VNC_AUTH_INVALID;
- }
- if (websocket) {
- vd->ws_auth = VNC_AUTH_SASL;
- } else {
- vd->ws_auth = VNC_AUTH_INVALID;
- }
- } else {
- if (vd->tlscreds) {
- vd->auth = VNC_AUTH_VENCRYPT;
- if (object_dynamic_cast(OBJECT(vd->tlscreds),
- TYPE_QCRYPTO_TLS_CREDS_X509)) {
+ if (is_x509) {
VNC_DEBUG("Initializing VNC server with x509 no auth\n");
- vd->subauth = VNC_AUTH_VENCRYPT_X509NONE;
- } else if (object_dynamic_cast(OBJECT(vd->tlscreds),
- TYPE_QCRYPTO_TLS_CREDS_ANON)) {
- VNC_DEBUG("Initializing VNC server with TLS no auth\n");
- vd->subauth = VNC_AUTH_VENCRYPT_TLSNONE;
+ *subauth = VNC_AUTH_VENCRYPT_X509NONE;
} else {
- error_setg(errp,
- "Unsupported TLS cred type %s",
- object_get_typename(OBJECT(vd->tlscreds)));
- return -1;
+ VNC_DEBUG("Initializing VNC server with TLS no auth\n");
+ *subauth = VNC_AUTH_VENCRYPT_TLSNONE;
}
- } else {
- VNC_DEBUG("Initializing VNC server with no auth\n");
- vd->auth = VNC_AUTH_NONE;
- vd->subauth = VNC_AUTH_INVALID;
- }
- if (websocket) {
- vd->ws_auth = VNC_AUTH_NONE;
- } else {
- vd->ws_auth = VNC_AUTH_INVALID;
}
}
return 0;
@@ -3769,7 +3741,15 @@ void vnc_display_open(const char *id, Error **errp)
}
#endif
- if (vnc_display_setup_auth(vd, password, sasl, ws_enabled, errp) < 0) {
+ if (vnc_display_setup_auth(&vd->auth, &vd->subauth,
+ vd->tlscreds, password,
+ sasl, false, errp) < 0) {
+ goto fail;
+ }
+
+ if (vnc_display_setup_auth(&vd->ws_auth, &vd->ws_subauth,
+ vd->tlscreds, password,
+ sasl, true, errp) < 0) {
goto fail;
}
diff --git a/ui/vnc.h b/ui/vnc.h
index 223af38..d191d88 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -172,6 +172,7 @@ struct VncDisplay
int auth;
int subauth; /* Used by VeNCrypt */
int ws_auth; /* Used by websockets */
+ int ws_subauth; /* Used by websockets */
bool lossy;
bool non_adaptive;
QCryptoTLSCreds *tlscreds;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 6/9] ui: remove bogus call to graphic_hw_update() in vnc_listen_io
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
` (4 preceding siblings ...)
2016-09-29 15:45 ` [Qemu-devel] [PATCH 5/9] ui: refactor method for setting up VncDisplay auth types Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 7/9] ui: remove bogus call to reset_keys() in vnc_init_state Daniel P. Berrange
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
Just before accepting a new client connection the vnc_listen_io
method calls graphic_hw_update(). This is bogus because there
is a call to this method already in vnc_state_init() and the
client doesn't need up2date graphics console before reaching
that.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 2f3ebdc..d1f33d3 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3102,8 +3102,6 @@ static gboolean vnc_listen_io(QIOChannel *ioc,
QIOChannelSocket *sioc = NULL;
Error *err = NULL;
- /* Catch-up */
- graphic_hw_update(vd->dcl.con);
sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc), &err);
if (sioc != NULL) {
qio_channel_set_delay(QIO_CHANNEL(sioc), false);
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 7/9] ui: remove bogus call to reset_keys() in vnc_init_state
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
` (5 preceding siblings ...)
2016-09-29 15:45 ` [Qemu-devel] [PATCH 6/9] ui: remove bogus call to graphic_hw_update() in vnc_listen_io Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 8/9] ui: move some initialization out of vnc_init_state Daniel P. Berrange
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
The vnc_init_state method calls reset_keys() to reset the
modifier key state. This was originally added in
commit 53762ddb277c690e486d0e17b10591774248c8cf
Author: malc <malc@c046a42c-6fe2-441c-8c8c-71466251a162>
Date: Mon Dec 1 20:57:52 2008 +0000
Reset the key modifiers upon client connect
This was valid at this time because there was only the
single VncState object which was persistent across client
connections and so needed resetting.
The persistent data was later split off into VncDisplay
and VncState was allocated at time of client connection:
commit 753b4053311ff1437d99726970b1e7e6bf38249b
Author: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>
Date: Mon Feb 16 14:59:30 2009 +0000
Support multiple VNC clients (Brian Kress)
at which point the modifier state is always 0 due to
use of g_new0. As such the reset_keys() call has been
a no-op ever since.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index d1f33d3..7c8f07d 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3086,7 +3086,6 @@ void vnc_init_state(VncState *vs)
vnc_write(vs, "RFB 003.008\n", 12);
vnc_flush(vs);
vnc_read_when(vs, protocol_version, 12);
- reset_keys(vs);
if (vs->vd->lock_key_sync)
vs->led = qemu_add_led_event_handler(kbd_leds, vs);
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 8/9] ui: move some initialization out of vnc_init_state
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
` (6 preceding siblings ...)
2016-09-29 15:45 ` [Qemu-devel] [PATCH 7/9] ui: remove bogus call to reset_keys() in vnc_init_state Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-09-29 15:45 ` [Qemu-devel] [PATCH 9/9] ui: rename vnc_init_state to vnc_start_protocol Daniel P. Berrange
2016-10-13 7:21 ` [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Gerd Hoffmann
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
Most of the fields in VncState are initialized in the
vnc_connect() method, but some are done in vnc_init_state()
instead.
The purpose of having vnc_init_state() is to delay starting
of the VNC wire protocol until after the websockets handshake
has completed. As such the vnc_init_state() method only needs
to be used for initialization that is dependant on the wire
protocol running.
This also lets us get rid of the initialized boolean flag
from the VncState struct.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc.c | 49 +++++++++++++++++++++++--------------------------
ui/vnc.h | 1 -
2 files changed, 23 insertions(+), 27 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 7c8f07d..c10a003 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1222,13 +1222,13 @@ void vnc_disconnect_finish(VncState *vs)
audio_del(vs);
vnc_release_modifiers(vs);
- if (vs->initialized) {
- QTAILQ_REMOVE(&vs->vd->clients, vs, next);
+ if (vs->mouse_mode_notifier.notify != NULL) {
qemu_remove_mouse_mode_change_notifier(&vs->mouse_mode_notifier);
- if (QTAILQ_EMPTY(&vs->vd->clients)) {
- /* last client gone */
- vnc_update_server_surface(vs->vd);
- }
+ }
+ QTAILQ_REMOVE(&vs->vd->clients, vs, next);
+ if (QTAILQ_EMPTY(&vs->vd->clients)) {
+ /* last client gone */
+ vnc_update_server_surface(vs->vd);
}
if (vs->vd->lock_key_sync)
@@ -2978,6 +2978,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
bool skipauth, bool websocket)
{
VncState *vs = g_new0(VncState, 1);
+ bool first_client = QTAILQ_EMPTY(&vd->clients);
int i;
vs->sioc = sioc;
@@ -3045,26 +3046,6 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
vnc_qmp_event(vs, QAPI_EVENT_VNC_CONNECTED);
vnc_set_share_mode(vs, VNC_SHARE_MODE_CONNECTING);
- if (!vs->websocket) {
- vnc_init_state(vs);
- }
-
- if (vd->num_connecting > vd->connections_limit) {
- QTAILQ_FOREACH(vs, &vd->clients, next) {
- if (vs->share_mode == VNC_SHARE_MODE_CONNECTING) {
- vnc_disconnect_start(vs);
- return;
- }
- }
- }
-}
-
-void vnc_init_state(VncState *vs)
-{
- vs->initialized = true;
- VncDisplay *vd = vs->vd;
- bool first_client = QTAILQ_EMPTY(&vd->clients);
-
vs->last_x = -1;
vs->last_y = -1;
@@ -3083,6 +3064,22 @@ void vnc_init_state(VncState *vs)
graphic_hw_update(vd->dcl.con);
+ if (!vs->websocket) {
+ vnc_init_state(vs);
+ }
+
+ if (vd->num_connecting > vd->connections_limit) {
+ QTAILQ_FOREACH(vs, &vd->clients, next) {
+ if (vs->share_mode == VNC_SHARE_MODE_CONNECTING) {
+ vnc_disconnect_start(vs);
+ return;
+ }
+ }
+ }
+}
+
+void vnc_init_state(VncState *vs)
+{
vnc_write(vs, "RFB 003.008\n", 12);
vnc_flush(vs);
vnc_read_when(vs, protocol_version, 12);
diff --git a/ui/vnc.h b/ui/vnc.h
index d191d88..e48e155 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -307,7 +307,6 @@ struct VncState
QEMUPutLEDEntry *led;
bool abort;
- bool initialized;
QemuMutex output_mutex;
QEMUBH *bh;
Buffer jobs_buffer;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH 9/9] ui: rename vnc_init_state to vnc_start_protocol
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
` (7 preceding siblings ...)
2016-09-29 15:45 ` [Qemu-devel] [PATCH 8/9] ui: move some initialization out of vnc_init_state Daniel P. Berrange
@ 2016-09-29 15:45 ` Daniel P. Berrange
2016-10-13 7:21 ` [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Gerd Hoffmann
9 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrange @ 2016-09-29 15:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Daniel P. Berrange
Rename the vnc_init_state method to reflect what its actual
purpose is, to discourage future devs from using it for more
general state initialization.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
ui/vnc-ws.c | 2 +-
ui/vnc.c | 4 ++--
ui/vnc.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 3bac46e..42a8e7b 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -92,7 +92,7 @@ static void vncws_handshake_done(Object *source,
vnc_client_error(vs);
} else {
VNC_DEBUG("Websock handshake complete, starting VNC protocol\n");
- vnc_init_state(vs);
+ vnc_start_protocol(vs);
vs->ioc_tag = qio_channel_add_watch(
vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
}
diff --git a/ui/vnc.c b/ui/vnc.c
index c10a003..c1e98fb 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3065,7 +3065,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
graphic_hw_update(vd->dcl.con);
if (!vs->websocket) {
- vnc_init_state(vs);
+ vnc_start_protocol(vs);
}
if (vd->num_connecting > vd->connections_limit) {
@@ -3078,7 +3078,7 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
}
}
-void vnc_init_state(VncState *vs)
+void vnc_start_protocol(VncState *vs)
{
vnc_write(vs, "RFB 003.008\n", 12);
vnc_flush(vs);
diff --git a/ui/vnc.h b/ui/vnc.h
index e48e155..d20b154 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -515,7 +515,7 @@ void vnc_write_u8(VncState *vs, uint8_t value);
void vnc_flush(VncState *vs);
void vnc_read_when(VncState *vs, VncReadEvent *func, size_t expecting);
void vnc_disconnect_finish(VncState *vs);
-void vnc_init_state(VncState *vs);
+void vnc_start_protocol(VncState *vs);
/* Buffer I/O functions */
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups
2016-09-29 15:45 [Qemu-devel] [PATCH 0/9] Misc VNC server code cleanups Daniel P. Berrange
` (8 preceding siblings ...)
2016-09-29 15:45 ` [Qemu-devel] [PATCH 9/9] ui: rename vnc_init_state to vnc_start_protocol Daniel P. Berrange
@ 2016-10-13 7:21 ` Gerd Hoffmann
9 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13 7:21 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: qemu-devel
On Do, 2016-09-29 at 16:45 +0100, Daniel P. Berrange wrote:
> This patches series should have no functional change, it is
> just a series of cleanups I've accumulated for the VNC server.
> It aims to remove misleading cruft and simplify some parts
> to make future work I'm experimenting with easier.
>
> Daniel P. Berrange (9):
> ui: remove misleading comment from vnc_init_state
> ui: remove 'enabled' and 'ws_enabled' fields from VncState
> ui: remove 'ws_tls' field from VncState
> ui: rename misleading 'VncDisplay' variables
> ui: refactor method for setting up VncDisplay auth types
> ui: remove bogus call to graphic_hw_update() in vnc_listen_io
> ui: remove bogus call to reset_keys() in vnc_init_state
> ui: move some initialization out of vnc_init_state
> ui: rename vnc_init_state to vnc_start_protocol
>
> ui/vnc-ws.c | 2 +-
> ui/vnc.c | 410 +++++++++++++++++++++++++++---------------------------------
> ui/vnc.h | 7 +-
> 3 files changed, 189 insertions(+), 230 deletions(-)
>
Added to UI patch queue.
thanks,
Gerd
^ permalink raw reply [flat|nested] 12+ messages in thread