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

Eric Blake eblake at redhat.com
Thu Dec 19 15:13:36 UTC 2013


I noticed that the virDomainQemuMonitorCommand debug output wasn't
telling me the name of the domain it was working on.  While it was
easy enough to determine which pointer matches the domain based on
other log messages, it is nicer to be consistent.  Along the same
lines, having virLibDomainError() take two arguments in libvirt.c
but three arguments in libvirt-qemu.c is not friendly to code
copying between the two files.

* src/libvirt.c (VIR_ARG15, VIR_HAS_COMMA)
(VIR_DOMAIN_DEBUG*, VIR_UUID_DEBUG, virLib*Error): Move...
* src/libvirt_internal.h: ...here.
* src/libvirt-qemu.c (virDomainQemuMonitorCommand)
(virDomainQemuAttach, virDomainQemuAgentCommand): Better debug
messages.
* src/libvirt-lxc.c (virDomainLxcOpenNamespace): Likewise.
* src/datatypes.c (virLibConnError): Drop duplicate definition.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 src/datatypes.c        |   6 +--
 src/libvirt-lxc.c      |  18 +++------
 src/libvirt-qemu.c     |  37 +++++++----------
 src/libvirt.c          | 105 ------------------------------------------------
 src/libvirt_internal.h | 107 +++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 145 deletions(-)

diff --git a/src/datatypes.c b/src/datatypes.c
index 161f1b0..f03f9b3 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -1,7 +1,7 @@
 /*
  * datatypes.h: management of structs for public data types
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -31,10 +31,6 @@

 #define VIR_FROM_THIS VIR_FROM_NONE

-#define virLibConnError(code, ...)                                \
-    virReportErrorHelper(VIR_FROM_THIS, code, __FILE__,           \
-                         __FUNCTION__, __LINE__, __VA_ARGS__)
-

 virClassPtr virConnectClass;
 virClassPtr virConnectCloseCallbackDataClass;
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index c8cdcea..19c6d66 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -28,6 +28,7 @@
 #include "virfile.h"
 #include "virlog.h"
 #include "virprocess.h"
+#include "viruuid.h"
 #include "datatypes.h"
 #ifdef WITH_SELINUX
 # include <selinux/selinux.h>
@@ -35,14 +36,6 @@

 #define VIR_FROM_THIS VIR_FROM_NONE

-#define virLibConnError(conn, error, info)                              \
-    virReportErrorHelper(VIR_FROM_NONE, error, __FILE__, __FUNCTION__,  \
-                         __LINE__, info)
-
-#define virLibDomainError(domain, error, info)                          \
-    virReportErrorHelper(VIR_FROM_DOM, error, __FILE__, __FUNCTION__,   \
-                         __LINE__, info)
-
 /**
  * virDomainLxcOpenNamespace:
  * @domain: a domain object
@@ -69,13 +62,12 @@ virDomainLxcOpenNamespace(virDomainPtr domain,
 {
     virConnectPtr conn;

-    VIR_DEBUG("domain=%p, fdlist=%p flags=%x",
-              domain, fdlist, flags);
+    VIR_DOMAIN_DEBUG(domain, "fdlist=%p flags=%x", fdlist, flags);

     virResetLastError();

     if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
-        virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+        virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
         virDispatchError(NULL);
         return -1;
     }
@@ -85,7 +77,7 @@ virDomainLxcOpenNamespace(virDomainPtr domain,
     virCheckNonNullArgGoto(fdlist, error);

     if (conn->flags & VIR_CONNECT_RO) {
-        virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
         goto error;
     }

@@ -99,7 +91,7 @@ virDomainLxcOpenNamespace(virDomainPtr domain,
         return ret;
     }

-    virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);

 error:
     virDispatchError(conn);
diff --git a/src/libvirt-qemu.c b/src/libvirt-qemu.c
index 83fb3b3..db52c65 100644
--- a/src/libvirt-qemu.c
+++ b/src/libvirt-qemu.c
@@ -2,7 +2,7 @@
  * libvirt-qemu.c: Interfaces for the libvirt library to handle qemu-specific
  *                 APIs.
  *
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -25,18 +25,11 @@

 #include "virerror.h"
 #include "virlog.h"
+#include "viruuid.h"
 #include "datatypes.h"

 #define VIR_FROM_THIS VIR_FROM_NONE

-#define virLibConnError(conn, error, info)                              \
-    virReportErrorHelper(VIR_FROM_NONE, error, __FILE__, __FUNCTION__,  \
-                         __LINE__, info)
-
-#define virLibDomainError(domain, error, info)                          \
-    virReportErrorHelper(VIR_FROM_DOM, error, __FILE__, __FUNCTION__,   \
-                         __LINE__, info)
-
 /**
  * virDomainQemuMonitorCommand:
  * @domain: a domain object
@@ -75,13 +68,13 @@ virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd,
 {
     virConnectPtr conn;

-    VIR_DEBUG("domain=%p, cmd=%s, result=%p, flags=%x",
-              domain, cmd, result, flags);
+    VIR_DOMAIN_DEBUG(domain, "cmd=%s, result=%p, flags=%x",
+                     cmd, result, flags);

     virResetLastError();

     if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
-        virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+        virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
         virDispatchError(NULL);
         return -1;
     }
@@ -91,7 +84,7 @@ virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd,
     virCheckNonNullArgGoto(result, error);

     if (conn->flags & VIR_CONNECT_RO) {
-        virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
         goto error;
     }

@@ -104,7 +97,7 @@ virDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd,
         return ret;
     }

-    virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);

 error:
     virDispatchError(conn);
@@ -152,7 +145,7 @@ virDomainQemuAttach(virConnectPtr conn,
     virResetLastError();

     if (!VIR_IS_CONNECT(conn)) {
-        virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
         virDispatchError(NULL);
         return NULL;
     }
@@ -166,7 +159,7 @@ virDomainQemuAttach(virConnectPtr conn,
     }

     if (conn->flags & VIR_CONNECT_RO) {
-        virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
         goto error;
     }

@@ -178,7 +171,7 @@ virDomainQemuAttach(virConnectPtr conn,
         return ret;
     }

-    virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);

 error:
     virDispatchError(conn);
@@ -213,11 +206,11 @@ virDomainQemuAgentCommand(virDomainPtr domain,
     virConnectPtr conn;
     char *ret;

-    VIR_DEBUG("domain=%p, cmd=%s, timeout=%d, flags=%x",
-              domain, cmd, timeout, flags);
+    VIR_DOMAIN_DEBUG(domain, "cmd=%s, timeout=%d, flags=%x",
+                     cmd, timeout, flags);

     if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
-        virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+        virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
         virDispatchError(NULL);
         return NULL;
     }
@@ -225,7 +218,7 @@ virDomainQemuAgentCommand(virDomainPtr domain,
     conn = domain->conn;

     if (conn->flags & VIR_CONNECT_RO) {
-        virLibDomainError(NULL, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+        virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
         goto error;
     }

@@ -237,7 +230,7 @@ virDomainQemuAgentCommand(virDomainPtr domain,
         return ret;
     }

-    virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+    virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__);

     /* Copy to connection error object for back compatibility */
 error:
diff --git a/src/libvirt.c b/src/libvirt.c
index 77f481e..a03c4b8 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -327,77 +327,6 @@ static struct gcry_thread_cbs virTLSThreadImpl = {
 };
 #endif /* WITH_GNUTLS_GCRYPT */

-/* 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__)
-
-/**
- * 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)
-

 static bool virGlobalError = false;
 static virOnceControl virGlobalOnce = VIR_ONCE_CONTROL_INITIALIZER;
@@ -566,40 +495,6 @@ DllMain(HINSTANCE instance ATTRIBUTE_UNUSED,
 }
 #endif

-#define virLibConnError(code, ...)                                \
-    virReportErrorHelper(VIR_FROM_NONE, 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__)
-

 /**
  * virRegisterNetworkDriver:
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__)
+
+/**
+ * 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)
+
+
+#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__)
+
+
 typedef void (*virStateInhibitCallback)(bool inhibit,
                                         void *opaque);

-- 
1.8.4.2




More information about the libvir-list mailing list