On 07/12/2012 09:45 AM, Guannan Ren wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> +++ 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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list