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(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c index 6a35064..e8d248f 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -141,30 +141,9 @@ VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST, "usb", "pci") -static void virDomainReportError(virConnectPtr conn, - int code, const char *fmt, ...) - ATTRIBUTE_FORMAT(printf, 3, 4); - -static void virDomainReportError(virConnectPtr conn, - int code, const char *fmt, ...) -{ - va_list args; - char errorMessage[1024]; - const char *virerr; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - - virerr = __virErrorMsg(code, (errorMessage[0] ? errorMessage : NULL)); - __virRaiseError(conn, NULL, NULL, VIR_FROM_DOMAIN, code, VIR_ERR_ERROR, - virerr, errorMessage, NULL, -1, -1, virerr, errorMessage); -} - +#define virDomainReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, 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 d4b8a74..3efda2c 100644 --- a/src/hash.c +++ b/src/hash.c @@ -31,6 +31,10 @@ /* #define DEBUG_GROW */ +#define virHashError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) + /* * A single entry in the hash table */ @@ -596,28 +600,6 @@ void *virHashSearch(virHashTablePtr table, virHashSearcher iter, const void *dat ************************************************************************/ /** - * virHashError: - * @conn: the connection if available - * @error: the error number - * @info: extra information string - * - * Handle an error at the connection level - */ -static void -virHashError(virConnectPtr conn, virErrorNumber error, const char *info) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = __virErrorMsg(error, info); - __virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, error, VIR_ERR_ERROR, - errmsg, info, NULL, 0, 0, errmsg, info); -} - - -/** * virDomainFreeName: * @domain: a domain object * diff --git a/src/internal.h b/src/internal.h index a3d48fa..2ae764d 100644 --- a/src/internal.h +++ b/src/internal.h @@ -313,6 +313,12 @@ 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, + const char *filename ATTRIBUTE_UNUSED, + const char *funcname ATTRIBUTE_UNUSED, + long long linenr ATTRIBUTE_UNUSED, + const char *fmt, ...) + ATTRIBUTE_FORMAT(printf, 7, 8); /************************************************************************ * * diff --git a/src/lxc_conf.c b/src/lxc_conf.c index bea6bbe..d522c4e 100644 --- a/src/lxc_conf.c +++ b/src/lxc_conf.c @@ -30,27 +30,6 @@ #include "lxc_conf.h" /* Functions */ -void lxcError(virConnectPtr conn, virDomainPtr dom, int code, - const char *fmt, ...) -{ - va_list args; - char errorMessage[1024]; - const char *codeErrorMessage; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - - codeErrorMessage = __virErrorMsg(code, fmt); - __virRaiseError(conn, dom, NULL, VIR_FROM_LXC, code, VIR_ERR_ERROR, - codeErrorMessage, errorMessage, NULL, 0, 0, - codeErrorMessage, errorMessage); -} - virCapsPtr lxcCapsInit(void) { struct utsname utsname; diff --git a/src/lxc_conf.h b/src/lxc_conf.h index 00442b4..4d57046 100644 --- a/src/lxc_conf.h +++ b/src/lxc_conf.h @@ -49,10 +49,9 @@ struct __lxc_driver { int lxcLoadDriverConfig(lxc_driver_t *driver); virCapsPtr lxcCapsInit(void); -void lxcError(virConnectPtr conn, - virDomainPtr dom, - int code, const char *fmt, ...) - ATTRIBUTE_FORMAT(printf,4,5); +#define lxcError(conn, dom, code, fmt...) \ + __virReportErrorHelper(conn, 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 4627fba..800ead9 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -48,26 +48,9 @@ VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, "none", "nat", "route" ) -static void virNetworkReportError(virConnectPtr conn, - int code, const char *fmt, ...) -{ - va_list args; - char errorMessage[1024]; - const char *virerr; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - - virerr = __virErrorMsg(code, (errorMessage[0] ? errorMessage : NULL)); - __virRaiseError(conn, NULL, NULL, VIR_FROM_NETWORK, code, VIR_ERR_ERROR, - virerr, errorMessage, NULL, -1, -1, virerr, errorMessage); -} - +#define virNetworkReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_NETWORK, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjPtr nets, const unsigned char *uuid) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index aa8493a..c48c219 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -53,28 +53,6 @@ static int openvzGetVPSUUID(int vpsid, char *uuidstr); static int openvzLocateConfFile(int vpsid, char *conffile, int maxlen); static int openvzAssignUUIDs(void); -void -openvzError (virConnectPtr conn, virErrorNumber code, const char *fmt, ...) -{ - va_list args; - char errorMessage[1024]; - const char *errmsg; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - - errmsg = __virErrorMsg(code, (errorMessage[0] ? errorMessage : NULL)); - __virRaiseError (conn, NULL, NULL, VIR_FROM_OPENVZ, - code, VIR_ERR_ERROR, errmsg, errorMessage, NULL, 0, 0, - errmsg, errorMessage); -} - - int strtoI(const char *str) { diff --git a/src/openvz_conf.h b/src/openvz_conf.h index 68c00ea..fbe4f69 100644 --- a/src/openvz_conf.h +++ b/src/openvz_conf.h @@ -41,6 +41,11 @@ 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) + + /* OpenVZ commands - Replace with wrapper scripts later? */ #define VZLIST "vzlist" #define VZCTL "vzctl" @@ -50,8 +55,6 @@ struct openvz_driver { virDomainObjPtr domains; }; -void openvzError (virConnectPtr conn, virErrorNumber code, const char *fmt, ...) - ATTRIBUTE_FORMAT(printf, 3, 4); int openvz_readline(int fd, char *ptr, int maxlen); int openvzReadConfigParam(int vpsid ,const char * param, char *value, int maxlen); virCapsPtr openvzCapsInit(void); diff --git a/src/proxy_internal.c b/src/proxy_internal.c index 1378559..0186eba 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -92,26 +92,9 @@ struct xenUnifiedDriver xenProxyDriver = { * * ************************************************************************/ -/** - * virProxyError: - * @conn: the connection if available - * @error: the error number - * @info: extra information string - * - * Handle an error at the xend daemon interface - */ -static void -virProxyError(virConnectPtr conn, virErrorNumber error, const char *info) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = __virErrorMsg(error, info); - __virRaiseError(conn, NULL, NULL, VIR_FROM_PROXY, error, VIR_ERR_ERROR, - errmsg, info, NULL, 0, 0, errmsg, info); -} +#define virProxyError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_PROXY, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) /************************************************************************ * * diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 5c41f9a..4c01f5c 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -59,27 +59,6 @@ VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, #define qemudLog(level, msg...) fprintf(stderr, msg) -void qemudReportError(virConnectPtr conn, - virDomainPtr dom, - virNetworkPtr net, - int code, const char *fmt, ...) { - va_list args; - char errorMessage[1024]; - const char *virerr; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - - virerr = __virErrorMsg(code, (errorMessage[0] ? errorMessage : NULL)); - __virRaiseError(conn, dom, net, VIR_FROM_QEMU, code, VIR_ERR_ERROR, - virerr, errorMessage, NULL, -1, -1, virerr, errorMessage); -} - int qemudLoadDriverConfig(struct qemud_driver *driver, const char *filename) { virConfPtr conf; diff --git a/src/qemu_conf.h b/src/qemu_conf.h index 88dfade..5ea57f0 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -71,11 +71,9 @@ struct qemud_driver { }; -void qemudReportError(virConnectPtr conn, - virDomainPtr dom, - virNetworkPtr net, - int code, const char *fmt, ...) - ATTRIBUTE_FORMAT(printf,5,6); +#define qemudReportError(conn, dom, net, code, fmt...) \ + __virReportErrorHelper(conn, 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 f068cb4..ed5bddc 100644 --- a/src/qparams.c +++ b/src/qparams.c @@ -30,13 +30,9 @@ #include "memory.h" #include "qparams.h" -static void -qparam_report_oom(void) -{ - const char *virerr = __virErrorMsg(VIR_ERR_NO_MEMORY, NULL); - __virRaiseError(NULL, NULL, NULL, VIR_FROM_NONE, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR, - virerr, NULL, NULL, -1, -1, virerr, NULL); -} +#define qparam_report_oom(void) \ + __virReportErrorHelper(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 b607e41..c168754 100644 --- a/src/sexpr.c +++ b/src/sexpr.c @@ -23,26 +23,9 @@ #include "util.h" #include "memory.h" -/** - * virSexprError: - * @conn: the connection if available - * @error: the error number - * @info: extra information string - * - * Handle an error in the S-Expression code - */ -static void -virSexprError(virErrorNumber error, const char *info) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = __virErrorMsg(error, info); - __virRaiseError(NULL, NULL, NULL, VIR_FROM_SEXPR, error, VIR_ERR_ERROR, - errmsg, info, NULL, 0, 0, errmsg, info); -} +#define virSexprError(code, fmt...) \ + __virReportErrorHelper(NULL, VIR_FROM_SEXPR, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) /** * sexpr_new: @@ -278,7 +261,7 @@ sexpr2string(const struct sexpr * sexpr, char *buffer, size_t n_buffer) return (ret); error: buffer[n_buffer - 1] = 0; - virSexprError(VIR_ERR_SEXPR_SERIAL, buffer); + virSexprError(VIR_ERR_SEXPR_SERIAL, "%s", buffer); return (0); } diff --git a/src/storage_conf.c b/src/storage_conf.c index 29d74c8..5884e9c 100644 --- a/src/storage_conf.c +++ b/src/storage_conf.c @@ -50,25 +50,6 @@ #define virStorageLog(msg...) fprintf(stderr, msg) void -virStorageReportError(virConnectPtr conn, int code, const char *fmt, ...) { - va_list args; - char errorMessage[1024]; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - virStorageLog("%s", errorMessage); - __virRaiseError(conn, NULL, NULL, VIR_FROM_STORAGE, code, VIR_ERR_ERROR, - NULL, NULL, NULL, -1, -1, "%s", errorMessage); -} - - - -void virStorageVolDefFree(virStorageVolDefPtr def) { int i; VIR_FREE(def->name); diff --git a/src/storage_conf.h b/src/storage_conf.h index 3500039..c86bf4b 100644 --- a/src/storage_conf.h +++ b/src/storage_conf.h @@ -247,10 +247,9 @@ static inline int virStoragePoolObjIsActive(virStoragePoolObjPtr pool) { return pool->active; } -void virStorageReportError(virConnectPtr conn, - int code, - const char *fmt, ...) - ATTRIBUTE_FORMAT(printf, 3, 4); +#define virStorageReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_STORAGE, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) int virStoragePoolObjScanConfigs(virStorageDriverStatePtr driver); diff --git a/src/test.c b/src/test.c index 69c9090..ad51736 100644 --- a/src/test.c +++ b/src/test.c @@ -114,22 +114,9 @@ static const virNodeInfo defaultNodeInfo = { privconn = (testConnPtr)conn->privateData; -static void -testError(virConnectPtr con, - virDomainPtr dom, - virNetworkPtr net, - virErrorNumber error, - const char *info) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = __virErrorMsg(error, info); - __virRaiseError(con, dom, net, VIR_FROM_TEST, error, VIR_ERR_ERROR, - errmsg, info, NULL, 0, 0, errmsg, info, 0); -} +#define testError(conn, dom, net, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_TEST, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) static virCapsPtr testBuildCapabilities(virConnectPtr conn) { @@ -600,12 +587,14 @@ static char *testGetHostname (virConnectPtr conn) r = gethostname (hostname, HOST_NAME_MAX+1); if (r == -1) { - testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, "%s", + strerror (errno)); return NULL; } str = strdup (hostname); if (str == NULL) { - testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, "%s", + strerror (errno)); return NULL; } return str; @@ -617,7 +606,8 @@ static char * testGetURI (virConnectPtr conn) GET_CONNECTION(conn); if (asprintf (&uri, "test://%s", privconn->path) == -1) { - testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, "%s", + strerror (errno)); return NULL; } return uri; diff --git a/src/util.c b/src/util.c index ca14be1..ebe86b2 100644 --- a/src/util.c +++ b/src/util.c @@ -62,32 +62,13 @@ # define MIN(a, b) ((a) < (b) ? (a) : (b)) #endif -#define MAX_ERROR_LEN 1024 - #define virLog(msg...) fprintf(stderr, msg) #ifndef PROXY -static void -ReportError(virConnectPtr conn, - int code, const char *fmt, ...) - ATTRIBUTE_FORMAT(printf, 3, 4); - -static void -ReportError(virConnectPtr conn, - int code, const char *fmt, ...) { - va_list args; - char errorMessage[MAX_ERROR_LEN]; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, MAX_ERROR_LEN-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - __virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, code, VIR_ERR_ERROR, - NULL, NULL, NULL, -1, -1, "%s", errorMessage); -} + +#define ReportError(conn, code, fmt...) \ + __virReportErrorHelper(conn, 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 4aa7f42..21c7339 100644 --- a/src/virterror.c +++ b/src/virterror.c @@ -722,3 +722,44 @@ __virErrorMsg(virErrorNumber error, const char *info) } return (errmsg); } + +/** + * __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 + * @errcode: the virErrorNumber code for the error + * @filename: Source file error is dispatched from + * @funcname: Function error is dispatched from + * @linenr: Line number error is dispatched from + * @msg: the message to display/transmit + * @...: extra parameters for the message display + * + * Helper function to do most of the grunt work for individual driver + * ReportError + */ +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, ...) +{ + va_list args; + char errorMessage[1024]; + const char *virerr; + + if (fmt) { + va_start(args, fmt); + vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); + va_end(args); + } else { + errorMessage[0] = '\0'; + } + + virerr = __virErrorMsg(errcode, (errorMessage[0] ? errorMessage : NULL)); + __virRaiseError(conn, NULL, NULL, 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 c3aaa60..f498cc3 100644 --- a/src/xen_internal.c +++ b/src/xen_internal.c @@ -717,28 +717,10 @@ struct xenUnifiedDriver xenHypervisorDriver = { }; #endif /* !PROXY */ -/** - * virXenError: - * @conn: connection, if known - * @error: the error number - * @info: extra information string - * @value: extra information number - * - * Handle an error at the xend daemon interface - */ -static void -virXenError(virConnectPtr conn, - virErrorNumber error, const char *info, int value) -{ - const char *errmsg; - - if ((error == VIR_ERR_OK) || (in_init != 0)) - return; - - errmsg = __virErrorMsg(error, info); - __virRaiseError(conn, NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR, - errmsg, info, NULL, value, 0, errmsg, info, value); -} +#define virXenError(conn, code, fmt...) \ + if (in_init == 0) \ + __virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) #ifndef PROXY @@ -781,29 +763,6 @@ virXenErrorFunc(virConnectPtr conn, #endif /* PROXY */ /** - * virXenPerror: - * @conn: the connection (if available) - * @msg: name of system call or file (as in perror(3)) - * - * Raise error from a failed system call, using errno as the source. - */ -static void -virXenPerror (virConnectPtr conn, const char *msg) -{ - char *msg_s; - - if (VIR_ALLOC_N(msg_s, strlen (msg) + 10) == 0) { - strcpy (msg_s, msg); - strcat (msg_s, ": %s"); - } - - __virRaiseError (conn, NULL, NULL, - VIR_FROM_XEN, VIR_ERR_SYSTEM_ERROR, VIR_ERR_ERROR, - msg, NULL, NULL, errno, 0, - msg_s ? msg_s : msg, strerror (errno)); -} - -/** * xenHypervisorDoV0Op: * @handle: the handle to the Xen hypervisor * @op: pointer to the hypervisor operation structure @@ -825,17 +784,18 @@ xenHypervisorDoV0Op(int handle, xen_op_v0 * op) hc.arg[0] = (unsigned long) op; if (lock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " locking", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " locking"); return (-1); } ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc); if (ret < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl ", xen_ioctl_hypercall_cmd); + virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl %d", + xen_ioctl_hypercall_cmd); } if (unlock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " releasing", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " releasing"); ret = -1; } @@ -866,17 +826,18 @@ xenHypervisorDoV1Op(int handle, xen_op_v1* op) hc.arg[0] = (unsigned long) op; if (lock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " locking", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " locking"); return (-1); } ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc); if (ret < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl ", xen_ioctl_hypercall_cmd); + virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl %d", + xen_ioctl_hypercall_cmd); } if (unlock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " releasing", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " releasing"); ret = -1; } @@ -908,17 +869,18 @@ xenHypervisorDoV2Sys(int handle, xen_op_v2_sys* op) hc.arg[0] = (unsigned long) op; if (lock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " locking", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " locking"); return (-1); } ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc); if (ret < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " sys ioctl ", xen_ioctl_hypercall_cmd); + virXenError(NULL, VIR_ERR_XEN_CALL, " sys ioctl %d", + xen_ioctl_hypercall_cmd); } if (unlock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " releasing", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " releasing"); ret = -1; } @@ -950,17 +912,18 @@ xenHypervisorDoV2Dom(int handle, xen_op_v2_dom* op) hc.arg[0] = (unsigned long) op; if (lock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " locking", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " locking"); return (-1); } ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc); if (ret < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl ", xen_ioctl_hypercall_cmd); + virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl %d", + xen_ioctl_hypercall_cmd); } if (unlock_pages(op, sizeof(dom0_op_t)) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " releasing", sizeof(*op)); + virXenError(NULL, VIR_ERR_XEN_CALL, " releasing"); ret = -1; } @@ -989,8 +952,7 @@ virXen_getdomaininfolist(int handle, int first_domain, int maxids, if (lock_pages(XEN_GETDOMAININFOLIST_DATA(dominfos), XEN_GETDOMAININFO_SIZE * maxids) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " locking", - XEN_GETDOMAININFO_SIZE * maxids); + virXenError(NULL, VIR_ERR_XEN_CALL, " locking"); return (-1); } if (hypervisor_version > 1) { @@ -1045,8 +1007,7 @@ virXen_getdomaininfolist(int handle, int first_domain, int maxids, } if (unlock_pages(XEN_GETDOMAININFOLIST_DATA(dominfos), XEN_GETDOMAININFO_SIZE * maxids) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " release", - XEN_GETDOMAININFO_SIZE * maxids); + virXenError(NULL, VIR_ERR_XEN_CALL, " release"); ret = -1; } return(ret); @@ -1644,7 +1605,7 @@ virXen_setvcpumap(int handle, int id, unsigned int vcpu, xen_op_v2_dom op; if (lock_pages(cpumap, maplen) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " locking", maplen); + virXenError(NULL, VIR_ERR_XEN_CALL, " locking"); return (-1); } memset(&op, 0, sizeof(op)); @@ -1680,7 +1641,7 @@ virXen_setvcpumap(int handle, int id, unsigned int vcpu, VIR_FREE(new); if (unlock_pages(cpumap, maplen) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " release", maplen); + virXenError(NULL, VIR_ERR_XEN_CALL, " release"); ret = -1; } } else { @@ -1777,7 +1738,7 @@ virXen_getvcpusinfo(int handle, int id, unsigned int vcpu, virVcpuInfoPtr ipt, } if ((cpumap != NULL) && (maplen > 0)) { if (lock_pages(cpumap, maplen) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " locking", maplen); + virXenError(NULL, VIR_ERR_XEN_CALL, " locking"); return (-1); } memset(cpumap, 0, maplen); @@ -1795,7 +1756,7 @@ virXen_getvcpusinfo(int handle, int id, unsigned int vcpu, virVcpuInfoPtr ipt, } ret = xenHypervisorDoV2Dom(handle, &op); if (unlock_pages(cpumap, maplen) < 0) { - virXenError(NULL, VIR_ERR_XEN_CALL, " release", maplen); + virXenError(NULL, VIR_ERR_XEN_CALL, " release"); ret = -1; } } @@ -1891,7 +1852,7 @@ xenHypervisorInit(void) char error[100]; regerror (errcode, &flags_hvm_rec, error, sizeof error); regfree (&flags_hvm_rec); - virXenError (NULL, VIR_ERR_INTERNAL_ERROR, error, 0); + virXenError (NULL, VIR_ERR_INTERNAL_ERROR, "%s", error); in_init = 0; return -1; } @@ -1901,7 +1862,7 @@ xenHypervisorInit(void) regerror (errcode, &flags_pae_rec, error, sizeof error); regfree (&flags_pae_rec); regfree (&flags_hvm_rec); - virXenError (NULL, VIR_ERR_INTERNAL_ERROR, error, 0); + virXenError (NULL, VIR_ERR_INTERNAL_ERROR, "%s", error); in_init = 0; return -1; } @@ -1912,7 +1873,7 @@ xenHypervisorInit(void) regfree (&xen_cap_rec); regfree (&flags_pae_rec); regfree (&flags_hvm_rec); - virXenError (NULL, VIR_ERR_INTERNAL_ERROR, error, 0); + virXenError (NULL, VIR_ERR_INTERNAL_ERROR, "%s", error); in_init = 0; return -1; } @@ -1966,7 +1927,7 @@ xenHypervisorInit(void) */ hypervisor_version = -1; - virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl ", IOCTL_PRIVCMD_HYPERCALL); + virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl %lu", IOCTL_PRIVCMD_HYPERCALL); close(fd); in_init = 0; return(-1); @@ -1980,7 +1941,7 @@ xenHypervisorInit(void) hypervisor_version = 2; if (VIR_ALLOC(ipt) < 0) { - virXenError(NULL, VIR_ERR_NO_MEMORY, __FUNCTION__, 0); + virXenError(NULL, VIR_ERR_NO_MEMORY, NULL); return(-1); } /* Currently consider RHEL5.0 Fedora7, xen-3.1, and xen-unstable */ @@ -2043,7 +2004,7 @@ xenHypervisorInit(void) DEBUG0("Failed to find any Xen hypervisor method\n"); hypervisor_version = -1; - virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl ", IOCTL_PRIVCMD_HYPERCALL); + virXenError(NULL, VIR_ERR_XEN_CALL, " ioctl %lu", IOCTL_PRIVCMD_HYPERCALL); close(fd); in_init = 0; VIR_FREE(ipt); @@ -2083,7 +2044,7 @@ xenHypervisorOpen(virConnectPtr conn, ret = open(XEN_HYPERVISOR_SOCKET, O_RDWR); if (ret < 0) { - virXenError(conn, VIR_ERR_NO_XEN, XEN_HYPERVISOR_SOCKET, 0); + virXenError(conn, VIR_ERR_NO_XEN, "%s", XEN_HYPERVISOR_SOCKET); return (-1); } @@ -2414,7 +2375,7 @@ xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, return caps; no_memory: - virXenError(NULL, VIR_ERR_NO_MEMORY, __FUNCTION__, 0); + virXenError(NULL, VIR_ERR_NO_MEMORY, NULL); virCapabilitiesFree(caps); return NULL; } @@ -2437,7 +2398,8 @@ xenHypervisorMakeCapabilities(virConnectPtr conn) cpuinfo = fopen ("/proc/cpuinfo", "r"); if (cpuinfo == NULL) { if (errno != ENOENT) { - virXenPerror (NULL, "/proc/cpuinfo"); + virXenError (conn, VIR_ERR_SYSTEM_ERROR, + "/proc/cpuinfo: %s", strerror(errno)); return NULL; } } @@ -2446,7 +2408,9 @@ xenHypervisorMakeCapabilities(virConnectPtr conn) if (capabilities == NULL) { if (errno != ENOENT) { fclose(cpuinfo); - virXenPerror (NULL, "/sys/hypervisor/properties/capabilities"); + virXenError (conn, VIR_ERR_SYSTEM_ERROR, + "/sys/hypervisor/properties/capabilities: %s", + strerror(errno)); return NULL; } } @@ -2479,7 +2443,7 @@ xenHypervisorGetCapabilities (virConnectPtr conn) char *xml; if (!(xml = virCapabilitiesFormatXML(priv->caps))) { - virXenError(conn, VIR_ERR_NO_MEMORY, NULL, 0); + virXenError(conn, VIR_ERR_NO_MEMORY, NULL); return NULL; } diff --git a/src/xen_unified.c b/src/xen_unified.c index dae68d3..016181e 100644 --- a/src/xen_unified.c +++ b/src/xen_unified.c @@ -58,23 +58,9 @@ static struct xenUnifiedDriver *drivers[XEN_UNIFIED_NR_DRIVERS] = { [XEN_UNIFIED_XM_OFFSET] = &xenXMDriver, }; -/** - * xenUnifiedError: - * @conn: the connection - * @error: the error number - * @info: extra information string - * - * Handle an error at the xend daemon interface - */ -static void -xenUnifiedError (virConnectPtr conn, virErrorNumber error, const char *info) -{ - const char *errmsg; - - errmsg = __virErrorMsg (error, info); - __virRaiseError (conn, NULL, NULL, VIR_FROM_XEN, error, VIR_ERR_ERROR, - errmsg, info, NULL, 0, 0, errmsg, info); -} +#define xenUnifiedError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) /* * Helper functions currently used in the NUMA code @@ -418,12 +404,12 @@ xenUnifiedGetHostname (virConnectPtr conn) r = gethostname (hostname, HOST_NAME_MAX+1); if (r == -1) { - xenUnifiedError (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + xenUnifiedError (conn, VIR_ERR_SYSTEM_ERROR, "%s", strerror(errno)); return NULL; } str = strdup (hostname); if (str == NULL) { - xenUnifiedError (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + xenUnifiedError (conn, VIR_ERR_SYSTEM_ERROR, "%s", strerror(errno)); return NULL; } return str; diff --git a/src/xend_internal.c b/src/xend_internal.c index 0125913..2d10e11 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -104,61 +104,12 @@ virDomainXMLDevID(virDomainPtr domain, int ref_len); #endif -static void virXendError(virConnectPtr conn, virErrorNumber error, - const char *fmt, ...) - ATTRIBUTE_FORMAT(printf,3,4); +#define virXendError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_XEND, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) -#define MAX_ERROR_MESSAGE_LEN 1024 - -/** - * virXendError: - * @conn: the connection if available - * @error: the error number - * @fmt: format string followed by variable args - * - * Handle an error at the xend daemon interface - */ -static void -virXendError(virConnectPtr conn, virErrorNumber error, - const char *fmt, ...) -{ - va_list args; - char msg[MAX_ERROR_MESSAGE_LEN]; - const char *msg2; - - if (fmt) { - va_start (args, fmt); - vsnprintf (msg, sizeof (msg), fmt, args); - va_end (args); - } else { - msg[0] = '\0'; - } - - msg2 = __virErrorMsg (error, fmt ? msg : NULL); - __virRaiseError(conn, NULL, NULL, VIR_FROM_XEND, error, VIR_ERR_ERROR, - msg2, msg, NULL, 0, 0, msg2, msg); -} - -/** - * virXendErrorInt: - * @conn: the connection if available - * @error: the error number - * @val: extra integer information - * - * Handle an error at the xend daemon interface - */ -static void -virXendErrorInt(virConnectPtr conn, virErrorNumber error, int val) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = __virErrorMsg(error, NULL); - __virRaiseError(conn, NULL, NULL, VIR_FROM_XEND, error, VIR_ERR_ERROR, - errmsg, NULL, NULL, val, 0, errmsg, val); -} +#define virXendErrorInt(conn, code, ival) \ + virXendError(conn, code, "%d", ival) /** * do_connect: diff --git a/src/xm_internal.c b/src/xm_internal.c index ad7f892..a2dd9aa 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -124,25 +124,9 @@ struct xenUnifiedDriver xenXMDriver = { NULL, /* domainSetSchedulerParameters */ }; -static void -xenXMError(virConnectPtr conn, int code, const char *fmt, ...) -{ - va_list args; - char errorMessage[1024]; - const char *virerr; - - if (fmt) { - va_start(args, fmt); - vsnprintf(errorMessage, sizeof(errorMessage)-1, fmt, args); - va_end(args); - } else { - errorMessage[0] = '\0'; - } - - virerr = __virErrorMsg(code, (errorMessage[0] ? errorMessage : NULL)); - __virRaiseError(conn, NULL, NULL, VIR_FROM_XENXM, code, VIR_ERR_ERROR, - virerr, errorMessage, NULL, -1, -1, virerr, errorMessage); -} +#define xenXMError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_XENXM, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) int xenXMInit (void) @@ -400,7 +384,7 @@ static int xenXMConfigCacheRefresh (virConnectPtr conn) { int ret = -1; if (now == ((time_t)-1)) { - xenXMError (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + xenXMError (conn, VIR_ERR_SYSTEM_ERROR, "%s", strerror(errno)); return (-1); } @@ -412,7 +396,7 @@ static int xenXMConfigCacheRefresh (virConnectPtr conn) { /* Process the files in the config dir */ if (!(dh = opendir(configDir))) { - xenXMError (conn, VIR_ERR_SYSTEM_ERROR, strerror (errno)); + xenXMError (conn, VIR_ERR_SYSTEM_ERROR, "%s", strerror(errno)); return (-1); } @@ -484,7 +468,7 @@ static int xenXMConfigCacheRefresh (virConnectPtr conn) { } else { /* Completely new entry */ newborn = 1; if (VIR_ALLOC(entry) < 0) { - xenXMError (conn, VIR_ERR_NO_MEMORY, strerror (errno)); + xenXMError (conn, VIR_ERR_NO_MEMORY, "%s", strerror(errno)); goto cleanup; } memcpy(entry->filename, path, PATH_MAX); diff --git a/src/xml.c b/src/xml.c index f7813ad..6e6e0f6 100644 --- a/src/xml.c +++ b/src/xml.c @@ -22,28 +22,9 @@ #include "util.h" #include "memory.h" -/** - * virXMLError: - * @conn: a connection if any - * @error: the error number - * @info: information/format string - * @value: extra integer parameter for the error string - * - * Report an error coming from the XML module. - */ -static void -virXMLError(virConnectPtr conn, virErrorNumber error, const char *info, - int value) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = __virErrorMsg(error, info); - __virRaiseError(conn, NULL, NULL, VIR_FROM_XML, error, VIR_ERR_ERROR, - errmsg, info, NULL, value, 0, errmsg, info, value); -} +#define virXMLError(conn, code, fmt...) \ + __virReportErrorHelper(conn, VIR_FROM_XML, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) /************************************************************************ @@ -73,7 +54,7 @@ virXPathString(virConnectPtr conn, if ((ctxt == NULL) || (xpath == NULL)) { virXMLError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid parameter to virXPathString()"), 0); + _("Invalid parameter to virXPathString()")); return (NULL); } relnode = ctxt->node; @@ -86,7 +67,7 @@ virXPathString(virConnectPtr conn, ret = strdup((char *) obj->stringval); xmlXPathFreeObject(obj); if (ret == NULL) { - virXMLError(conn, VIR_ERR_NO_MEMORY, _("strdup failed"), 0); + virXMLError(conn, VIR_ERR_NO_MEMORY, _("strdup failed")); } ctxt->node = relnode; return (ret); @@ -114,7 +95,7 @@ virXPathNumber(virConnectPtr conn, if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { virXMLError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid parameter to virXPathNumber()"), 0); + _("Invalid parameter to virXPathNumber()")); return (-1); } relnode = ctxt->node; @@ -156,7 +137,7 @@ virXPathLong(virConnectPtr conn, if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { virXMLError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid parameter to virXPathNumber()"), 0); + _("Invalid parameter to virXPathNumber()")); return (-1); } relnode = ctxt->node; @@ -211,7 +192,7 @@ virXPathULong(virConnectPtr conn, if ((ctxt == NULL) || (xpath == NULL) || (value == NULL)) { virXMLError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid parameter to virXPathNumber()"), 0); + _("Invalid parameter to virXPathNumber()")); return (-1); } relnode = ctxt->node; @@ -269,7 +250,7 @@ virXPathBoolean(virConnectPtr conn, if ((ctxt == NULL) || (xpath == NULL)) { virXMLError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid parameter to virXPathBoolean()"), 0); + _("Invalid parameter to virXPathBoolean()")); return (-1); } relnode = ctxt->node; @@ -307,7 +288,7 @@ virXPathNode(virConnectPtr conn, if ((ctxt == NULL) || (xpath == NULL)) { virXMLError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid parameter to virXPathNode()"), 0); + _("Invalid parameter to virXPathNode()")); return (NULL); } relnode = ctxt->node; @@ -349,7 +330,7 @@ virXPathNodeSet(virConnectPtr conn, if ((ctxt == NULL) || (xpath == NULL)) { virXMLError(conn, VIR_ERR_INTERNAL_ERROR, - _("Invalid parameter to virXPathNodeSet()"), 0); + _("Invalid parameter to virXPathNodeSet()")); return (-1); } @@ -369,7 +350,7 @@ virXPathNodeSet(virConnectPtr conn, if (list != NULL && ret) { if (VIR_ALLOC_N(*list, ret) < 0) { virXMLError(conn, VIR_ERR_NO_MEMORY, - _("allocate string array"), + _("allocate string array size %lu"), ret * sizeof(**list)); ret = -1; } else { diff --git a/src/xs_internal.c b/src/xs_internal.c index 316604a..fce7fa7 100644 --- a/src/xs_internal.c +++ b/src/xs_internal.c @@ -87,26 +87,9 @@ struct xenUnifiedDriver xenStoreDriver = { #endif /* ! PROXY */ -/** - * virXenStoreError: - * @conn: the connection if available - * @error: the error number - * @info: extra information string - * - * Handle an error at the xend store interface - */ -static void -virXenStoreError(virConnectPtr conn, virErrorNumber error, const char *info) -{ - const char *errmsg; - - if (error == VIR_ERR_OK) - return; - - errmsg = __virErrorMsg(error, info); - __virRaiseError(conn, NULL, NULL, VIR_FROM_XENSTORE, error, VIR_ERR_ERROR, - errmsg, info, NULL, 0, 0, errmsg, info); -} +#define virXenStoreError(conn, code, fmt...) \ + __virReportErrorHelper(NULL, VIR_FROM_XENSTORE, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) /************************************************************************ * *
-- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list