Easy dependency switch if only preload is in chain

qubes/api/internal.py

@@ -265,7 +265,7 @@ async def suspend_pre(self):
 
         preload_templates = qubes.vm.dispvm.get_preload_templates(self.app)
         for qube in preload_templates:
-            qube.remove_preload_excess(0)
+            qube.remove_preload_excess(0, reason="system wants to suspend")
 
         # first keep track of VMs which were paused before suspending
         previously_paused = [

qubes/app.py

@@ -1643,6 +1643,7 @@ def on_domain_pre_deleted(self, event, vm):
         :param qubes.vm.QubesVM name: Qube name.
         """
         # pylint: disable=unused-argument
+        dependencies = []
         for obj in itertools.chain(self.domains, (self,)):
             if obj is vm:
                 # allow removed VM to reference itself
@@ -1653,18 +1654,31 @@ def on_domain_pre_deleted(self, event, vm):
                         isinstance(prop, qubes.vm.VMProperty)
                         and getattr(obj, prop.__name__) == vm
                     ):
-                        self.log.error(
-                            "Cannot remove %s, used by %s.%s",
-                            vm,
-                            obj,
-                            prop.__name__,
-                        )
-                        raise qubes.exc.QubesVMInUseError(
-                            vm,
-                            "Domain is in use: {!r};"
-                            "see 'journalctl -u qubesd -e' in dom0 for "
-                            "details".format(vm.name),
-                        )
+                        if getattr(obj, "is_preload", False) and (
+                            prop.__name__ == "template"
+                            or (
+                                prop.__name__ == "default_dispvm"
+                                and getattr(obj, "template", None) == vm
+                            )
+                        ):
+                            continue
+                        if isinstance(obj, qubes.app.Qubes):
+                            dependencies.insert(0, ("@GLOBAL", prop.__name__))
+                        elif not obj.property_is_default(prop):
+                            dependencies.append((obj.name, prop.__name__))
+        if dependencies:
+            self.log.error(
+                "Cannot remove %s as it is used by %s",
+                vm,
+                ", ".join(
+                    ":".join(str(i) for i in tup) for tup in dependencies
+                ),
+            )
+            raise qubes.exc.QubesVMInUseError(
+                vm,
+                "Domain is in use: {!r};  see 'journalctl -u qubesd -e' in dom0"
+                " for details".format(vm.name),
+            )
         if isinstance(vm, qubes.vm.qubesvm.QubesVM):
             assignments = vm.get_provided_assignments()
         else:

qubes/ext/audio.py

@@ -69,10 +69,12 @@ def set_tag_and_qubesdb_entry(self, subject, event, newvalue=None):
     @qubes.ext.handler("domain-pre-shutdown")
     def on_domain_pre_shutdown(self, vm, event, **kwargs):
         attached_vms = [
-            domain for domain in self.attached_vms(vm) if domain.is_running()
+            domain
+            for domain in self.attached_vms(vm)
+            if domain.is_running() and not getattr(domain, "is_preload", False)
         ]
         if attached_vms and not kwargs.get("force", False):
-            raise qubes.exc.QubesVMError(
+            raise qubes.exc.QubesVMInUseError(
                 self,
                 "There are running VMs using this VM as AudioVM: "
                 "{}".format(", ".join(vm.name for vm in attached_vms)),

qubes/tests/api_internal.py

@@ -44,6 +44,7 @@ def setUp(self):
         self.app = mock.NonCallableMock()
         self.dom0 = mock.NonCallableMock(spec=qubes.vm.adminvm.AdminVM)
         self.dom0.name = "dom0"
+        self.dom0.klass = "AdminVM"
         self.dom0.features = {}
         self.domains = {
             "dom0": self.dom0,
@@ -88,17 +89,24 @@ def call_mgmt_func(self, method, arg=b"", payload=b""):
     def test_000_suspend_pre(self):
         running_vm = self.create_mockvm(features={"qrexec": True})
         running_vm.is_running.return_value = True
+        running_vm.klass = "AppVM"
 
-        not_running_vm = self.create_mockvm(features={"qrexec": True})
+        not_running_vm = self.create_mockvm(
+            features={"qrexec": True, "preload-dispvm-max": "1"}
+        )
         not_running_vm.is_running.return_value = False
+        not_running_vm.template_for_dispvms = True
+        not_running_vm.klass = "AppVM"
 
         no_qrexec_vm = self.create_mockvm()
         no_qrexec_vm.is_running.return_value = True
+        no_qrexec_vm.klass = "AppVM"
 
         paused_vm = self.create_mockvm(features={"qrexec": True})
         paused_vm.is_running.return_value = True
         paused_vm.get_power_state.return_value = "Paused"
         paused_vm.name = "SleepingBeauty"
+        paused_vm.klass = "AppVM"
 
         self.domains.update(
             {
@@ -113,8 +121,11 @@ def test_000_suspend_pre(self):
             qubes.api.internal,
             "PREVIOUSLY_PAUSED",
             "/tmp/qubes-previously-paused.tmp",
-        ):
+        ), mock.patch.object(
+            not_running_vm, "remove_preload_excess"
+        ) as mock_remove:
             ret = self.call_mgmt_func(b"internal.SuspendPre")
+            mock_remove.assert_called_once_with(0, reason=mock.ANY)
         self.assertIsNone(ret)
         self.assertFalse(self.dom0.called)
 

qubes/tests/app.py

@@ -1096,6 +1096,40 @@ def test_205_remove_appvm_dispvm(self):
             with self.assertRaises(qubes.exc.QubesVMInUseError):
                 del self.app.domains[appvm]
 
+    def test_205_remove_appvm_dispvm_preload(self):
+        appvm = self.app.add_new_vm(
+            "AppVM",
+            name="test-appvm",
+            template=self.template,
+            template_for_dispvms=True,
+            label="red",
+        )
+        dispvm = self.app.add_new_vm(
+            "DispVM", name="test-dispvm", template=appvm, label="red"
+        )
+        dispvm_alt = self.app.add_new_vm(
+            "DispVM", name="test-dispvm-alt", template=appvm, label="red"
+        )
+        with mock.patch.object(self.app, "vmm"):
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                del self.app.domains[appvm]
+
+        with mock.patch.object(self.app, "vmm"):
+            with mock.patch.object(appvm, "fire_event_async"):
+                appvm.features["preload-dispvm-max"] = "1"
+                appvm.features["preload-dispvm"] = str(dispvm.name)
+                with self.assertRaises(qubes.exc.QubesVMInUseError):
+                    del self.app.domains[appvm]
+
+        with mock.patch.object(self.app, "vmm"):
+            with mock.patch.object(appvm, "fire_event_async"):
+                appvm.features["preload-dispvm-max"] = "2"
+                appvm.features["preload-dispvm"] = dispvm.name
+                appvm.features["preload-dispvm"] = (
+                    dispvm.name + " " + dispvm_alt.name
+                )
+                del self.app.domains[appvm]
+
     def test_206_remove_attached(self):
         # See also qubes.tests.api_admin.
         vm = self.app.add_new_vm(

qubes/tests/ext.py

@@ -22,6 +22,7 @@
 import unittest.mock
 
 import qubes.ext.admin
+import qubes.ext.audio
 import qubes.ext.core_features
 import qubes.ext.custom_persist
 import qubes.ext.services
@@ -2610,3 +2611,58 @@ def test_000_tags_permission(self):
                 "admin-permission:admin.vm.tag.Set",
                 tag,
             )
+
+
+class TC_60_Audio(qubes.tests.QubesTestCase):
+    def setUp(self):
+        super().setUp()
+        self.ext = qubes.ext.audio.AUDIO()
+        self.audiovm = mock.MagicMock()
+        self.audiovm.name = "sys-audio"
+        self.client = mock.MagicMock()
+        self.client.name = "client"
+        self.client_alt = mock.MagicMock()
+        self.client_alt.name = "client"
+
+    def test_000_shutdown(self):
+        self.ext.on_domain_pre_shutdown(
+            self.audiovm,
+            "domain-pre-shutdown",
+        )
+
+    def test_000_shutdown_used(self):
+        with unittest.mock.patch.object(
+            self.ext, "attached_vms", return_value=[self.client]
+        ), unittest.mock.patch.object(
+            self.client, "is_running", return_value=True
+        ):
+            self.client.is_preload = False
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                self.ext.on_domain_pre_shutdown(
+                    self.audiovm,
+                    "domain-pre-shutdown",
+                )
+
+            self.client.is_preload = True
+            self.ext.on_domain_pre_shutdown(
+                self.audiovm,
+                "domain-pre-shutdown",
+            )
+
+    def test_000_shutdown_used_by_some(self):
+        with unittest.mock.patch.object(
+            self.ext,
+            "attached_vms",
+            return_value=[self.client, self.client_alt],
+        ), unittest.mock.patch.object(
+            self.client, "is_running", return_value=True
+        ), unittest.mock.patch.object(
+            self.client_alt, "is_running", return_value=True
+        ):
+            self.client.is_preload = False
+            self.client_alt.is_preload = True
+            with self.assertRaises(qubes.exc.QubesVMInUseError):
+                self.ext.on_domain_pre_shutdown(
+                    self.audiovm,
+                    "domain-pre-shutdown",
+                )

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")