Re: [Qemu-devel] [PATCH v4] ps2: migrate ledstate
On 10/17/2011 06:37 AM, Juan Quintela wrote:
From: Christophe Fergeau
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 fixes RH bug #729294
We only need to migrate the state when it is different of the default
one (0).
Signed-off-by: Christophe Fergeau
Signed-off-by: Juan Quintela
Applied. Thanks.
Regards,
Anthony Liguori
---
hw/ps2.c | 46 --
1 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/hw/ps2.c b/hw/ps2.c
index 24228c1..1d9057b 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;
@@ -557,6 +564,33 @@ static const VMStateDescription vmstate_ps2_common = {
}
};
+static bool ps2_keyboard_ledstate_needed(void *opaque)
+{
+PS2KbdState *s = opaque;
+
+return s->ledstate != 0; /* 0 is default state */
+}
+
+static int ps2_kbd_ledstate_post_load(void *opaque, int version_id)
+{
+PS2KbdState *s = opaque;
+
+kbd_put_ledstate(s->ledstate);
+return 0;
+}
+
+static const VMStateDescription vmstate_ps2_keyboard_ledstate = {
+.name = "ps2kbd/ledstate",
+.version_id = 3,
+.minimum_version_id = 2,
+.minimum_version_id_old = 2,
+.post_load = ps2_kbd_ledstate_post_load,
+.fields = (VMStateField []) {
+VMSTATE_INT32(ledstate, PS2KbdState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static int ps2_kbd_post_load(void* opaque, int version_id)
{
PS2KbdState *s = (PS2KbdState*)opaque;
@@ -578,6 +612,14 @@ static const VMStateDescription vmstate_ps2_keyboard = {
VMSTATE_INT32(translate, PS2KbdState),
VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
VMSTATE_END_OF_LIST()
+},
+.subsections = (VMStateSubsection []) {
+{
+.vmsd =&vmstate_ps2_keyboard_ledstate,
+.needed = ps2_keyboard_ledstate_needed,
+}, {
+/* empty */
+}
}
};
Re: [Qemu-devel] [PATCH v4] ps2: migrate ledstate
On 10/17/2011 10:15 AM, Gerd Hoffmann wrote: Hi, We only need to migrate the state when it is different of the default one (0). Er, isn't it going to be different from the default in like 99% of circumstances? Why? caps lock and scroll lock are off usually. numlock depends on what the user prefers. In case he cares in the first place of course. I expect most users don't and just go with the default. On my physical machines numlock is off by default. Likewise in my virtual machines: SeaBIOS keeps it off by default, and Linux+Windows guests don't muck with it at boot. I remember physical machines used to have numlock on by default in the 90ies. They even had a BIOS Setup option to pick the initial numlock state. That seems to be out-fashioned these days though, I havn't seen that for quite a while now ... Fair enough. Regards, Anthony Liguori cheers, Gerd
Re: [Qemu-devel] [PATCH v4] ps2: migrate ledstate
Gerd Hoffmann wrote: > Hi, > >>> We only need to migrate the state when it is different of the default >>> one (0). >> >> Er, isn't it going to be different from the default in like 99% of >> circumstances? > > Why? caps lock and scroll lock are off usually. numlock depends on > what the user prefers. In case he cares in the first place of > course. I expect most users don't and just go with the default. > > On my physical machines numlock is off by default. Likewise in my > virtual machines: SeaBIOS keeps it off by default, and Linux+Windows > guests don't muck with it at boot. > > I remember physical machines used to have numlock on by default in the > 90ies. They even had a BIOS Setup option to pick the initial numlock > state. That seems to be out-fashioned these days though, I havn't > seen that for quite a while now ... I have nothing else to add. This is exactly my point. I think that I have _never_ setup NumLock by default in any of my machines. And capslock use is very, very sporadic. In my case, it would be 0 about 99.9% of the time. Later, Juan.
Re: [Qemu-devel] [PATCH v4] ps2: migrate ledstate
Hi, We only need to migrate the state when it is different of the default one (0). Er, isn't it going to be different from the default in like 99% of circumstances? Why? caps lock and scroll lock are off usually. numlock depends on what the user prefers. In case he cares in the first place of course. I expect most users don't and just go with the default. On my physical machines numlock is off by default. Likewise in my virtual machines: SeaBIOS keeps it off by default, and Linux+Windows guests don't muck with it at boot. I remember physical machines used to have numlock on by default in the 90ies. They even had a BIOS Setup option to pick the initial numlock state. That seems to be out-fashioned these days though, I havn't seen that for quite a while now ... cheers, Gerd
Re: [Qemu-devel] [PATCH v4] ps2: migrate ledstate
On 10/17/2011 06:37 AM, Juan Quintela wrote:
From: Christophe Fergeau
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 fixes RH bug #729294
We only need to migrate the state when it is different of the default
one (0).
Er, isn't it going to be different from the default in like 99% of
circumstances?
I think in cases where you're usually going to send a subsection, bumping the
version is better.
Regards,
Anthony Liguori
Signed-off-by: Christophe Fergeau
Signed-off-by: Juan Quintela
---
hw/ps2.c | 46 --
1 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/hw/ps2.c b/hw/ps2.c
index 24228c1..1d9057b 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;
@@ -557,6 +564,33 @@ static const VMStateDescription vmstate_ps2_common = {
}
};
+static bool ps2_keyboard_ledstate_needed(void *opaque)
+{
+PS2KbdState *s = opaque;
+
+return s->ledstate != 0; /* 0 is default state */
+}
+
+static int ps2_kbd_ledstate_post_load(void *opaque, int version_id)
+{
+PS2KbdState *s = opaque;
+
+kbd_put_ledstate(s->ledstate);
+return 0;
+}
+
+static const VMStateDescription vmstate_ps2_keyboard_ledstate = {
+.name = "ps2kbd/ledstate",
+.version_id = 3,
+.minimum_version_id = 2,
+.minimum_version_id_old = 2,
+.post_load = ps2_kbd_ledstate_post_load,
+.fields = (VMStateField []) {
+VMSTATE_INT32(ledstate, PS2KbdState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static int ps2_kbd_post_load(void* opaque, int version_id)
{
PS2KbdState *s = (PS2KbdState*)opaque;
@@ -578,6 +612,14 @@ static const VMStateDescription vmstate_ps2_keyboard = {
VMSTATE_INT32(translate, PS2KbdState),
VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
VMSTATE_END_OF_LIST()
+},
+.subsections = (VMStateSubsection []) {
+{
+.vmsd =&vmstate_ps2_keyboard_ledstate,
+.needed = ps2_keyboard_ledstate_needed,
+}, {
+/* empty */
+}
}
};
Re: [Qemu-devel] [PATCH v4] ps2: migrate ledstate
Hey, On Mon, Oct 17, 2011 at 01:37:34PM +0200, Juan Quintela 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 fixes RH bug #729294 > > We only need to migrate the state when it is different of the default > one (0). I tested this version of the patch, and it works for me. Thanks! Christophe pgpVCentKvqMb.pgp Description: PGP signature
[Qemu-devel] [PATCH v4] ps2: migrate ledstate
From: Christophe Fergeau
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 fixes RH bug #729294
We only need to migrate the state when it is different of the default
one (0).
Signed-off-by: Christophe Fergeau
Signed-off-by: Juan Quintela
---
hw/ps2.c | 46 --
1 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/hw/ps2.c b/hw/ps2.c
index 24228c1..1d9057b 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;
@@ -557,6 +564,33 @@ static const VMStateDescription vmstate_ps2_common = {
}
};
+static bool ps2_keyboard_ledstate_needed(void *opaque)
+{
+PS2KbdState *s = opaque;
+
+return s->ledstate != 0; /* 0 is default state */
+}
+
+static int ps2_kbd_ledstate_post_load(void *opaque, int version_id)
+{
+PS2KbdState *s = opaque;
+
+kbd_put_ledstate(s->ledstate);
+return 0;
+}
+
+static const VMStateDescription vmstate_ps2_keyboard_ledstate = {
+.name = "ps2kbd/ledstate",
+.version_id = 3,
+.minimum_version_id = 2,
+.minimum_version_id_old = 2,
+.post_load = ps2_kbd_ledstate_post_load,
+.fields = (VMStateField []) {
+VMSTATE_INT32(ledstate, PS2KbdState),
+VMSTATE_END_OF_LIST()
+}
+};
+
static int ps2_kbd_post_load(void* opaque, int version_id)
{
PS2KbdState *s = (PS2KbdState*)opaque;
@@ -578,6 +612,14 @@ static const VMStateDescription vmstate_ps2_keyboard = {
VMSTATE_INT32(translate, PS2KbdState),
VMSTATE_INT32_V(scancode_set, PS2KbdState,3),
VMSTATE_END_OF_LIST()
+},
+.subsections = (VMStateSubsection []) {
+{
+.vmsd = &vmstate_ps2_keyboard_ledstate,
+.needed = ps2_keyboard_ledstate_needed,
+}, {
+/* empty */
+}
}
};
--
1.7.6.4
