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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list