[libvirt] [PATCH 02/19] qemu: Consolidate BeginJob{, WithDriver} into a single method

Jiri Denemark jdenemar at redhat.com
Thu Jul 7 23:34:07 UTC 2011


This avoids code duplication and also avoids relying on good luck that
ignore_value(virDomainObjUnref(obj)) doesn't set errno.
---
 src/qemu/qemu_domain.c |   93 ++++++++++++++++++++++--------------------------
 1 files changed, 43 insertions(+), 50 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bbdfdc4..8f3eaa7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -517,20 +517,17 @@ qemuDomainObjDiscardJob(virDomainObjPtr obj)
     qemuDomainObjSetJob(obj, QEMU_JOB_NONE);
 }
 
-/*
- * obj must be locked before calling, qemud_driver must NOT 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.
- *
- * Upon successful return, the object will have its ref count increased,
- * successful calls must be followed by EndJob eventually
- */
-
 /* Give up waiting for mutex after 30 seconds */
 #define QEMU_JOB_WAIT_TIME (1000ull * 30)
 
-int qemuDomainObjBeginJob(virDomainObjPtr obj)
+/*
+ * obj must be locked before calling; driver_locked says if qemu_driver is
+ * locked or not.
+ */
+static int
+qemuDomainObjBeginJobInternal(struct qemud_driver *driver,
+                              bool driver_locked,
+                              virDomainObjPtr obj)
 {
     qemuDomainObjPrivatePtr priv = obj->privateData;
     unsigned long long now;
@@ -541,17 +538,24 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj)
     then = now + QEMU_JOB_WAIT_TIME;
 
     virDomainObjRef(obj);
+    if (driver_locked)
+        qemuDriverUnlock(driver);
 
     while (priv->job.active) {
         if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) {
-            /* Safe to ignore value since ref count was incremented above */
-            ignore_value(virDomainObjUnref(obj));
             if (errno == ETIMEDOUT)
                 qemuReportError(VIR_ERR_OPERATION_TIMEOUT,
                                 "%s", _("cannot acquire state change lock"));
             else
                 virReportSystemError(errno,
                                      "%s", _("cannot acquire job mutex"));
+            if (driver_locked) {
+                virDomainObjUnlock(obj);
+                qemuDriverLock(driver);
+                virDomainObjLock(obj);
+            }
+            /* Safe to ignore value since ref count was incremented above */
+            ignore_value(virDomainObjUnref(obj));
             return -1;
         }
     }
@@ -559,54 +563,43 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj)
     qemuDomainObjSetJob(obj, QEMU_JOB_UNSPECIFIED);
     priv->job.start = now;
 
+    if (driver_locked) {
+        virDomainObjUnlock(obj);
+        qemuDriverLock(driver);
+        virDomainObjLock(obj);
+    }
+
     return 0;
 }
 
 /*
- * obj must be locked before calling, qemud_driver must be locked
+ * obj must be locked before calling, qemud_driver must NOT 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.
+ *
+ * Upon successful return, the object will have its ref count increased,
+ * successful calls must be followed by EndJob eventually
+ */
+int qemuDomainObjBeginJob(virDomainObjPtr obj)
+{
+    return qemuDomainObjBeginJobInternal(NULL, false, obj);
+}
+
+/*
+ * obj must be locked before calling. If qemud_driver is passed, it MUST be
+ * locked; otherwise it MUST NOT 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.
+ *
+ * Upon successful return, the object will have its ref count increased,
+ * successful calls must be followed by EndJob eventually
  */
 int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver,
                                     virDomainObjPtr obj)
 {
-    qemuDomainObjPrivatePtr priv = obj->privateData;
-    unsigned long long now;
-    unsigned long long then;
-
-    if (virTimeMs(&now) < 0)
-        return -1;
-    then = now + QEMU_JOB_WAIT_TIME;
-
-    virDomainObjRef(obj);
-    qemuDriverUnlock(driver);
-
-    while (priv->job.active) {
-        if (virCondWaitUntil(&priv->job.cond, &obj->lock, then) < 0) {
-            if (errno == ETIMEDOUT)
-                qemuReportError(VIR_ERR_OPERATION_TIMEOUT,
-                                "%s", _("cannot acquire state change lock"));
-            else
-                virReportSystemError(errno,
-                                     "%s", _("cannot acquire job mutex"));
-            virDomainObjUnlock(obj);
-            qemuDriverLock(driver);
-            virDomainObjLock(obj);
-            /* Safe to ignore value since ref count was incremented above */
-            ignore_value(virDomainObjUnref(obj));
-            return -1;
-        }
-    }
-    qemuDomainObjResetJob(priv);
-    qemuDomainObjSetJob(obj, QEMU_JOB_UNSPECIFIED);
-    priv->job.start = now;
-
-    virDomainObjUnlock(obj);
-    qemuDriverLock(driver);
-    virDomainObjLock(obj);
-
-    return 0;
+    return qemuDomainObjBeginJobInternal(driver, true, obj);
 }
 
 /*
-- 
1.7.6




More information about the libvir-list mailing list