On 07/12/2012 09:45 AM, Guannan Ren wrote:
From: "Daniel P. Berrange" <berrange@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
* 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@redhat.com>
+++ 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.
@@ -2838,7 +2843,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_HAS_METADATA = 272, /* autogen autogen */ REMOTE_PROC_CONNECT_LIST_ALL_DOMAINS = 273, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_LIST_ALL_SNAPSHOTS = 274, /* skipgen skipgen priority:high */ - REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275 /* skipgen skipgen priority:high */ + REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, /* skipgen skipgen priority:high */ + + REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276 /* autogen autogen */
Spurious blank line - we only group at sets of 10, not 5. Other than that, this looks fine to me. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org