Re: [RFC PATCH 2/5] hw/misc/led: Add LED_STATUS_CHANGED QAPI event

[Philippe Mathieu-Daudé]

Hi Eric,

On 6/9/20 4:29 PM, Eric Blake wrote:
> On 6/9/20 7:34 AM, Philippe Mathieu-Daudé wrote:
>> Allow LED devices to emit STATUS_CHANGED events on a QMP chardev.
>>
>> QMP event examples:
>>
>> {
>>      "timestamp": {
>>          "seconds": 1591704274,
>>          "microseconds": 520850
>>      },
>>      "event": "LED_STATUS_CHANGED",
>>      "data": {
>>          "name": "Green LED #0",
>>          "status": "on"
>>      }
>> }
>> {
>>      "timestamp": {
>>          "seconds": 1591704275,
>>          "microseconds": 530912
>>      },
>>      "event": "LED_STATUS_CHANGED",
>>      "data": {
>>          "name": "Green LED #0",
>>          "status": "off"
>>      }
>> }
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
> 
> The QAPI addition looks reasonable, however,
> 
>> +++ b/hw/misc/led.c
>> @@ -7,6 +7,7 @@
>>    */
>>   #include "qemu/osdep.h"
>>   #include "qapi/error.h"
>> +#include "qapi/qapi-events-led.h"
>>   #include "hw/qdev-properties.h"
>>   #include "hw/misc/led.h"
>>   #include "hw/irq.h"
>> @@ -19,6 +20,9 @@ static void led_set(void *opaque, int line, int
>> new_state)
>>         trace_led_set(s->name, s->current_state, new_state);
>>   +    /* FIXME QMP rate limite? */
> 
> s/limite/limit/
> 
> Yes, this is under guest control, so you MUST rate limit to avoid the
> guest being able to DoS qemu by changing the LED so frequently as to
> overwhelm the QMP connection with events.

Commits f544d174dfc and 7f1e7b23d5 refers to the qmp-events.txt
for documentation on rate-limiting QMP events, but I can't find
it in the codebase. Two files matches 'qmp-events' but don't have
documentation: qapi/qmp-event.c and include/qapi/qmp-event.h.

Last trace of it is in commit 231aaf3a8217. Apparently it was
somehow split qapi/event.json, then later c09656f1d392 move it
to qapi-schema.json, finally eb815e248f50 moved it to qapi/.

Is the referred documentation now in docs/devel/qapi-code-gen.txt?
There is only one occurence of 'limit' but it is unrelated to
rate-limit.

Thanks,

Phil.