[Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Amos Kong]

(qemu) sendkey a 1000

Current design is that qemu only send one down event to guest,
and delay sometime, then send one up event. In this case, only
key can be identified by guest.

This patch changed qemu to intervally send down events to guest
in the hold time, the interval is 100ms.

(qemu) sendkey a 1000

qemu will send 9 down events, 1 up event to guest, we can see
9 'a' in guest screen.

Signed-off-by: Amos Kong <akong@redhat.com>
---
This patch based on Luiz's qmp-unstable/queue/qmp

Signed-off-by: Amos Kong <akong@redhat.com>
---
 hmp-commands.hx |    4 +++-
 qmp-commands.hx |    3 ++-
 ui/input.c      |   38 ++++++++++++++++++++++++++------------
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index df44906..a16961e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -557,7 +557,9 @@ STEXI
 
 Send @var{keys} to the guest. @var{keys} could be the name of the
 key or the raw value in hexadecimal format. Use @code{-} to press
-several keys simultaneously. Example:
+several keys simultaneously. The default hold time is 100, in the
+hold time, qemu will intervally send down events to guest, the
+interval is 100ms. Example:
 @example
 sendkey ctrl-alt-f1
 @end example
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4d65422..081f736 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -348,7 +348,8 @@ Arguments:
 keys array:
     - "key": key sequence (a json-array of key enum values)
 
-- hold-time: time to delay key up events, milliseconds. Defaults to 100
+- hold-time: time to intervally send down events to guest, the interval
+             is 100ms. Defaults to 100 milliseconds
              (json-int, optional)
 
 Example:
diff --git a/ui/input.c b/ui/input.c
index ecfeb43..143c421 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -214,6 +214,7 @@ int index_from_keycode(int code)
 static int *keycodes;
 static int keycodes_size;
 static QEMUTimer *key_timer;
+static int rest_time;
 
 static int keycode_from_keyvalue(const KeyValue *value)
 {
@@ -232,8 +233,27 @@ static void free_keycodes(void)
     keycodes_size = 0;
 }
 
-static void release_keys(void *opaque)
+static void enter_keys(void *opaque)
 {
+    int i, time;
+
+    /* key down events */
+    for (i = 0; i < keycodes_size; i++) {
+        if (keycodes[i] & 0x80) {
+            kbd_put_keycode(0xe0);
+        }
+        kbd_put_keycode(keycodes[i] & 0x7f);
+    }
+
+    rest_time -= 100;
+    if (rest_time > 0) {
+        time = rest_time > 100 ? 100 : rest_time;
+        qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) +
+                       muldiv64(get_ticks_per_sec(), time, 1000));
+        return;
+    }
+
+    /* key up events */
     while (keycodes_size > 0) {
         if (keycodes[--keycodes_size] & 0x80) {
             kbd_put_keycode(0xe0);
@@ -251,20 +271,21 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time,
     KeyValueList *p;
 
     if (!key_timer) {
-        key_timer = qemu_new_timer_ns(vm_clock, release_keys, NULL);
+        key_timer = qemu_new_timer_ns(vm_clock, enter_keys, NULL);
     }
 
     if (keycodes != NULL) {
         qemu_del_timer(key_timer);
-        release_keys(NULL);
+        enter_keys(NULL);
     }
 
     if (!has_hold_time) {
         hold_time = 100;
     }
 
+    rest_time = hold_time;
+
     for (p = keys; p != NULL; p = p->next) {
-        /* key down events */
         keycode = keycode_from_keyvalue(p->value);
         if (keycode < 0x01 || keycode > 0xff) {
             error_setg(errp, "invalid hex keycode 0x%x", keycode);
@@ -272,18 +293,11 @@ void qmp_send_key(KeyValueList *keys, bool has_hold_time, int64_t hold_time,
             return;
         }
 
-        if (keycode & 0x80) {
-            kbd_put_keycode(0xe0);
-        }
-        kbd_put_keycode(keycode & 0x7f);
-
         keycodes = g_realloc(keycodes, sizeof(int) * (keycodes_size + 1));
         keycodes[keycodes_size++] = keycode;
     }
 
-    /* delayed key up events */
-    qemu_mod_timer(key_timer, qemu_get_clock_ns(vm_clock) +
-                   muldiv64(get_ticks_per_sec(), hold_time, 1000));
+    enter_keys(NULL);
 }
 
 void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
-- 
1.7.1

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

On 04/18/2013 10:44 PM, Amos Kong wrote:
> (qemu) sendkey a 1000
> 
> Current design is that qemu only send one down event to guest,
> and delay sometime, then send one up event. In this case, only
> key can be identified by guest.
> 
> This patch changed qemu to intervally send down events to guest
> in the hold time, the interval is 100ms.

I don't like this.  When you hold a key for a long time on bare metal,
there is only one down and one up event; if the console displays
multiple copies of the character being typed, it is because the console
does the repeats itself.  If the user wants multiple down and up events,
they should send multiple events, not rely on one command to send
multiple presses.

> 
> (qemu) sendkey a 1000
> 
> qemu will send 9 down events, 1 up event to guest, we can see
> 9 'a' in guest screen.


I'm inclined to NACK this unless you can give better explanation why
send-key should behave differently than bare metal.  If anything, the
behavior being complained about is a "feature" of the console of the
guest being tested, not something where we should change how the
hardware behaves.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Amos Kong]

On Sat, Apr 20, 2013 at 10:06:28AM -0600, Eric Blake wrote:
> On 04/18/2013 10:44 PM, Amos Kong wrote:
> > (qemu) sendkey a 1000
> > 
> > Current design is that qemu only send one down event to guest,
> > and delay sometime, then send one up event. In this case, only
> > key can be identified by guest.
> > 
> > This patch changed qemu to intervally send down events to guest
> > in the hold time, the interval is 100ms.
> 
> I don't like this.

> When you hold a key for a long time on bare metal,
> there is only one down and one up event;

Really? I do check events by 'showkey', the output of showkey is not the
events sent from keyboard?

# showkey -s (show keys' scancode)
I can always see many down scancodes, and one up scancode.
It's same when I disable / enable auto-repeat mode in system.

In the real host / vnc guest/ sdl guest, hold one key, many down
events can be checked by showkey.


http://msdn.microsoft.com/en-us/library/windows/desktop/gg153546(v=vs.85).aspx
"""
If you hold down a key long enough to start the keyboard's repeat
feature, the system sends multiple key-down messages, followed by a
single key-up message. """

key-down messages == key-down events?

> if the console displays
> multiple copies of the character being typed, it is because the console
> does the repeats itself.

> If the user wants multiple down and up events,
> they should send multiple events, not rely on one command to send
> multiple presses.
 

                Amos.

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Amos Kong]

On Mon, Apr 22, 2013 at 03:32:52PM +0800, Amos Kong wrote:
> On Sat, Apr 20, 2013 at 10:06:28AM -0600, Eric Blake wrote:
> > On 04/18/2013 10:44 PM, Amos Kong wrote:
> > > (qemu) sendkey a 1000
> > > 
> > > Current design is that qemu only send one down event to guest,
> > > and delay sometime, then send one up event. In this case, only
> > > key can be identified by guest.
> > > 
> > > This patch changed qemu to intervally send down events to guest
> > > in the hold time, the interval is 100ms.
> > 
> > I don't like this.
> 
> > When you hold a key for a long time on bare metal,
> > there is only one down and one up event;
> 
> Really? I do check events by 'showkey', the output of showkey is not the
> events sent from keyboard?
> 
> # showkey -s (show keys' scancode)
> I can always see many down scancodes, and one up scancode.
> It's same when I disable / enable auto-repeat mode in system.
> 
> In the real host / vnc guest/ sdl guest, hold one key, many down
> events can be checked by showkey.
 
# watch cat /proc/interrupts
          CPU0       CPU1       CPU2       CPU3
 1:       1692      40309       1462       1795   IO-APIC-edge  i8042

hit a botton without long-time holding, interrupt count increased 2.
hit a botton with long-time holding, interrupt count increased a lot (more than 2)


http://www.oocities.org/timessquare/2795/Files/keyboard.txt
"""
 If someone had just pressed the ESC key for instance, the port will
 show a value of 1 (1 is the ESC key's scan code). If they hold their finger
 on the button the keyboard will _keep_ generating interrupt 9's and each
 time the port will still show a value of 1. When the person releases the key
 a final interrupt will be generated and the port will return 129 (1 + 128,
 since the high bit will be set indicating the person has released the key).
"""

Hi Eric, it seems my change is same as real metal, please tell me if something
is wrong, Thanks.

 
> http://msdn.microsoft.com/en-us/library/windows/desktop/gg153546(v=vs.85).aspx
> """
> If you hold down a key long enough to start the keyboard's repeat
> feature, the system sends multiple key-down messages, followed by a
> single key-up message. """
> 
> key-down messages == key-down events?

-- 
			Amos.

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Paolo Bonzini]

Il 22/04/2013 10:09, Amos Kong ha scritto:
> On Mon, Apr 22, 2013 at 03:32:52PM +0800, Amos Kong wrote:
>> On Sat, Apr 20, 2013 at 10:06:28AM -0600, Eric Blake wrote:
>>> On 04/18/2013 10:44 PM, Amos Kong wrote:
>>>> (qemu) sendkey a 1000
>>>>
>>>> Current design is that qemu only send one down event to guest,
>>>> and delay sometime, then send one up event. In this case, only
>>>> key can be identified by guest.
>>>>
>>>> This patch changed qemu to intervally send down events to guest
>>>> in the hold time, the interval is 100ms.
>>>
>>> I don't like this.
>>
>>> When you hold a key for a long time on bare metal,
>>> there is only one down and one up event;
>>
>> Really? I do check events by 'showkey', the output of showkey is not the
>> events sent from keyboard?
>>
>> # showkey -s (show keys' scancode)
>> I can always see many down scancodes, and one up scancode.
>> It's same when I disable / enable auto-repeat mode in system.
>>
>> In the real host / vnc guest/ sdl guest, hold one key, many down
>> events can be checked by showkey.
>  
> # watch cat /proc/interrupts
>           CPU0       CPU1       CPU2       CPU3
>  1:       1692      40309       1462       1795   IO-APIC-edge  i8042
> 
> hit a botton without long-time holding, interrupt count increased 2.
> hit a botton with long-time holding, interrupt count increased a lot (more than 2)

You're right.  The typematic delay/rate is implemented within the i8042
keyboard microcontroller (QEMU does not implement that register).

It is possible that software ignores interrupts for a key that is
already down, and reimplements autorepeat in software, but your patch is
correct.

Paolo

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Luiz Capitulino]

On Mon, 22 Apr 2013 11:33:20 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 22/04/2013 10:09, Amos Kong ha scritto:
> > On Mon, Apr 22, 2013 at 03:32:52PM +0800, Amos Kong wrote:
> >> On Sat, Apr 20, 2013 at 10:06:28AM -0600, Eric Blake wrote:
> >>> On 04/18/2013 10:44 PM, Amos Kong wrote:
> >>>> (qemu) sendkey a 1000
> >>>>
> >>>> Current design is that qemu only send one down event to guest,
> >>>> and delay sometime, then send one up event. In this case, only
> >>>> key can be identified by guest.
> >>>>
> >>>> This patch changed qemu to intervally send down events to guest
> >>>> in the hold time, the interval is 100ms.
> >>>
> >>> I don't like this.
> >>
> >>> When you hold a key for a long time on bare metal,
> >>> there is only one down and one up event;
> >>
> >> Really? I do check events by 'showkey', the output of showkey is not the
> >> events sent from keyboard?
> >>
> >> # showkey -s (show keys' scancode)
> >> I can always see many down scancodes, and one up scancode.
> >> It's same when I disable / enable auto-repeat mode in system.
> >>
> >> In the real host / vnc guest/ sdl guest, hold one key, many down
> >> events can be checked by showkey.
> >  
> > # watch cat /proc/interrupts
> >           CPU0       CPU1       CPU2       CPU3
> >  1:       1692      40309       1462       1795   IO-APIC-edge  i8042
> > 
> > hit a botton without long-time holding, interrupt count increased 2.
> > hit a botton with long-time holding, interrupt count increased a lot (more than 2)
> 
> You're right.  The typematic delay/rate is implemented within the i8042
> keyboard microcontroller (QEMU does not implement that register).
> 
> It is possible that software ignores interrupts for a key that is
> already down, and reimplements autorepeat in software, but your patch is
> correct.

But isn't this patch the equivalent of repeatedly pressing and releasing a
key? Shouldn't this be implemented at a lower-level layer like the input
subsystem?

Say, the input subsystem detects a key is being hold and asks the keyboard
emulation driver to keep sending interrupts for that key like Amos described?

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Paolo Bonzini]

Il 22/04/2013 14:43, Luiz Capitulino ha scritto:
>> > 
>> > You're right.  The typematic delay/rate is implemented within the i8042
>> > keyboard microcontroller (QEMU does not implement that register).
>> > 
>> > It is possible that software ignores interrupts for a key that is
>> > already down, and reimplements autorepeat in software, but your patch is
>> > correct.
> But isn't this patch the equivalent of repeatedly pressing and releasing a
> key? Shouldn't this be implemented at a lower-level layer like the input
> subsystem?

No, this patch is implementing what the microcontroller does, i.e. 10
presses + 1 release.  I'm not sure it is the right level to do it (the
rate/delay should be at least customizable from the board), but the
logic is right and if someone else needs more configurability we can add
it later.

Paolo

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Gerd Hoffmann]

  Hi,

>> But isn't this patch the equivalent of repeatedly pressing and releasing a
>> key? Shouldn't this be implemented at a lower-level layer like the input
>> subsystem?

ps/2 keyboard emulation would probably the place to do it.

I'm pretty sure not all keyboard types have auto-repeat.  The linux
kernel input layer has code to generate the auto-repeat kbd events in
software, and that code is there for a reason I guess ...

> No, this patch is implementing what the microcontroller does, i.e. 10
> presses + 1 release.  I'm not sure it is the right level to do it (the
> rate/delay should be at least customizable from the board), but the
> logic is right and if someone else needs more configurability we can add
> it later.

IIRC the (ps/2) kbd controller can be programmed with rate+delay.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Paolo Bonzini]

Il 22/04/2013 15:35, Gerd Hoffmann ha scritto:
>   Hi,
> 
>>> But isn't this patch the equivalent of repeatedly pressing and releasing a
>>> key? Shouldn't this be implemented at a lower-level layer like the input
>>> subsystem?
> 
> ps/2 keyboard emulation would probably the place to do it.

Yes, if PS/2 keyboard emulation emulated the autorepeat rate/delay, then
the code we have in QMP would just work.  However it would need to be
done for all devices (ignoring repeated keydown events from the upper
layers, and creating its own repeated event).  So it makes sense to have
it in common code and have keyboard devices just tell common code the
desired rate/delay.

BTW, how do we currently handle stuck keys across migration (where the
key-up event never reaches the guest because the key was never pressed
in the first place on the destination)?

> I'm pretty sure not all keyboard types have auto-repeat.  The linux
> kernel input layer has code to generate the auto-repeat kbd events in
> software, and that code is there for a reason I guess ...

Yes.  Or simply it is easier to put it there than in all keyboard
drivers.  The USB keyboard for example has a hybrid hardware/software
autorepeat, where the OS must pass the delay to the next key after each
keypress.

>> No, this patch is implementing what the microcontroller does, i.e. 10
>> presses + 1 release.  I'm not sure it is the right level to do it (the
>> rate/delay should be at least customizable from the board), but the
>> logic is right and if someone else needs more configurability we can add
>> it later.
> 
> IIRC the (ps/2) kbd controller can be programmed with rate+delay.

Yes, but we ignore the command.  For the PS/2 keyboard, I think what we
send now to the guest is based on the rate/delay that is emulated in
software by the GUI layers (for Unix it should just be X11 for all of
SDL/VNC/Spice).

With a FreeDOS image it is easy to test, you can see that the USB
keyboard has a longer delay than the text console.  The PS/2 keyboard
instead has roughly the same delay.

Paolo

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Gerd Hoffmann]

  Hi,

> Yes, if PS/2 keyboard emulation emulated the autorepeat rate/delay, then
> the code we have in QMP would just work.  However it would need to be
> done for all devices (ignoring repeated keydown events from the upper
> layers, and creating its own repeated event).  So it makes sense to have
> it in common code and have keyboard devices just tell common code the
> desired rate/delay.

Yep, that'll work too.

> BTW, how do we currently handle stuck keys across migration (where the
> key-up event never reaches the guest because the key was never pressed
> in the first place on the destination)?

We don't.

>> IIRC the (ps/2) kbd controller can be programmed with rate+delay.
> 
> Yes, but we ignore the command.  For the PS/2 keyboard, I think what we
> send now to the guest is based on the rate/delay that is emulated in
> software by the GUI layers (for Unix it should just be X11 for all of
> SDL/VNC/Spice).

Exactly.  Thats why keys getting stuck on migration isn't a big issue in
practice.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Paolo Bonzini]

Il 22/04/2013 17:20, Gerd Hoffmann ha scritto:
>> > Yes, if PS/2 keyboard emulation emulated the autorepeat rate/delay, then
>> > the code we have in QMP would just work.  However it would need to be
>> > done for all devices (ignoring repeated keydown events from the upper
>> > layers, and creating its own repeated event).  So it makes sense to have
>> > it in common code and have keyboard devices just tell common code the
>> > desired rate/delay.
> Yep, that'll work too.

Ok, in that sense Amos's patch is not too bad.  The problems are that it
hardcodes 100/100 as the repeat/delay, and that the autorepeat is only
done for the send-key command.  If the first was replaced with at least
two #defines it would be acceptable IMO.

>>> >> IIRC the (ps/2) kbd controller can be programmed with rate+delay.
>> > 
>> > Yes, but we ignore the command.  For the PS/2 keyboard, I think what we
>> > send now to the guest is based on the rate/delay that is emulated in
>> > software by the GUI layers (for Unix it should just be X11 for all of
>> > SDL/VNC/Spice).
> Exactly.  Thats why keys getting stuck on migration isn't a big issue in
> practice.

Hmm, but if you press "a" and migrate, the receiver will see "key down
a" and no "key up a".  Software autorepeat will then generate an endless
stream of a's...

It isn't a big issue because it's just very unlikely to happen, or
perhaps because no one plays games on VDI during migration.

Paolo

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 22/04/2013 14:43, Luiz Capitulino ha scritto:
>>> > 
>>> > You're right.  The typematic delay/rate is implemented within the i8042
>>> > keyboard microcontroller (QEMU does not implement that register).
>>> > 
>>> > It is possible that software ignores interrupts for a key that is
>>> > already down, and reimplements autorepeat in software, but your patch is
>>> > correct.
>> But isn't this patch the equivalent of repeatedly pressing and releasing a
>> key? Shouldn't this be implemented at a lower-level layer like the input
>> subsystem?
>
> No, this patch is implementing what the microcontroller does, i.e. 10
> presses + 1 release.  I'm not sure it is the right level to do it (the
> rate/delay should be at least customizable from the board), but the
> logic is right and if someone else needs more configurability we can add
> it later.

Regardless, this is a compat breaker IMHO.  This is a dramatically
different semantic behavior.

What's the use-case here?

Regards,

Anthony Liguori

>
> Paolo

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Luiz Capitulino]

On Mon, 22 Apr 2013 09:02:41 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 22/04/2013 14:43, Luiz Capitulino ha scritto:
> >>> > 
> >>> > You're right.  The typematic delay/rate is implemented within the i8042
> >>> > keyboard microcontroller (QEMU does not implement that register).
> >>> > 
> >>> > It is possible that software ignores interrupts for a key that is
> >>> > already down, and reimplements autorepeat in software, but your patch is
> >>> > correct.
> >> But isn't this patch the equivalent of repeatedly pressing and releasing a
> >> key? Shouldn't this be implemented at a lower-level layer like the input
> >> subsystem?
> >
> > No, this patch is implementing what the microcontroller does, i.e. 10
> > presses + 1 release.  I'm not sure it is the right level to do it (the
> > rate/delay should be at least customizable from the board), but the
> > logic is right and if someone else needs more configurability we can add
> > it later.
> 
> Regardless, this is a compat breaker IMHO.  This is a dramatically
> different semantic behavior.
> 
> What's the use-case here?

This was reported by Zhenfeng Wang, my impression is that Zhenfeng had the
(resonable) expectation that this would work like in bare metal, but I don't
think there's a specific use-case behind here.

Am I correct Zhenfeng?

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Amos Kong]

On Mon, Apr 22, 2013 at 09:02:41AM -0500, Anthony Liguori wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 22/04/2013 14:43, Luiz Capitulino ha scritto:
> >>> > 
> >>> > You're right.  The typematic delay/rate is implemented within the i8042
> >>> > keyboard microcontroller (QEMU does not implement that register).
> >>> > 
> >>> > It is possible that software ignores interrupts for a key that is
> >>> > already down, and reimplements autorepeat in software, but your patch is
> >>> > correct.
> >> But isn't this patch the equivalent of repeatedly pressing and releasing a
> >> key? Shouldn't this be implemented at a lower-level layer like the input
> >> subsystem?
> >
> > No, this patch is implementing what the microcontroller does, i.e. 10
> > presses + 1 release.  I'm not sure it is the right level to do it (the
> > rate/delay should be at least customizable from the board), but the
> > logic is right and if someone else needs more configurability we can add
> > it later.
> 
> Regardless, this is a compat breaker IMHO.  This is a dramatically
> different semantic behavior.
>
> What's the use-case here?

I don't if we have some new use-cases now.

Original case:
| commit c8256f9d23bba4fac3b0b6a9e6e3dc12362cbe0b
| Author: balrog <balrog@c046a42c-6fe2-441c-8c8c-71466251a162>
| Date:   Sun Jun 8 22:45:01 2008 +0000
| 
|     Enhance sendkey with key hold time (Jan Kiszka).
|     
|     Current key injection via the monitor basically generates no key hold
|     time. This is fine for keyboard emulations that have their own queues,
|     but it causes troubles for those how don't (like the MusicPal - it
|     simply does not work with injected keys). Moreover, I would like to use
|     this mechanism to simulate pressed buttons during power-up.
|     
|     Therefore, this patch enhances the key injection with a configurable
|     release delay (by default 100 virtual milliseconds).

Original hold only casused the release delay, it didn't mention the
auto-repeate.

|     This feature allows to get rid of the initial sleep() in musicpal_init
|     because one can now simply start qemu with -S and issue "sendkey m 1000"
|     and "continue" in the monitor to achieve the desired effect of a pressed
|     menu button during power-up. So there is no need for a per-musicpal or
|     even qemu-wide "-hold-button" switch.

Before my patch, I started a guest with '-boot menu=on -S' option, executed
(qemu) sendkey f12 2000
(qemu) cont
Boot menu can't be enabled.

Applied my patch, do the same test, Boot menu can be enabled.
     
|     Signed-off-by: Jan Kiszka <jan.kiszka@web.de>

-- 
			Amos.

[Qemu-devel] [PATCH] ui/input.c: replace magic numbers by macros

[Amos Kong]

Signed-off-by: Amos Kong <akong@redhat.com>
---
It's based on http://lists.nongnu.org/archive/html/qemu-devel/2013-04/msg03822.html
---
 ui/input.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/ui/input.c b/ui/input.c
index 143c421..fac5d6d 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -28,6 +28,7 @@
 #include "qapi/error.h"
 #include "qmp-commands.h"
 #include "qapi-types.h"
+#include "ui/keymaps.h"
 
 static QEMUPutKBDEvent *qemu_put_kbd_event;
 static void *qemu_put_kbd_event_opaque;
@@ -239,10 +240,10 @@ static void enter_keys(void *opaque)
 
     /* key down events */
     for (i = 0; i < keycodes_size; i++) {
-        if (keycodes[i] & 0x80) {
-            kbd_put_keycode(0xe0);
+        if (keycodes[i] & SCANCODE_GREY) {
+            kbd_put_keycode(SCANCODE_EMUL0);
         }
-        kbd_put_keycode(keycodes[i] & 0x7f);
+        kbd_put_keycode(keycodes[i] & SCANCODE_KEYCODEMASK);
     }
 
     rest_time -= 100;
@@ -255,10 +256,10 @@ static void enter_keys(void *opaque)
 
     /* key up events */
     while (keycodes_size > 0) {
-        if (keycodes[--keycodes_size] & 0x80) {
-            kbd_put_keycode(0xe0);
+        if (keycodes[--keycodes_size] & SCANCODE_GREY) {
+            kbd_put_keycode(SCANCODE_EMUL0);
         }
-        kbd_put_keycode(keycodes[keycodes_size] | 0x80);
+        kbd_put_keycode(keycodes[keycodes_size] | SCANCODE_UP);
     }
 
     free_keycodes();
-- 
1.7.1

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2308 bytes --]

On 04/22/2013 01:32 AM, Amos Kong wrote:
> On Sat, Apr 20, 2013 at 10:06:28AM -0600, Eric Blake wrote:
>> On 04/18/2013 10:44 PM, Amos Kong wrote:
>>> (qemu) sendkey a 1000
>>>
>>> Current design is that qemu only send one down event to guest,
>>> and delay sometime, then send one up event. In this case, only
>>> key can be identified by guest.
>>>
>>> This patch changed qemu to intervally send down events to guest
>>> in the hold time, the interval is 100ms.
>>
>> I don't like this.
> 
>> When you hold a key for a long time on bare metal,
>> there is only one down and one up event;
> 
> Really? I do check events by 'showkey', the output of showkey is not the
> events sent from keyboard?

I didn't know that there is some bare metal hardware that sends more
than one event.  As long as you are matching bare metal behavior, then I
will be happy; but I still wonder if hard-coding repeat rates instead of
making it configurable is the best choice.

> 
> # showkey -s (show keys' scancode)
> I can always see many down scancodes, and one up scancode.
> It's same when I disable / enable auto-repeat mode in system.
> 
> In the real host / vnc guest/ sdl guest, hold one key, many down
> events can be checked by showkey.

Nice tool that I had not used before.  Thanks for pointing it out to me.

> 
> 
> http://msdn.microsoft.com/en-us/library/windows/desktop/gg153546(v=vs.85).aspx
> """
> If you hold down a key long enough to start the keyboard's repeat
> feature, the system sends multiple key-down messages, followed by a
> single key-up message. """
> 
> key-down messages == key-down events?
> 
>> if the console displays
>> multiple copies of the character being typed, it is because the console
>> does the repeats itself.
> 
>> If the user wants multiple down and up events,
>> they should send multiple events, not rely on one command to send
>> multiple presses.

So now that I have more information, I'm okay with sending multiple down
events and one up event for a long hold time (if we are emulating
hardware that does the same), and still making the user call send-key
multiple times if they want multiple up events.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Laszlo Ersek]

Hi,

On 04/19/13 06:44, Amos Kong wrote:
> (qemu) sendkey a 1000
> 
> Current design is that qemu only send one down event to guest,
> and delay sometime, then send one up event. In this case, only
> key can be identified by guest.
> 
> This patch changed qemu to intervally send down events to guest
> in the hold time, the interval is 100ms.
> 
> (qemu) sendkey a 1000
> 
> qemu will send 9 down events, 1 up event to guest, we can see
> 9 'a' in guest screen.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> This patch based on Luiz's qmp-unstable/queue/qmp
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  hmp-commands.hx |    4 +++-
>  qmp-commands.hx |    3 ++-
>  ui/input.c      |   38 ++++++++++++++++++++++++++------------
>  3 files changed, 31 insertions(+), 14 deletions(-)

What's the status of this patch if I may ask?

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Anthony Liguori]

Laszlo Ersek <lersek@redhat.com> writes:

> Hi,
>
> On 04/19/13 06:44, Amos Kong wrote:
>> (qemu) sendkey a 1000
>> 
>> Current design is that qemu only send one down event to guest,
>> and delay sometime, then send one up event. In this case, only
>> key can be identified by guest.
>> 
>> This patch changed qemu to intervally send down events to guest
>> in the hold time, the interval is 100ms.
>> 
>> (qemu) sendkey a 1000
>> 
>> qemu will send 9 down events, 1 up event to guest, we can see
>> 9 'a' in guest screen.
>> 
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> ---
>> This patch based on Luiz's qmp-unstable/queue/qmp
>> 
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> ---
>>  hmp-commands.hx |    4 +++-
>>  qmp-commands.hx |    3 ++-
>>  ui/input.c      |   38 ++++++++++++++++++++++++++------------
>>  3 files changed, 31 insertions(+), 14 deletions(-)
>
> What's the status of this patch if I may ask?

1) It's unclear if this is the right solution.  If key repeat is done in
the PS/2 controller, then that's where the logic here should be.

2) It's a compat breaker from a QMP perspective.

Regards,

Anthony Liguori

>
> Thanks,
> Laszlo

Re: [Qemu-devel] [PATCH] monitor: intervally send down events to guest in hold time

[Amos Kong]

On Tue, May 14, 2013 at 09:55:26AM -0500, Anthony Liguori wrote:
> Laszlo Ersek <lersek@redhat.com> writes:

<snip>

> > What's the status of this patch if I may ask?
> 
> 1) It's unclear if this is the right solution.  If key repeat is done in
> the PS/2 controller, then that's where the logic here should be.

It's best the implement auto-repeat feature in PS2 controller.
I'm investigating to implement this, bug was reported by our QE,
not from real user. So the priority is low.
 
> 2) It's a compat breaker from a QMP perspective.
>
> Regards,
> 
> Anthony Liguori

-- 
			Amos.