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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Wed, 16 May 2012 13:58:34 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

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

Yeah, I've said something along these lines below:

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