[libvirt] [PATCHv2 2/6] maint: avoid nested use of virConnect{Ref, Close}
Eric Blake
eblake at redhat.com
Tue Jan 14 21:43:33 UTC 2014
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 at redhat.com>
---
src/lxc/lxc_process.c | 11 ++++-------
src/qemu/qemu_driver.c | 12 ++++--------
src/qemu/qemu_migration.c | 4 ++--
src/qemu/qemu_process.c | 10 +++++-----
src/storage/storage_driver.c | 5 ++---
src/uml/uml_driver.c | 3 +--
6 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index cc9c1a2..ed729f6 100644
--- 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 1949abe..6389a8b 100644
--- 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 53a4ae6..a8a493a 100644
--- 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 9331744..28aa45c 100644
--- 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
@@ -3261,7 +3261,7 @@ endjob:
if (obj && virObjectUnref(obj))
virObjectUnlock(obj);
- virConnectClose(conn);
+ virObjectUnref(conn);
virObjectUnref(cfg);
return;
@@ -3296,7 +3296,7 @@ error:
virObjectUnlock(obj);
}
}
- virConnectClose(conn);
+ virObjectUnref(conn);
virObjectUnref(cfg);
}
@@ -3343,11 +3343,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 85fc0f2..36eb793 100644
--- 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 ad29ebf..6a1b129 100644
--- 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);
}
--
1.8.4.2
More information about the libvir-list
mailing list