[libvirt] [v3] API: Improve log for domain related APIs
Eric Blake
eblake at redhat.com
Thu Jan 6 16:50:43 UTC 2011
On 01/06/2011 08:43 AM, Osier Yang wrote:
> Add VM name/UUID in log for domain related APIs.
> Format: "dom=%p, (VM: name=%s, uuid=%s), param0=%s, param1=%s
>
> *src/libvirt.c (introduce two macros: VIR_DOMAIN_DEBUG, and
> VIR_DOMAIN_DEBUG0)
> ---
> src/libvirt.c | 243 +++++++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 169 insertions(+), 74 deletions(-)
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index ee2495a..bdf9896 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -312,6 +312,39 @@ static struct gcry_thread_cbs virTLSThreadImpl = {
> NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
> };
>
> +#define VIR_DOMAIN_DEBUG(dom, fmt, ...) { \
> + do { \
No need to use both an outer '{}' and an inner 'do {}while(0);'. The
more idiomatic expression is do{}while(0) (note the missing trailing ;
in the macro definition, since it is supplied instead by the statement
where the macro is used - you had one caller that missed this) and no
extra {} (if there is any worry about ever using this as a one-line
statement in a brace-less if statement, then it is essential to let the
caller supply a trailing ; without it expanding into two statements).
But in this case, we can go one step further - VIR_DOMAIN_DEBUG is
always called at the top level (never part of a compound statement), and
we require C99 (decl-after-statement is not an issue even if
VIR_DOMAIN_DEBUG(); is not the first statement), so we can avoid
introducing a new scope altogether. We already use compiler warnings to
ensure we don't accidentally shadow any variables, but we can be even
safer by making the macro-specific declarations less likely to collide.
> + virUUIDFormat(dom->uuid, uuidstr); \
Just in case dom is complex, we're safer to properly parenthesize within
the macro (although I don't think any of the existing uses of dom are
complex).
> +#define VIR_DOMAIN_DEBUG0(dom) { \
Shorter to define this as as a wrapper around VIR_DOMAIN_DEBUG, rather
than repeating logic (plus it means fewer places to touch if we change
the layout in the future).
> @@ -2100,7 +2133,10 @@ error:
> virDomainPtr
> virDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
> {
> - DEBUG("conn=%p, uuid=%s", conn, uuid);
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + virUUIDFormat(uuid, uuidstr);
> +
> + DEBUG("conn=%p, uuid=%s", conn, uuidstr);
Good catch. Maybe we should favor VIR_DEBUG over DEBUG, (they are
identical macros, but the latter is marked as legacy in logging.h), but
that doesn't impact this patch.
Everything else looks like straight-forward mechanical conversions.
Therefore, ACK with this squashed in (I used diff -b, to focus on the
non-indentation changes; the final patch looks better), and I'm pushing it.
diff --git -b i/src/libvirt.c w/src/libvirt.c
index 68873cf..89b37c5 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -2,7 +2,7 @@
* libvirt.c: Main interfaces for the libvirt library to handle
virtualization
* domains from a process running in domain 0
*
- * Copyright (C) 2005-2006, 2008-2010 Red Hat, Inc.
+ * Copyright (C) 2005-2006, 2008-2011 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -312,39 +312,24 @@ static struct gcry_thread_cbs virTLSThreadImpl = {
NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL
};
-#define VIR_DOMAIN_DEBUG(dom, fmt, ...) { \
- do { \
- char uuidstr[VIR_UUID_STRING_BUFLEN]; \
- const char *domname = NULL; \
+/* Helper macro to print debugging information about a domain DOM,
+ * followed by a literal string FMT and any other printf arguments.
+ */
+#define VIR_DOMAIN_DEBUG(dom, fmt, ...) \
+ char _uuidstr[VIR_UUID_STRING_BUFLEN]; \
+ const char *_domname = NULL; \
\
if (!VIR_IS_DOMAIN(dom)) { \
- memset(uuidstr, 0, sizeof(uuidstr)); \
+ memset(_uuidstr, 0, sizeof(_uuidstr)); \
} else { \
- virUUIDFormat(dom->uuid, uuidstr); \
- domname = dom->name; \
+ virUUIDFormat((dom)->uuid, _uuidstr); \
+ _domname = (dom)->name; \
} \
\
DEBUG("dom=%p, (VM: name=%s, uuid=%s), " fmt, \
- dom, NULLSTR(domname), uuidstr, __VA_ARGS__); \
- } while (0); \
-}
+ dom, NULLSTR(_domname), _uuidstr, __VA_ARGS__)
-#define VIR_DOMAIN_DEBUG0(dom) { \
- do { \
- char uuidstr[VIR_UUID_STRING_BUFLEN]; \
- const char *name = NULL; \
- \
- if (!VIR_IS_DOMAIN(dom)) { \
- memset(uuidstr, 0, sizeof(uuidstr)); \
- } else { \
- virUUIDFormat(dom->uuid, uuidstr); \
- name = dom->name; \
- } \
- \
- DEBUG("dom=%p, (VM: name=%s, uuid=%s)", \
- dom, NULLSTR(name), uuidstr); \
- } while (0); \
-}
+#define VIR_DOMAIN_DEBUG0(dom) VIR_DOMAIN_DEBUG(dom, "%s", "")
/**
* virInitialize:
@@ -4985,7 +4970,7 @@ int
virDomainUndefine(virDomainPtr domain) {
virConnectPtr conn;
- VIR_DOMAIN_DEBUG0(domain)
+ VIR_DOMAIN_DEBUG0(domain);
virResetLastError();
--
Eric Blake eblake at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110106/423b4ead/attachment-0001.sig>
More information about the libvir-list
mailing list