diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 418f765c5..34635529c 100644 --- a/qubes/tests/api_admin.py +++ b/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( diff --git a/qubes/tests/integ/dispvm.py b/qubes/tests/integ/dispvm.py index fe3a138a7..80a2edc1b 100644 --- a/qubes/tests/integ/dispvm.py +++ b/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") diff --git a/qubes/vm/dispvm.py b/qubes/vm/dispvm.py index cb74011e2..869f25f00 100644 --- a/qubes/vm/dispvm.py +++ b/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: diff --git a/qubes/vm/mix/dvmtemplate.py b/qubes/vm/mix/dvmtemplate.py index a14be4b8b..c2d18bcac 100644 --- a/qubes/vm/mix/dvmtemplate.py +++ b/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()