[libvirt] [PATCH 0/2] event cleanup patches
Noticed these while working on another patch. Although it is mostly cosmetic, it is not small enough to push under the trivial rule, so I'll wait for a review. Eric Blake (2): docs: improve event-related documentation maint: improve debug of libvirt-{qemu,lxc} apis include/libvirt/libvirt.h.in | 36 +++++++++--- src/datatypes.c | 6 +- src/libvirt-lxc.c | 18 ++---- src/libvirt-qemu.c | 37 +++++-------- src/libvirt.c | 129 ++++--------------------------------------- src/libvirt_internal.h | 107 +++++++++++++++++++++++++++++++++++ 6 files changed, 168 insertions(+), 165 deletions(-) -- 1.8.4.2
While looking at event code, I noticed that the documentation was trying to refer me to functions that don't exist. Also fix some typos and poor formatting. * src/libvirt.c (virConnectDomainEventDeregister) (virConnectDomainEventRegisterAny) (virConnectDomainEventDeregisterAny) (virConnectNetworkEventRegisterAny) (virConnectNetworkEventDeregisterAny): Link to correct function. * include/libvirt.h.in (VIR_DOMAIN_EVENT_CALLBACK) (VIR_NETWORK_EVENT_CALLBACK): Likewise. (virDomainEventID, virConnectDomainEventGenericCallback) (virNetworkEventID, virConnectNetworkEventGenericCallback): Improve docs. Signed-off-by: Eric Blake <eblake@redhat.com> --- include/libvirt/libvirt.h.in | 36 ++++++++++++++++++++++++++++-------- src/libvirt.c | 24 ++++++++++++------------ 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 6f79c49..b6c4cd0 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4507,14 +4507,17 @@ int virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, int virDomainSnapshotRef(virDomainSnapshotPtr snapshot); int virDomainSnapshotFree(virDomainSnapshotPtr snapshot); -/* +/** * virConnectDomainEventGenericCallback: * @conn: the connection pointer * @dom: the domain pointer * @opaque: application specified data * - * A generic domain event callback handler. Specific events usually - * have a customization with extra parameters + * A generic domain event callback handler, for use with + * virConnectDomainEventRegisterAny(). Specific events usually + * have a customization with extra parameters, often with @opaque being + * passed in a different parameter position; use VIR_DOMAIN_EVENT_CALLBACK() + * when registering an appropriate handler. */ typedef void (*virConnectDomainEventGenericCallback)(virConnectPtr conn, virDomainPtr dom, @@ -4929,11 +4932,18 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, * VIR_DOMAIN_EVENT_CALLBACK: * * Used to cast the event specific callback into the generic one - * for use for virDomainEventRegister + * for use for virConnectDomainEventRegisterAny() */ #define VIR_DOMAIN_EVENT_CALLBACK(cb) ((virConnectDomainEventGenericCallback)(cb)) +/** + * virDomainEventID: + * + * An enumeration of supported eventId parameters for + * virConnectDomainEventRegisterAny(). Each event id determines which + * signature of callback function will be used. + */ typedef enum { VIR_DOMAIN_EVENT_ID_LIFECYCLE = 0, /* virConnectDomainEventCallback */ VIR_DOMAIN_EVENT_ID_REBOOT = 1, /* virConnectDomainEventGenericCallback */ @@ -5014,10 +5024,17 @@ typedef void (*virConnectNetworkEventLifecycleCallback)(virConnectPtr conn, * VIR_NETWORK_EVENT_CALLBACK: * * Used to cast the event specific callback into the generic one - * for use for virNetworkEventRegister + * for use for virConnectNetworkEventRegisterAny() */ #define VIR_NETWORK_EVENT_CALLBACK(cb) ((virConnectNetworkEventGenericCallback)(cb)) +/** + * virNetworkEventID: + * + * An enumeration of supported eventId parameters for + * virConnectNetworkEventRegisterAny(). Each event id determines which + * signature of callback function will be used. + */ typedef enum { VIR_NETWORK_EVENT_ID_LIFECYCLE = 0, /* virConnectNetworkEventLifecycleCallback */ @@ -5031,14 +5048,17 @@ typedef enum { #endif } virNetworkEventID; -/* +/** * virConnectNetworkEventGenericCallback: * @conn: the connection pointer * @net: the network pointer * @opaque: application specified data * - * A generic network event callback handler. Specific events usually - * have a customization with extra parameters + * A generic network event callback handler, for use with + * virConnectNetworkEventRegisterAny(). Specific events usually + * have a customization with extra parameters, often with @opaque being + * passed in a different parameter position; use VIR_NETWORK_EVENT_CALLBACK() + * when registering an appropriate handler. */ typedef void (*virConnectNetworkEventGenericCallback)(virConnectPtr conn, virNetworkPtr net, diff --git a/src/libvirt.c b/src/libvirt.c index d15d617..77f481e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16127,7 +16127,7 @@ error: * occurring on a connection * * Use of this method is no longer recommended. Instead applications - * should try virConnectDomainEventRegisterAny which has a more flexible + * should try virConnectDomainEventRegisterAny() which has a more flexible * API contract * * The virDomainPtr object handle passed into the callback upon delivery @@ -16178,7 +16178,7 @@ error: * function. * * Use of this method is no longer recommended. Instead applications - * should try virConnectDomainEventUnregisterAny which has a more flexible + * should try virConnectDomainEventDeregisterAny() which has a more flexible * API contract * * Returns 0 on success, -1 on failure @@ -19069,7 +19069,7 @@ error: * Adds a callback to receive notifications of arbitrary domain events * occurring on a domain. * - * If dom is NULL, then events will be monitored for any domain. If dom + * If @dom is NULL, then events will be monitored for any domain. If @dom * is non-NULL, then only the specific domain will be monitored * * Most types of event have a callback providing a custom set of parameters @@ -19080,13 +19080,13 @@ error: * The virDomainPtr object handle passed into the callback upon delivery * of an event is only valid for the duration of execution of the callback. * If the callback wishes to keep the domain object after the callback returns, - * it shall take a reference to it, by calling virDomainRef. + * it shall take a reference to it, by calling virDomainRef(). * The reference can be released once the object is no longer required - * by calling virDomainFree. + * by calling virDomainFree(). * * The return value from this method is a positive integer identifier * for the callback. To unregister a callback, this callback ID should - * be passed to the virDomainEventUnregisterAny method + * be passed to the virConnectDomainEventDeregisterAny() method. * * Returns a callback identifier on success, -1 on failure */ @@ -19143,7 +19143,7 @@ error: * @callbackID: the callback identifier * * Removes an event callback. The callbackID parameter should be the - * vaule obtained from a previous virDomainEventRegisterAny method. + * value obtained from a previous virConnectDomainEventRegisterAny() method. * * Returns 0 on success, -1 on failure */ @@ -19188,7 +19188,7 @@ error: * Adds a callback to receive notifications of arbitrary network events * occurring on a network. * - * If net is NULL, then events will be monitored for any network. If net + * If @net is NULL, then events will be monitored for any network. If @net * is non-NULL, then only the specific network will be monitored * * Most types of event have a callback providing a custom set of parameters @@ -19199,13 +19199,13 @@ error: * The virNetworkPtr object handle passed into the callback upon delivery * of an event is only valid for the duration of execution of the callback. * If the callback wishes to keep the network object after the callback - * returns, it shall take a reference to it, by calling virNetworkRef. + * returns, it shall take a reference to it, by calling virNetworkRef(). * The reference can be released once the object is no longer required - * by calling virNetworkFree. + * by calling virNetworkFree(). * * The return value from this method is a positive integer identifier * for the callback. To unregister a callback, this callback ID should - * be passed to the virNetworkEventUnregisterAny method + * be passed to the virConnectNetworkEventDeregisterAny() method. * * Returns a callback identifier on success, -1 on failure */ @@ -19266,7 +19266,7 @@ error: * @callbackID: the callback identifier * * Removes an event callback. The callbackID parameter should be the - * vaule obtained from a previous virNetworkEventRegisterAny method. + * value obtained from a previous virConnectNetworkEventRegisterAny() method. * * Returns 0 on success, -1 on failure */ -- 1.8.4.2
On Thu, Dec 19, 2013 at 08:13:35AM -0700, Eric Blake wrote:
While looking at event code, I noticed that the documentation was trying to refer me to functions that don't exist. Also fix some typos and poor formatting.
* src/libvirt.c (virConnectDomainEventDeregister) (virConnectDomainEventRegisterAny) (virConnectDomainEventDeregisterAny) (virConnectNetworkEventRegisterAny) (virConnectNetworkEventDeregisterAny): Link to correct function. * include/libvirt.h.in (VIR_DOMAIN_EVENT_CALLBACK) (VIR_NETWORK_EVENT_CALLBACK): Likewise. (virDomainEventID, virConnectDomainEventGenericCallback) (virNetworkEventID, virConnectNetworkEventGenericCallback): Improve docs.
Signed-off-by: Eric Blake <eblake@redhat.com> --- include/libvirt/libvirt.h.in | 36 ++++++++++++++++++++++++++++-------- src/libvirt.c | 24 ++++++++++++------------ 2 files changed, 40 insertions(+), 20 deletions(-)
ACK 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 :|
On 12/19/2013 08:18 AM, Daniel P. Berrange wrote:
On Thu, Dec 19, 2013 at 08:13:35AM -0700, Eric Blake wrote:
While looking at event code, I noticed that the documentation was trying to refer me to functions that don't exist. Also fix some typos and poor formatting.
* src/libvirt.c (virConnectDomainEventDeregister) (virConnectDomainEventRegisterAny) (virConnectDomainEventDeregisterAny) (virConnectNetworkEventRegisterAny) (virConnectNetworkEventDeregisterAny): Link to correct function. * include/libvirt.h.in (VIR_DOMAIN_EVENT_CALLBACK) (VIR_NETWORK_EVENT_CALLBACK): Likewise. (virDomainEventID, virConnectDomainEventGenericCallback) (virNetworkEventID, virConnectNetworkEventGenericCallback): Improve docs.
Signed-off-by: Eric Blake <eblake@redhat.com> --- include/libvirt/libvirt.h.in | 36 ++++++++++++++++++++++++++++-------- src/libvirt.c | 24 ++++++++++++------------ 2 files changed, 40 insertions(+), 20 deletions(-)
ACK
Thanks; pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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@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
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 :|
On 12/19/2013 08:17 AM, Daniel P. Berrange wrote:
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"
+#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
Good idea.
+ +/** + * 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
Sure, although VIR_DOMAIN_DEBUG requires the use of viruuid.h in the first place.
+#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 ??
Perhaps, by using virReportError instead. I can give that a try; but just looking at it, virReportError() hardcodes VIR_FROM_THIS as its first argument, whereas virLib*Error() sets different VIR_FROM_* categories. Worse, we use multiple calls from within a single function (so continually redefining VIR_FROM_THIS would get painful) - for example, virDomainSnapshotCreateXML() uses both virLibDomainError() (VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE). I'll definitely split into 2 patches; the VIR_DOMAIN_DEBUG motion in one (since it was less controversial) and the potential virLib*Error cleanup in the other. v2 coming up later today. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Dec 19, 2013 at 09:35:23AM -0700, Eric Blake wrote:
On 12/19/2013 08:17 AM, Daniel P. Berrange wrote:
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"
+#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
Good idea.
+ +/** + * 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
Sure, although VIR_DOMAIN_DEBUG requires the use of viruuid.h in the first place.
+#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 ??
Perhaps, by using virReportError instead. I can give that a try; but just looking at it, virReportError() hardcodes VIR_FROM_THIS as its first argument, whereas virLib*Error() sets different VIR_FROM_* categories. Worse, we use multiple calls from within a single function (so continually redefining VIR_FROM_THIS would get painful) - for example, virDomainSnapshotCreateXML() uses both virLibDomainError() (VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE).
Oh true I'd forgotten it relied on VIR_FROM_THIS. How about just renaming them s/Lib/Report/ eg virLibDomainError -> virReportDomainError and having them in datatypes.h too. 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 :|
On 12/19/2013 09:37 AM, Daniel P. Berrange wrote:
+#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 ??
Perhaps, by using virReportError instead. I can give that a try; but just looking at it, virReportError() hardcodes VIR_FROM_THIS as its first argument, whereas virLib*Error() sets different VIR_FROM_* categories. Worse, we use multiple calls from within a single function (so continually redefining VIR_FROM_THIS would get painful) - for example, virDomainSnapshotCreateXML() uses both virLibDomainError() (VIR_FROM_DOMAIN) and virLibConnError() (VIR_FROM_NONE).
Oh true I'd forgotten it relied on VIR_FROM_THIS.
How about just renaming them s/Lib/Report/ eg virLibDomainError -> virReportDomainError and having them in datatypes.h too.
Sounds reasonable, but again, splitting into two patches to keep the review work easier. v2 coming up later today. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake