Remove preloaded disposables on test tearDown

qubes/tests/api_admin.py

@@ -3977,7 +3977,6 @@ def test_642_vm_create_disposable_not_allowed(self, storage_mock):
             self.call_mgmt_func(b"admin.vm.CreateDisposable", b"test-vm1")
         self.assertFalse(self.app.save.called)
 
-    @unittest.mock.patch("qubes.vm.dispvm.DispVM._bare_cleanup")
     @unittest.mock.patch("qubes.vm.dispvm.DispVM.start")
     @unittest.mock.patch("qubes.storage.Storage.verify")
     @unittest.mock.patch("qubes.storage.Storage.create")
@@ -3986,12 +3985,10 @@ def test_643_vm_create_disposable_preload_autostart(
         mock_storage_create,
         mock_storage_verify,
         mock_dispvm_start,
-        mock_bare_cleanup,
     ):
         mock_storage_create.side_effect = self.dummy_coro
         mock_storage_verify.side_effect = self.dummy_coro
         mock_dispvm_start.side_effect = self.dummy_coro
-        mock_bare_cleanup.side_effect = self.dummy_coro
         self.vm.template_for_dispvms = True
         self.app.default_dispvm = self.vm
         self.vm.add_handler(

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:
+    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,35 +765,41 @@ 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])
 
-    async def _bare_cleanup(self) -> None:
-        """
-        Cleanup bare disposable objects.
-        """
-        if self in self.app.domains:
-            del self.app.domains[self]
-            await self.remove_from_disk()
-            self.app.save()
-
-    async def _auto_cleanup(self) -> None:
+    async def _auto_cleanup(self, force: bool = False) -> None:
         """
         Do auto cleanup if enabled.
+
+        :param bool force: Auto clean up even if property is disabled
         """
-        if not self.auto_cleanup:
+        if not self.auto_cleanup and not force:
             return
         self._preload_cleanup()
-        if self in self.app.domains:
-            await self._bare_cleanup()
+        if self not in self.app.domains:
+            return
+        del self.app.domains[self]
+        await self.remove_from_disk()
+        self.app.save()
 
-    async def cleanup(self) -> None:
+    async def cleanup(self, force: bool = False) -> None:
         """
         Clean up after the disposable.
 
         This stops the disposable qube and removes it from the store.
         This method modifies :file:`qubes.xml` file.
+
+        :param bool force: Auto clean up if property is enabled and domain \
+                is not running, should be used in special circumstances only \
+                as the sole purpose of this option is because using it may not \
+                be reliable.
         """
         if self not in self.app.domains:
             return
@@ -804,10 +810,10 @@ 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:
-            self._preload_cleanup()
-            if self in self.app.domains:
-                await self._bare_cleanup()
+        if not self.auto_cleanup or (
+            force and not running and self.auto_cleanup
+        ):
+            await self._auto_cleanup(force=force)
 
     async def start(self, **kwargs):
         """
@@ -824,11 +830,7 @@ async def start(self, **kwargs):
             await super().start(**kwargs)
         except:
             # Cleanup also on failed startup
-            try:
-                await self.kill()
-            except qubes.exc.QubesVMNotStartedError:
-                pass
-            await self._auto_cleanup()
+            await self.cleanup()
             raise
 
     def create_qdb_entries(self) -> None:

qubes/vm/mix/dvmtemplate.py

@@ -105,7 +105,7 @@ def on_domain_loaded(self, event) -> None:
                 [qube.name for qube in preload_in_progress]
             )
             for dispvm in preload_in_progress:
-                asyncio.ensure_future(dispvm.cleanup())
+                asyncio.ensure_future(dispvm.cleanup(force=True))
 
         if changes:
             self.app.save()