Merge remote-tracking branch 'origin/pr/673'

* origin/pr/673:
  Assure paused domains prior to suspending remain paused

Author: marmarek

qubes/api/internal.py

@@ -22,6 +22,7 @@
 
 import asyncio
 import json
+import os
 import subprocess
 
 import qubes.api
@@ -30,6 +31,9 @@
 import qubes.vm.dispvm
 
 
+PREVIOUSLY_PAUSED = "/run/qubes/previously-paused.list"
+
+
 class SystemInfoCache:
     cache = None
     cache_for_app = None
@@ -258,13 +262,24 @@ async def suspend_pre(self):
         :return:
         """
 
-        # first notify all VMs
+        # first keep track of VMs which were paused before suspending
+        previously_paused = [
+            vm.name
+            for vm in self.app.domains
+            if vm.get_power_state() in ["Paused", "Suspended"]
+        ]
+        with open(PREVIOUSLY_PAUSED, "w", encoding="ascii") as file:
+            file.write("\n".join(previously_paused))
+
+        # then notify all VMs (except paused ones)
         processes = []
         for vm in self.app.domains:
             if isinstance(vm, qubes.vm.adminvm.AdminVM):
                 continue
             if not vm.is_running():
                 continue
+            if vm.name in previously_paused:
+                continue
             if not vm.features.check_with_template("qrexec", False):
                 continue
             try:
@@ -303,6 +318,8 @@ async def suspend_pre(self):
         for vm in self.app.domains:
             if isinstance(vm, qubes.vm.adminvm.AdminVM):
                 continue
+            if vm.name in previously_paused:
+                continue
             if vm.is_running():
                 coros.append(asyncio.create_task(vm.suspend()))
         if coros:
@@ -326,23 +343,37 @@ async def suspend_post(self):
         :return:
         """
 
+        # Reload list of previously paused qubes before suspending
+        previously_paused = []
+        try:
+            if os.path.isfile(PREVIOUSLY_PAUSED):
+                with open(PREVIOUSLY_PAUSED, encoding="ascii") as file:
+                    previously_paused = file.read().split("\n")
+                os.unlink(PREVIOUSLY_PAUSED)
+        except OSError:
+            previously_paused = []
+
         coros = []
         # first resume/unpause VMs
         for vm in self.app.domains:
             if isinstance(vm, qubes.vm.adminvm.AdminVM):
                 continue
+            if vm.name in previously_paused:
+                continue
             if vm.get_power_state() in ["Paused", "Suspended"]:
                 coros.append(asyncio.create_task(vm.resume()))
         if coros:
             await asyncio.wait(coros)
 
-        # then notify all VMs
+        # then notify all VMs (except previously paused ones)
         processes = []
         for vm in self.app.domains:
             if isinstance(vm, qubes.vm.adminvm.AdminVM):
                 continue
             if not vm.is_running():
                 continue
+            if vm.name in previously_paused:
+                continue
             if not vm.features.check_with_template("qrexec", False):
                 continue
             try:

qubes/tests/api_internal.py

@@ -93,15 +93,26 @@ def test_000_suspend_pre(self):
         no_qrexec_vm = self.create_mockvm()
         no_qrexec_vm.is_running.return_value = True
 
+        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"
+
         self.domains.update(
             {
                 "running": running_vm,
                 "not-running": not_running_vm,
                 "no-qrexec": no_qrexec_vm,
+                "paused": paused_vm,
             }
         )
 
-        ret = self.call_mgmt_func(b"internal.SuspendPre")
+        with mock.patch.object(
+            qubes.api.internal,
+            "PREVIOUSLY_PAUSED",
+            "/tmp/qubes-previously-paused.tmp",
+        ):
+            ret = self.call_mgmt_func(b"internal.SuspendPre")
         self.assertIsNone(ret)
         self.assertFalse(self.dom0.called)
 
@@ -121,6 +132,13 @@ def test_000_suspend_pre(self):
             ("run_service", ("qubes.SuspendPreAll",), mock.ANY),
             no_qrexec_vm.mock_calls,
         )
+
+        self.assertNotIn(
+            ("run_service", ("qubes.SuspendPreAll",), mock.ANY),
+            paused_vm.mock_calls,
+        )
+        self.assertIn(("suspend", (), {}), running_vm.mock_calls)
+
         self.assertIn(("suspend", (), {}), no_qrexec_vm.mock_calls)
 
     def test_001_suspend_post(self):
@@ -136,15 +154,26 @@ def test_001_suspend_post(self):
         no_qrexec_vm.is_running.return_value = True
         no_qrexec_vm.get_power_state.return_value = "Suspended"
 
+        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"
+
         self.domains.update(
             {
                 "running": running_vm,
                 "not-running": not_running_vm,
                 "no-qrexec": no_qrexec_vm,
+                "paused": paused_vm,
             }
         )
 
-        ret = self.call_mgmt_func(b"internal.SuspendPost")
+        with mock.patch.object(
+            qubes.api.internal,
+            "PREVIOUSLY_PAUSED",
+            "/tmp/qubes-previously-paused.tmp",
+        ):
+            ret = self.call_mgmt_func(b"internal.SuspendPost")
         self.assertIsNone(ret)
         self.assertFalse(self.dom0.called)
 
@@ -166,6 +195,12 @@ def test_001_suspend_post(self):
         )
         self.assertIn(("resume", (), {}), no_qrexec_vm.mock_calls)
 
+        self.assertNotIn(
+            ("run_service", ("qubes.SuspendPostAll",), mock.ANY),
+            paused_vm.mock_calls,
+        )
+        self.assertNotIn(("resume", (), {}), paused_vm.mock_calls)
+
     def test_010_get_system_info(self):
         self.dom0.name = "dom0"
         self.dom0.tags = ["tag1", "tag2"]