Patchwork ps2: migrate ledstate

login
register
mail settings
Submitter Christophe Fergeau
Date Oct. 12, 2011, 4:35 p.m.
Message ID <1318437328-16865-1-git-send-email-cfergeau@redhat.com>
Download mbox | patch
Permalink /patch/119253/
State New
Headers show

Comments

Christophe Fergeau - Oct. 12, 2011, 4:35 p.m.
Make the ps2 device track its ledstate so that we can migrate it.
Otherwise it gets lost across migration, and spice-server gets
confused about the actual keyboard state and sends bogus
caps/scroll/num key events.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
---
 hw/ps2.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)
Anthony Liguori - Oct. 12, 2011, 4:42 p.m.
On 10/12/2011 11:35 AM, Christophe Fergeau wrote:
> Make the ps2 device track its ledstate so that we can migrate it.
> Otherwise it gets lost across migration, and spice-server gets
> confused about the actual keyboard state and sends bogus
> caps/scroll/num key events.
>
> Signed-off-by: Christophe Fergeau<cfergeau@redhat.com>
> ---
>   hw/ps2.c |   13 +++++++++++--
>   1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ps2.c b/hw/ps2.c
> index 24228c1..f19ea18 100644
> --- a/hw/ps2.c
> +++ b/hw/ps2.c
> @@ -92,6 +92,7 @@ typedef struct {
>          not the keyboard controller.  */
>       int translate;
>       int scancode_set; /* 1=XT, 2=AT, 3=PS/2 */
> +    int ledstate;
>   } PS2KbdState;
>
>   typedef struct {
> @@ -195,11 +196,17 @@ uint32_t ps2_read_data(void *opaque)
>       return val;
>   }
>
> +static void ps2_set_ledstate(PS2KbdState *s, int ledstate)
> +{
> +    s->ledstate = ledstate;
> +    kbd_put_ledstate(ledstate);
> +}
> +
>   static void ps2_reset_keyboard(PS2KbdState *s)
>   {
>       s->scan_enabled = 1;
>       s->scancode_set = 2;
> -    kbd_put_ledstate(0);
> +    ps2_set_ledstate(s, 0);
>   }
>
>   void ps2_write_keyboard(void *opaque, int val)
> @@ -274,7 +281,7 @@ void ps2_write_keyboard(void *opaque, int val)
>           s->common.write_cmd = -1;
>           break;
>       case KBD_CMD_SET_LEDS:
> -        kbd_put_ledstate(val);
> +        ps2_set_ledstate(s, val);
>           ps2_queue(&s->common, KBD_REPLY_ACK);
>           s->common.write_cmd = -1;
>           break;
> @@ -563,6 +570,7 @@ static int ps2_kbd_post_load(void* opaque, int version_id)
>
>       if (version_id == 2)
>           s->scancode_set=2;
> +    kbd_put_ledstate(s->ledstate);
>       return 0;
>   }
>
> @@ -577,6 +585,7 @@ static const VMStateDescription vmstate_ps2_keyboard = {
>           VMSTATE_INT32(scan_enabled, PS2KbdState),
>           VMSTATE_INT32(translate, PS2KbdState),
>           VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
> +        VMSTATE_INT32(ledstate, PS2KbdState),

If you're adding a field, you need to bump the version.

Regards,

Anthony Liguori

>           VMSTATE_END_OF_LIST()
>       }
>   };
Christophe Fergeau - Oct. 12, 2011, 4:47 p.m.
Hey,

On Wed, Oct 12, 2011 at 06:35:28PM +0200, Christophe Fergeau wrote:
> Make the ps2 device track its ledstate so that we can migrate it.
> Otherwise it gets lost across migration, and spice-server gets
> confused about the actual keyboard state and sends bogus
> caps/scroll/num key events.

This patch adds a ledstate to the PS2 keyboard driver as an alternative
to the original patch. I forgot to mention in the changelog entry that
this fixes https://bugzilla.redhat.com/show_bug.cgi?id=729294
One thing that could be improved after this patch is that the ledstate
is now present both in spice-input and hw/ps2.c, but I couldn't find
an easy way to get the ps2 ledstate from spice-input so I left things
this way for now.

Christophe
Gerd Hoffmann - Oct. 13, 2011, 7:49 a.m.
Hi,

> One thing that could be improved after this patch is that the ledstate
> is now present both in spice-input and hw/ps2.c, but I couldn't find
> an easy way to get the ps2 ledstate from spice-input so I left things
> this way for now.

Directly linking ps/2 and spice-input would be wrong anyway as there are 
other keyboards (usb) too ...

input.c could cache the ledstate and offer a kbd_get_ledstate function, 
so anyone interested in the state doesn't need to keep it itself.  That 
is a pretty minor thing though IMHO.

cheers,
   Gerd

Patch

diff --git a/hw/ps2.c b/hw/ps2.c
index 24228c1..f19ea18 100644
--- a/hw/ps2.c
+++ b/hw/ps2.c
@@ -92,6 +92,7 @@  typedef struct {
        not the keyboard controller.  */
     int translate;
     int scancode_set; /* 1=XT, 2=AT, 3=PS/2 */
+    int ledstate;
 } PS2KbdState;
 
 typedef struct {
@@ -195,11 +196,17 @@  uint32_t ps2_read_data(void *opaque)
     return val;
 }
 
+static void ps2_set_ledstate(PS2KbdState *s, int ledstate)
+{
+    s->ledstate = ledstate;
+    kbd_put_ledstate(ledstate);
+}
+
 static void ps2_reset_keyboard(PS2KbdState *s)
 {
     s->scan_enabled = 1;
     s->scancode_set = 2;
-    kbd_put_ledstate(0);
+    ps2_set_ledstate(s, 0);
 }
 
 void ps2_write_keyboard(void *opaque, int val)
@@ -274,7 +281,7 @@  void ps2_write_keyboard(void *opaque, int val)
         s->common.write_cmd = -1;
         break;
     case KBD_CMD_SET_LEDS:
-        kbd_put_ledstate(val);
+        ps2_set_ledstate(s, val);
         ps2_queue(&s->common, KBD_REPLY_ACK);
         s->common.write_cmd = -1;
         break;
@@ -563,6 +570,7 @@  static int ps2_kbd_post_load(void* opaque, int version_id)
 
     if (version_id == 2)
         s->scancode_set=2;
+    kbd_put_ledstate(s->ledstate);
     return 0;
 }
 
@@ -577,6 +585,7 @@  static const VMStateDescription vmstate_ps2_keyboard = {
         VMSTATE_INT32(scan_enabled, PS2KbdState),
         VMSTATE_INT32(translate, PS2KbdState),
         VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
+        VMSTATE_INT32(ledstate, PS2KbdState),
         VMSTATE_END_OF_LIST()
     }
 };