lib: Grab write lock when modifying list of domains

In some places where virDomainObjListForEach() is called the
passed callback calls virDomainObjListRemoveLocked(). Well, this
is unsafe, because the former only grabs a read lock but the
latter modifies the list.
I've identified the following unsafe calls:

- qemuProcessReconnectAll()
- libxlReconnectDomains()

The rest seem to be safe.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

src/bhyve/bhyve_driver.c

@@ -114,7 +114,7 @@ bhyveAutostartDomains(bhyveConnPtr driver)
 
     struct bhyveAutostartData data = { driver, conn };
 
-    virDomainObjListForEach(driver->domains, bhyveAutostartDomain, &data);
+    virDomainObjListForEach(driver->domains, false, bhyveAutostartDomain, &data);
 
     virObjectUnref(conn);
 }

src/bhyve/bhyve_process.c

@@ -466,7 +466,7 @@ virBhyveProcessReconnectAll(bhyveConnPtr driver)
     data.driver = driver;
     data.kd = kd;
 
-    virDomainObjListForEach(driver->domains, virBhyveProcessReconnect, &data);
+    virDomainObjListForEach(driver->domains, false, virBhyveProcessReconnect, &data);
 
     kvm_close(kd);
 }

src/conf/virdomainobjlist.c

@@ -818,25 +818,33 @@ virDomainObjListHelper(void *payload,
 /**
  * virDomainObjListForEach:
  * @doms: Pointer to the domain object list
+ * @modify: Whether to lock @doms for modify operation
  * @callback: callback to run over each domain on the list
  * @opaque: opaque data to pass to @callback
  *
  * For every domain on the list (@doms) run @callback on it. If
  * @callback fails (i.e. returns a negative value), the iteration
- * carries still on until all domains are visited.
+ * carries still on until all domains are visited. Moreover, if
+ * @callback wants to modify the list of domains (@doms) then
+ * @modify must be set to true.
  *
  * Returns: 0 on success,
  *         -1 otherwise.
  */
 int
 virDomainObjListForEach(virDomainObjListPtr doms,
+                        bool modify,
                         virDomainObjListIterator callback,
                         void *opaque)
 {
     struct virDomainListIterData data = {
         callback, opaque, 0,
     };
-    virObjectRWLockRead(doms);
+
+    if (modify)
+        virObjectRWLockWrite(doms);
+    else
+        virObjectRWLockRead(doms);
     virHashForEach(doms->objs, virDomainObjListHelper, &data);
     virObjectRWUnlock(doms);
     return data.ret;

src/conf/virdomainobjlist.h

@@ -91,6 +91,7 @@ typedef int (*virDomainObjListIterator)(virDomainObjPtr dom,
                                         void *opaque);
 
 int virDomainObjListForEach(virDomainObjListPtr doms,
+                            bool modify,
                             virDomainObjListIterator callback,
                             void *opaque);
 

src/libxl/libxl_driver.c

@@ -497,7 +497,7 @@ libxlReconnectDomain(virDomainObjPtr vm,
 static void
 libxlReconnectDomains(libxlDriverPrivatePtr driver)
 {
-    virDomainObjListForEach(driver->domains, libxlReconnectDomain, driver);
+    virDomainObjListForEach(driver->domains, true, libxlReconnectDomain, driver);
 }
 
 static int
@@ -800,10 +800,12 @@ libxlStateInitialize(bool privileged,
                                        NULL, NULL) < 0)
         goto error;
 
-    virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
+    virDomainObjListForEach(libxl_driver->domains, false,
+                            libxlAutostartDomain,
                             libxl_driver);
 
-    virDomainObjListForEach(libxl_driver->domains, libxlDomainManagedSaveLoad,
+    virDomainObjListForEach(libxl_driver->domains, false,
+                            libxlDomainManagedSaveLoad,
                             libxl_driver);
 
     return VIR_DRV_STATE_INIT_COMPLETE;
@@ -832,7 +834,8 @@ libxlStateReload(void)
                                    libxl_driver->xmlopt,
                                    NULL, libxl_driver);
 
-    virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
+    virDomainObjListForEach(libxl_driver->domains, false,
+                            libxlAutostartDomain,
                             libxl_driver);
 
     virObjectUnref(cfg);

src/lxc/lxc_process.c

@@ -1639,7 +1639,7 @@ virLXCProcessAutostartAll(virLXCDriverPtr driver)
 
     struct virLXCProcessAutostartData data = { driver, conn };
 
-    virDomainObjListForEach(driver->domains,
+    virDomainObjListForEach(driver->domains, false,
                             virLXCProcessAutostartDomain,
                             &data);
 
@@ -1760,6 +1760,6 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
 int virLXCProcessReconnectAll(virLXCDriverPtr driver,
                               virDomainObjListPtr doms)
 {
-    virDomainObjListForEach(doms, virLXCProcessReconnectDomain, driver);
+    virDomainObjListForEach(doms, false, virLXCProcessReconnectDomain, driver);
     return 0;
 }

src/qemu/qemu_driver.c

@@ -304,7 +304,7 @@ qemuAutostartDomain(virDomainObjPtr vm,
 static void
 qemuAutostartDomains(virQEMUDriverPtr driver)
 {
-    virDomainObjListForEach(driver->domains, qemuAutostartDomain, driver);
+    virDomainObjListForEach(driver->domains, false, qemuAutostartDomain, driver);
 }
 
 
@@ -1055,10 +1055,12 @@ qemuStateInitialize(bool privileged,
      * the driver with. This is to avoid race between autostart and reconnect
      * threads */
     virDomainObjListForEach(qemu_driver->domains,
+                            false,
                             qemuDomainFindMaxID,
                             &qemu_driver->lastvmid);
 
     virDomainObjListForEach(qemu_driver->domains,
+                            false,
                             qemuDomainNetsRestart,
                             NULL);
 
@@ -1072,14 +1074,17 @@ qemuStateInitialize(bool privileged,
         goto error;
 
     virDomainObjListForEach(qemu_driver->domains,
+                            false,
                             qemuDomainSnapshotLoad,
                             cfg->snapshotDir);
 
     virDomainObjListForEach(qemu_driver->domains,
+                            false,
                             qemuDomainCheckpointLoad,
                             cfg->checkpointDir);
 
     virDomainObjListForEach(qemu_driver->domains,
+                            false,
                             qemuDomainManagedSaveLoad,
                             qemu_driver);
 

src/qemu/qemu_process.c

@@ -8369,7 +8369,8 @@ void
 qemuProcessReconnectAll(virQEMUDriverPtr driver)
 {
     struct qemuProcessReconnectData data = {.driver = driver};
-    virDomainObjListForEach(driver->domains, qemuProcessReconnectHelper, &data);
+    virDomainObjListForEach(driver->domains, true,
+                            qemuProcessReconnectHelper, &data);
 }
 
 

src/vmware/vmware_driver.c

@@ -990,7 +990,7 @@ static int vmwareDomainObjListUpdateDomain(virDomainObjPtr dom, void *data)
 static void
 vmwareDomainObjListUpdateAll(virDomainObjListPtr doms, struct vmware_driver *driver)
 {
-    virDomainObjListForEach(doms, vmwareDomainObjListUpdateDomain, driver);
+    virDomainObjListForEach(doms, false, vmwareDomainObjListUpdateDomain, driver);
 }
 
 static int