[libvirt] [PATCH] Improve error reporting in virsh

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



# HG changeset patch
# User john.levon@xxxxxxx
# 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@xxxxxxx>

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:

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]