On 18/06/12 23:30, Amos Kong wrote:
On 06/15/2012 09:35 PM, Luiz Capitulino wrote:
On Fri, 15 Jun 2012 09:57:49 +0200
Gerd Hoffmann<kra...@redhat.com> wrote:
Hi,
It seems we need to notice user when inputted keys are more than 16.
Hi Gerd,
When I use 'sendkey' command to send key-series to guest, some keyboard
events will be send. There is a limitation (16) that was introduced by this
old commit c8256f9d (without description). Do you know the reason?
Probably hardware limitation, ps/2 keyboards can buffer up to 16 keys IIRC.
Then the perfect thing to do would be to drop the MAX_KEYCODES check from
the sendkey command and move bounds checking down to the device emulation code.
However, this will require a bit of code churn if we do it for all devices,
and won't buy us much, as the most likely reason for the error is a client/user
trying to send too many keys in parallel to the guest, right?
Agree, we can notice in stderr when the redundant keys are ignored as hid.
#define QUEUE_LENGTH 16 /* should be enough for a triple-click */
static void hid_keyboard_event(void *opaque, int keycode)
{
...
if (hs->n == QUEUE_LENGTH) {
fprintf(stderr, "usb-kbd: warning: key event queue full\n");
return;
}
I dropped the limitation in sendkey command,
and didn't change current ps2.c, executed some
tests in different environments.
environment max inputted key number
------------ -----------------------
win7 notepad 100
rhel6 grub 15
rhel6 pxe 15
rhel6 login window 10
rhel6 vim 16
rhel6 terminal(init 3) 200
It seems original 256 queue limitation in ps2.c is fine.
I would only drop limitation(16) in old sendkey command,
it's secure.
If this is right, then I think that the best thing to do would be to drop the
MAX_KEYCODES check from the sendkey command and document that devices can drop
keys if too many of them are sent in parallel or too fast (we can mention ps/2
as an example of a 16 bytes limit).
Likewise the usb hid devices can buffer up to 16 events. In that case
it is just a qemu implementation detail and not a property of the
hardware we are emulating, so it can be changed. Not trivially though
as the buffer is part of the migration data, so it is more work that
just changing a #define.
--
Amos.
