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

On 07/12/2012 09:45 AM, Guannan Ren wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > 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 > > * include/libvirt/libvirt.h.in: Define the > virConnectDomainEventBalloonChangeCallback callback > and VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE constant > * python/libvirt-override-virConnect.py, > python/libvirt-override.c: Wire up helpers for new event > * daemon/remote.c: Helper for serializing balloon event > * examples/domain-events/events-c/event-test.c, > examples/domain-events/events-python/event-test.py: Add > example of balloon event usage > * src/conf/domain_event.c, src/conf/domain_event.h: Handling > of balloon events > * src/remote/remote_driver.c: Add handler of balloon events > * src/remote/remote_protocol.x: Define wire protocol for > balloon events > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; > +++ b/examples/domain-events/events-c/event-test.c > @@ -208,16 +208,29 @@ static int myDomainEventRTCChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, > long long offset, > 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 */ > - if (asprintf(&str, "%s EVENT: Domain %s(%d) rtc change %lld\n", > - __func__, virDomainGetName(dom), > - virDomainGetID(dom), offset) < 0) > - return 0; > - > - printf("%s", str); > - free(str); > +#ifdef WIN32 > + printf("%s EVENT: Domain %s(%d) rtc change %I64d\n", > + __func__, virDomainGetName(dom), virDomainGetID(dom), offset); > +#else > + printf("%s EVENT: Domain %s(%d) rtc change %lld\n", > + __func__, virDomainGetName(dom), virDomainGetID(dom), offset); > +#endif This hunk has nothing to do with the new code, and I detest the use of %I64d. > +static int myDomainEventBalloonChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, > + virDomainPtr dom, > + unsigned long long actual, > + void *opaque ATTRIBUTE_UNUSED) > +{ > +#ifdef WIN32 > + printf("%s EVENT: Domain %s(%d) balloon change %I64d KB\n", > + __func__, virDomainGetName(dom), virDomainGetID(dom), actual); > +#else > + printf("%s EVENT: Domain %s(%d) balloon change %lld KB\n", > + __func__, virDomainGetName(dom), virDomainGetID(dom), actual); > +#endif Here, you're repeating the same idiom. And on a pedantic point, %lld is for 'signed long long', but 'actual' is 'unsigned long long'. Can we instead just rely on <inttypes.h> and PRIuMAX here (and PRIdMAX earlier), along with a cast to [u]intmax_t? Mingw has a working <stdint.h>, so that would get rid of our #ifdef, as well as avoid the asprintf() hack as well as the ugly %I64d format string.