[PATCH 1/2] qemu: drop needless acquiring job on removing domain

Nikolay Shirokovskiy nikolay.shirokovskiy at openvz.org
Wed Apr 13 08:43:31 UTC 2022


Acquiring job introduced in commit [1] to fix a race described in the
commit. Actually it does not help because we get domain in create API
before acuiring job. Then [2] fix the race but [1] was not reverted even
is does not required by [2] to work properly.

[1] commit b629c64e5e0a32ef439b8eeb3a697e2cd76f3248
Author: Martin Kletzander <mkletzan at redhat.com>
Date:   Thu Oct 30 14:38:35 2014 +0100

    qemu: avoid rare race when undefining domain

[2] commit c7d1c139ca3402e875002753952e80ce8054374e
Author: Martin Kletzander <mkletzan at redhat.com>
Date:   Thu Dec 11 11:14:08 2014 +0100

    qemu: avoid rare race when undefining domain

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at openvz.org>
---
 src/qemu/qemu_domain.c    | 45 +--------------------------------------
 src/qemu/qemu_domain.h    | 10 ++++-----
 src/qemu/qemu_driver.c    | 12 +++++------
 src/qemu/qemu_migration.c | 10 ++++-----
 src/qemu/qemu_process.c   | 12 ++++-------
 5 files changed, 20 insertions(+), 69 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a4334de158..95134a3570 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7343,7 +7343,7 @@ qemuDomainRemoveInactive(virQEMUDriver *driver,
  * lock on driver->domains in order to call the remove obj
  * from locked list method.
  */
-static void
+void
 qemuDomainRemoveInactiveLocked(virQEMUDriver *driver,
                                virDomainObj *vm)
 {
@@ -7357,49 +7357,6 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver,
     virDomainObjListRemoveLocked(driver->domains, vm);
 }
 
-/**
- * qemuDomainRemoveInactiveJob:
- *
- * Just like qemuDomainRemoveInactive but it tries to grab a
- * VIR_JOB_MODIFY first. Even though it doesn't succeed in
- * grabbing the job the control carries with
- * qemuDomainRemoveInactive call.
- */
-void
-qemuDomainRemoveInactiveJob(virQEMUDriver *driver,
-                            virDomainObj *vm)
-{
-    bool haveJob;
-
-    haveJob = qemuDomainObjBeginJob(driver, vm, VIR_JOB_MODIFY) >= 0;
-
-    qemuDomainRemoveInactive(driver, vm);
-
-    if (haveJob)
-        qemuDomainObjEndJob(vm);
-}
-
-
-/**
- * qemuDomainRemoveInactiveJobLocked:
- *
- * Similar to qemuDomainRemoveInactiveJob, except that the caller must
- * also hold the lock @driver->domains
- */
-void
-qemuDomainRemoveInactiveJobLocked(virQEMUDriver *driver,
-                                  virDomainObj *vm)
-{
-    bool haveJob;
-
-    haveJob = qemuDomainObjBeginJob(driver, vm, VIR_JOB_MODIFY) >= 0;
-
-    qemuDomainRemoveInactiveLocked(driver, vm);
-
-    if (haveJob)
-        qemuDomainObjEndJob(vm);
-}
-
 
 void
 qemuDomainSetFakeReboot(virDomainObj *vm,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index f4d84ca43a..0415a34908 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -672,6 +672,10 @@ int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriver *driver,
 void qemuDomainRemoveInactive(virQEMUDriver *driver,
                               virDomainObj *vm);
 
+void
+qemuDomainRemoveInactiveLocked(virQEMUDriver *driver,
+                               virDomainObj *vm);
+
 void qemuDomainSetFakeReboot(virDomainObj *vm,
                              bool value);
 
@@ -1036,12 +1040,6 @@ int
 qemuDomainDefNumaCPUsRectify(virDomainDef *def,
                              virQEMUCaps *qemuCaps);
 
-void qemuDomainRemoveInactiveJob(virQEMUDriver *driver,
-                                 virDomainObj *vm);
-
-void qemuDomainRemoveInactiveJobLocked(virQEMUDriver *driver,
-                                       virDomainObj *vm);
-
 int virQEMUFileOpenAs(uid_t fallback_uid,
                       gid_t fallback_gid,
                       bool dynamicOwnership,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8c0e36e9b2..ee0963c30d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1625,7 +1625,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn,
 
     if (qemuProcessBeginJob(driver, vm, VIR_DOMAIN_JOB_OPERATION_START,
                             flags) < 0) {
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
         goto cleanup;
     }
 
@@ -2751,7 +2751,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
     }
     qemuDomainObjEndAsyncJob(vm);
     if (ret == 0)
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
     virQEMUSaveDataFree(data);
@@ -3228,7 +3228,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
 
     qemuDomainObjEndAsyncJob(vm);
     if (ret == 0 && flags & VIR_DUMP_CRASH)
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
     virDomainObjEndAPI(&vm);
@@ -3536,7 +3536,7 @@ processGuestPanicEvent(virQEMUDriver *driver,
  endjob:
     qemuDomainObjEndAsyncJob(vm);
     if (removeInactive)
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
 }
 
 
@@ -5851,7 +5851,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
     virFileWrapperFdFree(wrapperFd);
     virQEMUSaveDataFree(data);
     if (vm && ret < 0)
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
     virDomainObjEndAPI(&vm);
     return ret;
 }
@@ -6510,7 +6510,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
         } else {
             /* Brand new domain. Remove it */
             VIR_INFO("Deleting domain '%s'", vm->def->name);
-            qemuDomainRemoveInactiveJob(driver, vm);
+            qemuDomainRemoveInactive(driver, vm);
         }
     }
 
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 30a6202d85..b735bdb391 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3100,7 +3100,7 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver,
             virPortAllocatorRelease(priv->nbdPort);
         priv->nbdPort = 0;
         virDomainObjRemoveTransientDef(vm);
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
     }
     virDomainObjEndAPI(&vm);
     virObjectEventStateQueue(driver->domainEventState, event);
@@ -3517,7 +3517,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver,
             virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
             vm->persistent = 0;
         }
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
     }
 
  cleanup:
@@ -5342,7 +5342,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver,
             virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
             vm->persistent = 0;
         }
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
     }
 
     virErrorRestore(&orig_err);
@@ -5416,7 +5416,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver,
     }
 
     if (!virDomainObjIsActive(vm))
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
 
     return ret;
 }
@@ -5877,7 +5877,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
 
     qemuMigrationJobFinish(vm);
     if (!virDomainObjIsActive(vm))
-        qemuDomainRemoveInactiveJob(driver, vm);
+        qemuDomainRemoveInactive(driver, vm);
 
  cleanup:
     g_clear_pointer(&jobData, virDomainJobDataFree);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2e11d24be2..8f0d1ae7e4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8979,14 +8979,10 @@ qemuProcessReconnect(void *opaque)
         driver->inhibitCallback(true, driver->inhibitOpaque);
 
  cleanup:
-    if (jobStarted) {
-        if (!virDomainObjIsActive(obj))
-            qemuDomainRemoveInactive(driver, obj);
+    if (jobStarted)
         qemuDomainObjEndJob(obj);
-    } else {
-        if (!virDomainObjIsActive(obj))
-            qemuDomainRemoveInactiveJob(driver, obj);
-    }
+    if (!virDomainObjIsActive(obj))
+        qemuDomainRemoveInactive(driver, obj);
     virDomainObjEndAPI(&obj);
     virIdentitySetCurrent(NULL);
     return;
@@ -9058,7 +9054,7 @@ qemuProcessReconnectHelper(virDomainObj *obj,
          */
         qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED,
                         VIR_ASYNC_JOB_NONE, 0);
-        qemuDomainRemoveInactiveJobLocked(src->driver, obj);
+        qemuDomainRemoveInactiveLocked(src->driver, obj);
 
         virDomainObjEndAPI(&obj);
         g_clear_object(&data->identity);
-- 
2.35.1



More information about the libvir-list mailing list