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

[libvirt] PATCH: Ensure errors are guarenteed reported in virConnectOpen



The virConnectOpen method is unfortuantely rather special. While there is
a virConnect object available, the current rule is that drivers must report
errors against the global error object, because upon failure no virConnect
object will be returned to the caller. Unforatunately, despite frequent
cleanups of code getting this wrong, I've just audited the remote driver
and found another 20 or so places where its wrong. This is clearly not a
sustainable approach to error reporting.

The guarenteed correct solution is actually rather simple

 - Always report errors against the virConnect object, even in the driver
   open method

 - In the cleanup patch of do_open() in libvirt.c, if no global error is
   set, copy the error from the virConnect object's virError, into the 
   global virError.

With this in place we can change all the drivers back to always report
against the error object, and thus cleanup some disgusting code like

        __virRaiseError (in_open ? NULL : conn, ...

To just

        __virRaiseError (conn, ...

Daniel

Index: src/libvirt.c
===================================================================
RCS file: /data/cvs/libvirt/src/libvirt.c,v
retrieving revision 1.151
diff -u -r1.151 libvirt.c
--- src/libvirt.c	18 Aug 2008 09:24:46 -0000	1.151
+++ src/libvirt.c	19 Aug 2008 10:29:51 -0000
@@ -735,10 +735,8 @@
         name = "xen:///";
 
     ret = virGetConnect();
-    if (ret == NULL) {
-        virLibConnError(NULL, VIR_ERR_NO_MEMORY, _("allocating connection"));
+    if (ret == NULL)
         return NULL;
-    }
 
     uri = xmlParseURI (name);
     if (!uri) {
@@ -839,7 +837,21 @@
 failed:
     if (ret->driver) ret->driver->close (ret);
     if (uri) xmlFreeURI(uri);
-        virUnrefConnect(ret);
+
+    /* If not global error was set, copy any error set
+       in the connection object we're about to dispose of */
+    if (__lastErr.code == VIR_ERR_OK) {
+        memcpy(&__lastErr, &ret->err, sizeof(ret->err));
+        memset(&ret->err, 0, sizeof(ret->err));
+    }
+
+    /* Still no error set, then raise a generic error */
+    if (__lastErr.code == VIR_ERR_OK)
+        virLibConnError (NULL, VIR_ERR_INTERNAL_ERROR,
+                         _("unable to open connection"));
+
+    virUnrefConnect(ret);
+
     return NULL;
 }
 

-- 
|: 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 :|


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