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

[Eric Blake <eblake@redhat.com>]

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org