Force auto cleanup conditionally

Relying simple on domain not being running resulted in storage errors
of attempting to remove it while still in use (domain still running).
Didn't identify the cause, which is unfortunate as now it requires an
extra parameter.

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

Author: ben-grande

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/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_remove_from_disk(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
@@ -774,31 +774,32 @@ def _preload_cleanup(self) -> None:
             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
@@ -809,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 self.auto_cleanup or (not running and 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):
         """
@@ -829,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()