1:17 p.m.
The attached patch is a more complete cut at unifying
error reporting among the different source files.
Most files have their own custom Error function, each
with varying degrees of functionality, and lots of
code duplication.
The attached patch adds a helper function to virterror.c
that centralizes all this logic, and redefines each
local error function as a macro that calls out to
the helper. The fixed files now offer printf style
reporting, and the macros pass off function name,
file name, and line number of the reported error
(currently not used, but handy to have).
This change centralizes probably 90% of the error calls.
Some files have pretty custom error functions that
don't map easily to the helper, or call __virRaiseError
directly in a number of places, so I skipped these for
now. Eventually we should move all these edge cases over
so any src/ wide error changes will be pretty easy to
make.
The one thing we lose here is that some some places
were logging info in the err{1,2,3} and int{1,2} fields
of raised error. I don't consider this a loss: even
where it was used, it was used inconsistently and
rarely for anything that would be useful to a user.
In the places where the data had some value, I
included it in the error string.
One side question: is src/xmlrpc.{c,h} even used? And
if not, can it be removed?
Thanks,
Cole
domain_conf.c | 27 +-----------
hash.c | 26 +-----------
internal.h | 6 ++
lxc_conf.c | 21 ---------
lxc_conf.h | 7 +--
network_conf.c | 23 +---------
openvz_conf.c | 22 ----------
openvz_conf.h | 7 ++-
proxy_internal.c | 23 +---------
qemu_conf.c | 21 ---------
qemu_conf.h | 8 +--
qparams.c | 10 +---
sexpr.c | 25 +----------
storage_conf.c | 19 ---------
storage_conf.h | 7 +--
test.c | 28 ++++---------
util.c | 27 +-----------
virterror.c | 40 ++++++++++++++++++
xen_internal.c | 116 ++++++++++++++++++-------------------------------------
xen_unified.c | 24 ++---------
xend_internal.c | 59 ++-------------------------
xm_internal.c | 28 ++-----------
xml.c | 43 +++++---------------
xs_internal.c | 23 +---------
24 files changed, 164 insertions(+), 476 deletions(-)
2:35 p.m.
On Wed, Oct 08, 2008 at 02:17:45PM -0400, Cole Robinson wrote:
The attached patch is a more complete cut at unifying
error reporting among the different source files.
Most files have their own custom Error function, each
with varying degrees of functionality, and lots of
code duplication.
The attached patch adds a helper function to virterror.c
that centralizes all this logic, and redefines each
local error function as a macro that calls out to
the helper. The fixed files now offer printf style
reporting, and the macros pass off function name,
file name, and line number of the reported error
(currently not used, but handy to have).
This change centralizes probably 90% of the error calls.
Some files have pretty custom error functions that
don't map easily to the helper, or call __virRaiseError
directly in a number of places, so I skipped these for
now. Eventually we should move all these edge cases over
so any src/ wide error changes will be pretty easy to
make.
Ok.
The one thing we lose here is that some some places
were logging info in the err{1,2,3} and int{1,2} fields
of raised error. I don't consider this a loss: even
where it was used, it was used inconsistently and
rarely for anything that would be useful to a user.
In the places where the data had some value, I
included it in the error string.
Agreed - it had no documented use either.
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.
ACK to this patch
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
3:50 a.m.
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
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
10:44 a.m.
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.
- Cole
3:31 a.m.
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
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
4:47 a.m.
Daniel Veillard <veillard(a)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(a)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
4:51 a.m.
On Fri, Oct 10, 2008 at 11:47:49AM +0200, Jim Meyering wrote:
Daniel Veillard <veillard(a)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:
>
> 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:
We removed them because they're deprecated and 90% of the driver code
never provides them - the test driver was the exception to the rule here
/**
* __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.
That's not really solving the problem because very little code passes
in a non-NULL dom/net parameter.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:59 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Fri, Oct 10, 2008 at 11:47:49AM +0200, Jim Meyering wrote:
> Daniel Veillard <veillard(a)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:
> >
> > 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:
We removed them because they're deprecated and 90% of the driver code
never provides them - the test driver was the exception to the rule here
Oh well.
FYI, the other exceptions:
lxc_conf.h: lxcError (dom-only)
qemu_conf.h: qemudReportError
...
That's not really solving the problem because very little code
passes
in a non-NULL dom/net parameter.
7:13 a.m.
On Fri, Oct 10, 2008 at 11:59:34AM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Fri, Oct 10, 2008 at 11:47:49AM +0200, Jim Meyering wrote:
>> Daniel Veillard <veillard(a)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:
>> >
>> > 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:
>
> We removed them because they're deprecated and 90% of the driver code
> never provides them - the test driver was the exception to the rule here
Oh well.
FYI, the other exceptions:
lxc_conf.h: lxcError (dom-only)
qemu_conf.h: qemudReportError
Some parts of qemu supply it, many other parts do not since they
have no access to the virDomainPtr object. This is one of the
reasons for deprecating this field - it was impossible to reliably
provide it when raising errors.
In this particular test case error, we are better off supplying the
domain name in the format string - it'll improve the error message
to have it placed in context of the description
eg, instead of
libvir: Test error test: internal error Domain is still running
We'd have
libvir: Test error: internal error Domain 'test' is still running
Which could be done by changing
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
_("Domain is still running"));
to be
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Domain '%s' is still running"), domain->name);
We already follow this pattern throughout the QEMU driver
eg
vm = virDomainFindByName(&driver->domains, def->name);
if (vm) {
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
_("domain '%s' is already defined"),
def->name);
goto cleanup;
}
eg
virUUIDFormat(def->uuid, uuidstr);
qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
_("domain with uuid '%s' is already defined"),
uuidstr);
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
7:19 a.m.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
...
Some parts of qemu supply it, many other parts do not since they
have no access to the virDomainPtr object. This is one of the
reasons for deprecating this field - it was impossible to reliably
provide it when raising errors.
In this particular test case error, we are better off supplying the
domain name in the format string - it'll improve the error message
to have it placed in context of the description
eg, instead of
libvir: Test error test: internal error Domain is still running
We'd have
libvir: Test error: internal error Domain 'test' is still running
Which could be done by changing
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
_("Domain is still running"));
to be
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Domain '%s' is still running"), domain->name);
Yep. I began doing that about 15 minutes ago ;-)
8:16 a.m.
Jim Meyering <jim(a)meyering.net> wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
...
> Some parts of qemu supply it, many other parts do not since they
> have no access to the virDomainPtr object. This is one of the
> reasons for deprecating this field - it was impossible to reliably
> provide it when raising errors.
>
> In this particular test case error, we are better off supplying the
> domain name in the format string - it'll improve the error message
> to have it placed in context of the description
>
> eg, instead of
>
> libvir: Test error test: internal error Domain is still running
>
>
> We'd have
>
> libvir: Test error: internal error Domain 'test' is still running
>
> Which could be done by changing
>
> if (privdom->state != VIR_DOMAIN_SHUTOFF) {
> testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
> _("Domain is still running"));
>
>
> to be
>
> if (privdom->state != VIR_DOMAIN_SHUTOFF) {
> testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> _("Domain '%s' is still running"),
domain->name);
Yep. I began doing that about 15 minutes ago ;-)
Here's the change to adjust src/test.c.
Two change sets:
- Fix the "make check" failure by adjusting uses of testError,
including domain->name (and a few net->name) strings via format.
- Remove now-ignored dom and net parameters.
From e6e5387e1e8f8ea7f60bf8666c188b1ee9852700 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 10 Oct 2008 14:45:42 +0200
Subject: [PATCH] change testError format strings to explicitly include domain and network
names
* src/test.c (testResumeDomain, testPauseDomain): Likewise.
(testShutdownDomain, testGetDomainInfo, testDomainSave): Likewise.
(testSetMemory, testSetVcpus, testDomainCreate, testDomainUndefine)
(testDomainGetSchedulerType, testDomainGetSchedulerParams): Likewise.
(testDomainSetSchedulerParams, testNetworkUndefine): Likewise.
(testNetworkStart, testNetworkGetBridgeName): Likewise.
(testDomainCoreDump): Likewise.
Name the file we failed to open or write.
Use strerror(errno) in diagnostics.
* tests/undefine: Adjust expected output.
---
src/test.c | 87 ++++++++++++++++++++++++++++++-------------------------
tests/undefine | 2 +-
2 files changed, 48 insertions(+), 41 deletions(-)
diff --git a/src/test.c b/src/test.c
index ad51736..7ca50eb 100644
--- a/src/test.c
+++ b/src/test.c
@@ -776,8 +776,9 @@ static int testResumeDomain (virDomainPtr domain)
GET_DOMAIN(domain, -1);
if (privdom->state != VIR_DOMAIN_PAUSED) {
- testError(domain->conn, domain, NULL,
- VIR_ERR_INTERNAL_ERROR, _("domain not paused"));
+ testError(domain->conn, NULL, NULL,
+ VIR_ERR_INTERNAL_ERROR, _("%s: domain not paused"),
+ domain->name);
return -1;
}
@@ -791,8 +792,9 @@ static int testPauseDomain (virDomainPtr domain)
if (privdom->state == VIR_DOMAIN_SHUTOFF ||
privdom->state == VIR_DOMAIN_PAUSED) {
- testError(domain->conn, domain, NULL,
- VIR_ERR_INTERNAL_ERROR, _("domain not running"));
+ testError(domain->conn, NULL, NULL,
+ VIR_ERR_INTERNAL_ERROR, _("%s: domain not running"),
+ domain->name);
return -1;
}
@@ -805,7 +807,8 @@ static int testShutdownDomain (virDomainPtr domain)
GET_DOMAIN(domain, -1);
if (privdom->state == VIR_DOMAIN_SHUTOFF) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not
running");
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("%s: domain not running"), domain->name);
return -1;
}
@@ -861,7 +864,8 @@ static int testGetDomainInfo (virDomainPtr domain,
GET_DOMAIN(domain, -1);
if (gettimeofday(&tv, NULL) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, _("getting
time of day"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("getting time of day"));
return (-1);
}
@@ -886,40 +890,40 @@ static int testDomainSave(virDomainPtr domain,
xml = testDomainDumpXML(domain, 0);
if (xml == NULL) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot allocate space for metadata"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("%s: cannot allocate space for metadata"),
domain->name);
return (-1);
}
if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot save domain"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("%s: cannot save domain"), domain->name);
return (-1);
}
len = strlen(xml);
if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot write header"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("%s: cannot write header"), domain->name);
close(fd);
return (-1);
}
if (safewrite(fd, (char*)&len, sizeof(len)) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot write metadata length"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("%s: cannot write metadata length"), domain->name);
close(fd);
return (-1);
}
if (safewrite(fd, xml, len) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot write metadata"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("%s: cannot write metadata"), domain->name);
VIR_FREE(xml);
close(fd);
return (-1);
}
VIR_FREE(xml);
if (close(fd) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot save domain data"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("%s: cannot save domain data"), domain->name);
close(fd);
return (-1);
}
@@ -1007,19 +1011,22 @@ static int testDomainCoreDump(virDomainPtr domain,
GET_DOMAIN(domain, -1);
if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot save domain core"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("domain '%s' coredump: failed to open %s: %s"),
+ domain->name, to, strerror (errno));
return (-1);
}
if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot write header"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("domain '%s' coredump: failed to write header to %s:
%s"),
+ domain->name, to, strerror (errno));
close(fd);
return (-1);
}
if (close(fd) < 0) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("cannot save domain data"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("domain '%s' coredump: write failed: %s: %s"),
+ domain->name, to, strerror (errno));
close(fd);
return (-1);
}
@@ -1060,7 +1067,7 @@ static int testSetMemory(virDomainPtr domain,
GET_DOMAIN(domain, -1);
if (memory > privdom->def->maxmem) {
- testError(domain->conn, domain, NULL,
+ testError(domain->conn, NULL, NULL,
VIR_ERR_INVALID_ARG, __FUNCTION__);
return (-1);
}
@@ -1075,7 +1082,7 @@ static int testSetVcpus(virDomainPtr domain,
/* We allow more cpus in guest than host */
if (nrCpus > 32) {
- testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
return (-1);
}
@@ -1186,8 +1193,8 @@ static int testDomainCreate(virDomainPtr domain) {
GET_DOMAIN(domain, -1);
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Domain is already running"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Domain '%s' is already running"),
domain->name);
return (-1);
}
@@ -1201,8 +1208,8 @@ static int testDomainUndefine(virDomainPtr domain) {
GET_DOMAIN(domain, -1);
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
- testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Domain is still running"));
+ testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Domain '%s' is still running"), domain->name);
return (-1);
}
@@ -1237,7 +1244,7 @@ static char *testDomainGetSchedulerType(virDomainPtr domain,
*nparams = 1;
type = strdup("fair");
if (!type) {
- testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY,
"schedular");
+ testError(domain->conn, NULL, NULL, VIR_ERR_NO_MEMORY,
"schedular");
return (NULL);
}
return type;
@@ -1249,7 +1256,7 @@ static int testDomainGetSchedulerParams(virDomainPtr domain,
{
GET_DOMAIN(domain, -1);
if (*nparams != 1) {
- testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+ testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG,
"nparams");
return (-1);
}
strcpy(params[0].field, "weight");
@@ -1267,15 +1274,15 @@ static int testDomainSetSchedulerParams(virDomainPtr domain,
{
GET_DOMAIN(domain, -1);
if (nparams != 1) {
- testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+ testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG,
"nparams");
return (-1);
}
if (STRNEQ(params[0].field, "weight")) {
- testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG,
"field");
+ testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG, "field");
return (-1);
}
if (params[0].type != VIR_DOMAIN_SCHED_FIELD_UINT) {
- testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, "type");
+ testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG, "type");
return (-1);
}
/* XXX */
@@ -1441,8 +1448,8 @@ static int testNetworkUndefine(virNetworkPtr network) {
GET_NETWORK(network, -1);
if (virNetworkIsActive(privnet)) {
- testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
- _("Network is still running"));
+ testError(network->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Network '%s' is still running"),
network->name);
return (-1);
}
@@ -1456,8 +1463,8 @@ static int testNetworkStart(virNetworkPtr network) {
GET_NETWORK(network, -1);
if (virNetworkIsActive(privnet)) {
- testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR,
- _("Network is already running"));
+ testError(network->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("Network '%s' is already running"),
network->name);
return (-1);
}
@@ -1488,7 +1495,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) {
GET_NETWORK(network, NULL);
if (privnet->def->bridge &&
!(bridge = strdup(privnet->def->bridge))) {
- testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY,
"network");
+ testError(network->conn, NULL, NULL, VIR_ERR_NO_MEMORY, "network");
return NULL;
}
return bridge;
diff --git a/tests/undefine b/tests/undefine
index 3936e22..d745241 100755
--- a/tests/undefine
+++ b/tests/undefine
@@ -29,7 +29,7 @@ fail=0
virsh -q -c test:///default undefine test > out 2>&1
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
-libvir: Test error test: internal error Domain is still running
+libvir: Test error : internal error Domain 'test' is still running
error: Failed to undefine domain test
EOF
compare out exp || fail=1
--
1.6.0.2.307.gc4275
From 6e10cf8adc031e11adfdaf10553dc3a6b38ed0ab Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 10 Oct 2008 15:10:35 +0200
Subject: [PATCH] test.c (testError): Remove now-ignored dom and net parameters.
This is a sytnax-only change: removing the two unused parameters
and updating all callers:
* src/test.c (GET_DOMAIN, GET_NETWORK, testError)
(testBuildCapabilities, testOpenDefault, testOpenFromFile)
(testOpen, testGetHostname, testGetURI, testGetCapabilities)
(testLookupDomainByID, testLookupDomainByUUID)
(testLookupDomainByName, testResumeDomain, testPauseDomain)
(testShutdownDomain, testGetDomainInfo, testDomainSave)
(testDomainRestore, testDomainCoreDump, testGetOSType)
(testSetMemory, testSetVcpus, testListDefinedDomains)
(testNodeGetCellsFreeMemory, testDomainCreate)
(testDomainUndefine, testDomainGetSchedulerType)
(testDomainGetSchedulerParams, testDomainSetSchedulerParams)
(testLookupNetworkByUUID, testLookupNetworkByName)
(testListNetworks, testListDefinedNetworks, testNetworkUndefine)
(testNetworkStart, testNetworkGetBridgeName): Update callers.
---
src/test.c | 142 +++++++++++++++++++++++++++++------------------------------
1 files changed, 70 insertions(+), 72 deletions(-)
diff --git a/src/test.c b/src/test.c
index 7ca50eb..00d5541 100644
--- a/src/test.c
+++ b/src/test.c
@@ -88,8 +88,7 @@ static const virNodeInfo defaultNodeInfo = {
do { \
if ((privdom = virDomainFindByName(privconn->domains, \
(dom)->name)) == NULL) { \
- testError((dom)->conn, (dom), NULL, VIR_ERR_INVALID_ARG, \
- __FUNCTION__); \
+ testError((dom)->conn, VIR_ERR_INVALID_ARG, __FUNCTION__); \
return (ret); \
} \
} while (0)
@@ -102,8 +101,7 @@ static const virNodeInfo defaultNodeInfo = {
do { \
if ((privnet = virNetworkFindByName(privconn->networks, \
(net)->name)) == NULL) { \
- testError((net)->conn, NULL, (net), VIR_ERR_INVALID_ARG, \
- __FUNCTION__); \
+ testError((net)->conn, VIR_ERR_INVALID_ARG, __FUNCTION__); \
return (ret); \
} \
} while (0)
@@ -114,8 +112,8 @@ static const virNodeInfo defaultNodeInfo = {
privconn = (testConnPtr)conn->privateData;
-#define testError(conn, dom, net, code, fmt...) \
- __virReportErrorHelper(conn, VIR_FROM_TEST, code, __FILE__, \
+#define testError(conn, code, fmt...) \
+ __virReportErrorHelper(conn, VIR_FROM_TEST, code, __FILE__, \
__FUNCTION__, __LINE__, fmt)
static virCapsPtr
@@ -168,7 +166,7 @@ testBuildCapabilities(virConnectPtr conn) {
return caps;
no_memory:
- testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+ testError(conn, VIR_ERR_NO_MEMORY, NULL);
virCapabilitiesFree(caps);
return NULL;
}
@@ -209,13 +207,13 @@ static int testOpenDefault(virConnectPtr conn) {
virNetworkObjPtr netobj = NULL;
if (VIR_ALLOC(privconn) < 0) {
- testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "testConn");
+ testError(conn, VIR_ERR_NO_MEMORY, "testConn");
return VIR_DRV_OPEN_ERROR;
}
conn->privateData = privconn;
if (gettimeofday(&tv, NULL) < 0) {
- testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("getting time of
day"));
+ testError(NULL, VIR_ERR_INTERNAL_ERROR, _("getting time of day"));
goto error;
}
@@ -304,7 +302,7 @@ static int testOpenFromFile(virConnectPtr conn,
virDomainObjPtr dom;
testConnPtr privconn;
if (VIR_ALLOC(privconn) < 0) {
- testError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, "testConn");
+ testError(NULL, VIR_ERR_NO_MEMORY, "testConn");
return VIR_DRV_OPEN_ERROR;
}
conn->privateData = privconn;
@@ -313,14 +311,14 @@ static int testOpenFromFile(virConnectPtr conn,
goto error;
if ((fd = open(file, O_RDONLY)) < 0) {
- testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("loading host
definition file"));
+ testError(NULL, VIR_ERR_INTERNAL_ERROR, _("loading host definition
file"));
goto error;
}
if (!(xml = xmlReadFd(fd, file, NULL,
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
- testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("host"));
+ testError(NULL, VIR_ERR_INTERNAL_ERROR, _("host"));
goto error;
}
close(fd);
@@ -328,13 +326,13 @@ static int testOpenFromFile(virConnectPtr conn,
root = xmlDocGetRootElement(xml);
if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "node"))) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node"));
goto error;
}
ctxt = xmlXPathNewContext(xml);
if (ctxt == NULL) {
- testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("creating xpath
context"));
+ testError(NULL, VIR_ERR_INTERNAL_ERROR, _("creating xpath context"));
goto error;
}
@@ -349,7 +347,7 @@ static int testOpenFromFile(virConnectPtr conn,
if (ret == 0) {
nodeInfo->nodes = l;
} else if (ret == -2) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu numa
nodes"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node cpu numa nodes"));
goto error;
}
@@ -357,7 +355,7 @@ static int testOpenFromFile(virConnectPtr conn,
if (ret == 0) {
nodeInfo->sockets = l;
} else if (ret == -2) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu sockets"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node cpu sockets"));
goto error;
}
@@ -365,7 +363,7 @@ static int testOpenFromFile(virConnectPtr conn,
if (ret == 0) {
nodeInfo->cores = l;
} else if (ret == -2) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu cores"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node cpu cores"));
goto error;
}
@@ -373,7 +371,7 @@ static int testOpenFromFile(virConnectPtr conn,
if (ret == 0) {
nodeInfo->threads = l;
} else if (ret == -2) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu threads"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node cpu threads"));
goto error;
}
@@ -384,14 +382,14 @@ static int testOpenFromFile(virConnectPtr conn,
nodeInfo->cpus = l;
}
} else if (ret == -2) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node active cpu"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node active cpu"));
goto error;
}
ret = virXPathLong(conn, "string(/node/cpu/mhz[1])", ctxt, &l);
if (ret == 0) {
nodeInfo->mhz = l;
} else if (ret == -2) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node cpu mhz"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node cpu mhz"));
goto error;
}
@@ -406,13 +404,13 @@ static int testOpenFromFile(virConnectPtr conn,
if (ret == 0) {
nodeInfo->memory = l;
} else if (ret == -2) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node memory"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node memory"));
goto error;
}
ret = virXPathNodeSet(conn, "/node/domain", ctxt, &domains);
if (ret < 0) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node domain list"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node domain list"));
goto error;
}
@@ -423,7 +421,7 @@ static int testOpenFromFile(virConnectPtr conn,
char *absFile = testBuildFilename(file, relFile);
VIR_FREE(relFile);
if (!absFile) {
- testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving
domain filename"));
+ testError(NULL, VIR_ERR_INTERNAL_ERROR, _("resolving domain
filename"));
goto error;
}
def = virDomainDefParseFile(conn, privconn->caps, absFile);
@@ -449,7 +447,7 @@ static int testOpenFromFile(virConnectPtr conn,
ret = virXPathNodeSet(conn, "/node/network", ctxt, &networks);
if (ret < 0) {
- testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node network
list"));
+ testError(NULL, VIR_ERR_XML_ERROR, _("node network list"));
goto error;
}
for (i = 0 ; i < ret ; i++) {
@@ -459,7 +457,7 @@ static int testOpenFromFile(virConnectPtr conn,
char *absFile = testBuildFilename(file, relFile);
VIR_FREE(relFile);
if (!absFile) {
- testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving
network filename"));
+ testError(NULL, VIR_ERR_INTERNAL_ERROR, _("resolving network
filename"));
goto error;
}
@@ -536,7 +534,7 @@ static int testOpen(virConnectPtr conn,
if (!uri->path
|| uri->path[0] == '\0'
|| (uri->path[0] == '/' && uri->path[1] == '\0'))
{
- testError (NULL, NULL, NULL, VIR_ERR_INVALID_ARG,
+ testError (NULL, VIR_ERR_INVALID_ARG,
_("testOpen: supply a path or use test:///default"));
return VIR_DRV_OPEN_ERROR;
}
@@ -587,13 +585,13 @@ static char *testGetHostname (virConnectPtr conn)
r = gethostname (hostname, HOST_NAME_MAX+1);
if (r == -1) {
- testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, "%s",
+ testError (conn, VIR_ERR_SYSTEM_ERROR, "%s",
strerror (errno));
return NULL;
}
str = strdup (hostname);
if (str == NULL) {
- testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, "%s",
+ testError (conn, VIR_ERR_SYSTEM_ERROR, "%s",
strerror (errno));
return NULL;
}
@@ -606,7 +604,7 @@ static char * testGetURI (virConnectPtr conn)
GET_CONNECTION(conn);
if (asprintf (&uri, "test://%s", privconn->path) == -1) {
- testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, "%s",
+ testError (conn, VIR_ERR_SYSTEM_ERROR, "%s",
strerror (errno));
return NULL;
}
@@ -633,7 +631,7 @@ static char *testGetCapabilities (virConnectPtr conn)
GET_CONNECTION(conn);
if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL) {
- testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+ testError(conn, VIR_ERR_NO_MEMORY, NULL);
return NULL;
}
@@ -691,7 +689,7 @@ static virDomainPtr testLookupDomainByID(virConnectPtr conn,
GET_CONNECTION(conn);
if ((dom = virDomainFindByID(privconn->domains, id)) == NULL) {
- testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ testError (conn, VIR_ERR_NO_DOMAIN, NULL);
return NULL;
}
@@ -710,7 +708,7 @@ static virDomainPtr testLookupDomainByUUID(virConnectPtr conn,
GET_CONNECTION(conn);
if ((dom = virDomainFindByUUID(privconn->domains, uuid)) == NULL) {
- testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ testError (conn, VIR_ERR_NO_DOMAIN, NULL);
return NULL;
}
@@ -729,7 +727,7 @@ static virDomainPtr testLookupDomainByName(virConnectPtr conn,
GET_CONNECTION(conn);
if ((dom = virDomainFindByName(privconn->domains, name)) == NULL) {
- testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
+ testError (conn, VIR_ERR_NO_DOMAIN, NULL);
return NULL;
}
@@ -776,7 +774,7 @@ static int testResumeDomain (virDomainPtr domain)
GET_DOMAIN(domain, -1);
if (privdom->state != VIR_DOMAIN_PAUSED) {
- testError(domain->conn, NULL, NULL,
+ testError(domain->conn,
VIR_ERR_INTERNAL_ERROR, _("%s: domain not paused"),
domain->name);
return -1;
@@ -792,7 +790,7 @@ static int testPauseDomain (virDomainPtr domain)
if (privdom->state == VIR_DOMAIN_SHUTOFF ||
privdom->state == VIR_DOMAIN_PAUSED) {
- testError(domain->conn, NULL, NULL,
+ testError(domain->conn,
VIR_ERR_INTERNAL_ERROR, _("%s: domain not running"),
domain->name);
return -1;
@@ -807,7 +805,7 @@ static int testShutdownDomain (virDomainPtr domain)
GET_DOMAIN(domain, -1);
if (privdom->state == VIR_DOMAIN_SHUTOFF) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("%s: domain not running"), domain->name);
return -1;
}
@@ -864,7 +862,7 @@ static int testGetDomainInfo (virDomainPtr domain,
GET_DOMAIN(domain, -1);
if (gettimeofday(&tv, NULL) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("getting time of day"));
return (-1);
}
@@ -890,31 +888,31 @@ static int testDomainSave(virDomainPtr domain,
xml = testDomainDumpXML(domain, 0);
if (xml == NULL) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("%s: cannot allocate space for metadata"),
domain->name);
return (-1);
}
if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("%s: cannot save domain"), domain->name);
return (-1);
}
len = strlen(xml);
if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("%s: cannot write header"), domain->name);
close(fd);
return (-1);
}
if (safewrite(fd, (char*)&len, sizeof(len)) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("%s: cannot write metadata length"), domain->name);
close(fd);
return (-1);
}
if (safewrite(fd, xml, len) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("%s: cannot write metadata"), domain->name);
VIR_FREE(xml);
close(fd);
@@ -922,7 +920,7 @@ static int testDomainSave(virDomainPtr domain,
}
VIR_FREE(xml);
if (close(fd) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("%s: cannot save domain data"), domain->name);
close(fd);
return (-1);
@@ -946,41 +944,41 @@ static int testDomainRestore(virConnectPtr conn,
GET_CONNECTION(conn);
if ((fd = open(path, O_RDONLY)) < 0) {
- testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(conn, VIR_ERR_INTERNAL_ERROR,
_("cannot read domain image"));
return (-1);
}
if (read(fd, magic, sizeof(magic)) != sizeof(magic)) {
- testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(conn, VIR_ERR_INTERNAL_ERROR,
_("incomplete save header"));
close(fd);
return (-1);
}
if (memcmp(magic, TEST_SAVE_MAGIC, sizeof(magic))) {
- testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(conn, VIR_ERR_INTERNAL_ERROR,
_("mismatched header magic"));
close(fd);
return (-1);
}
if (read(fd, (char*)&len, sizeof(len)) != sizeof(len)) {
- testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to read metadata length"));
close(fd);
return (-1);
}
if (len < 1 || len > 8192) {
- testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(conn, VIR_ERR_INTERNAL_ERROR,
_("length of metadata out of range"));
close(fd);
return (-1);
}
if (VIR_ALLOC_N(xml, len+1) < 0) {
- testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "xml");
+ testError(conn, VIR_ERR_NO_MEMORY, "xml");
close(fd);
return (-1);
}
if (read(fd, xml, len) != len) {
- testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(conn, VIR_ERR_INTERNAL_ERROR,
_("incomplete metdata"));
close(fd);
return (-1);
@@ -1011,20 +1009,20 @@ static int testDomainCoreDump(virDomainPtr domain,
GET_DOMAIN(domain, -1);
if ((fd = open(to, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("domain '%s' coredump: failed to open %s: %s"),
domain->name, to, strerror (errno));
return (-1);
}
if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("domain '%s' coredump: failed to write header to %s:
%s"),
domain->name, to, strerror (errno));
close(fd);
return (-1);
}
if (close(fd) < 0) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("domain '%s' coredump: write failed: %s: %s"),
domain->name, to, strerror (errno));
close(fd);
@@ -1041,7 +1039,7 @@ static int testDomainCoreDump(virDomainPtr domain,
static char *testGetOSType(virDomainPtr dom) {
char *ret = strdup("linux");
if (!ret)
- testError(dom->conn, dom, NULL, VIR_ERR_NO_MEMORY, NULL);
+ testError(dom->conn, VIR_ERR_NO_MEMORY, NULL);
return ret;
}
@@ -1067,7 +1065,7 @@ static int testSetMemory(virDomainPtr domain,
GET_DOMAIN(domain, -1);
if (memory > privdom->def->maxmem) {
- testError(domain->conn, NULL, NULL,
+ testError(domain->conn,
VIR_ERR_INVALID_ARG, __FUNCTION__);
return (-1);
}
@@ -1082,7 +1080,7 @@ static int testSetVcpus(virDomainPtr domain,
/* We allow more cpus in guest than host */
if (nrCpus > 32) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__);
+ testError(domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return (-1);
}
@@ -1135,7 +1133,7 @@ static int testListDefinedDomains(virConnectPtr conn,
return n;
no_memory:
- testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+ testError(conn, VIR_ERR_NO_MEMORY, NULL);
for (n = 0 ; n < maxnames ; n++)
VIR_FREE(names[n]);
return -1;
@@ -1174,7 +1172,7 @@ static int testNodeGetCellsFreeMemory(virConnectPtr conn,
GET_CONNECTION(conn);
if (startCell > privconn->numCells) {
- testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG,
+ testError(conn, VIR_ERR_INVALID_ARG,
_("Range exceeds available cells"));
return -1;
}
@@ -1193,7 +1191,7 @@ static int testDomainCreate(virDomainPtr domain) {
GET_DOMAIN(domain, -1);
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("Domain '%s' is already running"),
domain->name);
return (-1);
}
@@ -1208,7 +1206,7 @@ static int testDomainUndefine(virDomainPtr domain) {
GET_DOMAIN(domain, -1);
if (privdom->state != VIR_DOMAIN_SHUTOFF) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(domain->conn, VIR_ERR_INTERNAL_ERROR,
_("Domain '%s' is still running"), domain->name);
return (-1);
}
@@ -1244,7 +1242,7 @@ static char *testDomainGetSchedulerType(virDomainPtr domain,
*nparams = 1;
type = strdup("fair");
if (!type) {
- testError(domain->conn, NULL, NULL, VIR_ERR_NO_MEMORY,
"schedular");
+ testError(domain->conn, VIR_ERR_NO_MEMORY, "schedular");
return (NULL);
}
return type;
@@ -1256,7 +1254,7 @@ static int testDomainGetSchedulerParams(virDomainPtr domain,
{
GET_DOMAIN(domain, -1);
if (*nparams != 1) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+ testError(domain->conn, VIR_ERR_INVALID_ARG, "nparams");
return (-1);
}
strcpy(params[0].field, "weight");
@@ -1274,15 +1272,15 @@ static int testDomainSetSchedulerParams(virDomainPtr domain,
{
GET_DOMAIN(domain, -1);
if (nparams != 1) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG,
"nparams");
+ testError(domain->conn, VIR_ERR_INVALID_ARG, "nparams");
return (-1);
}
if (STRNEQ(params[0].field, "weight")) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG, "field");
+ testError(domain->conn, VIR_ERR_INVALID_ARG, "field");
return (-1);
}
if (params[0].type != VIR_DOMAIN_SCHED_FIELD_UINT) {
- testError(domain->conn, NULL, NULL, VIR_ERR_INVALID_ARG, "type");
+ testError(domain->conn, VIR_ERR_INVALID_ARG, "type");
return (-1);
}
/* XXX */
@@ -1314,7 +1312,7 @@ static virNetworkPtr testLookupNetworkByUUID(virConnectPtr conn,
GET_CONNECTION(conn);
if ((net = virNetworkFindByUUID(privconn->networks, uuid)) == NULL) {
- testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL);
+ testError (conn, VIR_ERR_NO_NETWORK, NULL);
return NULL;
}
@@ -1328,7 +1326,7 @@ static virNetworkPtr testLookupNetworkByName(virConnectPtr conn,
GET_CONNECTION(conn);
if ((net = virNetworkFindByName(privconn->networks, name)) == NULL) {
- testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL);
+ testError (conn, VIR_ERR_NO_NETWORK, NULL);
return NULL;
}
@@ -1366,7 +1364,7 @@ static int testListNetworks(virConnectPtr conn, char **const names,
int nnames)
return n;
no_memory:
- testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+ testError(conn, VIR_ERR_NO_MEMORY, NULL);
for (n = 0 ; n < nnames ; n++)
VIR_FREE(names[n]);
return (-1);
@@ -1402,7 +1400,7 @@ static int testListDefinedNetworks(virConnectPtr conn, char **const
names, int n
return n;
no_memory:
- testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL);
+ testError(conn, VIR_ERR_NO_MEMORY, NULL);
for (n = 0 ; n < nnames ; n++)
VIR_FREE(names[n]);
return (-1);
@@ -1448,7 +1446,7 @@ static int testNetworkUndefine(virNetworkPtr network) {
GET_NETWORK(network, -1);
if (virNetworkIsActive(privnet)) {
- testError(network->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(network->conn, VIR_ERR_INTERNAL_ERROR,
_("Network '%s' is still running"),
network->name);
return (-1);
}
@@ -1463,7 +1461,7 @@ static int testNetworkStart(virNetworkPtr network) {
GET_NETWORK(network, -1);
if (virNetworkIsActive(privnet)) {
- testError(network->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ testError(network->conn, VIR_ERR_INTERNAL_ERROR,
_("Network '%s' is already running"),
network->name);
return (-1);
}
@@ -1495,7 +1493,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) {
GET_NETWORK(network, NULL);
if (privnet->def->bridge &&
!(bridge = strdup(privnet->def->bridge))) {
- testError(network->conn, NULL, NULL, VIR_ERR_NO_MEMORY, "network");
+ testError(network->conn, VIR_ERR_NO_MEMORY, "network");
return NULL;
}
return bridge;
--
1.6.0.2.307.gc4275
9:16 a.m.
On Fri, Oct 10, 2008 at 03:16:09PM +0200, Jim Meyering wrote:
Jim Meyering <jim(a)meyering.net> wrote:
> "Daniel P. Berrange" <berrange(a)redhat.com> wrote:
Here's the change to adjust src/test.c.
Two change sets:
- Fix the "make check" failure by adjusting uses of testError,
including domain->name (and a few net->name) strings via format.
- Remove now-ignored dom and net parameters.
Sounds fine to me.
I saw 2 concatenated patches (mutt here), I didn't spot any problem, +1
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
9:58 a.m.
On Fri, Oct 10, 2008 at 03:16:09PM +0200, Jim Meyering wrote:
Jim Meyering <jim(a)meyering.net> wrote:
> "Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> ...
>> Some parts of qemu supply it, many other parts do not since they
>> have no access to the virDomainPtr object. This is one of the
>> reasons for deprecating this field - it was impossible to reliably
>> provide it when raising errors.
>>
>> In this particular test case error, we are better off supplying the
>> domain name in the format string - it'll improve the error message
>> to have it placed in context of the description
>>
>> eg, instead of
>>
>> libvir: Test error test: internal error Domain is still running
>>
>>
>> We'd have
>>
>> libvir: Test error: internal error Domain 'test' is still running
>>
>> Which could be done by changing
>>
>> if (privdom->state != VIR_DOMAIN_SHUTOFF) {
>> testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
>> _("Domain is still running"));
>>
>>
>> to be
>>
>> if (privdom->state != VIR_DOMAIN_SHUTOFF) {
>> testError(domain->conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
>> _("Domain '%s' is still running"),
domain->name);
>
> Yep. I began doing that about 15 minutes ago ;-)
Here's the change to adjust src/test.c.
Two change sets:
- Fix the "make check" failure by adjusting uses of testError,
including domain->name (and a few net->name) strings via format.
- Remove now-ignored dom and net parameters.
ACK, these both look good.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5922
days inactive
5924
days old
12 comments
4 participants
participants (4)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering