[Qemu-devel] [PATCH 0/3 RFC v2] Support for LED state extension to Qemu VNC server

[Qemu-devel] [PATCH 0/3 RFC v2] Support for LED state extension to Qemu VNC server

[Lei Li]

Hi guys,

This patch series tries to add support for LED state
extension to Qemu VNC server. The proposal has been sent
few days ago as link below:

http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg01421.html

Hi Anthony,

This series has not been completely tested, it takes longer than
expected for that I found that it needs to add hack on gtk-vnc 
not just gvncfb, so still mark it as RFC. Send it out first to make
sure I understood your previous suggestions correctly. Please let
me know if this seems like the direction we should be headed.

Your suggestions and comments are very appreciated!

Thanks

Changes since v1:
  - Address the comments from Anthony includes:
    - Use Pseudo-encoding for led state;
    - Get rid of send_ext_leds_state_ack;
    - Add document for the led state Pseudo-encoding.

[Qemu-devel] [PATCH 1/3] vnc: Add SCROLL lock key to kbd_leds

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 ui/vnc.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 8ee66b7..f574962 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1525,10 +1525,11 @@ static void press_key(VncState *vs, int keysym)
 static void kbd_leds(void *opaque, int ledstate)
 {
     VncState *vs = opaque;
-    int caps, num;
+    int caps, num, scr;
 
     caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
     num  = ledstate & QEMU_NUM_LOCK_LED  ? 1 : 0;
+    scr  = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0;
 
     if (vs->modifiers_state[0x3a] != caps) {
         vs->modifiers_state[0x3a] = caps;
@@ -1536,6 +1537,9 @@ static void kbd_leds(void *opaque, int ledstate)
     if (vs->modifiers_state[0x45] != num) {
         vs->modifiers_state[0x45] = num;
     }
+    if (vs->modifiers_state[0x46] != scr) {
+        vs->modifiers_state[0x46] = scr;
+    }
 }
 
 static void do_key_event(VncState *vs, int down, int keycode, int sym)
-- 
1.7.7.6

[Qemu-devel] [PATCH 2/3] vnc: Support for LED state extension

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 ui/vnc.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 ui/vnc.h |    4 +++-
 2 files changed, 51 insertions(+), 1 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index f574962..e22cca1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1522,6 +1522,45 @@ static void press_key(VncState *vs, int keysym)
     kbd_put_keycode(keycode | SCANCODE_UP);
 }
 
+static int current_led_state(VncState *vs)
+{
+    int ledstate = 0;
+
+    if (vs->modifiers_state[0x46]) {
+        ledstate |= QEMU_SCROLL_LOCK_LED;
+    }
+    if (vs->modifiers_state[0x45]) {
+        ledstate |= QEMU_NUM_LOCK_LED;
+    }
+    if (vs->modifiers_state[0x3a]) {
+        ledstate |= QEMU_CAPS_LOCK_LED;
+    }
+
+    return ledstate;
+}
+
+static void vnc_led_state_change(VncState *vs)
+{
+    int ledstate = 0;
+
+    if (!vnc_has_feature(vs, VNC_FEATURE_LED_STATE)) {
+        return;
+    }
+
+    ledstate = current_led_state(vs);
+    vnc_lock_output(vs);
+    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
+    vnc_write_u8(vs, 0);
+    vnc_write_u16(vs, 1);
+    vnc_framebuffer_update(vs, 0, 0,
+                           surface_width(vs->vd->ds),
+                           surface_height(vs->vd->ds),
+                           VNC_ENCODING_LED_STATE);
+    vnc_write_u8(vs, ledstate);
+    vnc_unlock_output(vs);
+    vnc_flush(vs);
+}
+
 static void kbd_leds(void *opaque, int ledstate)
 {
     VncState *vs = opaque;
@@ -1540,6 +1579,11 @@ static void kbd_leds(void *opaque, int ledstate)
     if (vs->modifiers_state[0x46] != scr) {
         vs->modifiers_state[0x46] = scr;
     }
+
+    /* Sending the current led state message to the client */
+    if (ledstate != current_led_state(vs)) {
+        vnc_led_state_change(vs);
+    }
 }
 
 static void do_key_event(VncState *vs, int down, int keycode, int sym)
@@ -1893,6 +1937,9 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
         case VNC_ENCODING_WMVi:
             vs->features |= VNC_FEATURE_WMVI_MASK;
             break;
+        case VNC_ENCODING_LED_STATE:
+            vs->features |= VNC_FEATURE_LED_STATE_MASK;
+            break;
         case VNC_ENCODING_COMPRESSLEVEL0 ... VNC_ENCODING_COMPRESSLEVEL0 + 9:
             vs->tight.compression = (enc & 0x0F);
             break;
@@ -1908,6 +1955,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
     }
     vnc_desktop_resize(vs);
     check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
+    vnc_led_state_change(vs);
 }
 
 static void set_pixel_conversion(VncState *vs)
diff --git a/ui/vnc.h b/ui/vnc.h
index ad1dec2..e873377 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -384,6 +384,7 @@ enum {
 #define VNC_ENCODING_EXT_KEY_EVENT        0XFFFFFEFE /* -258 */
 #define VNC_ENCODING_AUDIO                0XFFFFFEFD /* -259 */
 #define VNC_ENCODING_TIGHT_PNG            0xFFFFFEFC /* -260 */
+#define VNC_ENCODING_LED_STATE            0XFFFFFEFB /* -261 */
 #define VNC_ENCODING_WMVi                 0x574D5669
 
 /*****************************************************************************
@@ -422,6 +423,7 @@ enum {
 #define VNC_FEATURE_TIGHT_PNG                8
 #define VNC_FEATURE_ZRLE                     9
 #define VNC_FEATURE_ZYWRLE                  10
+#define VNC_FEATURE_LED_STATE               11
 
 #define VNC_FEATURE_RESIZE_MASK              (1 << VNC_FEATURE_RESIZE)
 #define VNC_FEATURE_HEXTILE_MASK             (1 << VNC_FEATURE_HEXTILE)
@@ -434,7 +436,7 @@ enum {
 #define VNC_FEATURE_TIGHT_PNG_MASK           (1 << VNC_FEATURE_TIGHT_PNG)
 #define VNC_FEATURE_ZRLE_MASK                (1 << VNC_FEATURE_ZRLE)
 #define VNC_FEATURE_ZYWRLE_MASK              (1 << VNC_FEATURE_ZYWRLE)
-
+#define VNC_FEATURE_LED_STATE_MASK           (1 << VNC_FEATURE_LED_STATE)
 
 /* Client -> Server message IDs */
 #define VNC_MSG_CLIENT_SET_PIXEL_FORMAT           0
-- 
1.7.7.6

[Qemu-devel] [PATCH 3/3] doc: document the Pseudo-encoding of LED state

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 docs/vnc-ledstate-Pseudo-encoding.txt |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
 create mode 100644 docs/vnc-ledstate-Pseudo-encoding.txt

diff --git a/docs/vnc-ledstate-Pseudo-encoding.txt b/docs/vnc-ledstate-Pseudo-encoding.txt
new file mode 100644
index 0000000..3f3fd15
--- /dev/null
+++ b/docs/vnc-ledstate-Pseudo-encoding.txt
@@ -0,0 +1,33 @@
+VNC LED state Pseudo-encoding
+=============================
+
+Introduction
+------------
+
+This document describes the Pseudo-encoding of LED state for RFB which
+is the protocol used in VNC as reference link below:
+
+http://tigervnc.svn.sourceforge.net/viewvc/tigervnc/rfbproto/rfbproto.rst?content-type=text/plain
+
+When accessing a guest by console through VNC, there might be mismatch
+between the lock keys notification LED on the computer running the VNC
+client session and the current status of the lock keys on the guest
+machine.
+
+To solve this problem it attempts to add LED state Pseudo-encoding
+extension to VNC protocol to deal with setting LED state.
+
+Example
+-------
+
+The example psuedo-encodings for LED state defined as following:
+
+======= ===============================================================
+Code    Description
+======= ===============================================================
+100     CapsLock is set
+010     NumLock is set
+001     ScrollLock is set
+110     CapsLock and NumLock are set
+111     CapsLock, NumLock and ScrollLock are set
+======= ===============================================================
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 2/3] vnc: Support for LED state extension

[Anthony Liguori]

Lei Li <lilei@linux.vnet.ibm.com> writes:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  ui/vnc.c |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  ui/vnc.h |    4 +++-
>  2 files changed, 51 insertions(+), 1 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index f574962..e22cca1 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1522,6 +1522,45 @@ static void press_key(VncState *vs, int keysym)
>      kbd_put_keycode(keycode | SCANCODE_UP);
>  }
>  
> +static int current_led_state(VncState *vs)
> +{
> +    int ledstate = 0;
> +
> +    if (vs->modifiers_state[0x46]) {
> +        ledstate |= QEMU_SCROLL_LOCK_LED;
> +    }
> +    if (vs->modifiers_state[0x45]) {
> +        ledstate |= QEMU_NUM_LOCK_LED;
> +    }
> +    if (vs->modifiers_state[0x3a]) {
> +        ledstate |= QEMU_CAPS_LOCK_LED;
> +    }
> +
> +    return ledstate;
> +}
> +
> +static void vnc_led_state_change(VncState *vs)
> +{
> +    int ledstate = 0;
> +
> +    if (!vnc_has_feature(vs, VNC_FEATURE_LED_STATE)) {
> +        return;
> +    }
> +
> +    ledstate = current_led_state(vs);
> +    vnc_lock_output(vs);
> +    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
> +    vnc_write_u8(vs, 0);
> +    vnc_write_u16(vs, 1);
> +    vnc_framebuffer_update(vs, 0, 0,
> +                           surface_width(vs->vd->ds),
> +                           surface_height(vs->vd->ds),

This can just be 1, 1.  You don't have to send the actual width/height.

Otherwise, looks pretty good.

Regards,

Anthony Liguori

> +                           VNC_ENCODING_LED_STATE);
> +    vnc_write_u8(vs, ledstate);
> +    vnc_unlock_output(vs);
> +    vnc_flush(vs);
> +}
> +
>  static void kbd_leds(void *opaque, int ledstate)
>  {
>      VncState *vs = opaque;
> @@ -1540,6 +1579,11 @@ static void kbd_leds(void *opaque, int ledstate)
>      if (vs->modifiers_state[0x46] != scr) {
>          vs->modifiers_state[0x46] = scr;
>      }
> +
> +    /* Sending the current led state message to the client */
> +    if (ledstate != current_led_state(vs)) {
> +        vnc_led_state_change(vs);
> +    }
>  }
>  
>  static void do_key_event(VncState *vs, int down, int keycode, int sym)
> @@ -1893,6 +1937,9 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
>          case VNC_ENCODING_WMVi:
>              vs->features |= VNC_FEATURE_WMVI_MASK;
>              break;
> +        case VNC_ENCODING_LED_STATE:
> +            vs->features |= VNC_FEATURE_LED_STATE_MASK;
> +            break;
>          case VNC_ENCODING_COMPRESSLEVEL0 ... VNC_ENCODING_COMPRESSLEVEL0 + 9:
>              vs->tight.compression = (enc & 0x0F);
>              break;
> @@ -1908,6 +1955,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
>      }
>      vnc_desktop_resize(vs);
>      check_pointer_type_change(&vs->mouse_mode_notifier, NULL);
> +    vnc_led_state_change(vs);
>  }
>  
>  static void set_pixel_conversion(VncState *vs)
> diff --git a/ui/vnc.h b/ui/vnc.h
> index ad1dec2..e873377 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -384,6 +384,7 @@ enum {
>  #define VNC_ENCODING_EXT_KEY_EVENT        0XFFFFFEFE /* -258 */
>  #define VNC_ENCODING_AUDIO                0XFFFFFEFD /* -259 */
>  #define VNC_ENCODING_TIGHT_PNG            0xFFFFFEFC /* -260 */
> +#define VNC_ENCODING_LED_STATE            0XFFFFFEFB /* -261 */
>  #define VNC_ENCODING_WMVi                 0x574D5669
>  
>  /*****************************************************************************
> @@ -422,6 +423,7 @@ enum {
>  #define VNC_FEATURE_TIGHT_PNG                8
>  #define VNC_FEATURE_ZRLE                     9
>  #define VNC_FEATURE_ZYWRLE                  10
> +#define VNC_FEATURE_LED_STATE               11
>  
>  #define VNC_FEATURE_RESIZE_MASK              (1 << VNC_FEATURE_RESIZE)
>  #define VNC_FEATURE_HEXTILE_MASK             (1 << VNC_FEATURE_HEXTILE)
> @@ -434,7 +436,7 @@ enum {
>  #define VNC_FEATURE_TIGHT_PNG_MASK           (1 << VNC_FEATURE_TIGHT_PNG)
>  #define VNC_FEATURE_ZRLE_MASK                (1 << VNC_FEATURE_ZRLE)
>  #define VNC_FEATURE_ZYWRLE_MASK              (1 << VNC_FEATURE_ZYWRLE)
> -
> +#define VNC_FEATURE_LED_STATE_MASK           (1 << VNC_FEATURE_LED_STATE)
>  
>  /* Client -> Server message IDs */
>  #define VNC_MSG_CLIENT_SET_PIXEL_FORMAT           0
> -- 
> 1.7.7.6

Re: [Qemu-devel] [PATCH 3/3] doc: document the Pseudo-encoding of LED state

[Anthony Liguori]

Lei Li <lilei@linux.vnet.ibm.com> writes:

> Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
> ---
>  docs/vnc-ledstate-Pseudo-encoding.txt |   33 +++++++++++++++++++++++++++++++++
>  1 files changed, 33 insertions(+), 0 deletions(-)
>  create mode 100644 docs/vnc-ledstate-Pseudo-encoding.txt
>
> diff --git a/docs/vnc-ledstate-Pseudo-encoding.txt b/docs/vnc-ledstate-Pseudo-encoding.txt
> new file mode 100644
> index 0000000..3f3fd15
> --- /dev/null
> +++ b/docs/vnc-ledstate-Pseudo-encoding.txt
> @@ -0,0 +1,33 @@
> +VNC LED state Pseudo-encoding
> +=============================
> +
> +Introduction
> +------------
> +
> +This document describes the Pseudo-encoding of LED state for RFB which
> +is the protocol used in VNC as reference link below:
> +
> +http://tigervnc.svn.sourceforge.net/viewvc/tigervnc/rfbproto/rfbproto.rst?content-type=text/plain
> +
> +When accessing a guest by console through VNC, there might be mismatch
> +between the lock keys notification LED on the computer running the VNC
> +client session and the current status of the lock keys on the guest
> +machine.
> +
> +To solve this problem it attempts to add LED state Pseudo-encoding
> +extension to VNC protocol to deal with setting LED state.
> +
> +Example
> +-------
> +
> +The example psuedo-encodings for LED state defined as following:
> +
> +======= ===============================================================
> +Code    Description
> +======= ===============================================================
> +100     CapsLock is set
> +010     NumLock is set
> +001     ScrollLock is set
> +110     CapsLock and NumLock are set
> +111     CapsLock, NumLock and ScrollLock are set
> +=======
> ===============================================================

You can just describe that each bit represents the Caps, Num, and Scroll
lock key respectively and that '1' indicates that the LED should be on
and '0' should be off.

You should also list the psuedo-encoding number somewhere in this document.

Regards,

Anthony Liguori

> -- 
> 1.7.7.6

[Qemu-devel] [PATCH 1/3] vnc: Add SCROLL lock key to kbd_leds

[Lei Li]

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 ui/vnc.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 8ee66b7..f574962 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1525,10 +1525,11 @@ static void press_key(VncState *vs, int keysym)
 static void kbd_leds(void *opaque, int ledstate)
 {
     VncState *vs = opaque;
-    int caps, num;
+    int caps, num, scr;
 
     caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
     num  = ledstate & QEMU_NUM_LOCK_LED  ? 1 : 0;
+    scr  = ledstate & QEMU_SCROLL_LOCK_LED ? 1 : 0;
 
     if (vs->modifiers_state[0x3a] != caps) {
         vs->modifiers_state[0x3a] = caps;
@@ -1536,6 +1537,9 @@ static void kbd_leds(void *opaque, int ledstate)
     if (vs->modifiers_state[0x45] != num) {
         vs->modifiers_state[0x45] = num;
     }
+    if (vs->modifiers_state[0x46] != scr) {
+        vs->modifiers_state[0x46] = scr;
+    }
 }
 
 static void do_key_event(VncState *vs, int down, int keycode, int sym)
-- 
1.7.7.6