[libvirt] [PATCH 1/2] Define public API for receiving guest memory balloon events

[Eric Blake]

On 05/16/2012 08:35 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> When the guest changes its memory balloon applications may want
> to know what the new value is, without having to periodically
> poll on XML / domain info. Introduce a "balloon change" event
> to let apps see this
> 

> +++ b/daemon/remote.c
> @@ -582,6 +582,32 @@ static int remoteRelayDomainEventPMSuspend(virConnectPtr conn ATTRIBUTE_UNUSED,
>      return 0;
>  }
>  
> +static int remoteRelayDomainEventBalloonChange(virConnectPtr conn ATTRIBUTE_UNUSED,

Formatting nit: should the function name start on a new line, leaving
'static int' on a line of its own?

> +                                               virDomainPtr dom,
> +                                               unsigned long long actual,
> +                                               void *opaque)
> +{
> +    virNetServerClientPtr client = opaque;
> +    remote_domain_event_balloon_change_msg data;
> +
> +    if (!client)
> +        return -1;
> +
> +    VIR_DEBUG("Relaying domain ballon change event %s %d %lld", dom->name, dom->id, actual);

s/ballon/balloon/

> +++ b/examples/domain-events/events-c/event-test.c
> @@ -222,6 +222,25 @@ static int myDomainEventRTCChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
>      return 0;
>  }
>  
> +static int myDomainEventBalloonChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                                              virDomainPtr dom,
> +                                              unsigned long long actual,
> +                                              void *opaque ATTRIBUTE_UNUSED)
> +{
> +    char *str = NULL;
> +    /* HACK: use asprintf since we have gnulib's wrapper for %lld on Win32
> +     * but don't have a printf() replacement with %lld */

Yep, I can see why you did this.  And since our example code can't use
virAsprintf (which is internal to libvirt, but our examples are supposed
to be standalone), we definitely need to keep this comment.

I'm a bit worried, though, that we are making this example harder than
necessary to use on BSD (that is, avoiding our virAsprintf wrapper but
then relying on our gnulib wrapper seems risky).  Maybe we should do:

#ifdef WIN32
use printf("%I64d")
#else
use printf("%lld")
#endif

so that the example code is self-standing without asprintf.

I don't know which approach is worse, though, and at least your comment
should be sufficient to give someone a hint of how to work around things
on a system that lacks asprintf when trying to use the example code in a
standalone manner.


> +++ b/include/libvirt/libvirt.h.in
> @@ -3780,6 +3780,22 @@ typedef void (*virConnectDomainEventPMSuspendCallback)(virConnectPtr conn,
>                                                         int reason,
>                                                         void *opaque);
>  
> +
> +/**
> + * virConnectDomainEventBalloonChangeCallback:
> + * @conn: connection object
> + * @dom: domain on which the event occurred
> + * @actual: the new balloon level measured in kilobytes

s/kilobytes/kibibytes (blocks of 1024 bytes)/

At any rate, I agree with keeping this in blocks of 1024, to match our
XML for memory balloon sizing.  If we had a time machine, I would have
designed that value in bytes instead; oh well.


> +++ b/src/conf/domain_event.c
> @@ -121,6 +121,9 @@ struct _virDomainEvent {
>              char *devAlias;
>              int reason;
>          } trayChange;
> +        struct {
> +            unsigned long long actual;

add a comment, documenting that this is in units of 1024 (otherwise, we
may end up with a future patch goofing and using bytes or megabytes
instead of kilobytes, and messing things up)

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120516/4d91795b/attachment-0001.sig>