5:56 p.m.
# HG changeset patch
# User john.levon(a)sun.com
# Date 1233791330 28800
# Node ID 6588879d8cc9a3af1147a5edd624aee5155493ae
# Parent da162569b5f7e132c4ccbfc56fc3670fb5ecee10
Improve error reporting in virsh
Rather than verbosely printing every error, save the last error and
report that only if the entire command fails. Additionally, avoid
over-writing an existing error in various places in the library.
Signed-off-by: John Levon <john.levon(a)sun.com>
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -187,6 +187,8 @@ void virConnSetErrorFunc (virConnectPt
virErrorFunc handler);
int virConnCopyLastError (virConnectPtr conn,
virErrorPtr to);
+
+virErrorPtr virCloneError (virErrorPtr from);
#ifdef __cplusplus
}
#endif
diff --git a/src/domain_conf.c b/src/domain_conf.c
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -2432,8 +2432,7 @@ catchXMLError (void *ctx, const char *ms
if (ctxt) {
virConnectPtr conn = ctxt->_private;
- if (conn &&
- conn->err.code == VIR_ERR_NONE &&
+ if (virGetLastError() == NULL &&
ctxt->lastError.level == XML_ERR_FATAL &&
ctxt->lastError.message != NULL) {
virDomainReportError (conn, VIR_ERR_XML_DETAIL,
@@ -2466,7 +2465,7 @@ virDomainDefPtr virDomainDefParseString(
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
if (!xml) {
- if (conn && conn->err.code == VIR_ERR_NONE)
+ if (virGetLastError() == NULL)
virDomainReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("failed to parse xml
document"));
goto cleanup;
@@ -2507,7 +2506,7 @@ virDomainDefPtr virDomainDefParseFile(vi
XML_PARSE_NOENT | XML_PARSE_NONET |
XML_PARSE_NOWARNING);
if (!xml) {
- if (conn && conn->err.code == VIR_ERR_NONE)
+ if (virGetLastError() == NULL)
virDomainReportError(conn, VIR_ERR_XML_ERROR,
"%s", _("failed to parse xml
document"));
goto cleanup;
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -244,6 +244,7 @@ LIBVIRT_0.6.0 {
virStoragePoolRef;
virStorageVolRef;
virNodeDeviceRef;
+ virCloneError;
} LIBVIRT_0.5.0;
diff --git a/src/virsh.c b/src/virsh.c
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -93,22 +93,6 @@ typedef enum {
} vshErrorLevel;
/*
- * The error handler for virsh
- */
-static void
-virshErrorHandler(void *unused, virErrorPtr error)
-{
- if ((unused != NULL) || (error == NULL))
- return;
-
- /* Suppress the VIR_ERR_NO_XEN error which fails as non-root */
- if ((error->code == VIR_ERR_NO_XEN) || (error->code == VIR_ERR_OK))
- return;
-
- virDefaultErrorFunc(error);
-}
-
-/*
* virsh command line grammar:
*
* command_line = <command>\n | <command>; <command>; ...
@@ -319,6 +303,41 @@ static int namesorter(const void *a, con
const char **sb = (const char**)b;
return strcasecmp(*sa, *sb);
+}
+
+static virErrorPtr last_error;
+
+/*
+ * Quieten libvirt until we're done with the command.
+ */
+static void
+virshErrorHandler(void *unused, virErrorPtr error)
+{
+ last_error = virCloneError(error);
+ if (getenv("VIRSH_DEBUG") != NULL)
+ virDefaultErrorFunc(error);
+}
+
+/*
+ * Report an error when a command finishes. This is better than before
+ * (when correct operation would report errors), but it has some
+ * problems: we lose the smarter formatting of virDefaultErrorFunc(),
+ * and it can become harder to debug problems, if errors get reported
+ * twice during one command. This case shouldn't really happen anyway,
+ * and it's IMHO a bug that libvirt does that sometimes.
+ */
+static void
+virshReportError(vshControl *ctl)
+{
+ if (last_error == NULL)
+ return;
+
+ if (last_error->code == VIR_ERR_OK) {
+ vshError(ctl, TRUE, "%s", _("unknown error"));
+ return;
+ }
+
+ vshError(ctl, TRUE, "%s", last_error->message);
}
@@ -6102,6 +6121,9 @@ vshCommandRun(vshControl *ctl, const vsh
if (ctl->timing)
GETTIMEOFDAY(&after);
+
+ if (ret == FALSE)
+ virshReportError(ctl);
if (STREQ(cmd->def->name, "quit")) /* hack ... */
return ret;
diff --git a/src/virterror.c b/src/virterror.c
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -259,6 +259,32 @@ virGetLastError(void)
}
/**
+ * virCloneError:
+ * @from: source error to copy from
+ *
+ * Allocate a new error object and deep-copy it from the given error
+ * object. The destination error is reset before the copy. If NULL is
+ * passed, a new error object is allocated.
+ *
+ * Returns a pointer to the copied error or NULL if allocation failed.
+ */
+virErrorPtr
+virCloneError(virErrorPtr from)
+{
+ virErrorPtr to;
+
+ if (VIR_ALLOC(to) < 0)
+ return NULL;
+
+ if (from)
+ virCopyError(from, to);
+ else
+ virResetError(to);
+
+ return to;
+}
+
+/**
* virCopyLastError:
* @to: target to receive the copy
*
diff --git a/src/xend_internal.c b/src/xend_internal.c
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -105,12 +105,16 @@ virDomainDevID(virDomainPtr domain,
int devid_len);
#endif
-#define virXendError(conn, code, fmt...) \
- virReportErrorHelper(conn, VIR_FROM_XEND, code, __FILE__, \
- __FUNCTION__, __LINE__, fmt)
-
+#define virXendError(conn, codeval, fmt...) \
+ do { \
+ if (virGetLastError() == NULL) { \
+ virReportErrorHelper(conn, VIR_FROM_XEND, codeval, __FILE__, \
+ __FUNCTION__, __LINE__, fmt); \
+ } \
+ } while (0)
+
#define virXendErrorInt(conn, code, ival) \
- virXendError(conn, code, "%d", ival)
+ virXendError(conn, code, "%d", ival)
/**
* do_connect: