[Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Gerd Hoffmann]

For any modifier key (shift, ctrl, alt) still pressed on disconnect
inject a key-up event into the guest.  The vnc client is gone, it will
not do that, so qemu has to do it instead.

Without this keys will get stuck, making the guest act in weird ways
after reconnecting.  Reproducer: exit vnc client via Alt-F4, guest
continues to see the pressed alt key and will not react to key events
in any useful way until you tap the alt key once to unstuck it.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 83a9b15..02b71bc 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -46,6 +46,7 @@ static VncDisplay *vnc_display; /* needed for info vnc */
 static DisplayChangeListener *dcl;
 
 static int vnc_cursor_define(VncState *vs);
+static void vnc_release_modifiers(VncState *vs);
 
 static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
 {
@@ -1051,6 +1052,7 @@ static void vnc_disconnect_finish(VncState *vs)
     vnc_sasl_client_cleanup(vs);
 #endif /* CONFIG_VNC_SASL */
     audio_del(vs);
+    vnc_release_modifiers(vs);
 
     QTAILQ_REMOVE(&vs->vd->clients, vs, next);
 
@@ -1679,6 +1681,29 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
     }
 }
 
+static void vnc_release_modifiers(VncState *vs)
+{
+    static const int keycodes[] = {
+        /* shift, control, alt keys, both left & right */
+        0x2a, 0x36, 0x1d, 0x9d, 0x38, 0xb8,
+    };
+    int i, keycode;
+
+    if (!is_graphic_console()) {
+        return;
+    }
+    for (i = 0; i < ARRAY_SIZE(keycodes); i++) {
+        keycode = keycodes[i];
+        if (!vs->modifiers_state[keycode]) {
+            continue;
+        }
+        if (keycode & SCANCODE_GREY) {
+            kbd_put_keycode(SCANCODE_EMUL0);
+        }
+        kbd_put_keycode(keycode | SCANCODE_UP);
+    }
+}
+
 static void key_event(VncState *vs, int down, uint32_t sym)
 {
     int keycode;
-- 
1.7.1

Re: [Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Peter Lieven]

Hi Gerd,

just looking at your patch. It works fine so far, but I am curious how
to handle the lock keys? I have the problem that if I press caps lock
and then create a new vnc session with exclusive access (from another
terminal), the caps lock is still there.

Wouldn't it be right to release at least also caps lock if not all lock
keys?

Peter

Gerd Hoffmann wrote:
> For any modifier key (shift, ctrl, alt) still pressed on disconnect
> inject a key-up event into the guest.  The vnc client is gone, it will
> not do that, so qemu has to do it instead.
>
> Without this keys will get stuck, making the guest act in weird ways
> after reconnecting.  Reproducer: exit vnc client via Alt-F4, guest
> continues to see the pressed alt key and will not react to key events
> in any useful way until you tap the alt key once to unstuck it.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc.c |   25 +++++++++++++++++++++++++
>  1 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 83a9b15..02b71bc 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -46,6 +46,7 @@ static VncDisplay *vnc_display; /* needed for info vnc
> */
>  static DisplayChangeListener *dcl;
>
>  static int vnc_cursor_define(VncState *vs);
> +static void vnc_release_modifiers(VncState *vs);
>
>  static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
>  {
> @@ -1051,6 +1052,7 @@ static void vnc_disconnect_finish(VncState *vs)
>      vnc_sasl_client_cleanup(vs);
>  #endif /* CONFIG_VNC_SASL */
>      audio_del(vs);
> +    vnc_release_modifiers(vs);
>
>      QTAILQ_REMOVE(&vs->vd->clients, vs, next);
>
> @@ -1679,6 +1681,29 @@ static void do_key_event(VncState *vs, int down,
> int keycode, int sym)
>      }
>  }
>
> +static void vnc_release_modifiers(VncState *vs)
> +{
> +    static const int keycodes[] = {
> +        /* shift, control, alt keys, both left & right */
> +        0x2a, 0x36, 0x1d, 0x9d, 0x38, 0xb8,
> +    };
> +    int i, keycode;
> +
> +    if (!is_graphic_console()) {
> +        return;
> +    }
> +    for (i = 0; i < ARRAY_SIZE(keycodes); i++) {
> +        keycode = keycodes[i];
> +        if (!vs->modifiers_state[keycode]) {
> +            continue;
> +        }
> +        if (keycode & SCANCODE_GREY) {
> +            kbd_put_keycode(SCANCODE_EMUL0);
> +        }
> +        kbd_put_keycode(keycode | SCANCODE_UP);
> +    }
> +}
> +
>  static void key_event(VncState *vs, int down, uint32_t sym)
>  {
>      int keycode;
> --
> 1.7.1
>
>
>

Re: [Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Gerd Hoffmann]

On 06/17/13 10:10, Peter Lieven wrote:
> Hi Gerd,
> 
> just looking at your patch. It works fine so far, but I am curious how
> to handle the lock keys? I have the problem that if I press caps lock
> and then create a new vnc session with exclusive access (from another
> terminal), the caps lock is still there.

There is some logic for that in the vnc server, next time you press a
letter key it should get synced up.  capslock+numlock can actually
easier go out of sync than the other modifiers, you don't need a
reconnect to trigger that:  Activate some other window so the vnc client
window so it hasn't the keyboard focus, press capslock or numlock, move
focus back to the vnc window.

HTH,
  Gerd

Re: [Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Peter Lieven]

Am 17.06.2013 10:49, schrieb Gerd Hoffmann:
> On 06/17/13 10:10, Peter Lieven wrote:
>> Hi Gerd,
>>
>> just looking at your patch. It works fine so far, but I am curious how
>> to handle the lock keys? I have the problem that if I press caps lock
>> and then create a new vnc session with exclusive access (from another
>> terminal), the caps lock is still there.
> 
> There is some logic for that in the vnc server, next time you press a
> letter key it should get synced up.  capslock+numlock can actually
> easier go out of sync than the other modifiers, you don't need a
> reconnect to trigger that:  Activate some other window so the vnc client
> window so it hasn't the keyboard focus, press capslock or numlock, move
> focus back to the vnc window.

It seems the sync logic fails iff.
 Caps Lock enabled -> VNC Disconnect -> Caps Lock disabled -> VNC Reconnect.
 In this case the guest OS is still typing uppercase letters.

Peter

Re: [Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Gerd Hoffmann]

On 06/17/13 13:05, Peter Lieven wrote:
> Am 17.06.2013 10:49, schrieb Gerd Hoffmann:
>> On 06/17/13 10:10, Peter Lieven wrote:
>>> Hi Gerd,
>>>
>>> just looking at your patch. It works fine so far, but I am curious how
>>> to handle the lock keys? I have the problem that if I press caps lock
>>> and then create a new vnc session with exclusive access (from another
>>> terminal), the caps lock is still there.
>>
>> There is some logic for that in the vnc server, next time you press a
>> letter key it should get synced up.  capslock+numlock can actually
>> easier go out of sync than the other modifiers, you don't need a
>> reconnect to trigger that:  Activate some other window so the vnc client
>> window so it hasn't the keyboard focus, press capslock or numlock, move
>> focus back to the vnc window.
> 
> It seems the sync logic fails iff.
>  Caps Lock enabled -> VNC Disconnect -> Caps Lock disabled -> VNC Reconnect.
>  In this case the guest OS is still typing uppercase letters.

I guess reset_keys() should skip capslock+numlock to not disturb the
sync logic, can you try that?

cheers,
 Gerd

Re: [Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Peter Lieven]

Am 17.06.2013 14:09, schrieb Gerd Hoffmann:
> On 06/17/13 13:05, Peter Lieven wrote:
>> Am 17.06.2013 10:49, schrieb Gerd Hoffmann:
>>> On 06/17/13 10:10, Peter Lieven wrote:
>>>> Hi Gerd,
>>>>
>>>> just looking at your patch. It works fine so far, but I am curious how
>>>> to handle the lock keys? I have the problem that if I press caps lock
>>>> and then create a new vnc session with exclusive access (from another
>>>> terminal), the caps lock is still there.
>>>
>>> There is some logic for that in the vnc server, next time you press a
>>> letter key it should get synced up.  capslock+numlock can actually
>>> easier go out of sync than the other modifiers, you don't need a
>>> reconnect to trigger that:  Activate some other window so the vnc client
>>> window so it hasn't the keyboard focus, press capslock or numlock, move
>>> focus back to the vnc window.
>>
>> It seems the sync logic fails iff.
>>  Caps Lock enabled -> VNC Disconnect -> Caps Lock disabled -> VNC Reconnect.
>>  In this case the guest OS is still typing uppercase letters.
> 
> I guess reset_keys() should skip capslock+numlock to not disturb the
> sync logic, can you try that?

mhh, i might be wrong, but where could the vs->modifiers_state be not zero
on a new connection. I only find a g_malloc0 initializing the VNCState.

Peter

Re: [Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Gerd Hoffmann]

  Hi,

>> I guess reset_keys() should skip capslock+numlock to not disturb the
>> sync logic, can you try that?
> 
> mhh, i might be wrong, but where could the vs->modifiers_state be not zero
> on a new connection. I only find a g_malloc0 initializing the VNCState.

Ah, right, it is in the per-connection vnc state not in the
server-global vnc state.  Yes, I guess we should store ledstate in
VncDisplay then, so we can call kbd_leds() on new connections.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] vnc: lift modifier keys on client disconnect.

[Peter Lieven]

Am 17.06.2013 15:58, schrieb Gerd Hoffmann:
>   Hi,
> 
>>> I guess reset_keys() should skip capslock+numlock to not disturb the
>>> sync logic, can you try that?
>>
>> mhh, i might be wrong, but where could the vs->modifiers_state be not zero
>> on a new connection. I only find a g_malloc0 initializing the VNCState.
> 
> Ah, right, it is in the per-connection vnc state not in the
> server-global vnc state.  Yes, I guess we should store ledstate in
> VncDisplay then, so we can call kbd_leds() on new connections.

maybe all modifiers should be stored global?

anyway, what also seems to work is to clear capslock / numlock on disconnect
like shift,alt,ctrl if the sync logic is enabled.

Peter

> 
> cheers,
>   Gerd
> 
>