From 25221a1b2103dd4be8a056de5c11705bce037f47 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Fri, 3 Jan 2014 08:08:52 -0700 Subject: [PATCH] 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 --- 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 cc9c1a200b..ed729f65d4 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 ebb77dca08..b964b3c22b 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 53a4ae6a34..a8a493aa06 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 684893977f..8bcd98e772 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 @@ -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 bb13e8e954..c83aa8a6e8 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 31ebf4ab6e..f286f41e6c 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); }