Re: [Qemu-devel] [PATCH v2 1/2] ps2: add support of auto-repeat

[Amos Kong <akong@redhat.com>]

On Thu, May 30, 2013 at 11:48:46AM -0500, Anthony Liguori wrote:
> Amos Kong <akong@redhat.com> writes:
> 
> > Guest driver sets repeat rate and delay time by KBD_CMD_SET_RATE,
> > but ps2 backend doesn't process it and no auto-repeat implementation.
> > This patch adds support of auto-repeat feature. The repeated events
> > from host are ignored and re-implements ps2's auto-repeat.
> >
> > Guest ps2 driver sets autorepeat to fastest possible in reset,
> > period: 250ms, delay: 33ms
> >
> > Tested by 'sendkey' monitor command.
> > Tested by Linux & Windows guests with SDL, VNC, SPICE, GTK+
> >
> > referenced: http://www.computer-engineering.org/ps2keyboard/
> >
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  hw/input/ps2.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >
> > diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> > index 3412079..8adbb4a 100644
> > --- a/hw/input/ps2.c
> > +++ b/hw/input/ps2.c
> > @@ -94,6 +94,10 @@ typedef struct {
> >      int translate;
> >      int scancode_set; /* 1=XT, 2=AT, 3=PS/2 */
> >      int ledstate;
> > +    int repeat_period; /* typematic period, ms */
> > +    int repeat_delay; /* typematic delay, ms */
> > +    int repeat_key; /* keycode to repeat */
> > +    QEMUTimer *repeat_timer;
> 
> This state needs to be migrated, no?  I suspect it can/should be done
> via a subsection too.

It sounds only reasonable for 'sendkey' command. We want to repeat one
key for 100 times, the key should be continaully repeated in the dest
vm until it reaches to 100 times.

For implement this, we should also migrate key_timer in ui/input.c,
then it will send a release event to ps2 queue when the key_timer
is expired. The bottom patch migrates repeat_timer & repeat_key,
where should we save key_timer for migration?

----

For the VNC/SPICE/SDL/GTK+ windows, qemu gets repeated press events
from host. After vm migrates to dest host, if we don't touch the
keyboard, there will be no repeat / release event comes from host,

1) continue to repeat? but qemu no long gets press event from host
2) stop to repeat? but qemu has not got the release event, auto-repeat
should continue in real keyboard in this condition.

I prefect 2), because we need to emulate a release event in the
prepare stage of migrate.

For implement 2), we should not load/restart the repeat_timer if
the migrated key_timer is already expired.

---

Or just migrate the repeat_rate/repeat_period. We don't have
real request of auto-repeat in libvirt.

This might be the best solution as Paolo said ;)
http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg02207.html



diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index cdb18e6..fdb9912 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -615,7 +615,17 @@ static bool ps2_keyboard_repeatstate_needed(void
*opaque)
 {
     PS2KbdState *s = opaque;
 
-    return s->repeat_period || s->repeat_delay;
+    return s->repeat_period || s->repeat_delay || s->repeat_key ||
s->repeat_timer;
+}
+
+static int ps2_kbd_repeatstate_load(QEMUFile *f, void *opaque, int
version_id)
+{
+    PS2KbdState *s = opaque;
+    qemu_get_timer(f, s->repeat_timer);
+    qemu_mod_timer(s->repeat_timer, qemu_get_clock_ns(vm_clock) +
+                   muldiv64(get_ticks_per_sec(), s->repeat_period,
1000));
+
+    return 0;
 }
 
 static bool ps2_keyboard_ledstate_needed(void *opaque)
@@ -638,9 +648,12 @@ static const VMStateDescription
vmstate_ps2_keyboard_repeatstate = {
     .version_id = 3,
     .minimum_version_id = 2,
     .minimum_version_id_old = 2,
+    .load_state_old = ps2_kbd_repeatstate_load,
     .fields      = (VMStateField[]) {
         VMSTATE_INT32(repeat_period, PS2KbdState),
         VMSTATE_INT32(repeat_delay, PS2KbdState),
+        VMSTATE_INT32(repeat_key, PS2KbdState),
+        VMSTATE_TIMER(repeat_timer, PS2KbdState),
         VMSTATE_END_OF_LIST()
     }
 };

-- 
			Amos.