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

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> On Fri, 14 Jun 2013 13:46:41 +0800
> Amos Kong <address@hidden> wrote:
>
>> On Fri, May 31, 2013 at 08:31:17PM +0800, Amos Kong wrote:
>> > On Thu, May 30, 2013 at 11:48:46AM -0500, Anthony Liguori wrote:
>> > > Amos Kong <address@hidden> writes:
>> 
>> 
>> > > > 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?
>> 
>> Luiz, any suggestion about migrate the key_timer in ui/input.c?
>
> I don't have any. Maybe Markus or Juan can help (CC'ed).
>
>> 
>> We need to migrate it, then sendkey can continually work in dest vm
>> until the timer is expired.

I read the thread, but I'm not sure I have enough context for a sensible
answer.  Let me try to piece it together.

On a real PC keyboard, key down generates that key's make code, key up
generates the key's break code.  If the key is typematic, the make code
repeats while the key is down.  First repeat is after a programmable
delay, subsequent repeats happen at a programmable rate.

Which keys are typematic is programmable in scan code set 3, but we
don't implement the commands to do so.  Oh well, the world is full of
crappy clone keyboards, this is just one more.

What problem are you trying to solve?  Your cover letter mentions the
sendkey command.  Takes an array of keys and an optional hold-time,
defaulting to 100ms.

Aside: array of keys + a single hold time is a rotten design.  Outside
the scope of this patch.

100ms is well below the smallest typematic delay, so by default no
repeat happens.  But if you specify a sufficiently large hold-time, it
arguably should.  Is that the problem you're trying to fix?

Why is this problem worth fixing?

Does your patch affect anything but the sendkey command?  I'm asking
because I'm not at all sure injecting emulated repeat between the user's
keyboard and the guest OS is a good idea.  I'd expect the user's
keyboard to repeat according to the user's wishes already, and I'm
concerned a second repeat could mess up things.