[libvirt] [PATCH 2/2] maint: improve debug of libvirt-{qemu, lxc} apis

Daniel P. Berrange berrange at redhat.com
Thu Dec 19 15:17:55 UTC 2013


On Thu, Dec 19, 2013 at 08:13:36AM -0700, Eric Blake wrote:
> diff --git a/src/libvirt_internal.h b/src/libvirt_internal.h
> index 115d8d1..b8c842d 100644
> --- a/src/libvirt_internal.h
> +++ b/src/libvirt_internal.h
> @@ -27,6 +27,113 @@
> 
>  # include "internal.h"
> 
> +/* Helper macros to implement VIR_DOMAIN_DEBUG using just C99.  This
> + * assumes you pass fewer than 15 arguments to VIR_DOMAIN_DEBUG, but
> + * can easily be expanded if needed.
> + *
> + * Note that gcc provides extensions of "define a(b...) b" or
> + * "define a(b,...) b,##__VA_ARGS__" as a means of eliding a comma
> + * when no var-args are present, but we don't want to require gcc.
> + */
> +#define VIR_ARG15(_1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, _15, ...) _15
> +#define VIR_HAS_COMMA(...) VIR_ARG15(__VA_ARGS__, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0)
> +
> +/* Form the name VIR_DOMAIN_DEBUG_[01], then call that macro,
> + * according to how many arguments are present.  Two-phase due to
> + * macro expansion rules.  */
> +#define VIR_DOMAIN_DEBUG_EXPAND(a, b, ...)      \
> +    VIR_DOMAIN_DEBUG_PASTE(a, b, __VA_ARGS__)
> +#define VIR_DOMAIN_DEBUG_PASTE(a, b, ...)       \
> +    a##b(__VA_ARGS__)
> +
> +/* Internal use only, when VIR_DOMAIN_DEBUG has one argument.  */
> +#define VIR_DOMAIN_DEBUG_0(dom)                 \
> +    VIR_DOMAIN_DEBUG_2(dom, "%s", "")
> +
> +/* Internal use only, when VIR_DOMAIN_DEBUG has three or more arguments.  */
> +#define VIR_DOMAIN_DEBUG_1(dom, fmt, ...)       \
> +    VIR_DOMAIN_DEBUG_2(dom, ", " fmt, __VA_ARGS__)
> +
> +/* Internal use only, with final format.  */
> +#define VIR_DOMAIN_DEBUG_2(dom, fmt, ...)                               \
> +    do {                                                                \
> +        char _uuidstr[VIR_UUID_STRING_BUFLEN];                          \
> +        const char *_domname = NULL;                                    \
> +                                                                        \
> +        if (!VIR_IS_DOMAIN(dom)) {                                      \
> +            memset(_uuidstr, 0, sizeof(_uuidstr));                      \
> +        } else {                                                        \
> +            virUUIDFormat((dom)->uuid, _uuidstr);                       \
> +            _domname = (dom)->name;                                     \
> +        }                                                               \
> +                                                                        \
> +        VIR_DEBUG("dom=%p, (VM: name=%s, uuid=%s)" fmt,                 \
> +                  dom, NULLSTR(_domname), _uuidstr, __VA_ARGS__);       \
> +    } while (0)
> +
> +/**
> + * VIR_DOMAIN_DEBUG:
> + * @dom: domain
> + * @fmt: optional format for additional information
> + * @...: optional arguments corresponding to @fmt.
> + */
> +#define VIR_DOMAIN_DEBUG(...)                           \
> +    VIR_DOMAIN_DEBUG_EXPAND(VIR_DOMAIN_DEBUG_,          \
> +                            VIR_HAS_COMMA(__VA_ARGS__), \
> +                            __VA_ARGS__)

I'd suggest these go in  datatypes.h


> +
> +/**
> + * VIR_UUID_DEBUG:
> + * @conn: connection
> + * @uuid: possibly null UUID array
> + */
> +#define VIR_UUID_DEBUG(conn, uuid)                              \
> +    do {                                                        \
> +        if (uuid) {                                             \
> +            char _uuidstr[VIR_UUID_STRING_BUFLEN];              \
> +            virUUIDFormat(uuid, _uuidstr);                      \
> +            VIR_DEBUG("conn=%p, uuid=%s", conn, _uuidstr);      \
> +        } else {                                                \
> +            VIR_DEBUG("conn=%p, uuid=(null)", conn);            \
> +        }                                                       \
> +    } while (0)


And this in viruuid.h

So we avoid turning internal.h into even more of a dumping ground.

> +
> +
> +#define virLibConnError(code, ...)                                \
> +    virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,           \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibDomainError(code, ...)                              \
> +    virReportErrorHelper(VIR_FROM_DOM, code, __FILE__,            \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibNetworkError(code, ...)                             \
> +    virReportErrorHelper(VIR_FROM_NETWORK, code, __FILE__,        \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibStoragePoolError(code, ...)                         \
> +    virReportErrorHelper(VIR_FROM_STORAGE, code, __FILE__,        \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibStorageVolError(code, ...)                          \
> +    virReportErrorHelper(VIR_FROM_STORAGE, code, __FILE__,        \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibInterfaceError(code, ...)                           \
> +    virReportErrorHelper(VIR_FROM_INTERFACE, code, __FILE__,      \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibNodeDeviceError(code, ...)                          \
> +    virReportErrorHelper(VIR_FROM_NODEDEV, code, __FILE__,        \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibSecretError(code, ...)                              \
> +    virReportErrorHelper(VIR_FROM_SECRET, code, __FILE__,         \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibStreamError(code, ...)                              \
> +    virReportErrorHelper(VIR_FROM_STREAMS, code, __FILE__,        \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibNWFilterError(code, ...)                            \
> +    virReportErrorHelper(VIR_FROM_NWFILTER, code, __FILE__,       \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)
> +#define virLibDomainSnapshotError(code, ...)                       \
> +    virReportErrorHelper(VIR_FROM_DOMAIN_SNAPSHOT, code, __FILE__, \
> +                         __FUNCTION__, __LINE__, __VA_ARGS__)

I'd venture to sugggest that these functions really have little
to no benefit / reason to exist. They aren't really simplifying
like, just making it different. They're historical baggage from
the old time when they were actual functions which did extra
sprintf formatting work. Could we just remove them ??


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|




More information about the libvir-list mailing list