Re: [Qemu-devel] [PATCH] Add event notification for guest balloon changes

[Anthony Liguori <anthony@codemonkey.ws>]

On 05/16/2012 01:42 PM, Luiz Capitulino wrote:
> On Wed, 16 May 2012 11:10:47 +0100
> "Daniel P. Berrange"<berrange@redhat.com>  wrote:
>
>> From: "Daniel P. Berrange"<berrange@redhat.com>
>>
>> After setting a balloon target value, applications have to
>> continually poll 'query-balloon' to determine whether the
>> guest has reacted to this request. The virtio-balloon backend
>> knows exactly when the guest has reacted though, and thus it
>> is possible to emit a JSON event to tell the mgmt application
>> whenever the guest balloon changes.
>>
>> This introduces a new 'qemu_balloon_change()' API which is
>> to be called by balloon driver backends, whenever they have
>> a change in balloon value. This takes the 'actual' balloon
>> value, as would be found in the BalloonInfo struct.
>>
>> The qemu_balloon_change API emits a JSON monitor event which
>> looks like:
>>
>>    {"timestamp": {"seconds": 1337162462, "microseconds": 814521},
>>     "event": "BALLOON_CHANGE", "data": {"actual": 944766976}}
>
> It's missing an entry in QMP/qmp-events.txt and I have a comment below,
> but in general looks good.
>
> Amit, would be good to get your ack.

I think it would be safer to limit this event to (1) only firing once target has 
been reached (2) firing if target is deviated from without a corresponding 
change in target.

Otherwise, a guest could just flood libvirt with events.  This would queue 
memory in QEMU indefinitely as the events got queued up to potentially serving 
as a DoS against other guests.

Regards,

Anthony LIguori

>
>>
>> * balloon.c, balloon.h: Introduce qemu_balloon_change() for
>>    emitting balloon change events on the monitor
>> * hw/virtio-balloon.c: Invoke qemu_balloon_change() whenever
>>    the guest changes the balloon actual value
>> * monitor.c, monitor.h: Define QEVENT_BALLOON_CHANGE
>>
>> Signed-off-by: Daniel P. Berrange<berrange@redhat.com>
>> ---
>>   balloon.c           |   14 ++++++++++++++
>>   balloon.h           |    2 ++
>>   hw/virtio-balloon.c |    5 +++++
>>   monitor.c           |    3 +++
>>   monitor.h           |    1 +
>>   5 files changed, 25 insertions(+), 0 deletions(-)
>>
>> diff --git a/balloon.c b/balloon.c
>> index aa354f7..913862b 100644
>> --- a/balloon.c
>> +++ b/balloon.c
>> @@ -30,6 +30,7 @@
>>   #include "balloon.h"
>>   #include "trace.h"
>>   #include "qmp-commands.h"
>> +#include "qjson.h"
>>
>>   static QEMUBalloonEvent *balloon_event_fn;
>>   static QEMUBalloonStatus *balloon_stat_fn;
>> @@ -80,6 +81,19 @@ static int qemu_balloon_status(BalloonInfo *info)
>>       return 1;
>>   }
>>
>> +void qemu_balloon_change(int64_t actual)
>> +{
>> +    QObject *data;
>> +
>> +    data = qobject_from_jsonf("{ 'actual': %" PRId64 " }",
>> +                              actual);
>> +
>> +    monitor_protocol_event(QEVENT_BALLOON_CHANGE, data);
>> +
>> +    qobject_decref(data);
>> +}
>> +
>> +
>>   BalloonInfo *qmp_query_balloon(Error **errp)
>>   {
>>       BalloonInfo *info;
>> diff --git a/balloon.h b/balloon.h
>> index b60fd5d..2ebac0d 100644
>> --- a/balloon.h
>> +++ b/balloon.h
>> @@ -24,4 +24,6 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
>>   			     QEMUBalloonStatus *stat_func, void *opaque);
>>   void qemu_remove_balloon_handler(void *opaque);
>>
>> +void qemu_balloon_change(int64_t actual);
>> +
>>   #endif
>> diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
>> index ce9d2c9..9137573 100644
>> --- a/hw/virtio-balloon.c
>> +++ b/hw/virtio-balloon.c
>> @@ -146,8 +146,13 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
>>   {
>>       VirtIOBalloon *dev = to_virtio_balloon(vdev);
>>       struct virtio_balloon_config config;
>> +    uint32_t oldactual = dev->actual;
>>       memcpy(&config, config_data, 8);
>>       dev->actual = le32_to_cpu(config.actual);
>> +    if (dev->actual != oldactual) {
>> +        qemu_balloon_change(ram_size -
>> +                            (dev->actual<<  VIRTIO_BALLOON_PFN_SHIFT));
>> +    }
>
> This can cause several events to be emitted until the memory is adjusted
> to the value asked by the user. I'm undecided if this is a feature, but
> if I were a client issuing the balloon command I'd expect to get the event
> only when the memory is fully adjusted to the value I asked.
>
> Not sure if this possible to implement though, or if we really want it.
>
>
>>   }
>>
>>   static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
>> diff --git a/monitor.c b/monitor.c
>> index 12a6fe2..ef59cd9 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>>           case QEVENT_WAKEUP:
>>               event_name = "WAKEUP";
>>               break;
>> +        case QEVENT_BALLOON_CHANGE:
>> +            event_name = "BALLOON_CHANGE";
>> +            break;
>>           default:
>>               abort();
>>               break;
>> diff --git a/monitor.h b/monitor.h
>> index 0d49800..8de0160 100644
>> --- a/monitor.h
>> +++ b/monitor.h
>> @@ -41,6 +41,7 @@ typedef enum MonitorEvent {
>>       QEVENT_DEVICE_TRAY_MOVED,
>>       QEVENT_SUSPEND,
>>       QEVENT_WAKEUP,
>> +    QEVENT_BALLOON_CHANGE,
>>       QEVENT_MAX,
>>   } MonitorEvent;
>>
>