All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix.
@ 2016-10-13  9:32 Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 01/10] ui: remove misleading comment from vnc_init_state Gerd Hoffmann
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi,

Here is the ui patch queue, shipping mostly vnc cleanups, also a input-linux kbd fix.

please pull,
  Gerd

The following changes since commit c264a8807299852fc45562768ae60ccc886cea91:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-20161012-1' into staging (2016-10-12 14:05:23 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-ui-20161013-1

for you to fetch changes up to 2a57c55f26f7ba6dcea6d01ef74bae7069150f6f:

  input-linux: initialize key state (2016-10-13 09:25:24 +0200)

----------------------------------------------------------------
ui: vnc cleanups, input-linux kbd fix.

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

Gerd Hoffmann (1):
      input-linux: initialize key state

 ui/input-linux.c |  15 +-
 ui/vnc-ws.c      |   2 +-
 ui/vnc.c         | 412 +++++++++++++++++++++++++------------------------------
 ui/vnc.h         |   7 +-
 4 files changed, 203 insertions(+), 233 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 01/10] ui: remove misleading comment from vnc_init_state
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 02/10] ui: remove 'enabled' and 'ws_enabled' fields from VncState Gerd Hoffmann
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-2-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 02/10] ui: remove 'enabled' and 'ws_enabled' fields from VncState
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 01/10] ui: remove misleading comment from vnc_init_state Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 03/10] ui: remove 'ws_tls' field " Gerd Hoffmann
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-3-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 03/10] ui: remove 'ws_tls' field from VncState
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 01/10] ui: remove misleading comment from vnc_init_state Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 02/10] ui: remove 'enabled' and 'ws_enabled' fields from VncState Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 04/10] ui: rename misleading 'VncDisplay' variables Gerd Hoffmann
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-4-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 04/10] ui: rename misleading 'VncDisplay' variables
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 03/10] ui: remove 'ws_tls' field " Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 05/10] ui: refactor method for setting up VncDisplay auth types Gerd Hoffmann
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-5-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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));
     }
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 05/10] ui: refactor method for setting up VncDisplay auth types
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 04/10] ui: rename misleading 'VncDisplay' variables Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 06/10] ui: remove bogus call to graphic_hw_update() in vnc_listen_io Gerd Hoffmann
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

There is a lot 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>
Message-id: 1475163940-26094-6-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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)) {
+    if (websocket || !tlscreds) {
+        if (password) {
+            VNC_DEBUG("Initializing VNC server with password auth\n");
+            *auth = VNC_AUTH_VNC;
+        } else if (sasl) {
+            VNC_DEBUG("Initializing VNC server with SASL auth\n");
+            *auth = VNC_AUTH_SASL;
+        } else {
+            VNC_DEBUG("Initializing VNC server with no auth\n");
+            *auth = VNC_AUTH_NONE;
+        }
+        *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");
-                vd->subauth = VNC_AUTH_VENCRYPT_X509VNC;
-            } else if (object_dynamic_cast(OBJECT(vd->tlscreds),
-                                           TYPE_QCRYPTO_TLS_CREDS_ANON)) {
+                *subauth = VNC_AUTH_VENCRYPT_X509VNC;
+            } else {
                 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;
+                *subauth = VNC_AUTH_VENCRYPT_TLSVNC;
             }
-        } else {
-            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;
-        } else {
-            vd->ws_auth = VNC_AUTH_INVALID;
-        }
-    } else if (sasl) {
-        if (vd->tlscreds) {
-            vd->auth = VNC_AUTH_VENCRYPT;
-            if (object_dynamic_cast(OBJECT(vd->tlscreds),
-                                    TYPE_QCRYPTO_TLS_CREDS_X509)) {
+
+        } 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)) {
+                *subauth = VNC_AUTH_VENCRYPT_X509SASL;
+            } else {
                 VNC_DEBUG("Initializing VNC server with TLS SASL auth\n");
-                vd->subauth = VNC_AUTH_VENCRYPT_TLSSASL;
-            } else {
-                error_setg(errp,
-                           "Unsupported TLS cred type %s",
-                           object_get_typename(OBJECT(vd->tlscreds)));
-                return -1;
+                *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)) {
+                *subauth = VNC_AUTH_VENCRYPT_X509NONE;
+            } else {
                 VNC_DEBUG("Initializing VNC server with TLS no auth\n");
-                vd->subauth = VNC_AUTH_VENCRYPT_TLSNONE;
-            } else {
-                error_setg(errp,
-                           "Unsupported TLS cred type %s",
-                           object_get_typename(OBJECT(vd->tlscreds)));
-                return -1;
+                *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;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 06/10] ui: remove bogus call to graphic_hw_update() in vnc_listen_io
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 05/10] ui: refactor method for setting up VncDisplay auth types Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 07/10] ui: remove bogus call to reset_keys() in vnc_init_state Gerd Hoffmann
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-7-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 07/10] ui: remove bogus call to reset_keys() in vnc_init_state
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 06/10] ui: remove bogus call to graphic_hw_update() in vnc_listen_io Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 08/10] ui: move some initialization out of vnc_init_state Gerd Hoffmann
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-8-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 08/10] ui: move some initialization out of vnc_init_state
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 07/10] ui: remove bogus call to reset_keys() in vnc_init_state Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 09/10] ui: rename vnc_init_state to vnc_start_protocol Gerd Hoffmann
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-9-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 09/10] ui: rename vnc_init_state to vnc_start_protocol
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (7 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 08/10] ui: move some initialization out of vnc_init_state Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13  9:32 ` [Qemu-devel] [PULL 10/10] input-linux: initialize key state Gerd Hoffmann
  2016-10-13 15:08 ` [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrange, Gerd Hoffmann

From: "Daniel P. Berrange" <berrange@redhat.com>

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>
Message-id: 1475163940-26094-10-git-send-email-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@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 */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PULL 10/10] input-linux: initialize key state
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (8 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 09/10] ui: rename vnc_init_state to vnc_start_protocol Gerd Hoffmann
@ 2016-10-13  9:32 ` Gerd Hoffmann
  2016-10-13 15:08 ` [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2016-10-13  9:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Query input device keys, initialize state accordingly, so the correct
state is reflected in case any key is pressed at initialization time.
There is a high chance for this to actually happen for the 'enter' key
in case you start qemu with a terminal command (directly or virsh).

When finding any pressed keys the input grab is delayed until all keys
are lifted, to avoid confusing guest and host with appearently stuck
keys.

Reported-by: Muted Bytes <mutedbytes@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1476277384-30365-1-git-send-email-kraxel@redhat.com
---
 ui/input-linux.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/ui/input-linux.c b/ui/input-linux.c
index 0e230ce..f345317 100644
--- a/ui/input-linux.c
+++ b/ui/input-linux.c
@@ -347,7 +347,8 @@ static void input_linux_event(void *opaque)
 static void input_linux_complete(UserCreatable *uc, Error **errp)
 {
     InputLinux *il = INPUT_LINUX(uc);
-    uint8_t evtmap, relmap, absmap, keymap[KEY_CNT / 8];
+    uint8_t evtmap, relmap, absmap;
+    uint8_t keymap[KEY_CNT / 8], keystate[KEY_CNT / 8];
     unsigned int i;
     int rc, ver;
 
@@ -394,6 +395,7 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
     if (evtmap & (1 << EV_KEY)) {
         memset(keymap, 0, sizeof(keymap));
         rc = ioctl(il->fd, EVIOCGBIT(EV_KEY, sizeof(keymap)), keymap);
+        rc = ioctl(il->fd, EVIOCGKEY(sizeof(keystate)), keystate);
         for (i = 0; i < KEY_CNT; i++) {
             if (keymap[i / 8] & (1 << (i % 8))) {
                 if (linux_is_button(i)) {
@@ -401,12 +403,21 @@ static void input_linux_complete(UserCreatable *uc, Error **errp)
                 } else {
                     il->num_keys++;
                 }
+                if (keystate[i / 8] & (1 << (i % 8))) {
+                    il->keydown[i] = true;
+                    il->keycount++;
+                }
             }
         }
     }
 
     qemu_set_fd_handler(il->fd, input_linux_event, NULL, il);
-    input_linux_toggle_grab(il);
+    if (il->keycount) {
+        /* delay grab until all keys are released */
+        il->grab_request = true;
+    } else {
+        input_linux_toggle_grab(il);
+    }
     QTAILQ_INSERT_TAIL(&inputs, il, next);
     il->initialized = true;
     return;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix.
  2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
                   ` (9 preceding siblings ...)
  2016-10-13  9:32 ` [Qemu-devel] [PULL 10/10] input-linux: initialize key state Gerd Hoffmann
@ 2016-10-13 15:08 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2016-10-13 15:08 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: QEMU Developers

On 13 October 2016 at 10:32, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here is the ui patch queue, shipping mostly vnc cleanups, also a input-linux kbd fix.
>
> please pull,
>   Gerd
>
> The following changes since commit c264a8807299852fc45562768ae60ccc886cea91:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-20161012-1' into staging (2016-10-12 14:05:23 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-ui-20161013-1
>
> for you to fetch changes up to 2a57c55f26f7ba6dcea6d01ef74bae7069150f6f:
>
>   input-linux: initialize key state (2016-10-13 09:25:24 +0200)
>
> ----------------------------------------------------------------
> ui: vnc cleanups, input-linux kbd fix.
>
> ----------------------------------------------------------------
> 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
>
> Gerd Hoffmann (1):
>       input-linux: initialize key state

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-10-13 15:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-13  9:32 [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 01/10] ui: remove misleading comment from vnc_init_state Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 02/10] ui: remove 'enabled' and 'ws_enabled' fields from VncState Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 03/10] ui: remove 'ws_tls' field " Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 04/10] ui: rename misleading 'VncDisplay' variables Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 05/10] ui: refactor method for setting up VncDisplay auth types Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 06/10] ui: remove bogus call to graphic_hw_update() in vnc_listen_io Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 07/10] ui: remove bogus call to reset_keys() in vnc_init_state Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 08/10] ui: move some initialization out of vnc_init_state Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 09/10] ui: rename vnc_init_state to vnc_start_protocol Gerd Hoffmann
2016-10-13  9:32 ` [Qemu-devel] [PULL 10/10] input-linux: initialize key state Gerd Hoffmann
2016-10-13 15:08 ` [Qemu-devel] [PULL 00/10] ui: vnc cleanups, input-linux kbd fix Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.