More preloaded dispvm tests

Author: ben-grande

linux/aux-tools/preload-dispvm

@@ -14,7 +14,7 @@ def main():
     ]
     method = "admin.vm.CreateDisposable"
     for qube in appvms:
-        qube.qubesd_call("dom0", method, "preload-autostart")
+        qube.qubesd_call(qube.name, method, "preload-autostart")
 
 
 if __name__ == "__main__":

linux/systemd/qubes-vm@.service

@@ -1,7 +1,7 @@
 [Unit]
 Description=Start Qubes VM %i
 After=qubesd.service qubes-meminfo-writer-dom0.service
-Before=qubes-vm@.service
+Before=qubes-preload-dispvm.service
 ConditionKernelCommandLine=!qubes.skip_autostart
 
 [Service]

qubes/api/admin.py

@@ -298,9 +298,7 @@ async def vm_property_set(self, untrusted_payload):
     async def property_set(self, untrusted_payload):
         """Set property value"""
         self.enforce(self.dest.name == "dom0")
-        return self._property_set(
-            self.app, untrusted_payload=untrusted_payload
-        )
+        return self._property_set(self.app, untrusted_payload=untrusted_payload)
 
     def _property_set(self, dest, untrusted_payload):
         if self.arg not in dest.property_list():
@@ -486,9 +484,7 @@ async def vm_volume_clone_to(self, untrusted_payload):
 
         # make sure the volume still exists, but invalidate token anyway
         self.enforce(str(src_volume.pool) in self.app.pools)
-        self.enforce(
-            src_volume in self.app.pools[str(src_volume.pool)].volumes
-        )
+        self.enforce(src_volume in self.app.pools[str(src_volume.pool)].volumes)
 
         dst_volume = self.dest.volumes[self.arg]
 
@@ -770,9 +766,7 @@ async def pool_add(self, untrusted_payload):
 
         driver_parameters = qubes.storage.driver_parameters(self.arg)
         unexpected_parameters = [
-            key
-            for key in untrusted_pool_config
-            if key not in driver_parameters
+            key for key in untrusted_pool_config if key not in driver_parameters
         ]
         if unexpected_parameters:
             raise qubes.exc.QubesException(
@@ -1120,9 +1114,7 @@ async def vm_feature_checkwithtpladminvm(self):
 
         self.fire_event_for_permission()
         try:
-            value = self.dest.features.check_with_template_and_adminvm(
-                self.arg
-            )
+            value = self.dest.features.check_with_template_and_adminvm(self.arg)
         except KeyError:
             raise qubes.exc.QubesFeatureNotFoundError(self.dest, self.arg)
         return value
@@ -1280,7 +1272,7 @@ async def _vm_create(
 
     @qubes.api.method("admin.vm.CreateDisposable", scope="global", write=True)
     async def create_disposable(self, untrusted_payload):
-        self.enforce(self.arg in [None, "preload", "preload-autostart"])
+        self.enforce(self.arg in [None, "", "preload", "preload-autostart"])
         preload = False
         preload_autostart = False
         if self.arg == "preload":
@@ -1304,9 +1296,9 @@ async def create_disposable(self, untrusted_payload):
             await appvm.fire_event_async("domain-preloaded-dispvm-autostart")
             return
 
-        dispvm = await qubes.vm.dispvm.DispVM.from_appvm(
-            appvm, preload=preload
-        )
+        dispvm = await qubes.vm.dispvm.DispVM.from_appvm(appvm, preload=preload)
+        if preload:
+            await dispvm.start()
         # TODO: move this to extension (in race-free fashion, better than here)
         dispvm.tags.add("created-by-" + str(self.src))
         dispvm.tags.add("disp-created-by-" + str(self.src))
@@ -1967,9 +1959,7 @@ async def backup_info(self):
                 "Backup profile {} does not exist".format(self.arg)
             )
 
-        backup = await self._load_backup_profile(
-            self.arg, skip_passphrase=True
-        )
+        backup = await self._load_backup_profile(self.arg, skip_passphrase=True)
         return backup.get_backup_summary()
 
     def _send_stats_single(

qubes/tests/__init__.py

@@ -1796,6 +1796,8 @@ def load_tests(loader, tests, pattern):  # pylint: disable=unused-argument
         "qubes.tests.ext",
         "qubes.tests.vm.qubesvm",
         "qubes.tests.vm.mix.net",
+        # TODO: Ben
+        #"qubes.tests.vm.mix.dvmtemplate",
         "qubes.tests.vm.adminvm",
         "qubes.tests.vm.appvm",
         "qubes.tests.vm.dispvm",

qubes/tests/api_admin.py

@@ -578,9 +578,9 @@ def test_080_vm_volume_info(self):
             volumes_conf["__getitem__.return_value.{}".format(prop)] = (
                 prop + "-value"
             )
-        volumes_conf["__getitem__.return_value.is_outdated.return_value"] = (
-            False
-        )
+        volumes_conf[
+            "__getitem__.return_value.is_outdated.return_value"
+        ] = False
         self.vm.volumes.configure_mock(**volumes_conf)
         value = self.call_mgmt_func(
             b"admin.vm.volume.Info", b"test-vm1", b"private"
@@ -608,9 +608,9 @@ def test_081_vm_volume_info_unsupported_is_outdated(self):
             volumes_conf["__getitem__.return_value.{}".format(prop)] = (
                 prop + "-value"
             )
-        volumes_conf["__getitem__.return_value.is_outdated.side_effect"] = (
-            NotImplementedError
-        )
+        volumes_conf[
+            "__getitem__.return_value.is_outdated.side_effect"
+        ] = NotImplementedError
         self.vm.volumes.configure_mock(**volumes_conf)
         value = self.call_mgmt_func(
             b"admin.vm.volume.Info", b"test-vm1", b"private"
@@ -672,7 +672,9 @@ def test_090_vm_volume_listsnapshots_invalid_volume(self):
         self.vm.volumes.configure_mock(**volumes_conf)
         with self.assertRaises(qubes.exc.PermissionDenied):
             self.call_mgmt_func(
-                b"admin.vm.volume.ListSnapshots", b"test-vm1", b"no-such-volume"
+                b"admin.vm.volume.ListSnapshots",
+                b"test-vm1",
+                b"no-such-volume",
             )
         self.assertEqual(
             self.vm.volumes.mock_calls, [unittest.mock.call.keys()]
@@ -1293,7 +1295,10 @@ def test_200_label_create_invalid_name(self):
             )
         with self.assertRaises(qubes.exc.PermissionDenied):
             self.call_mgmt_func(
-                b"admin.label.Create", b"dom0", b"strange-name!@#$", b"0xff0000"
+                b"admin.label.Create",
+                b"dom0",
+                b"strange-name!@#$",
+                b"0xff0000",
             )
 
         self.assertEqual(self.app.get_label.mock_calls, [])
@@ -1675,7 +1680,9 @@ def test_316_feature_checkwithnetvm_netvm(self):
     def test_317_feature_checkwithnetvm_none(self):
         with self.assertRaises(qubes.exc.QubesFeatureNotFoundError):
             self.call_mgmt_func(
-                b"admin.vm.feature.CheckWithNetvm", b"test-vm1", b"test-feature"
+                b"admin.vm.feature.CheckWithNetvm",
+                b"test-vm1",
+                b"test-feature",
             )
         self.assertFalse(self.app.save.called)
 
@@ -1708,7 +1715,10 @@ def test_319_feature_checkwithtpladminvm(self):
 
     def test_320_feature_set(self):
         value = self.call_mgmt_func(
-            b"admin.vm.feature.Set", b"test-vm1", b"test-feature", b"some-value"
+            b"admin.vm.feature.Set",
+            b"test-vm1",
+            b"test-feature",
+            b"some-value",
         )
         self.assertIsNone(value)
         self.assertEqual(self.vm.features["test-feature"], "some-value")
@@ -1961,7 +1971,8 @@ def test_340_vm_create_in_pool_app(self, storage_mock):
         # setting pool= affect only volumes actually created for this VM,
         # not used from a template or so
         self.assertEqual(
-            vm.volume_config["root"]["pool"], self.template.volumes["root"].pool
+            vm.volume_config["root"]["pool"],
+            self.template.volumes["root"].pool,
         )
         self.assertEqual(vm.volume_config["private"]["pool"], "test")
         self.assertEqual(vm.volume_config["volatile"]["pool"], "test")
@@ -1993,11 +2004,13 @@ def test_341_vm_create_in_pool_private(self, storage_mock):
         self.assertEqual(vm.label, self.app.get_label("red"))
         self.assertEqual(vm.template, self.app.domains["test-template"])
         self.assertEqual(
-            vm.volume_config["root"]["pool"], self.template.volumes["root"].pool
+            vm.volume_config["root"]["pool"],
+            self.template.volumes["root"].pool,
         )
         self.assertEqual(vm.volume_config["private"]["pool"], "test")
         self.assertEqual(
-            vm.volume_config["volatile"]["pool"], self.app.default_pool_volatile
+            vm.volume_config["volatile"]["pool"],
+            self.app.default_pool_volatile,
         )
         self.assertEqual(vm.volume_config["kernel"]["pool"], "linux-kernel")
         self.assertEqual(
@@ -2283,7 +2296,9 @@ def test_473_vm_device_list_assigned_specific(self):
             self.vm.devices["testclass"].assign(assignment)
         )
         value = self.call_mgmt_func(
-            b"admin.vm.device.testclass.Assigned", b"test-vm1", b"test-vm1+1234"
+            b"admin.vm.device.testclass.Assigned",
+            b"test-vm1",
+            b"test-vm1+1234",
         )
         self.assertEqual(
             value,
@@ -2310,7 +2325,9 @@ def test_474_vm_device_list_attached_specific(self):
             self.device_list_multiple_attached_testclass,
         )
         value = self.call_mgmt_func(
-            b"admin.vm.device.testclass.Attached", b"test-vm1", b"test-vm1+1234"
+            b"admin.vm.device.testclass.Attached",
+            b"test-vm1",
+            b"test-vm1+1234",
         )
         self.assertEqual(
             value,
@@ -2946,9 +2963,10 @@ def test_516_vm_volume_import_fire_event_with_size(self):
         )
 
     def test_510_vm_volume_import_end_success(self):
-        import_data_end_mock, self.vm.storage.import_data_end = (
-            self.coroutine_mock()
-        )
+        (
+            import_data_end_mock,
+            self.vm.storage.import_data_end,
+        ) = self.coroutine_mock()
         self.call_internal_mgmt_func(
             b"internal.vm.volume.ImportEnd",
             b"test-vm1",
@@ -2961,9 +2979,10 @@ def test_510_vm_volume_import_end_success(self):
         )
 
     def test_510_vm_volume_import_end_failure(self):
-        import_data_end_mock, self.vm.storage.import_data_end = (
-            self.coroutine_mock()
-        )
+        (
+            import_data_end_mock,
+            self.vm.storage.import_data_end,
+        ) = self.coroutine_mock()
         with self.assertRaisesRegex(qubes.exc.QubesException, "error message"):
             self.call_internal_mgmt_func(
                 b"internal.vm.volume.ImportEnd",
@@ -3008,9 +3027,9 @@ def setup_for_clone(self):
         self.vm.volumes["private"].import_volume.return_value = self.vm.volumes[
             "private"
         ]
-        self.vm2.volumes["private"].import_volume.return_value = (
-            self.vm2.volumes["private"]
-        )
+        self.vm2.volumes[
+            "private"
+        ].import_volume.return_value = self.vm2.volumes["private"]
 
         self.addCleanup(self.cleanup_for_clone)
 
@@ -3716,9 +3735,13 @@ 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.start")
     @unittest.mock.patch("qubes.storage.Storage.create")
-    def test_643_vm_create_disposable_preload(self, mock_storage):
+    def test_643_vm_create_disposable_preload(
+        self, mock_storage, mock_dispvm_start
+    ):
         mock_storage.side_effect = self.dummy_coro
+        mock_dispvm_start.side_effect = self.dummy_coro
         self.vm.template_for_dispvms = True
         self.vm.features["preload-dispvm-max"] = "1"
         self.app.default_dispvm = self.vm
@@ -3731,29 +3754,58 @@ def test_643_vm_create_disposable_preload(self, mock_storage):
         self.assertIn("created-by-dom0", dispvm.tags)
         self.assertIn("disp-created-by-dom0", dispvm.tags)
         mock_storage.assert_called_once_with()
+        mock_dispvm_start.assert_called_once_with()
         self.assertTrue(self.app.save.called)
 
+    @unittest.mock.patch("qubes.vm.dispvm.DispVM.start")
     @unittest.mock.patch("qubes.storage.Storage.create")
-    def test_643_vm_create_disposable_preload_autostart(self, mock_storage):
+    def test_643_vm_create_disposable_preload_autostart(
+        self, mock_storage, mock_dispvm_start
+    ):
         mock_storage.side_effect = self.dummy_coro
+        mock_dispvm_start.side_effect = self.dummy_coro
         self.vm.template_for_dispvms = True
         self.app.domains[self.vm].fire_event = self.emitter.fire_event
         self.vm.features["preload-dispvm-max"] = "1"
         self.app.default_dispvm = self.vm
-        # TODO: how to mock start of a qube that we don't have its object?
-        # Looked as test_220_start(), but 'self.vm.start = corountine_mock'
-        # won't help if we don't have the dispvm object.
         retval = self.call_mgmt_func(
             b"admin.vm.CreateDisposable", b"dom0", arg=b"preload-autostart"
         )
+        # TODO: why event is not being fired?
         self.assertEventFired(self.emitter, "domain-preloaded-dispvm-autostart")
         dispvm_preload = self.vm.get_feat_preload()
         self.assertEqual(len(dispvm_preload), 1)
         self.assertIsNone(retval)
         mock_storage.assert_called_once_with()
+        mock_dispvm_start.assert_called_once_with()
         self.assertTrue(self.app.save.called)
 
-    ## TODO: test return of the preloaded dispvm
+    # TODO: test return of the preloaded dispvm
+    @unittest.mock.patch("qubes.vm.dispvm.DispVM.start")
+    @unittest.mock.patch("qubes.storage.Storage.create")
+    def test_643_vm_create_disposable_preload_use(
+        self, mock_storage, mock_dispvm_start
+    ):
+        mock_storage.side_effect = self.dummy_coro
+        mock_dispvm_start.side_effect = self.dummy_coro
+        self.vm.template_for_dispvms = True
+        self.app.domains[self.vm].fire_event = self.emitter.fire_event
+        self.vm.features["preload-dispvm-max"] = "1"
+        self.app.default_dispvm = self.vm
+        self.call_mgmt_func(
+            b"admin.vm.CreateDisposable", b"dom0", arg=b"preload"
+        )
+        dispvm_preload = self.vm.get_feat_preload()
+        dispvm = dispvm_preload[0]
+        self.assertEqual(len(dispvm_preload), 1)
+        mock_storage.assert_called_once_with()
+        mock_dispvm_start.assert_called_once_with()
+        self.assertTrue(self.app.save.called)
+        retval = self.call_mgmt_func(b"admin.vm.CreateDisposable", b"dom0")
+        # TODO: why event is not being fired?
+        self.assertEventFired(self.emitter, "domain-preloaded-dispvm-used")
+        self.assertEqual(retval, dispvm)
+        self.assertFalse(self.vm.get_feat_preload())
 
     def test_650_vm_device_set_mode_required(self):
         assignment = DeviceAssignment(
@@ -4104,7 +4156,10 @@ def test_702_pool_set_revisions_to_keep_not_a_number(self):
     def test_703_pool_set_ephemeral(self):
         self.app.pools["test-pool"] = unittest.mock.Mock()
         value = self.call_mgmt_func(
-            b"admin.pool.Set.ephemeral_volatile", b"dom0", b"test-pool", b"true"
+            b"admin.pool.Set.ephemeral_volatile",
+            b"dom0",
+            b"test-pool",
+            b"true",
         )
         self.assertIsNone(value)
         self.assertEqual(self.app.pools["test-pool"].mock_calls, [])
@@ -4273,7 +4328,10 @@ def test_731_vm_console_not_running(self):
             "</domain>\n"
         )
         self.vm._libvirt_domain.configure_mock(
-            **{"XMLDesc.return_value": xml_desc, "isActive.return_value": False}
+            **{
+                "XMLDesc.return_value": xml_desc,
+                "isActive.return_value": False,
+            }
         )
         with self.assertRaises(qubes.exc.QubesVMNotRunningError):
             self.call_mgmt_func(b"admin.vm.Console", b"test-vm1")
@@ -4353,7 +4411,8 @@ def test_901_current_state_changed(self):
         self.vm.get_power_state = lambda: "Running"
         value = self.call_mgmt_func(b"admin.vm.CurrentState", b"test-vm1")
         self.assertEqual(
-            value, "mem=512 mem_static_max=1024 cputime=100 power_state=Running"
+            value,
+            "mem=512 mem_static_max=1024 cputime=100 power_state=Running",
         )
 
     def test_990_vm_unexpected_payload(self):