maint: avoid nested use of virConnect{Ref,Close}

The public virConnectRef and virConnectClose API are just thin
wrappers around virObjectRef/virObjectRef, with added object
validation and an error reset.  Within our backend drivers, use
of the object validation is just an inefficiency since we always
pass valid objects.  More important to think about is what
happens with the error reset; our uses of virConnectRef happened
to be safe (since we hadn't encountered any earlier errors), but
in several cases the use of virConnectClose could lose a real
error.

Ideally, we should also avoid calling virConnectOpen() from
within backend drivers - but that is a known situation that
needs much more design work.

* src/qemu/qemu_process.c (qemuProcessReconnectHelper)
(qemuProcessReconnect): Avoid nested public API call.
* src/qemu/qemu_driver.c (qemuAutostartDomains)
(qemuStateInitialize, qemuStateStop): Likewise.
* src/qemu/qemu_migration.c (doPeer2PeerMigrate): Likewise.
* src/storage/storage_driver.c (storageDriverAutostart):
Likewise.
* src/uml/uml_driver.c (umlAutostartConfigs): Likewise.
* src/lxc/lxc_process.c (virLXCProcessAutostartAll): Likewise.
(virLXCProcessReboot): Likewise, and avoid leaking conn on error.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index cc9c1a200b55f876cabd450e09e70066bb92cbdf..ed729f65d4a7614d4044e7e5502c0d4a8cb08681 100644 (file)
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010-2013 Red Hat, Inc.
+ * Copyright (C) 2010-2014 Red Hat, Inc.
  * Copyright IBM Corp. 2008
  *
  * lxc_process.c: LXC process lifecycle management
@@ -103,7 +103,7 @@ virLXCProcessReboot(virLXCDriverPtr driver,
     VIR_DEBUG("Faking reboot");
 
     if (conn) {
-        virConnectRef(conn);
+        virObjectRef(conn);
         autodestroy = true;
     } else {
         conn = virConnectOpen("lxc:///");
@@ -125,12 +125,10 @@ virLXCProcessReboot(virLXCDriverPtr driver,
         goto cleanup;
     }
 
-    if (conn)
-        virConnectClose(conn);
-
     ret = 0;
 
 cleanup:
+    virObjectUnref(conn);
     return ret;
 }
 
@@ -1428,8 +1426,7 @@ virLXCProcessAutostartAll(virLXCDriverPtr driver)
                             virLXCProcessAutostartDomain,
                             &data);
 
-    if (conn)
-        virConnectClose(conn);
+    virObjectUnref(conn);
 }
 
 static int

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ebb77dca08c91b18fe4ce096c6abdb710ca551c0..b964b3c22bef2b40761d68af59362c7fe56c45b6 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -320,8 +320,7 @@ qemuAutostartDomains(virQEMUDriverPtr driver)
 
     virDomainObjListForEach(driver->domains, qemuAutostartDomain, &data);
 
-    if (conn)
-        virConnectClose(conn);
+    virObjectUnref(conn);
     virObjectUnref(cfg);
 }
 
@@ -843,15 +842,13 @@ qemuStateInitialize(bool privileged,
     if (!qemu_driver->workerPool)
         goto error;
 
-    if (conn)
-        virConnectClose(conn);
+    virObjectUnref(conn);
 
     virNWFilterRegisterCallbackDriver(&qemuCallbackDriver);
     return 0;
 
 error:
-    if (conn)
-        virConnectClose(conn);
+    virObjectUnref(conn);
     VIR_FREE(driverConf);
     VIR_FREE(membase);
     VIR_FREE(mempath);
@@ -970,8 +967,7 @@ qemuStateStop(void) {
         virDomainFree(domains[i]);
     VIR_FREE(domains);
     VIR_FREE(flags);
-    if (conn)
-        virConnectClose(conn);
+    virObjectUnref(conn);
     virObjectUnref(cfg);
 
     return ret;

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 53a4ae6a348ebbef35d4717cfbb44989b9940590..a8a493aa0606a96b1f1553c224296c102468300f 100644 (file)
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2,7 +2,7 @@
 /*
  * qemu_migration.c: QEMU migration handling
  *
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 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
@@ -4021,7 +4021,7 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
 cleanup:
     orig_err = virSaveLastError();
     qemuDomainObjEnterRemote(vm);
-    virConnectClose(dconn);
+    virObjectUnref(dconn);
     qemuDomainObjExitRemote(vm);
     if (orig_err) {
         virSetError(orig_err);

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 684893977f03b24977153e37438968951e1bf4f0..8bcd98e772de5845fc78418d40176bde470bb157 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1,7 +1,7 @@
 /*
  * qemu_process.c: QEMU process management
  *
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 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
@@ -3270,7 +3270,7 @@ endjob:
     if (obj && virObjectUnref(obj))
         virObjectUnlock(obj);
 
-    virConnectClose(conn);
+    virObjectUnref(conn);
     virObjectUnref(cfg);
 
     return;
@@ -3305,7 +3305,7 @@ error:
                 virObjectUnlock(obj);
         }
     }
-    virConnectClose(conn);
+    virObjectUnref(conn);
     virObjectUnref(cfg);
 }
 
@@ -3352,11 +3352,11 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
      * that the threads we start see a valid connection throughout their
      * lifetime. We simply increase the reference counter here.
      */
-    virConnectRef(data->conn);
+    virObjectRef(data->conn);
 
     if (virThreadCreate(&thread, false, qemuProcessReconnect, data) < 0) {
 
-        virConnectClose(data->conn);
+        virObjectUnref(data->conn);
 
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Could not create thread. QEMU initialization "

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index bb13e8e954f4ad7cddcb2b6d3dc9031b535a84ae..c83aa8a6e8aec210746f51e6d6e7a24d82556510 100644 (file)
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1,7 +1,7 @@
 /*
  * storage_driver.c: core driver for storage APIs
  *
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -130,8 +130,7 @@ storageDriverAutostart(virStorageDriverStatePtr driver) {
         virStoragePoolObjUnlock(pool);
     }
 
-    if (conn)
-        virConnectClose(conn);
+    virObjectUnref(conn);
 }
 
 /**

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 31ebf4ab6e1b69046b6d0ffb213f60cd5bc88629..f286f41e6c5c2573a36921eacefbadae51b4d5aa 100644 (file)
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -224,8 +224,7 @@ umlAutostartConfigs(struct uml_driver *driver) {
     virDomainObjListForEach(driver->domains, umlAutostartDomain, &data);
     umlDriverUnlock(driver);
 
-    if (conn)
-        virConnectClose(conn);
+    virObjectUnref(conn);
 }