Re: [Qemu-devel] [PATCH v2 1/6] qerror: add MAX_KEYCODES 16

[Amos Kong]

On 06/15/2012 09:35 PM, Luiz Capitulino wrote:
> On Fri, 15 Jun 2012 09:57:49 +0200
> Gerd Hoffmann <address@hidden> 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;
    }


> 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.