4:13 a.m.
On Thu, Apr 23, 2015 at 19:14:57 +0200, Michal Privoznik wrote:
Every domain that grabs a domain object to work over should
reference it to make sure it won't disappear meanwhile.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/bhyve/bhyve_driver.c | 3 +-
src/conf/domain_conf.c | 1 +
src/libxl/libxl_driver.c | 10 ++---
src/lxc/lxc_driver.c | 3 +-
src/openvz/openvz_driver.c | 11 +++--
src/parallels/parallels_driver.c | 3 +-
src/qemu/qemu_driver.c | 14 ++----
src/test/test_driver.c | 93 +++++++++++++---------------------------
src/uml/uml_driver.c | 15 +++----
9 files changed, 51 insertions(+), 102 deletions(-)
...
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 686c614..6666d03 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1157,6 +1157,7 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr
doms,
virDomainObjPtr obj;
virObjectLock(doms);
obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
+ virObjectRef(obj);
if (obj) {
virObjectLock(obj);
if (obj->removing) {
Here this code that is below in the context checks if the @removing flag
is set and if so the function returns NULL. With your addition the
reference you've added wouldn't be removed.
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index 1bb8973..10d94ff 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
...
@@ -1007,6 +1006,7 @@ openvzDomainDefineXMLFlags(virConnectPtr conn,
const char *xml, unsigned int fla
virReportError(VIR_ERR_OPERATION_FAILED,
_("Already an OPENVZ VM active with the id
'%s'"),
vmdef->name);
+ virDomainObjEndAPI(&vm);
goto cleanup;
}
if (!(vm = virDomainObjListAdd(driver->domains, vmdef,
The whole piece of code that looks up the domain and reports the error
above could be removed as virDomainObjListAdd does the same check.
@@ -1103,6 +1103,7 @@ openvzDomainCreateXML(virConnectPtr conn, const
char *xml,
virReportError(VIR_ERR_OPERATION_FAILED,
_("Already an OPENVZ VM defined with the id
'%s'"),
vmdef->name);
+ virDomainObjEndAPI(&vm);
goto cleanup;
}
if (!(vm = virDomainObjListAdd(driver->domains,
Same here. Not worth changing though probably.
...
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 9cee541..f8a84e4 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -339,7 +339,7 @@ umlInotifyEvent(int watch,
if (e.mask & IN_DELETE) {
VIR_DEBUG("Got inotify domain shutdown '%s'", name);
if (!virDomainObjIsActive(dom)) {
- virObjectUnlock(dom);
+ virDomainObjEndAPI(&dom);
continue;
}
@@ -351,17 +351,16 @@ umlInotifyEvent(int watch,
if (!dom->persistent) {
virDomainObjListRemove(driver->domains,
dom);
- dom = NULL;
}
This now leaves a single statement in an if with braces.
} else if (e.mask & (IN_CREATE | IN_MODIFY)) {
VIR_DEBUG("Got inotify domain startup '%s'", name);
if (virDomainObjIsActive(dom)) {
- virObjectUnlock(dom);
+ virDomainObjEndAPI(&dom);
continue;
}
if (umlReadPidFile(driver, dom) < 0) {
- virObjectUnlock(dom);
+ virDomainObjEndAPI(&dom);
continue;
}
@@ -385,7 +384,6 @@ umlInotifyEvent(int watch,
if (!dom->persistent) {
virDomainObjListRemove(driver->domains,
dom);
- dom = NULL;
Again, single statement in an if with braces
}
} else if (umlIdentifyChrPTY(driver, dom) < 0) {
VIR_WARN("Could not identify character devices for new
domain");
@@ -398,12 +396,10 @@ umlInotifyEvent(int watch,
if (!dom->persistent) {
virDomainObjListRemove(driver->domains,
dom);
- dom = NULL;
Here too
}
}
}
- if (dom)
- virObjectUnlock(dom);
+ virDomainObjEndAPI(&dom);
if (event) {
umlDomainEventQueue(driver, event);
event = NULL;
ACK, if you fix the broken reference counting in case the domain is
being removed.
Peter