Browse Source

tests: remove VM under startup_lock

Prevent starting a VM while it's being removed. Something could try to
start a VM just after it's being killer but before removing it (Whonix
example from previous commit is real-life case). The window specifically
is between kill() call and removing it from collection
(`del app.domains[vm.qid]`). Grab a startup_lock for the whole operation
to prevent it.
Marek Marczykowski-Górecki 4 years ago
parent
commit
e0e0c7eaf9
1 changed files with 17 additions and 5 deletions
  1. 17 5
      qubes/tests/__init__.py

+ 17 - 5
qubes/tests/__init__.py

@@ -823,14 +823,15 @@ class SystemTestCase(QubesTestCase):
         app = vm.app
 
         try:
-            self.loop.run_until_complete(vm.remove_from_disk())
-        except:  # pylint: disable=bare-except
+            del app.domains[vm.qid]
+        except KeyError:
             pass
 
         try:
-            del app.domains[vm.qid]
-        except KeyError:
+            self.loop.run_until_complete(vm.remove_from_disk())
+        except:  # pylint: disable=bare-except
             pass
+
         vm.close()
         del vm
 
@@ -914,12 +915,18 @@ class SystemTestCase(QubesTestCase):
                 except:
                     pass
 
+        locked_vms = set()
+        # first take startup lock
+        for vm in vms:
+            self.loop.run_until_complete(vm.startup_lock.acquire())
+            locked_vms.add(vm)
+
         # first kill all the domains, to avoid side effects of changing netvm
         for vm in vms:
             try:
                 # XXX .is_running() may throw libvirtError if undefined
                 if vm.is_running():
-                    self.loop.run_until_complete(vm.kill())
+                    self.loop.run_until_complete(vm._kill_locked())
             except:  # pylint: disable=bare-except
                 pass
         # break dependencies
@@ -950,6 +957,11 @@ class SystemTestCase(QubesTestCase):
                 continue
             self._remove_vm_qubes(vm)
 
+        # release startup_lock, if anything was waiting at vm.start(),
+        # it will detect the VM is gone
+        for vm in locked_vms:
+            vm.startup_lock.release()
+
     def remove_test_vms(self, xmlpath=XMLPATH, prefix=VMPREFIX):
         '''Aggressively remove any domain that has name in testing namespace.