[libvirt] [PATCH 3/6] use virObject to manage reference-count of virDomainObj

Hu Tao hutao at cn.fujitsu.com
Wed Apr 6 07:19:51 UTC 2011


This patch also eliminates a dead-lock bug in
qemuDomainObjBeginJobWithDriver: if virCondWaitUntil() timeouts, the
thread tries to acquire qemu driver lock while holding virDomainObj
lock.
---
 src/conf/domain_conf.c    |   56 ++++----
 src/conf/domain_conf.h    |    6 +-
 src/openvz/openvz_conf.c  |    8 +-
 src/qemu/qemu_domain.c    |   32 ++---
 src/qemu/qemu_domain.h    |    2 +-
 src/qemu/qemu_driver.c    |  304 ++++++++++++++++++++-------------------------
 src/qemu/qemu_migration.c |   45 +++----
 src/qemu/qemu_process.c   |   33 ++---
 src/vmware/vmware_conf.c  |    2 +-
 9 files changed, 215 insertions(+), 273 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 90a1317..fc76a00 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -47,6 +47,7 @@
 #include "storage_file.h"
 #include "files.h"
 #include "bitmap.h"
+#include "virobject.h"
 
 #define VIR_FROM_THIS VIR_FROM_DOMAIN
 
@@ -395,9 +396,7 @@ static void
 virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
 {
     virDomainObjPtr obj = payload;
-    virDomainObjLock(obj);
-    if (virDomainObjUnref(obj) > 0)
-        virDomainObjUnlock(obj);
+    virDomainObjUnref(obj);
 }
 
 int virDomainObjListInit(virDomainObjListPtr doms)
@@ -437,7 +436,7 @@ virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms,
     virDomainObjPtr obj;
     obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
     if (obj)
-        virDomainObjLock(obj);
+        virDomainObjRef(obj);
     return obj;
 }
 
@@ -452,7 +451,7 @@ virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms,
 
     obj = virHashLookup(doms->objs, uuidstr);
     if (obj)
-        virDomainObjLock(obj);
+        virDomainObjRef(obj);
     return obj;
 }
 
@@ -476,7 +475,7 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
     virDomainObjPtr obj;
     obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
     if (obj)
-        virDomainObjLock(obj);
+        virDomainObjRef(obj);
     return obj;
 }
 
@@ -967,6 +966,12 @@ static void virDomainObjFree(virDomainObjPtr dom)
 {
     if (!dom)
         return;
+    virDomainObjUnref(dom);
+}
+
+static void doDomainObjFree(virObjectPtr obj)
+{
+    virDomainObjPtr dom = (virDomainObjPtr)obj;
 
     VIR_DEBUG("obj=%p", dom);
     virDomainDefFree(dom->def);
@@ -984,21 +989,13 @@ static void virDomainObjFree(virDomainObjPtr dom)
 
 void virDomainObjRef(virDomainObjPtr dom)
 {
-    dom->refs++;
-    VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
+    virObjectRef(&dom->obj);
 }
 
 
-int virDomainObjUnref(virDomainObjPtr dom)
+void virDomainObjUnref(virDomainObjPtr dom)
 {
-    dom->refs--;
-    VIR_DEBUG("obj=%p refs=%d", dom, dom->refs);
-    if (dom->refs == 0) {
-        virDomainObjUnlock(dom);
-        virDomainObjFree(dom);
-        return 0;
-    }
-    return dom->refs;
+    virObjectUnref(&dom->obj);
 }
 
 static virDomainObjPtr virDomainObjNew(virCapsPtr caps)
@@ -1010,6 +1007,11 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps)
         return NULL;
     }
 
+    if (virObjectInit(&domain->obj, doDomainObjFree)) {
+        VIR_FREE(domain);
+        return NULL;
+    }
+
     if (caps->privateDataAllocFunc &&
         !(domain->privateData = (caps->privateDataAllocFunc)())) {
         virReportOOMError();
@@ -1027,9 +1029,7 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps)
         return NULL;
     }
 
-    virDomainObjLock(domain);
     domain->state = VIR_DOMAIN_SHUTOFF;
-    domain->refs = 1;
 
     virDomainSnapshotObjListInit(&domain->snapshots);
 
@@ -1075,8 +1075,10 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps,
     domain->def = def;
 
     virUUIDFormat(def->uuid, uuidstr);
+    virDomainObjRef(domain);
     if (virHashAddEntry(doms->objs, uuidstr, domain) < 0) {
-        VIR_FREE(domain);
+        virDomainObjUnref(domain);
+        virDomainObjFree(domain);
         return NULL;
     }
 
@@ -1149,9 +1151,7 @@ virDomainObjGetPersistentDef(virCapsPtr caps,
 }
 
 /*
- * The caller must hold a lock  on the driver owning 'doms',
- * and must also have locked 'dom', to ensure no one else
- * is either waiting for 'dom' or still usingn it
+ * The caller must hold a lock  on the driver owning 'doms'.
  */
 void virDomainRemoveInactive(virDomainObjListPtr doms,
                              virDomainObjPtr dom)
@@ -1159,9 +1159,8 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virUUIDFormat(dom->def->uuid, uuidstr);
 
-    virDomainObjUnlock(dom);
-
     virHashRemoveEntry(doms->objs, uuidstr);
+    virDomainObjUnref(dom);
 }
 
 
@@ -6146,7 +6145,7 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps,
 
 error:
     /* obj was never shared, so unref should return 0 */
-    ignore_value(virDomainObjUnref(obj));
+    virDomainObjUnref(obj);
     return NULL;
 }
 
@@ -8449,10 +8448,12 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps,
     if (!(dom = virDomainAssignDef(caps, doms, def, false)))
         goto error;
 
+    virDomainObjLock(dom);
     dom->autostart = autostart;
 
     if (notify)
         (*notify)(dom, newVM, opaque);
+    virDomainObjUnlock(dom);
 
     VIR_FREE(configFile);
     VIR_FREE(autostartLink);
@@ -8501,9 +8502,8 @@ static virDomainObjPtr virDomainLoadStatus(virCapsPtr caps,
     return obj;
 
 error:
-    /* obj was never shared, so unref should return 0 */
     if (obj)
-        ignore_value(virDomainObjUnref(obj));
+        virDomainObjUnref(obj);
     VIR_FREE(statusFile);
     return NULL;
 }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 10e73cb..3218672 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -40,6 +40,7 @@
 # include "nwfilter_conf.h"
 # include "macvtap.h"
 # include "sysinfo.h"
+# include "virobject.h"
 
 /* Private component of virDomainXMLFlags */
 typedef enum {
@@ -1144,8 +1145,8 @@ struct _virDomainDef {
 typedef struct _virDomainObj virDomainObj;
 typedef virDomainObj *virDomainObjPtr;
 struct _virDomainObj {
+    virObject obj;
     virMutex lock;
-    int refs;
 
     int pid;
     int state;
@@ -1226,8 +1227,7 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def,
 
 void virDomainDefFree(virDomainDefPtr vm);
 void virDomainObjRef(virDomainObjPtr vm);
-/* Returns 1 if the object was freed, 0 if more refs exist */
-int virDomainObjUnref(virDomainObjPtr vm) ATTRIBUTE_RETURN_CHECK;
+void virDomainObjUnref(virDomainObjPtr vm);
 
 /* live == true means def describes an active domain (being migrated or
  * restored) as opposed to a new persistent configuration of the domain */
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 88cd4c8..c08ed3b 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -472,6 +472,11 @@ int openvzLoadDomains(struct openvz_driver *driver) {
         if (VIR_ALLOC(dom) < 0)
             goto no_memory;
 
+        if (virObjectInit(&dom->obj, NULL)) {
+            VIR_FREE(dom);
+            goto cleanup;
+        }
+
         if (virMutexInit(&dom->lock) < 0) {
             openvzError(VIR_ERR_INTERNAL_ERROR, "%s",
                         _("cannot initialize mutex"));
@@ -489,7 +494,6 @@ int openvzLoadDomains(struct openvz_driver *driver) {
         else
             dom->state = VIR_DOMAIN_RUNNING;
 
-        dom->refs = 1;
         dom->pid = veid;
         dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid;
         /* XXX OpenVZ doesn't appear to have concept of a transient domain */
@@ -554,7 +558,7 @@ int openvzLoadDomains(struct openvz_driver *driver) {
     VIR_FREE(outbuf);
     /* dom hasn't been shared yet, so unref should return 0 */
     if (dom)
-        ignore_value(virDomainObjUnref(dom));
+        virDomainObjUnref(dom);
     return -1;
 }
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c2a1f9a..3a3c953 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -457,13 +457,13 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj)
     }
     then = timeval_to_ms(now) + QEMU_JOB_WAIT_TIME;
 
-    virDomainObjRef(obj);
+    virDomainObjLock(obj);
 
     while (priv->jobActive) {
         if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) {
-            /* Safe to ignore value since ref count was incremented above */
-            ignore_value(virDomainObjUnref(obj));
-            if (errno == ETIMEDOUT)
+            int err = errno;
+            virDomainObjUnlock(obj);
+            if (err == ETIMEDOUT)
                 qemuReportError(VIR_ERR_OPERATION_TIMEOUT,
                                 "%s", _("cannot acquire state change lock"));
             else
@@ -482,12 +482,10 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj)
 }
 
 /*
- * obj must be locked before calling, qemud_driver must be locked
- *
  * This must be called by anything that will change the VM state
  * in any way, or anything that will use the QEMU monitor.
  */
-int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
+int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver ATTRIBUTE_UNUSED,
                                     virDomainObjPtr obj)
 {
     qemuDomainObjPrivatePtr priv = obj->privateData;
@@ -501,20 +499,18 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
     }
     then = timeval_to_ms(now) + QEMU_JOB_WAIT_TIME;
 
-    virDomainObjRef(obj);
-    qemuDriverUnlock(driver);
+    virDomainObjLock(obj);
 
     while (priv->jobActive) {
         if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) {
-            /* Safe to ignore value since ref count was incremented above */
-            ignore_value(virDomainObjUnref(obj));
-            if (errno == ETIMEDOUT)
+            int err = errno;
+            virDomainObjUnlock(obj);
+            if (err == ETIMEDOUT)
                 qemuReportError(VIR_ERR_OPERATION_TIMEOUT,
                                 "%s", _("cannot acquire state change lock"));
             else
                 virReportSystemError(errno,
                                      "%s", _("cannot acquire job mutex"));
-            qemuDriverLock(driver);
             return -1;
         }
     }
@@ -524,10 +520,6 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
     priv->jobStart = timeval_to_ms(now);
     memset(&priv->jobInfo, 0, sizeof(priv->jobInfo));
 
-    virDomainObjUnlock(obj);
-    qemuDriverLock(driver);
-    virDomainObjLock(obj);
-
     return 0;
 }
 
@@ -540,7 +532,7 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
  * Returns remaining refcount on 'obj', maybe 0 to indicated it
  * was deleted
  */
-int qemuDomainObjEndJob(virDomainObjPtr obj)
+void qemuDomainObjEndJob(virDomainObjPtr obj)
 {
     qemuDomainObjPrivatePtr priv = obj->privateData;
 
@@ -551,7 +543,7 @@ int qemuDomainObjEndJob(virDomainObjPtr obj)
     memset(&priv->jobInfo, 0, sizeof(priv->jobInfo));
     virCondSignal(&priv->jobCond);
 
-    return virDomainObjUnref(obj);
+    virDomainObjUnlock(obj);
 }
 
 
@@ -655,7 +647,7 @@ void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver,
     virDomainObjLock(obj);
     /* Safe to ignore value, since we incremented ref in
      * qemuDomainObjEnterRemoteWithDriver */
-    ignore_value(virDomainObjUnref(obj));
+    virDomainObjUnref(obj);
 }
 
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 8258900..12fd21d 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -97,7 +97,7 @@ void qemuDomainSetNamespaceHooks(virCapsPtr caps);
 int qemuDomainObjBeginJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK;
 int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
                                     virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK;
-int qemuDomainObjEndJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK;
+void qemuDomainObjEndJob(virDomainObjPtr obj);
 void qemuDomainObjEnterMonitor(virDomainObjPtr obj);
 void qemuDomainObjExitMonitor(virDomainObjPtr obj);
 void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 48fe266..db89402 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -139,7 +139,6 @@ qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaq
     struct qemuAutostartData *data = opaque;
     virErrorPtr err;
 
-    virDomainObjLock(vm);
     virResetLastError();
     if (qemuDomainObjBeginJobWithDriver(data->driver, vm) < 0) {
         err = virGetLastError();
@@ -156,12 +155,8 @@ qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaq
                       err ? err->message : _("unknown error"));
         }
 
-        if (qemuDomainObjEndJob(vm) == 0)
-            vm = NULL;
+        qemuDomainObjEndJob(vm);
     }
-
-    if (vm)
-        virDomainObjUnlock(vm);
 }
 
 
@@ -1055,12 +1050,13 @@ static virDomainPtr qemudDomainLookupByID(virConnectPtr conn,
         goto cleanup;
     }
 
+    virDomainObjLock(vm);
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
     if (dom) dom->id = vm->def->id;
+    virDomainObjUnlock(vm);
 
+    virDomainObjUnref(vm);
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -1082,12 +1078,13 @@ static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn,
         goto cleanup;
     }
 
+    virDomainObjLock(vm);
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
     if (dom) dom->id = vm->def->id;
+    virDomainObjUnlock(vm);
 
+    virDomainObjUnref(vm);
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -1107,12 +1104,13 @@ static virDomainPtr qemudDomainLookupByName(virConnectPtr conn,
         goto cleanup;
     }
 
+    virDomainObjLock(vm);
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
     if (dom) dom->id = vm->def->id;
+    virDomainObjUnlock(vm);
 
+    virDomainObjUnref(vm);
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     return dom;
 }
 
@@ -1133,11 +1131,13 @@ static int qemuDomainIsActive(virDomainPtr dom)
                         _("no domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }
+
+    virDomainObjLock(obj);
     ret = virDomainObjIsActive(obj);
+    virDomainObjUnlock(obj);
 
+    virDomainObjUnref(obj);
 cleanup:
-    if (obj)
-        virDomainObjUnlock(obj);
     return ret;
 }
 
@@ -1157,11 +1157,13 @@ static int qemuDomainIsPersistent(virDomainPtr dom)
                         _("no domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }
+    virDomainObjLock(obj);
     ret = obj->persistent;
+    virDomainObjUnlock(obj);
 
 cleanup:
     if (obj)
-        virDomainObjUnlock(obj);
+        virDomainObjUnref(obj);
     return ret;
 }
 
@@ -1181,11 +1183,13 @@ static int qemuDomainIsUpdated(virDomainPtr dom)
                         _("no domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }
+    virDomainObjLock(obj);
     ret = obj->updated;
+    virDomainObjUnlock(obj);
 
 cleanup:
     if (obj)
-        virDomainObjUnlock(obj);
+        virDomainObjUnref(obj);
     return ret;
 }
 
@@ -1239,39 +1243,57 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
 
     qemuDriverLock(driver);
     if (!(def = virDomainDefParseString(driver->caps, xml,
-                                        VIR_DOMAIN_XML_INACTIVE)))
+                                        VIR_DOMAIN_XML_INACTIVE))) {
+        qemuDriverUnlock(driver);
         goto cleanup;
+    }
 
-    if (virSecurityManagerVerify(driver->securityManager, def) < 0)
+    if (virSecurityManagerVerify(driver->securityManager, def) < 0) {
+        qemuDriverUnlock(driver);
         goto cleanup;
+    }
 
-    if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0)
+    if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) {
+        qemuDriverUnlock(driver);
         goto cleanup;
+    }
 
-    if (qemudCanonicalizeMachine(driver, def) < 0)
+    if (qemudCanonicalizeMachine(driver, def) < 0) {
+        qemuDriverUnlock(driver);
         goto cleanup;
+    }
 
-    if (qemuDomainAssignPCIAddresses(def) < 0)
+    if (qemuDomainAssignPCIAddresses(def) < 0) {
+        qemuDriverUnlock(driver);
         goto cleanup;
+    }
 
     if (!(vm = virDomainAssignDef(driver->caps,
                                   &driver->domains,
-                                  def, false)))
+                                  def, false))) {
+        qemuDriverUnlock(driver);
         goto cleanup;
+    }
+
+    qemuDriverUnlock(driver);
 
     def = NULL;
 
-    if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+    if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) {
+        virDomainObjUnref(vm);
         goto cleanup; /* XXXX free the 'vm' we created ? */
+    }
 
     if (qemuProcessStart(conn, driver, vm, NULL,
                          (flags & VIR_DOMAIN_START_PAUSED) != 0,
                          -1, NULL, VIR_VM_OP_CREATE) < 0) {
         qemuAuditDomainStart(vm, "booted", false);
-        if (qemuDomainObjEndJob(vm) > 0)
-            virDomainRemoveInactive(&driver->domains,
-                                    vm);
-        vm = NULL;
+        qemuDomainObjEndJob(vm);
+        qemuDriverLock(driver);
+        virDomainRemoveInactive(&driver->domains,
+                                vm);
+        qemuDriverUnlock(driver);
+        virDomainObjUnref(vm);
         goto cleanup;
     }
 
@@ -1283,17 +1305,13 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
     if (dom) dom->id = vm->def->id;
 
-    if (vm &&
-        qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
+    virDomainObjUnref(vm);
 
 cleanup:
     virDomainDefFree(def);
-    if (vm)
-        virDomainObjUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
-    qemuDriverUnlock(driver);
     return dom;
 }
 
@@ -1307,13 +1325,14 @@ static int qemudDomainSuspend(virDomainPtr dom) {
 
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    qemuDriverUnlock(driver);
 
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
     if (!virDomainObjIsActive(vm)) {
         qemuReportError(VIR_ERR_OPERATION_INVALID,
@@ -1354,16 +1373,12 @@ static int qemudDomainSuspend(virDomainPtr dom) {
     }
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
-
+    virDomainObjUnref(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
-    qemuDriverUnlock(driver);
     return ret;
 }
 
@@ -1376,6 +1391,7 @@ static int qemudDomainResume(virDomainPtr dom) {
 
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    qemuDriverUnlock(driver);
 
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -1409,15 +1425,12 @@ static int qemudDomainResume(virDomainPtr dom) {
     ret = 0;
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
+    virDomainObjUnref(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
-    qemuDriverUnlock(driver);
     return ret;
 }
 
@@ -1437,7 +1450,7 @@ static int qemudDomainShutdown(virDomainPtr dom) {
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
 
     if (qemuDomainObjBeginJob(vm) < 0)
@@ -1455,12 +1468,10 @@ static int qemudDomainShutdown(virDomainPtr dom) {
     qemuDomainObjExitMonitor(vm);
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
+    virDomainObjUnref(vm);
     return ret;
 }
 
@@ -1473,12 +1484,13 @@ static int qemudDomainDestroy(virDomainPtr dom) {
 
     qemuDriverLock(driver);
     vm  = virDomainFindByUUID(&driver->domains, dom->uuid);
+    qemuDriverUnlock(driver);
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
 
     if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
@@ -1497,24 +1509,24 @@ static int qemudDomainDestroy(virDomainPtr dom) {
     qemuAuditDomainStop(vm, "destroyed");
 
     if (!vm->persistent) {
-        if (qemuDomainObjEndJob(vm) > 0)
-            virDomainRemoveInactive(&driver->domains,
-                                    vm);
+        qemuDomainObjEndJob(vm);
+        virDomainObjUnref(vm);
+        qemuDriverLock(driver);
+        virDomainRemoveInactive(&driver->domains, vm);
+        qemuDriverUnlock(driver);
         vm = NULL;
     }
     ret = 0;
 
 endjob:
-    if (vm &&
-        qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    if (vm)
+        qemuDomainObjEndJob(vm);
 
 cleanup:
     if (vm)
-        virDomainObjUnlock(vm);
+        virDomainObjUnref(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
-    qemuDriverUnlock(driver);
     return ret;
 }
 
@@ -1535,12 +1547,14 @@ static char *qemudDomainGetOSType(virDomainPtr dom) {
         goto cleanup;
     }
 
+    virDomainObjLock(vm);
     if (!(type = strdup(vm->def->os.type)))
         virReportOOMError();
+    virDomainObjUnlock(vm);
 
 cleanup:
     if (vm)
-        virDomainObjUnlock(vm);
+        virDomainObjUnref(vm);
     return type;
 }
 
@@ -1562,11 +1576,13 @@ static unsigned long qemudDomainGetMaxMemory(virDomainPtr dom) {
         goto cleanup;
     }
 
+    virDomainObjLock(vm);
     ret = vm->def->mem.max_balloon;
+    virDomainObjUnlock(vm);
 
 cleanup:
     if (vm)
-        virDomainObjUnlock(vm);
+        virDomainObjUnref(vm);
     return ret;
 }
 
@@ -1594,18 +1610,18 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
 
+    if (qemuDomainObjBeginJob(vm) < 0)
+        goto cleanup;
+
     if (newmem > vm->def->mem.max_balloon) {
         qemuReportError(VIR_ERR_INVALID_ARG,
                         "%s", _("cannot set memory higher than max memory"));
-        goto cleanup;
+        goto endjob;
     }
 
-    if (qemuDomainObjBeginJob(vm) < 0)
-        goto cleanup;
-
     if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_MEM_LIVE)) {
         qemuReportError(VIR_ERR_OPERATION_INVALID,
                         "%s", _("domain is not running"));
@@ -1647,12 +1663,10 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
 
     ret = 0;
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
+    virDomainObjUnref(vm);
     return ret;
 }
 
@@ -1676,9 +1690,12 @@ static int qemudDomainGetInfo(virDomainPtr dom,
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
 
+    if (qemuDomainObjBeginJob(vm) < 0)
+        goto cleanup;
+
     info->state = vm->state;
 
     if (!virDomainObjIsActive(vm)) {
@@ -1687,7 +1704,7 @@ static int qemudDomainGetInfo(virDomainPtr dom,
         if (qemudGetProcessInfo(&(info->cpuTime), NULL, vm->pid, 0) < 0) {
             qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
                             _("cannot read cputime for domain"));
-            goto cleanup;
+            goto endjob;
         }
     }
 
@@ -1700,8 +1717,6 @@ static int qemudDomainGetInfo(virDomainPtr dom,
             (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) {
             info->memory = vm->def->mem.max_balloon;
         } else if (!priv->jobActive) {
-            if (qemuDomainObjBeginJob(vm) < 0)
-                goto cleanup;
             if (!virDomainObjIsActive(vm))
                 err = 0;
             else {
@@ -1709,10 +1724,6 @@ static int qemudDomainGetInfo(virDomainPtr dom,
                 err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
                 qemuDomainObjExitMonitor(vm);
             }
-            if (qemuDomainObjEndJob(vm) == 0) {
-                vm = NULL;
-                goto cleanup;
-            }
 
             if (err < 0)
                 goto cleanup;
@@ -1731,9 +1742,10 @@ static int qemudDomainGetInfo(virDomainPtr dom,
     info->nrVirtCpu = vm->def->vcpus;
     ret = 0;
 
+endjob:
+    qemuDomainObjEndJob(vm);
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
+    virDomainObjUnref(vm);
     return ret;
 }
 
@@ -2006,9 +2018,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_SAVED);
     if (!vm->persistent) {
-        if (qemuDomainObjEndJob(vm) > 0)
-            virDomainRemoveInactive(&driver->domains,
-                                    vm);
+        qemuDomainObjEndJob(vm);
+        virDomainRemoveInactive(&driver->domains, vm);
         vm = NULL;
     }
 
@@ -2021,8 +2032,8 @@ endjob:
                     VIR_WARN0("Unable to resume guest CPUs after save failure");
             }
         }
-        if (qemuDomainObjEndJob(vm) == 0)
-            vm = NULL;
+
+        qemuDomainObjEndJob(vm);
     }
 
 cleanup:
@@ -2303,6 +2314,7 @@ static int qemudDomainCoreDump(virDomainPtr dom,
 
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+    qemuDriverUnlock(driver);
 
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -2311,11 +2323,12 @@ static int qemudDomainCoreDump(virDomainPtr dom,
                         _("no domain with matching uuid '%s'"), uuidstr);
         goto cleanup;
     }
-    priv = vm->privateData;
 
     if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
         goto cleanup;
 
+    priv = vm->privateData;
+
     if (!virDomainObjIsActive(vm)) {
         qemuReportError(VIR_ERR_OPERATION_INVALID,
                         "%s", _("domain is not running"));
@@ -2367,20 +2380,20 @@ endjob:
         }
     }
 
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
-    else if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) {
+    qemuDomainObjEndJob(vm);
+    if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) {
+        qemuDriverLock(driver);
         virDomainRemoveInactive(&driver->domains,
                                 vm);
+        qemuDriverUnlock(driver);
         vm = NULL;
     }
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
-    qemuDriverUnlock(driver);
+    if (vm)
+        virDomainObjUnref(vm);
     return ret;
 }
 
@@ -2403,9 +2416,6 @@ static void processWatchdogEvent(void *data, void *opaque)
                 break;
             }
 
-            qemuDriverLock(driver);
-            virDomainObjLock(wdEvent->vm);
-
             if (qemuDomainObjBeginJobWithDriver(driver, wdEvent->vm) < 0)
                 break;
 
@@ -2429,10 +2439,7 @@ static void processWatchdogEvent(void *data, void *opaque)
                 qemuReportError(VIR_ERR_OPERATION_FAILED,
                                 "%s", _("Resuming after dump failed"));
 
-            if (qemuDomainObjEndJob(wdEvent->vm) > 0)
-                virDomainObjUnlock(wdEvent->vm);
-
-            qemuDriverUnlock(driver);
+            qemuDomainObjEndJob(wdEvent->vm);
 
             VIR_FREE(dumpfile);
         }
@@ -2533,7 +2540,7 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
 
     if (qemuDomainObjBeginJob(vm) < 0)
@@ -2608,12 +2615,10 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
         ret = virDomainSaveConfig(driver->configDir, persistentDef);
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
+    virDomainObjUnref(vm);
     return ret;
 }
 
@@ -2645,7 +2650,7 @@ qemudDomainPinVcpu(virDomainPtr dom,
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
 
     if (!virDomainObjIsActive(vm)) {
@@ -2690,8 +2695,7 @@ qemudDomainPinVcpu(virDomainPtr dom,
     ret = 0;
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
+    virDomainObjUnref(vm);
     return ret;
 }
 
@@ -2717,7 +2721,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
         virUUIDFormat(dom->uuid, uuidstr);
         qemuReportError(VIR_ERR_NO_DOMAIN,
                         _("no domain with matching uuid '%s'"), uuidstr);
-        goto cleanup;
+        return -1;
     }
 
     if (!virDomainObjIsActive(vm)) {
@@ -2780,8 +2784,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
     ret = maxinfo;
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
+    virDomainObjUnref(vm);
     return ret;
 }
 
@@ -3164,9 +3167,8 @@ qemuDomainRestore(virConnectPtr conn,
 
     ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path);
 
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
-    else if (ret < 0 && !vm->persistent) {
+    qemuDomainObjEndJob(vm);
+    if (ret < 0 && !vm->persistent) {
         virDomainRemoveInactive(&driver->domains, vm);
         vm = NULL;
     }
@@ -3174,8 +3176,6 @@ qemuDomainRestore(virConnectPtr conn,
 cleanup:
     virDomainDefFree(def);
     VIR_FORCE_CLOSE(fd);
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -3254,10 +3254,7 @@ static char *qemudDomainDumpXML(virDomainPtr dom,
             qemuDomainObjEnterMonitorWithDriver(driver, vm);
             err = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
             qemuDomainObjExitMonitorWithDriver(driver, vm);
-            if (qemuDomainObjEndJob(vm) == 0) {
-                vm = NULL;
-                goto cleanup;
-            }
+            qemuDomainObjEndJob(vm);
             if (err < 0)
                 goto cleanup;
             if (err > 0)
@@ -3269,8 +3266,6 @@ static char *qemudDomainDumpXML(virDomainPtr dom,
     ret = qemuDomainFormatXML(driver, vm, flags);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -3487,12 +3482,9 @@ qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags)
                               (flags & VIR_DOMAIN_START_PAUSED) != 0);
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -3859,8 +3851,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
         ret = -1;
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
     if (cgroup)
@@ -3868,8 +3859,6 @@ cleanup:
 
     qemuCapsFree(qemuCaps);
     virDomainDeviceDefFree(dev);
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -3993,8 +3982,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
         ret = -1;
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
     if (cgroup)
@@ -4002,8 +3990,6 @@ cleanup:
 
     qemuCapsFree(qemuCaps);
     virDomainDeviceDefFree(dev);
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -4083,14 +4069,11 @@ static int qemudDomainDetachDevice(virDomainPtr dom,
         ret = -1;
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
     qemuCapsFree(qemuCaps);
     virDomainDeviceDefFree(dev);
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -4802,12 +4785,9 @@ qemudDomainBlockStats (virDomainPtr dom,
     qemuDomainObjExitMonitor(vm);
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -4906,12 +4886,9 @@ qemudDomainMemoryStats (virDomainPtr dom,
                         "%s", _("domain is not running"));
     }
 
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -5064,15 +5041,12 @@ qemudDomainMemoryPeek (virDomainPtr dom,
     ret = 0;
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
     VIR_FORCE_CLOSE(fd);
     unlink (tmp);
     VIR_FREE(tmp);
-    if (vm)
-        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -5218,16 +5192,13 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
             qemuDomainObjExitMonitor(vm);
         }
 
-        if (qemuDomainObjEndJob(vm) == 0)
-            vm = NULL;
+        qemuDomainObjEndJob(vm);
     } else {
         ret = 0;
     }
 
 cleanup:
     VIR_FORCE_CLOSE(fd);
-    if (vm)
-        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -6067,8 +6038,8 @@ cleanup:
                         _("resuming after snapshot failed"));
     }
 
-    if (qemuDomainObjEndJob(vm) == 0)
-        *vmptr = NULL;
+    qemuDomainObjEndJob(vm);
+    *vmptr = NULL;
 
     return ret;
 }
@@ -6438,9 +6409,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                                              VIR_DOMAIN_EVENT_STOPPED,
                                              VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
             if (!vm->persistent) {
-                if (qemuDomainObjEndJob(vm) > 0)
-                    virDomainRemoveInactive(&driver->domains, vm);
-                vm = NULL;
+                qemuDomainObjEndJob(vm);
+                virDomainRemoveInactive(&driver->domains, vm);
                 goto cleanup;
             }
         }
@@ -6454,14 +6424,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
     ret = 0;
 
 endjob:
-    if (vm && qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    if (vm)
+        qemuDomainObjEndJob(vm);
 
 cleanup:
     if (event)
         qemuDomainEventQueue(driver, event);
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
 
     return ret;
@@ -6675,12 +6643,9 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
     ret = qemuDomainSnapshotDiscard(driver, vm, snap);
 
 endjob:
-    if (qemuDomainObjEndJob(vm) == 0)
-        vm = NULL;
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
@@ -6727,14 +6692,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd,
     qemuDomainObjEnterMonitorWithDriver(driver, vm);
     ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp);
     qemuDomainObjExitMonitorWithDriver(driver, vm);
-    if (qemuDomainObjEndJob(vm) == 0) {
-        vm = NULL;
-        goto cleanup;
-    }
+    qemuDomainObjEndJob(vm);
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
     return ret;
 }
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 43741e1..462e6be 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -336,8 +336,8 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
         qemuAuditDomainStart(vm, "migrated", false);
         qemuProcessStop(driver, vm, 0);
         if (!vm->persistent) {
-            if (qemuDomainObjEndJob(vm) > 0)
-                virDomainRemoveInactive(&driver->domains, vm);
+            qemuDomainObjEndJob(vm);
+            virDomainRemoveInactive(&driver->domains, vm);
             vm = NULL;
         }
         virReportSystemError(errno, "%s",
@@ -353,9 +353,10 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver,
     ret = 0;
 
 endjob:
-    if (vm &&
-        qemuDomainObjEndJob(vm) == 0)
+    if (vm) {
+        qemuDomainObjEndJob(vm);
         vm = NULL;
+    }
 
     /* We set a fake job active which is held across
      * API calls until the finish() call. This prevents
@@ -374,11 +375,8 @@ cleanup:
     virDomainDefFree(def);
     VIR_FORCE_CLOSE(dataFD[0]);
     VIR_FORCE_CLOSE(dataFD[1]);
-    if (vm)
-        virDomainObjUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
-    qemuDriverUnlock(driver);
     return ret;
 }
 
@@ -530,8 +528,8 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver,
          * should have already done that.
          */
         if (!vm->persistent) {
-            if (qemuDomainObjEndJob(vm) > 0)
-                virDomainRemoveInactive(&driver->domains, vm);
+            qemuDomainObjEndJob(vm);
+            virDomainRemoveInactive(&driver->domains, vm);
             vm = NULL;
         }
         goto endjob;
@@ -544,9 +542,10 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver,
     ret = 0;
 
 endjob:
-    if (vm &&
-        qemuDomainObjEndJob(vm) == 0)
+    if (vm) {
+        qemuDomainObjEndJob(vm);
         vm = NULL;
+    }
 
     /* We set a fake job active which is held across
      * API calls until the finish() call. This prevents
@@ -565,8 +564,6 @@ cleanup:
     virDomainDefFree(def);
     if (ret != 0)
         VIR_FREE(*uri_out);
-    if (vm)
-        virDomainObjUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
     return ret;
@@ -1090,8 +1087,8 @@ int qemuMigrationPerform(struct qemud_driver *driver,
                                      VIR_DOMAIN_EVENT_STOPPED_MIGRATED);
     if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) {
         virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm);
-        if (qemuDomainObjEndJob(vm) > 0)
-            virDomainRemoveInactive(&driver->domains, vm);
+        qemuDomainObjEndJob(vm);
+         virDomainRemoveInactive(&driver->domains, vm);
         vm = NULL;
     }
     ret = 0;
@@ -1112,13 +1109,12 @@ endjob:
                                          VIR_DOMAIN_EVENT_RESUMED,
                                          VIR_DOMAIN_EVENT_RESUMED_MIGRATED);
     }
-    if (vm &&
-        qemuDomainObjEndJob(vm) == 0)
+    if (vm) {
+        qemuDomainObjEndJob(vm);
         vm = NULL;
+    }
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
     return ret;
@@ -1266,20 +1262,19 @@ qemuMigrationFinish(struct qemud_driver *driver,
                                          VIR_DOMAIN_EVENT_STOPPED,
                                          VIR_DOMAIN_EVENT_STOPPED_FAILED);
         if (!vm->persistent) {
-            if (qemuDomainObjEndJob(vm) > 0)
-                virDomainRemoveInactive(&driver->domains, vm);
+            qemuDomainObjEndJob(vm);
+            virDomainRemoveInactive(&driver->domains, vm);
             vm = NULL;
         }
     }
 
 endjob:
-    if (vm &&
-        qemuDomainObjEndJob(vm) == 0)
+    if (vm) {
+        qemuDomainObjEndJob(vm);
         vm = NULL;
+    }
 
 cleanup:
-    if (vm)
-        virDomainObjUnlock(vm);
     if (event)
         qemuDomainEventQueue(driver, event);
     return dom;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 48ecd5c..244b22a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -604,8 +604,8 @@ static void qemuProcessHandleMonitorDestroy(qemuMonitorPtr mon,
     priv = vm->privateData;
     if (priv->mon == mon)
         priv->mon = NULL;
-    if (virDomainObjUnref(vm) > 0)
-        virDomainObjUnlock(vm);
+    virDomainObjUnlock(vm);
+    virDomainObjUnref(vm);
 }
 
 static qemuMonitorCallbacks monitorCallbacks = {
@@ -644,7 +644,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm)
 
     /* Safe to ignore value since ref count was incremented above */
     if (priv->mon == NULL)
-        ignore_value(virDomainObjUnref(vm));
+        virDomainObjUnref(vm);
 
     if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0) {
         VIR_ERROR(_("Failed to clear security context for monitor for %s"),
@@ -1940,10 +1940,6 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa
 
     priv = obj->privateData;
 
-    /* Hold an extra reference because we can't allow 'vm' to be
-     * deleted if qemuConnectMonitor() failed */
-    virDomainObjRef(obj);
-
     /* XXX check PID liveliness & EXE path */
     if (qemuConnectMonitor(driver, obj) < 0)
         goto error;
@@ -1975,8 +1971,7 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa
     if (obj->def->id >= driver->nextvmid)
         driver->nextvmid = obj->def->id + 1;
 
-    if (virDomainObjUnref(obj) > 0)
-        virDomainObjUnlock(obj);
+    virDomainObjUnlock(obj);
 
     qemuCapsFree(qemuCaps);
     return;
@@ -1984,21 +1979,17 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa
 error:
     qemuCapsFree(qemuCaps);
     if (!virDomainObjIsActive(obj)) {
-        if (virDomainObjUnref(obj) > 0)
-            virDomainObjUnlock(obj);
+        virDomainObjUnlock(obj);
         return;
     }
 
-    if (virDomainObjUnref(obj) > 0) {
-        /* We can't get the monitor back, so must kill the VM
-         * to remove danger of it ending up running twice if
-         * user tries to start it again later */
-        qemuProcessStop(driver, obj, 0);
-        if (!obj->persistent)
-            virDomainRemoveInactive(&driver->domains, obj);
-        else
-            virDomainObjUnlock(obj);
-    }
+    /* We can't get the monitor back, so must kill the VM
+     * to remove danger of it ending up running twice if
+     * user tries to start it again later */
+    qemuProcessStop(driver, obj, 0);
+    virDomainObjUnlock(obj);
+    if (!obj->persistent)
+        virDomainRemoveInactive(&driver->domains, obj);
 }
 
 /**
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
index 6339248..6b59b18 100644
--- a/src/vmware/vmware_conf.c
+++ b/src/vmware/vmware_conf.c
@@ -205,7 +205,7 @@ cleanup:
     VIR_FREE(vmx);
     /* any non-NULL vm here has not been shared, so unref will return 0 */
     if (vm)
-        ignore_value(virDomainObjUnref(vm));
+        virDomainObjUnref(vm);
     return ret;
 }
 
-- 
1.7.3.1


-- 
Thanks,
Hu Tao




More information about the libvir-list mailing list