[Qemu-devel] Re: [PATCH v2] add event queueing to USB HID

[Ian Jackson]

Paolo Bonzini writes ("Re: [PATCH v2] add event queueing to USB HID"):
> It's all pretty academic as in practice it worked well.  The queue-full 
> code would never trigger in usb_pointer_event, and instead the queue 
> would be instantly emptied when a 17th event arrived.  This is lucky 
> actually, because the device would also stop unqueuing from a full queue 
> if it had SET_IDLE 0.  Both bugs happened because head==tail could mean 
> both "empty queue" and "full queue".  I'm not even sure it's possible to 
> fill the queue in practice; even with a very high latency you'd have to 
> do 8 clicks in say half a second.  I didn't try with a shorter queue, 
> which of course would have made the problems evident.

Ah.  head==tail was supposed to mean empty (as is conventional); I
guess my "full" test just had an off-by-one error.

> I don't disagree, but I think this is better left for a separate patch. 
>   I see three reasons for this:
> 
> 1) I would like to understand better the relation between GET_REPORT and 
> TOKEN_IN.  If an event occurs between two TOKEN_IN messages, and the OS 
> sends a GET_REPORT in between, should the second TOKEN_IN return 
> USB_RET_NAK or not?  With your patch it will, with current QEMU and my 
> patch it won't.  In this sense, the "changed" flag is recording slightly 
> different information at least in my version of the code.

Hrm.

> 2) if buffering is implemented for the keyboard device (and it probably 
> should) most of the differences would go away again.

True.

> 3) Secondarily, this is the only part of your patch that would need 
> adjustments, since finite idle delays are now implemented.

Right.

OK.  Thanks for your work.


Acked-by: Ian Jackson <address@hidden>

Ian.