Discard preloaded disposables with outdated props

If a qube property is changed on the disposable template, it is not
replicated to the preloaded disposable, no refresh occurs, which means,
if a disposable template has the netvm changed, the preloaded disposable
would still remain with the old setting. While changing netvm on the fly
is possible after unpause, several other settings requires a restart.

Fixes: QubesOS/qubes-issues#10525
For: QubesOS/qubes-issues#1512

Author: ben-grande

qubes/tests/integ/dispvm.py

@@ -832,8 +832,29 @@ async def _test_019_preload_refresh(self):
         self.log_preload()
         logger.info("end")
 
+    def test_020_preload_discard_outdated(self):
+        """Discard preload if properties differ from the disposable template."""
+        self.loop.run_until_complete(self._test_020_preload_discard_outdated())
+
+    async def _test_020_preload_discard_outdated(self):
+        logger.info("start")
+        self.log_preload()
+        preload_max = 1
+        self.disp_base.features["preload-dispvm-max"] = str(preload_max)
+        await self.wait_preload(preload_max)
+        preload_dispvm = self.disp_base.get_feat_preload()
+        self.disp_base.netvm = None
+        try:
+            dispvm = await asyncio.wait_for(
+                qubes.vm.dispvm.DispVM.from_appvm(self.disp_base), 30
+            )
+            self.assertNotIn(dispvm.name, preload_dispvm)
+        finally:
+            await dispvm.cleanup()
+        logger.info("end")
+
     @unittest.skipUnless(which("xdotool"), "xdotool not installed")
-    def test_020_gui_app(self):
+    def test_080_gui_app(self):
         dispvm = self.loop.run_until_complete(
             qubes.vm.dispvm.DispVM.from_appvm(self.disp_base)
         )
@@ -1057,7 +1078,7 @@ def _whonix_ws_dispvm_confirm(self, action_str):
         return (True, "")
 
     @unittest.skipUnless(which("xdotool"), "xdotool not installed")
-    def test_030_edit_file(self):
+    def test_090_edit_file(self):
         self.testvm1 = self.app.add_new_vm(
             qubes.vm.appvm.AppVM,
             name=self.make_vm_name("vm1"),

qubes/tests/vm/dispvm.py

@@ -181,6 +181,7 @@ def test_000_from_appvm_preload_use(
         mock_symlink,
         mock_start,
     ):
+        # pylint: disable=unused-argument
         mock_storage.return_value.create.side_effect = self.mock_coro
         mock_start.side_effect = self.mock_coro
         self.appvm.template_for_dispvms = True
@@ -222,6 +223,7 @@ def test_000_from_appvm_preload_use(
             mock_qube.features = dispvm.features
             mock_qube.unpause = self.mock_coro
             mock_qube.request_preload.return_value = dispvm
+            mock_qube.is_preload_outdated = dispvm.is_preload_outdated
             mock_qube.get_preload = mock.AsyncMock()
             mock_qube.volumes = {}
             fresh_dispvm = self.loop.run_until_complete(
@@ -656,3 +658,61 @@ def test_023_inherit_ephemeral(self, _mock_makedirs, _mock_symlink):
             self.loop.run_until_complete(dispvm.create_on_disk())
         self.assertIs(dispvm.template, self.appvm)
         self.assertTrue(dispvm.volumes["volatile"].ephemeral)
+
+    @mock.patch("qubes.vm.qubesvm.QubesVM.start")
+    @mock.patch("os.symlink")
+    @mock.patch("os.makedirs")
+    @mock.patch("qubes.storage.Storage")
+    def test_024_is_preload_outdated(
+        self,
+        mock_storage,
+        mock_makedirs,
+        mock_symlink,
+        mock_start,
+    ):
+        mock_storage.return_value.create.side_effect = self.mock_coro
+        mock_makedirs.return_value = self.mock_coro
+        mock_symlink.return_value = self.mock_coro
+        mock_start.side_effect = self.mock_coro
+        self.appvm.template_for_dispvms = True
+
+        self.appvm.features["supported-rpc.qubes.WaitForRunningSystem"] = True
+        self.appvm.features["preload-dispvm-max"] = "1"
+        orig_getitem = self.app.domains.__getitem__
+        with mock.patch.object(
+            self.app, "domains", wraps=self.app.domains
+        ) as mock_domains:
+            mock_qube = mock.Mock()
+            mock_qube.template = self.appvm
+            mock_qube.qrexec_timeout = self.appvm.qrexec_timeout
+            mock_qube.preload_complete = mock.Mock(spec=asyncio.Event)
+            mock_qube.preload_complete.is_set.return_value = True
+            mock_qube.preload_complete.set = self.mock_coro
+            mock_qube.preload_complete.clear = self.mock_coro
+            mock_qube.preload_complete.wait = self.mock_coro
+            mock_domains.configure_mock(
+                **{
+                    "get_new_unused_dispid": mock.Mock(return_value=42),
+                    "__contains__.return_value": True,
+                    "__getitem__.side_effect": lambda key: (
+                        mock_qube if key == "disp42" else orig_getitem(key)
+                    ),
+                }
+            )
+            dispvm = self.loop.run_until_complete(
+                qubes.vm.dispvm.DispVM.from_appvm(self.appvm, preload=True)
+            )
+
+        self.assertFalse(dispvm.is_preload_outdated())
+        self.appvm.debug = not self.appvm.debug
+        self.assertEqual(list(dispvm.is_preload_outdated().keys()), ["debug"])
+        self.appvm.debug = not self.appvm.debug
+
+        self.assertFalse(dispvm.is_preload_outdated())
+        self.appvm_alt.provides_network = True
+        self.assertFalse(dispvm.is_preload_outdated())
+        self.appvm.netvm = self.appvm_alt
+        self.assertEqual(
+            sorted(list(dispvm.is_preload_outdated().keys())),
+            sorted(["netvm", "dns", "visible_netmask"]),
+        )

qubes/vm/dispvm.py

@@ -417,6 +417,52 @@ def is_preload(self) -> bool:
             return True
         return False
 
+    def is_preload_outdated(self) -> dict:
+        """
+        Show properties that differ on disposable compared to its template.
+
+        :rtype: dict
+        """
+        if not self.is_preload:
+            return {}
+        appvm = self.template
+        appvm_props = appvm.property_dict()
+        props = self.property_dict()
+        exclude_props = [
+            "backup_timestamp",
+            "default_dispvm",
+            "dispid",
+            "gateway",
+            "gateway6",
+            "icon",
+            "include_in_backups",
+            "installed_by_rpm",
+            "ip",
+            "ip6",
+            "klass",
+            "name",
+            "qid",
+            "start_time",
+            "stubdom_uuid",
+            "stubdom_xid",
+            "template",
+            "template_for_dispvms",
+            "updateable",
+            "uuid",
+            "visible_gateway",
+            "visible_gateway6",
+            "visible_ip",
+            "visible_ip6",
+            "xid",
+        ]
+        differed_props = {
+            k: v
+            for k, v in props.items() & appvm_props.items()
+            if k not in exclude_props
+            and getattr(self, k, None) != getattr(appvm, k, None)
+        }
+        return differed_props
+
     @qubes.events.handler("domain-load")
     def on_domain_loaded(self, event) -> None:
         """

qubes/vm/mix/dvmtemplate.py

@@ -682,16 +682,24 @@ def request_preload(self) -> Optional["qubes.vm.dispvm.DispVM"]:
         dispvm = None
         for item in preload_dispvm:
             qube = self.app.domains[item]
-            if any(vol.is_outdated() for vol in qube.volumes.values()):
+            outdated_settings = None
+            if (outdated_settings := qube.is_preload_outdated()) or any(
+                vol.is_outdated() for vol in qube.volumes.values()
+            ):
+                if outdated_settings:
+                    discard_reason = "settings(s): " + ", ".join(
+                        map(str, outdated_settings.keys())
+                    )
+                else:
+                    discard_reason = "volume(s)"
                 qube.log.warning(
-                    "Requested preloaded qube but it is outdated, trying "
-                    "another one if available"
+                    "Requested preloaded qube but it has outdated %s. Trying "
+                    "another one if available",
+                    discard_reason,
                 )
-                # The gap is filled after the delay set by the
-                # 'domain-shutdown' of its ancestors. Not refilling now to
-                # deliver a disposable faster.
+                # Not refilling now to deliver a disposable faster.
                 self.remove_preload_from_list(
-                    [qube.name], reason="of outdated volume(s)"
+                    [qube.name], reason="of outdated " + discard_reason
                 )
                 # Delay to not  affect this run.
                 asyncio.ensure_future(
@@ -706,6 +714,7 @@ def request_preload(self) -> Optional["qubes.vm.dispvm.DispVM"]:
                 "Found only outdated preloaded qube(s), falling back to "
                 "normal disposable"
             )
+            self.fill_preload_gap()
             return None
         dispvm.mark_preload_requested()
         return dispvm