Re: [libvirt] [PATCH] Unify most error reporting (ver 2)

Daniel Veillard <veillard(a)redhat.com&gt; wrote: The documentation suggests that the new interface was initially supposed to have dom and net parameters: /** * __virReportErrorHelper * * @conn: the connection to the hypervisor if available * @dom: the domain if available * @net: the network if available * @domcode: the virErrorDomain indicating where it's coming from ... void __virReportErrorHelper(virConnectPtr conn, int domcode, int errcode, const char *filename ATTRIBUTE_UNUSED, const char *funcname ATTRIBUTE_UNUSED, long long linenr ATTRIBUTE_UNUSED, const char *fmt, ...) { ... virerr = __virErrorMsg(errcode, (errorMessage[0] ? errorMessage : NULL)); __virRaiseError(conn, NULL, NULL, domcode, errcode, VIR_ERR_ERROR, virerr, errorMessage, NULL, -1, -1, virerr, errorMessage); } Shouldn't be hard to make the code agree with the documentation. There are fewer than 20 uses of __virReportErrorHelper so far. In fact, I've just done it. While doing it, I found that there were a few other places where dom- and net-info would have been lost. Along the way, there was a macro that asked to be wrapped with the classic do {...} while(0), so I obliged. Here's the patch. With it, "make check" passes once again. Adjust __virReportErrorHelper to accept "dom" and "net", parameters, since some callers require passing those to __virRaiseError. * src/virterror.c (__virReportErrorHelper): Add dom and net parameters, to match documentation. * src/internal.h: Update its prototype. * src/domain_conf.c (virDomainReportError): Update caller. * src/hash.c (virHashError): Likewise. * src/lxc_conf.h (lxcError): Likewise. * src/network_conf.c (virNetworkReportError): Likewise. * src/openvz_conf.h (openvzError): Likewise. * src/proxy_internal.c (virProxyError): Likewise. * src/qemu_conf.h (qemudReportError): Likewise. * src/qparams.c (qparam_report_oom): Likewise. * src/sexpr.c (virSexprError): Likewise. * src/storage_conf.h (virStorageReportError): Likewise. * src/test.c (testError): Likewise. * src/util.c (ReportError): Likewise. * src/xen_internal.c (virXenError): Likewise. * src/xen_unified.c (xenUnifiedError): Likewise. * src/xend_internal.c (virXendError): Likewise. * src/xm_internal.c (xenXMError): Likewise. * src/xml.c (virXMLError): Likewise. * src/xs_internal.c (virXenStoreError): Likewise. From: Jim Meyering <meyering(a)redhat.com&gt; Date: Fri, 10 Oct 2008 11:45:16 +0200 Subject: [PATCH] Adjust __virReportErrorHelper to accept "dom" and "net" parameters, since some callers require passing those to __virRaiseError. * src/virterror.c (__virReportErrorHelper): Add dom and net parameters, to match documentation. * src/internal.h: Update its prototype. * src/domain_conf.c (virDomainReportError): Update caller. * src/hash.c (virHashError): Likewise. * src/lxc_conf.h (lxcError): Likewise. * src/network_conf.c (virNetworkReportError): Likewise. * src/openvz_conf.h (openvzError): Likewise. * src/proxy_internal.c (virProxyError): Likewise. * src/qemu_conf.h (qemudReportError): Likewise. * src/qparams.c (qparam_report_oom): Likewise. * src/sexpr.c (virSexprError): Likewise. * src/storage_conf.h (virStorageReportError): Likewise. * src/test.c (testError): Likewise. * src/util.c (ReportError): Likewise. * src/xen_internal.c (virXenError): Likewise. * src/xen_unified.c (xenUnifiedError): Likewise. * src/xend_internal.c (virXendError): Likewise. * src/xm_internal.c (xenXMError): Likewise. * src/xml.c (virXMLError): Likewise. * src/xs_internal.c (virXenStoreError): Likewise. --- src/domain_conf.c | 6 +++--- src/hash.c | 6 +++--- src/internal.h | 7 +++++-- src/lxc_conf.h | 7 +++---- src/network_conf.c | 6 +++--- src/openvz_conf.h | 6 +++--- src/proxy_internal.c | 6 +++--- src/qemu_conf.h | 8 ++++---- src/qparams.c | 9 +++++---- src/sexpr.c | 6 +++--- src/storage_conf.h | 6 +++--- src/test.c | 6 +++--- src/util.c | 6 +++--- src/virterror.c | 9 ++++++--- src/xen_internal.c | 10 ++++++---- src/xen_unified.c | 6 +++--- src/xend_internal.c | 6 +++--- src/xm_internal.c | 6 +++--- src/xml.c | 6 +++--- src/xs_internal.c | 8 ++++---- 20 files changed, 72 insertions(+), 64 deletions(-) diff --git a/src/domain_conf.c b/src/domain_conf.c index e8d248f..9e5482f 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -141,9 +141,9 @@ VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST, "usb", "pci") -#define virDomainReportError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_DOMAIN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virDomainReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_DOMAIN, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) virDomainObjPtr virDomainFindByID(const virDomainObjPtr doms, int id) diff --git a/src/hash.c b/src/hash.c index 3efda2c..a395d58 100644 --- a/src/hash.c +++ b/src/hash.c @@ -31,9 +31,9 @@ /* #define DEBUG_GROW */ -#define virHashError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virHashError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_NONE, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) /* * A single entry in the hash table diff --git a/src/internal.h b/src/internal.h index 2ae764d..04d4d3c 100644 --- a/src/internal.h +++ b/src/internal.h @@ -313,12 +313,15 @@ void __virRaiseError(virConnectPtr conn, int int1, int int2, const char *msg, ...) ATTRIBUTE_FORMAT(printf, 12, 13); const char *__virErrorMsg(virErrorNumber error, const char *info); -void __virReportErrorHelper(virConnectPtr conn, int domcode, int errcode, +void __virReportErrorHelper(virConnectPtr conn, + virDomainPtr dom, + virNetworkPtr net, + int domcode, int errcode, const char *filename ATTRIBUTE_UNUSED, const char *funcname ATTRIBUTE_UNUSED, long long linenr ATTRIBUTE_UNUSED, const char *fmt, ...) - ATTRIBUTE_FORMAT(printf, 7, 8); + ATTRIBUTE_FORMAT(printf, 9, 10); /************************************************************************ * * diff --git a/src/lxc_conf.h b/src/lxc_conf.h index 4d57046..b15ef5c 100644 --- a/src/lxc_conf.h +++ b/src/lxc_conf.h @@ -49,9 +49,8 @@ struct __lxc_driver { int lxcLoadDriverConfig(lxc_driver_t *driver); virCapsPtr lxcCapsInit(void); -#define lxcError(conn, dom, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_LXC, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define lxcError(conn, dom, code, fmt...) \ + __virReportErrorHelper(conn, dom, NULL, VIR_FROM_LXC, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) #endif /* LXC_CONF_H */ - diff --git a/src/network_conf.c b/src/network_conf.c index 800ead9..3b9100e 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -48,9 +48,9 @@ VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, "none", "nat", "route" ) -#define virNetworkReportError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_NETWORK, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virNetworkReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_NETWORK, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjPtr nets, const unsigned char *uuid) diff --git a/src/openvz_conf.h b/src/openvz_conf.h index fbe4f69..828d807 100644 --- a/src/openvz_conf.h +++ b/src/openvz_conf.h @@ -41,9 +41,9 @@ enum { OPENVZ_WARN, OPENVZ_ERR }; fprintf(stderr, msg);\ fprintf(stderr, "\n"); } -#define openvzError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_OPENVZ, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define openvzError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_OPENVZ, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) /* OpenVZ commands - Replace with wrapper scripts later? */ diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 0186eba..2cb1862 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -92,9 +92,9 @@ struct xenUnifiedDriver xenProxyDriver = { * * ************************************************************************/ -#define virProxyError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_PROXY, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virProxyError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_PROXY, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) /************************************************************************ * * diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 5ea57f0..75d17de 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -1,7 +1,7 @@ /* * config.h: VM configuration management * - * Copyright (C) 2006, 2007 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -71,9 +71,9 @@ struct qemud_driver { }; -#define qemudReportError(conn, dom, net, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_QEMU, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define qemudReportError(conn, dom, net, code, fmt...) \ + __virReportErrorHelper(conn, dom, net, VIR_FROM_QEMU, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) int qemudLoadDriverConfig(struct qemud_driver *driver, diff --git a/src/qparams.c b/src/qparams.c index ed5bddc..2628163 100644 --- a/src/qparams.c +++ b/src/qparams.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007 Red Hat, Inc. +/* Copyright (C) 2007, 2008 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 @@ -30,9 +30,10 @@ #include "memory.h" #include "qparams.h" -#define qparam_report_oom(void) \ - __virReportErrorHelper(NULL, VIR_FROM_NONE, VIR_ERR_NO_MEMORY, \ - __FILE__, __FUNCTION__, __LINE__, NULL) +#define qparam_report_oom(void) \ + __virReportErrorHelper(NULL, NULL, NULL, VIR_FROM_NONE, \ + VIR_ERR_NO_MEMORY, \ + __FILE__, __FUNCTION__, __LINE__, NULL) struct qparam_set * new_qparam_set (int init_alloc, ...) diff --git a/src/sexpr.c b/src/sexpr.c index c168754..f8eb39a 100644 --- a/src/sexpr.c +++ b/src/sexpr.c @@ -23,9 +23,9 @@ #include "util.h" #include "memory.h" -#define virSexprError(code, fmt...) \ - __virReportErrorHelper(NULL, VIR_FROM_SEXPR, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virSexprError(code, fmt...) \ + __virReportErrorHelper(NULL, NULL, NULL, VIR_FROM_SEXPR, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) /** * sexpr_new: diff --git a/src/storage_conf.h b/src/storage_conf.h index c86bf4b..599b5e3 100644 --- a/src/storage_conf.h +++ b/src/storage_conf.h @@ -247,9 +247,9 @@ static inline int virStoragePoolObjIsActive(virStoragePoolObjPtr pool) { return pool->active; } -#define virStorageReportError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_STORAGE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virStorageReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_STORAGE, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) int virStoragePoolObjScanConfigs(virStorageDriverStatePtr driver); diff --git a/src/test.c b/src/test.c index ad51736..7fd2145 100644 --- a/src/test.c +++ b/src/test.c @@ -114,9 +114,9 @@ static const virNodeInfo defaultNodeInfo = { privconn = (testConnPtr)conn->privateData; -#define testError(conn, dom, net, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_TEST, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define testError(conn, dom, net, code, fmt...) \ + __virReportErrorHelper(conn, dom, net, VIR_FROM_TEST, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) static virCapsPtr testBuildCapabilities(virConnectPtr conn) { diff --git a/src/util.c b/src/util.c index ebe86b2..bb6dc9b 100644 --- a/src/util.c +++ b/src/util.c @@ -66,9 +66,9 @@ #ifndef PROXY -#define ReportError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define ReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_NONE, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) int virFileStripSuffix(char *str, const char *suffix) diff --git a/src/virterror.c b/src/virterror.c index 21c7339..22de342 100644 --- a/src/virterror.c +++ b/src/virterror.c @@ -1,7 +1,7 @@ /* * virterror.c: implements error handling and reporting code for libvirt * - * Copy: Copyright (C) 2006 Red Hat, Inc. + * Copy: Copyright (C) 2006, 2008 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -740,7 +740,10 @@ __virErrorMsg(virErrorNumber error, const char *info) * Helper function to do most of the grunt work for individual driver * ReportError */ -void __virReportErrorHelper(virConnectPtr conn, int domcode, int errcode, +void __virReportErrorHelper(virConnectPtr conn, + virDomainPtr dom, + virNetworkPtr net, + int domcode, int errcode, const char *filename ATTRIBUTE_UNUSED, const char *funcname ATTRIBUTE_UNUSED, long long linenr ATTRIBUTE_UNUSED, @@ -759,7 +762,7 @@ void __virReportErrorHelper(virConnectPtr conn, int domcode, int errcode, } virerr = __virErrorMsg(errcode, (errorMessage[0] ? errorMessage : NULL)); - __virRaiseError(conn, NULL, NULL, domcode, errcode, VIR_ERR_ERROR, + __virRaiseError(conn, dom, net, domcode, errcode, VIR_ERR_ERROR, virerr, errorMessage, NULL, -1, -1, virerr, errorMessage); } diff --git a/src/xen_internal.c b/src/xen_internal.c index f498cc3..3e223cb 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -717,10 +717,12 @@ struct xenUnifiedDriver xenHypervisorDriver = { }; #endif /* !PROXY */ -#define virXenError(conn, code, fmt...) \ - if (in_init == 0) \ - __virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virXenError(conn, code, fmt...) \ + do { \ + if (in_init == 0) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_XEN, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt); \ + } while (0) #ifndef PROXY diff --git a/src/xen_unified.c b/src/xen_unified.c index 016181e..884a29d 100644 --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -58,9 +58,9 @@ static struct xenUnifiedDriver *drivers[XEN_UNIFIED_NR_DRIVERS] = { [XEN_UNIFIED_XM_OFFSET] = &xenXMDriver, }; -#define xenUnifiedError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define xenUnifiedError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_XEN, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) /* * Helper functions currently used in the NUMA code diff --git a/src/xend_internal.c b/src/xend_internal.c index 2d10e11..aacf8db 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -104,9 +104,9 @@ virDomainXMLDevID(virDomainPtr domain, int ref_len); #endif -#define virXendError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_XEND, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virXendError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_XEND, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) #define virXendErrorInt(conn, code, ival) \ virXendError(conn, code, "%d", ival) diff --git a/src/xm_internal.c b/src/xm_internal.c index a2dd9aa..1b783f9 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -124,9 +124,9 @@ struct xenUnifiedDriver xenXMDriver = { NULL, /* domainSetSchedulerParameters */ }; -#define xenXMError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_XENXM, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define xenXMError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_XENXM, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) int xenXMInit (void) diff --git a/src/xml.c b/src/xml.c index 6e6e0f6..a0cfdd7 100644 --- a/src/xml.c +++ b/src/xml.c @@ -22,9 +22,9 @@ #include "util.h" #include "memory.h" -#define virXMLError(conn, code, fmt...) \ - __virReportErrorHelper(conn, VIR_FROM_XML, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virXMLError(conn, code, fmt...) \ + __virReportErrorHelper(conn, NULL, NULL, VIR_FROM_XML, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) /************************************************************************ diff --git a/src/xs_internal.c b/src/xs_internal.c index fce7fa7..3f796ad 100644 --- a/src/xs_internal.c +++ b/src/xs_internal.c @@ -1,7 +1,7 @@ /* * xs_internal.c: access to Xen Store * - * Copyright (C) 2006 Red Hat, Inc. + * Copyright (C) 2006, 2008 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -87,9 +87,9 @@ struct xenUnifiedDriver xenStoreDriver = { #endif /* ! PROXY */ -#define virXenStoreError(conn, code, fmt...) \ - __virReportErrorHelper(NULL, VIR_FROM_XENSTORE, code, __FILE__, \ - __FUNCTION__, __LINE__, fmt) +#define virXenStoreError(conn, code, fmt...) \ + __virReportErrorHelper(NULL, NULL, NULL, VIR_FROM_XENSTORE, code, \ + __FILE__, __FUNCTION__, __LINE__, fmt) /************************************************************************ * * -- 1.6.0.2.307.gc4275