| Message ID | 1318350500-13896-1-git-send-email-alevy@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10/11/2011 11:28 AM, Alon Levy wrote: > The caps/scroll/num lock state is not tracked by the ps2 device, but by > spice-input QemuSpiceKbd->ledstate. To fix losing it across migration, > and then having the server send caps/scroll/nums keys when the client > sends a SPICE_MSGC_INPUTS_KEY_MODIFIERS, migrate it. > > RHBZ# 729294 > > Signed-off-by: Alon Levy<alevy@redhat.com> > --- > trace-events | 3 +++ > ui/spice-input.c | 24 ++++++++++++++++++++++++ > 2 files changed, 27 insertions(+), 0 deletions(-) > > diff --git a/trace-events b/trace-events > index 63d8c8e..605dc79 100644 > --- a/trace-events > +++ b/trace-events > @@ -538,3 +538,6 @@ esp_mem_writeb_cmd_ensel(uint32_t val) "Enable selection (%2.2x)" > # monitor.c > handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\"" > monitor_protocol_emitter(void *mon) "mon %p" > + > +# ui/spice-input.c > +disable spice_input_post_load(int ledstate) "leds %d" > diff --git a/ui/spice-input.c b/ui/spice-input.c > index af4223d..2f0d3af 100644 > --- a/ui/spice-input.c > +++ b/ui/spice-input.c > @@ -26,6 +26,8 @@ > #include "qemu-common.h" > #include "qemu-spice.h" > #include "console.h" > +#include "hw/hw.h" > +#include "trace.h" > > /* keyboard bits */ > > @@ -195,6 +197,27 @@ static void mouse_mode_notifier(Notifier *notifier, void *data) > pointer->absolute = is_absolute; > } > > +static int qemu_spice_input_post_load(void *opaque, int version_id) > +{ > + QemuSpiceKbd *s = (QemuSpiceKbd *)opaque; > + > + trace_spice_input_post_load(s->ledstate); > + kbd_leds(s, s->ledstate); > + return 0; > +} > + > +static const VMStateDescription vmstate_spice_kbd = { > + .name = "spice-input", > + .version_id = 0, > + .minimum_version_id = 0, > + .minimum_version_id_old = 0, > + .post_load = qemu_spice_input_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_INT32(ledstate, QemuSpiceKbd), > + VMSTATE_END_OF_LIST() > + } > +}; > + This is wrong IMHO. Spice is not a device. Why not just add LED state to the migration of the PS/2 keyboard? Regards, Anthony Liguori > void qemu_spice_input_init(void) > { > QemuSpiceKbd *kbd; > @@ -204,6 +227,7 @@ void qemu_spice_input_init(void) > kbd->sin.base.sif =&kbd_interface.base; > qemu_spice_add_interface(&kbd->sin.base); > qemu_add_led_event_handler(kbd_leds, kbd); > + vmstate_register(NULL, 0,&vmstate_spice_kbd, kbd); > > pointer = g_malloc0(sizeof(*pointer)); > pointer->mouse.base.sif =&mouse_interface.base;
On Tue, Oct 11, 2011 at 01:23:00PM -0500, Anthony Liguori wrote: > On 10/11/2011 11:28 AM, Alon Levy wrote: > >The caps/scroll/num lock state is not tracked by the ps2 device, but by > >spice-input QemuSpiceKbd->ledstate. To fix losing it across migration, > >and then having the server send caps/scroll/nums keys when the client > >sends a SPICE_MSGC_INPUTS_KEY_MODIFIERS, migrate it. > > > >RHBZ# 729294 > > > >Signed-off-by: Alon Levy<alevy@redhat.com> > >--- > > trace-events | 3 +++ > > ui/spice-input.c | 24 ++++++++++++++++++++++++ > > 2 files changed, 27 insertions(+), 0 deletions(-) > > > >diff --git a/trace-events b/trace-events > >index 63d8c8e..605dc79 100644 > >--- a/trace-events > >+++ b/trace-events > >@@ -538,3 +538,6 @@ esp_mem_writeb_cmd_ensel(uint32_t val) "Enable selection (%2.2x)" > > # monitor.c > > handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\"" > > monitor_protocol_emitter(void *mon) "mon %p" > >+ > >+# ui/spice-input.c > >+disable spice_input_post_load(int ledstate) "leds %d" > >diff --git a/ui/spice-input.c b/ui/spice-input.c > >index af4223d..2f0d3af 100644 > >--- a/ui/spice-input.c > >+++ b/ui/spice-input.c > >@@ -26,6 +26,8 @@ > > #include "qemu-common.h" > > #include "qemu-spice.h" > > #include "console.h" > >+#include "hw/hw.h" > >+#include "trace.h" > > > > /* keyboard bits */ > > > >@@ -195,6 +197,27 @@ static void mouse_mode_notifier(Notifier *notifier, void *data) > > pointer->absolute = is_absolute; > > } > > > >+static int qemu_spice_input_post_load(void *opaque, int version_id) > >+{ > >+ QemuSpiceKbd *s = (QemuSpiceKbd *)opaque; > >+ > >+ trace_spice_input_post_load(s->ledstate); > >+ kbd_leds(s, s->ledstate); > >+ return 0; > >+} > >+ > >+static const VMStateDescription vmstate_spice_kbd = { > >+ .name = "spice-input", > >+ .version_id = 0, > >+ .minimum_version_id = 0, > >+ .minimum_version_id_old = 0, > >+ .post_load = qemu_spice_input_post_load, > >+ .fields = (VMStateField[]) { > >+ VMSTATE_INT32(ledstate, QemuSpiceKbd), > >+ VMSTATE_END_OF_LIST() > >+ } > >+}; > >+ > > This is wrong IMHO. Spice is not a device. Why not just add LED > state to the migration of the PS/2 keyboard? > > Regards, > > Anthony Liguori There is no ledstate in a PS/2 keyboard (or I'm reading too much into the implementation in qemu). usb-hid does have it, but PS/2 just passes fragments to the PC, and get's led setting commands back (for some specific keys). Or are you suggesting to put this stuff in there anyway? > > > void qemu_spice_input_init(void) > > { > > QemuSpiceKbd *kbd; > >@@ -204,6 +227,7 @@ void qemu_spice_input_init(void) > > kbd->sin.base.sif =&kbd_interface.base; > > qemu_spice_add_interface(&kbd->sin.base); > > qemu_add_led_event_handler(kbd_leds, kbd); > >+ vmstate_register(NULL, 0,&vmstate_spice_kbd, kbd); > > > > pointer = g_malloc0(sizeof(*pointer)); > > pointer->mouse.base.sif =&mouse_interface.base; >
Hi, > There is no ledstate in a PS/2 keyboard (or I'm reading too much into > the implementation in qemu). There is. It isn't explicitly stored into the state struct though because the ps/2 keyboard itself doesn't use it, it just calls kbd_put_ledstate() to inform others about it. cheers, Gerd
diff --git a/trace-events b/trace-events index 63d8c8e..605dc79 100644 --- a/trace-events +++ b/trace-events @@ -538,3 +538,6 @@ esp_mem_writeb_cmd_ensel(uint32_t val) "Enable selection (%2.2x)" # monitor.c handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\"" monitor_protocol_emitter(void *mon) "mon %p" + +# ui/spice-input.c +disable spice_input_post_load(int ledstate) "leds %d" diff --git a/ui/spice-input.c b/ui/spice-input.c index af4223d..2f0d3af 100644 --- a/ui/spice-input.c +++ b/ui/spice-input.c @@ -26,6 +26,8 @@ #include "qemu-common.h" #include "qemu-spice.h" #include "console.h" +#include "hw/hw.h" +#include "trace.h" /* keyboard bits */ @@ -195,6 +197,27 @@ static void mouse_mode_notifier(Notifier *notifier, void *data) pointer->absolute = is_absolute; } +static int qemu_spice_input_post_load(void *opaque, int version_id) +{ + QemuSpiceKbd *s = (QemuSpiceKbd *)opaque; + + trace_spice_input_post_load(s->ledstate); + kbd_leds(s, s->ledstate); + return 0; +} + +static const VMStateDescription vmstate_spice_kbd = { + .name = "spice-input", + .version_id = 0, + .minimum_version_id = 0, + .minimum_version_id_old = 0, + .post_load = qemu_spice_input_post_load, + .fields = (VMStateField[]) { + VMSTATE_INT32(ledstate, QemuSpiceKbd), + VMSTATE_END_OF_LIST() + } +}; + void qemu_spice_input_init(void) { QemuSpiceKbd *kbd; @@ -204,6 +227,7 @@ void qemu_spice_input_init(void) kbd->sin.base.sif = &kbd_interface.base; qemu_spice_add_interface(&kbd->sin.base); qemu_add_led_event_handler(kbd_leds, kbd); + vmstate_register(NULL, 0, &vmstate_spice_kbd, kbd); pointer = g_malloc0(sizeof(*pointer)); pointer->mouse.base.sif = &mouse_interface.base;
The caps/scroll/num lock state is not tracked by the ps2 device, but by spice-input QemuSpiceKbd->ledstate. To fix losing it across migration, and then having the server send caps/scroll/nums keys when the client sends a SPICE_MSGC_INPUTS_KEY_MODIFIERS, migrate it. RHBZ# 729294 Signed-off-by: Alon Levy <alevy@redhat.com> --- trace-events | 3 +++ ui/spice-input.c | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 0 deletions(-)