[libvirt] [PATCH] Unify most error reporting (ver 2)
Jim Meyering
jim at meyering.net
Fri Oct 10 09:47:49 UTC 2008
Daniel Veillard <veillard at redhat.com> wrote:
> On Thu, Oct 09, 2008 at 11:44:02AM -0400, Cole Robinson wrote:
>> Daniel Veillard wrote:
>> > On Wed, Oct 08, 2008 at 08:35:38PM +0100, Daniel P. Berrange wrote:
>> >>> One side question: is src/xmlrpc.{c,h} even used? And
>> >>> if not, can it be removed?
>> >> It is not used at this time. It was for the hypothetical
>> >> alternative Xen driver which talks XML-RPC which we never
>> >> got around to writing. Perhaps it really is time to kill
>> >> it -we can get it back from CVS if ever required.
>> >
>> > Agreed,
>> >
>> >> ACK to this patch
>> >
>> > +1 too,
>> >
>> > Daniel
>> >
>>
>> Thanks, pushed now.
>
> Hum, this breaks "make check"
>
> PASS: read-non-seekable
> --- out 2008-10-10 10:23:57.000000000 +0200
> +++ exp 2008-10-10 10:23:57.000000000 +0200
> @@ -1,2 +1,2 @@
> -libvir: Test error : internal error Domain is still running
> +libvir: Test error test: internal error Domain is still running
> error: Failed to undefine domain test
> FAIL: undefine
>
> Seems we loose the domain name in the error message when trying to
> undefine a running domain:
>
> paphio:~/libvirt/tests -> /usr/bin/virsh -q -c test:///default undefine
> test
> libvir: Test error test: internal error Domain is still running
> error: Failed to undefine domain test
> paphio:~/libvirt/tests -> ../src/virsh -q -c test:///default undefine
> test
> libvir: Test error : internal error Domain is still running
> error: Failed to undefine domain test
> paphio:~/libvirt/tests ->
>
> I should be possible to restore the old behaviour of reporting the domain
> name there
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 63ab950ba9b8065de3036dcf502e14c05c6f6461 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
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
More information about the libvir-list
mailing list