Remove preloaded disposables on test tearDown

If it is not done on the test, it will be handled by
"qubes/tests/__init__.py", which will attempt to kill the domain. If the
preloaded disposable was still starting, exceptions will be handled by
also attempting to kill the domain. Both methods will trigger the
"_bare_cleanup()", sometimes indirectly via "cleanup()" or
"_auto_cleanup()", but if "_bare_cleanup()" happens on the other call,
not the one that called kill, it will not await and will attempt to
remove the domain from the disk while it is still running (not
completely killed).

For: #742
For: QubesOS/qubes-issues#1512
Fixes: QubesOS/qubes-issues#10369

Author: ben-grande

qubes/tests/integ/dispvm.py

@@ -305,9 +305,16 @@ def _on_domain_add(self, app, event, vm):  # pylint: disable=unused-argument
         self._register_handlers(vm)
 
     async def cleanup_preload_run(self, qube):
-        old_preload = qube.get_feat_preload()
+        old_preload = qube.features.get("preload-dispvm", "")
+        old_preload = old_preload.split(" ") if old_preload else []
+        if not old_preload:
+            return
+        logger.info(
+            "cleaning up preloaded disposables: %s:%s", qube.name, old_preload
+        )
         tasks = [self.app.domains[x].cleanup() for x in old_preload]
         await asyncio.gather(*tasks)
+        self.wait_for_dispvm_destroy(old_preload)
 
     def cleanup_preload(self):
         logger.info("start")
@@ -319,13 +326,24 @@ def cleanup_preload(self):
             logger.info("deleting global threshold feature")
             del self.app.domains["dom0"].features["preload-dispvm-threshold"]
         for qube in self.app.domains:
-            if "preload-dispvm-max" not in qube.features:
+            if "preload-dispvm-max" not in qube.features or qube not in [
+                self.app.domains["dom0"],
+                default_dispvm,
+                self.disp_base,
+                self.disp_base_alt,
+            ]:
                 continue
             logger.info("removing preloaded disposables: '%s'", qube.name)
-            if qube == default_dispvm:
-                self.loop.run_until_complete(
-                    self.cleanup_preload_run(default_dispvm)
+            target = qube
+            if qube.klass == "AdminVM" and default_dispvm:
+                target = default_dispvm
+            old_preload_max = qube.features.get("preload-dispvm-max") or 0
+            self.loop.run_until_complete(
+                self.wait_preload(
+                    old_preload_max, fail_on_timeout=False, timeout=20
                 )
+            )
+            self.loop.run_until_complete(self.cleanup_preload_run(target))
             logger.info("deleting max preload feature")
             del qube.features["preload-dispvm-max"]
         logger.info("end")

qubes/vm/dispvm.py

@@ -540,7 +540,7 @@ async def on_domain_shutdown(self, _event, **_kwargs) -> None:
         await self._auto_cleanup()
 
     @qubes.events.handler("domain-remove-from-disk")
-    async def on_domain_delete(self, _event, **_kwargs) -> None:
+    async def on_domain_remove_from_disk(self, _event, **_kwargs) -> None:
         """
         On volume removal, remove preloaded disposable from ``preload-dispvm``
         feature in disposable template. If the feature is still here, it means
@@ -765,7 +765,12 @@ def _preload_cleanup(self) -> None:
         """
         Cleanup preload from list.
         """
-        if self.name in self.template.get_feat_preload():
+        name = getattr(self, "name", None)
+        template = getattr(self, "template", None)
+        if not (name and template):
+            # Objects from self may be absent.
+            return
+        if name in template.get_feat_preload():
             self.log.info("Automatic cleanup removes qube from preload list")
             self.template.remove_preload_from_list([self.name])
 
@@ -804,7 +809,7 @@ async def cleanup(self) -> None:
             running = False
         # Full cleanup will be done automatically if event 'domain-shutdown' is
         # triggered and "auto_cleanup=True".
-        if not running or not self.auto_cleanup:
+        if not self.auto_cleanup or (not running and self.auto_cleanup):
             self._preload_cleanup()
             if self in self.app.domains:
                 await self._bare_cleanup()