[libvirt] [PATCH RFC 2/5] qemu: job: drop nested job

Nikolay Shirokovskiy nshirokovskiy at virtuozzo.com
Mon Nov 28 08:15:43 UTC 2016


After the drop we don't need checks that nested job is requested
by the same thread that run async job and that there is async job
at all. Or that async job does not try to use monitor without
aquiring nested job.

Dropping this value from enum does not affect persistent
status xml too as nested job is not trackable and serialized/deserialized
just as QEMU_JOB_NONE.
---
 src/qemu/qemu_domain.c  | 81 +++++++++----------------------------------------
 src/qemu/qemu_domain.h  |  7 +----
 src/qemu/qemu_process.c | 32 +++++--------------
 3 files changed, 23 insertions(+), 97 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 137d4d5..40d46d6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -73,7 +73,6 @@ VIR_ENUM_IMPL(qemuDomainJob, QEMU_JOB_LAST,
               "abort",
               "migration operation",
               "none",   /* async job is never stored in job.active */
-              "async nested",
 );
 
 VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST,
@@ -3199,8 +3198,6 @@ qemuDomainObjDiscardAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj)
 {
     qemuDomainObjPrivatePtr priv = obj->privateData;
 
-    if (priv->job.active == QEMU_JOB_ASYNC_NESTED)
-        qemuDomainObjResetJob(priv);
     qemuDomainObjResetAsyncJob(priv);
     qemuDomainObjSaveJob(driver, obj);
 }
@@ -3248,7 +3245,6 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
     qemuDomainObjPrivatePtr priv = obj->privateData;
     unsigned long long now;
     unsigned long long then;
-    bool nested = job == QEMU_JOB_ASYNC_NESTED;
     bool async = job == QEMU_JOB_ASYNC;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     const char *blocker = NULL;
@@ -3281,7 +3277,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
         goto error;
     }
 
-    while (!nested && !qemuDomainNestedJobAllowed(priv, job)) {
+    while (!qemuDomainNestedJobAllowed(priv, job)) {
         VIR_DEBUG("Waiting for async job (vm=%p name=%s)", obj, obj->def->name);
         if (virCondWaitUntil(&priv->job.asyncCond, &obj->parent.lock, then) < 0)
             goto error;
@@ -3295,7 +3291,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
 
     /* No job is active but a new async job could have been started while obj
      * was unlocked, so we need to recheck it. */
-    if (!nested && !qemuDomainNestedJobAllowed(priv, job))
+    if (!qemuDomainNestedJobAllowed(priv, job))
         goto retry;
 
     qemuDomainObjResetJob(priv);
@@ -3350,7 +3346,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver,
              priv->job.asyncOwner, NULLSTR(priv->job.asyncOwnerAPI),
              duration / 1000, asyncDuration / 1000);
 
-    if (nested || qemuDomainNestedJobAllowed(priv, job))
+    if (qemuDomainNestedJobAllowed(priv, job))
         blocker = priv->job.ownerAPI;
     else
         blocker = priv->job.asyncOwnerAPI;
@@ -3419,30 +3415,6 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
         return 0;
 }
 
-int
-qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
-                            virDomainObjPtr obj,
-                            qemuDomainAsyncJob asyncJob)
-{
-    qemuDomainObjPrivatePtr priv = obj->privateData;
-
-    if (asyncJob != priv->job.asyncJob) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unexpected async job %d"), asyncJob);
-        return -1;
-    }
-
-    if (priv->job.asyncOwner != virThreadSelfID()) {
-        VIR_WARN("This thread doesn't seem to be the async job owner: %llu",
-                 priv->job.asyncOwner);
-    }
-
-    return qemuDomainObjBeginJobInternal(driver, obj,
-                                         QEMU_JOB_ASYNC_NESTED,
-                                         QEMU_ASYNC_JOB_NONE);
-}
-
-
 /*
  * obj must be locked and have a reference before calling
  *
@@ -3502,45 +3474,25 @@ qemuDomainObjAbortAsyncJob(virDomainObjPtr obj)
  *
  * To be called immediately before any QEMU monitor API call
  * Must have already either called qemuDomainObjBeginJob() and checked
- * that the VM is still active; may not be used for nested async jobs.
+ * that the VM is still active
  *
  * To be followed with qemuDomainObjExitMonitor() once complete
  */
-static int
-qemuDomainObjEnterMonitorInternal(virQEMUDriverPtr driver,
-                                  virDomainObjPtr obj,
-                                  qemuDomainAsyncJob asyncJob)
+static void
+qemuDomainObjEnterMonitorInternal(virDomainObjPtr obj)
 {
     qemuDomainObjPrivatePtr priv = obj->privateData;
 
-    if (asyncJob != QEMU_ASYNC_JOB_NONE) {
-        int ret;
-        if ((ret = qemuDomainObjBeginNestedJob(driver, obj, asyncJob)) < 0)
-            return ret;
-        if (!virDomainObjIsActive(obj)) {
-            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                           _("domain is no longer running"));
-            qemuDomainObjEndJob(driver, obj);
-            return -1;
-        }
-    } else if (priv->job.asyncOwner == virThreadSelfID()) {
-        VIR_WARN("This thread seems to be the async job owner; entering"
-                 " monitor without asking for a nested job is dangerous");
-    }
-
     VIR_DEBUG("Entering monitor (mon=%p vm=%p name=%s)",
               priv->mon, obj, obj->def->name);
     virObjectLock(priv->mon);
     virObjectRef(priv->mon);
     ignore_value(virTimeMillisNow(&priv->monStart));
     virObjectUnlock(obj);
-
-    return 0;
 }
 
 static void ATTRIBUTE_NONNULL(1)
-qemuDomainObjExitMonitorInternal(virQEMUDriverPtr driver,
-                                 virDomainObjPtr obj)
+qemuDomainObjExitMonitorInternal(virDomainObjPtr obj)
 {
     qemuDomainObjPrivatePtr priv = obj->privateData;
     bool hasRefs;
@@ -3557,16 +3509,12 @@ qemuDomainObjExitMonitorInternal(virQEMUDriverPtr driver,
     priv->monStart = 0;
     if (!hasRefs)
         priv->mon = NULL;
-
-    if (priv->job.active == QEMU_JOB_ASYNC_NESTED)
-        qemuDomainObjEndJob(driver, obj);
 }
 
-void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver,
+void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
                                virDomainObjPtr obj)
 {
-    ignore_value(qemuDomainObjEnterMonitorInternal(driver, obj,
-                                                   QEMU_ASYNC_JOB_NONE));
+    qemuDomainObjEnterMonitorInternal(obj);
 }
 
 /* obj must NOT be locked before calling
@@ -3579,10 +3527,10 @@ void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver,
  * and replaced by the persistent definition, so pointers stolen
  * from the live definition could no longer be valid.
  */
-int qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
+int qemuDomainObjExitMonitor(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
                              virDomainObjPtr obj)
 {
-    qemuDomainObjExitMonitorInternal(driver, obj);
+    qemuDomainObjExitMonitorInternal(obj);
     if (!virDomainObjIsActive(obj)) {
         if (!virGetLastError())
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
@@ -3607,11 +3555,12 @@ int qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
  * in the meantime).
  */
 int
-qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver,
+qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
                                virDomainObjPtr obj,
-                               qemuDomainAsyncJob asyncJob)
+                               qemuDomainAsyncJob asyncJob ATTRIBUTE_UNUSED)
 {
-    return qemuDomainObjEnterMonitorInternal(driver, obj, asyncJob);
+    qemuDomainObjEnterMonitorInternal(obj);
+    return 0;
 }
 
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7650ff3..7fd8cd6 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -73,9 +73,8 @@ typedef enum {
     QEMU_JOB_ABORT,         /* Abort current async job */
     QEMU_JOB_MIGRATION_OP,  /* Operation influencing outgoing migration */
 
-    /* The following two items must always be the last items before JOB_LAST */
+    /* The following item must always be the last item before JOB_LAST */
     QEMU_JOB_ASYNC,         /* Asynchronous job */
-    QEMU_JOB_ASYNC_NESTED,  /* Normal job within an async job */
 
     QEMU_JOB_LAST
 } qemuDomainJob;
@@ -413,10 +412,6 @@ int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver,
                                virDomainObjPtr obj,
                                qemuDomainAsyncJob asyncJob)
     ATTRIBUTE_RETURN_CHECK;
-int qemuDomainObjBeginNestedJob(virQEMUDriverPtr driver,
-                                virDomainObjPtr obj,
-                                qemuDomainAsyncJob asyncJob)
-    ATTRIBUTE_RETURN_CHECK;
 
 void qemuDomainObjEndJob(virQEMUDriverPtr driver,
                          virDomainObjPtr obj);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f8f379a..d9776c4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3132,8 +3132,6 @@ qemuProcessRecoverJob(virQEMUDriverPtr driver,
     case QEMU_JOB_MIGRATION_OP:
     case QEMU_JOB_ABORT:
     case QEMU_JOB_ASYNC:
-    case QEMU_JOB_ASYNC_NESTED:
-        /* async job was already handled above */
     case QEMU_JOB_NONE:
     case QEMU_JOB_LAST:
         break;
@@ -5916,7 +5914,7 @@ qemuProcessBeginStopJob(virQEMUDriverPtr driver,
 void qemuProcessStop(virQEMUDriverPtr driver,
                      virDomainObjPtr vm,
                      virDomainShutoffReason reason,
-                     qemuDomainAsyncJob asyncJob,
+                     qemuDomainAsyncJob asyncJob ATTRIBUTE_UNUSED,
                      unsigned int flags)
 {
     int ret;
@@ -5930,32 +5928,21 @@ void qemuProcessStop(virQEMUDriverPtr driver,
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
     VIR_DEBUG("Shutting down vm=%p name=%s id=%d pid=%lld, "
-              "reason=%s, asyncJob=%s, flags=%x",
+              "reason=%s, flags=%x",
               vm, vm->def->name, vm->def->id,
               (long long) vm->pid,
               virDomainShutoffReasonTypeToString(reason),
-              qemuDomainAsyncJobTypeToString(asyncJob),
               flags);
 
+    if (!virDomainObjIsActive(vm)) {
+        VIR_DEBUG("VM '%s' not active", vm->def->name);
+        return;
+    }
+
     /* This method is routinely used in clean up paths. Disable error
      * reporting so we don't squash a legit error. */
     orig_err = virSaveLastError();
 
-    if (asyncJob != QEMU_ASYNC_JOB_NONE) {
-        if (qemuDomainObjBeginNestedJob(driver, vm, asyncJob) < 0)
-            goto cleanup;
-    } else if (priv->job.asyncJob != QEMU_ASYNC_JOB_NONE &&
-               priv->job.asyncOwner == virThreadSelfID() &&
-               priv->job.active != QEMU_JOB_ASYNC_NESTED) {
-        VIR_WARN("qemuProcessStop called without a nested job (async=%s)",
-                 qemuDomainAsyncJobTypeToString(asyncJob));
-    }
-
-    if (!virDomainObjIsActive(vm)) {
-        VIR_DEBUG("VM '%s' not active", vm->def->name);
-        goto endjob;
-    }
-
     vm->def->id = -1;
 
     if (virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback)
@@ -6209,11 +6196,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
 
     virDomainObjRemoveTransientDef(vm);
 
- endjob:
-    if (asyncJob != QEMU_ASYNC_JOB_NONE)
-        qemuDomainObjEndJob(driver, vm);
-
- cleanup:
     if (orig_err) {
         virSetError(orig_err);
         virFreeError(orig_err);
-- 
1.8.3.1




More information about the libvir-list mailing list