Make qvm_shutdown reusable for other tools

It was necessary to make alternative main async functions, as Python is
picky about running async code when mixing async and sync functions.

Author: ben-grande

qubesadmin/tests/tools/qvm_kill.py

@@ -20,6 +20,8 @@
 
 # pylint: disable=missing-docstring
 
+import asyncio
+
 import qubesadmin.tests
 import qubesadmin.tests.tools
 import qubesadmin.tools.qvm_kill
@@ -35,6 +37,20 @@ def test_000_with_vm(self):
         qubesadmin.tools.qvm_kill.main(['some-vm'], app=self.app)
         self.assertAllCalled()
 
+    def test_000_with_vm_async(self):
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(loop)
+
+        self.app.expected_calls[
+            ('dom0', 'admin.vm.List', None, None)] = \
+            b'0\x00some-vm class=AppVM state=Running\n'
+        self.app.expected_calls[
+            ('some-vm', 'admin.vm.Kill', None, None)] = b'0\x00'
+        loop.run_until_complete(
+            qubesadmin.tools.qvm_kill.main_async(['some-vm'], app=self.app)
+        )
+        self.assertAllCalled()
+
     def test_001_missing_vm(self):
         with self.assertRaises(SystemExit):
             with qubesadmin.tests.tools.StderrBuffer() as stderr:
@@ -74,9 +90,9 @@ def test_004_other_error(self):
             ('dom0', 'admin.vm.List', None, None)] = \
             b'0\x00some-vm class=AppVM state=Running\n'
         with qubesadmin.tests.tools.StderrBuffer() as stderr:
-            self.assertEqual(
-                qubesadmin.tools.qvm_kill.main(['some-vm'], app=self.app),
-                1)
+            with self.assertRaises(SystemExit):
+                self.assertEqual(
+                    qubesadmin.tools.qvm_kill.main(['some-vm'], app=self.app),
+                    1)
         self.assertAllCalled()
-        self.assertIn("Failed to kill 'some-vm': Error message",
-                      stderr.getvalue())
+        self.assertIn("Failed to kill: some-vm", stderr.getvalue())

qubesadmin/tests/tools/qvm_shutdown.py

@@ -96,6 +96,24 @@ def test_010_wait(self):
         qubesadmin.tools.qvm_shutdown.main(['--wait', 'some-vm'], app=self.app)
         self.assertAllCalled()
 
+    def test_011_wait_async(self):
+        '''test --wait option with async main'''
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(loop)
+
+        self.app.expected_calls[
+            ('some-vm', 'admin.vm.Shutdown', 'wait', None)] = \
+            b'0\x00'
+        self.app.expected_calls[
+            ('dom0', 'admin.vm.List', None, None)] = \
+            b'0\x00some-vm class=AppVM state=Running\n'
+        loop.run_until_complete(
+            qubesadmin.tools.qvm_shutdown.main_async(
+                ['--wait', 'some-vm'], app=self.app
+            )
+        )
+        self.assertAllCalled()
+
     def test_012_wait_all(self):
         '''test --wait option, with multiple VMs'''
         loop = asyncio.new_event_loop()
@@ -199,6 +217,21 @@ def test_006_dry_run(self):
             ['--dry-run', 'some-vm'], app=self.app)
         self.assertAllCalled()
 
+    def test_006_dry_run_async(self):
+        '''test --dry-run skips shutdown calls'''
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(loop)
+
+        self.app.expected_calls[
+            ('dom0', 'admin.vm.List', None, None)] = \
+            b'0\x00some-vm class=AppVM state=Running\n'
+        loop.run_until_complete(
+            qubesadmin.tools.qvm_shutdown.main_async(
+                ['--dry-run', 'some-vm'], app=self.app
+            )
+        )
+        self.assertAllCalled()
+
     def test_011_wait_retry(self):
         '''test --wait retries VMs whose shutdown request failed'''
         loop = asyncio.new_event_loop()

qubesadmin/tests/tools/qvm_template_postprocess.py

@@ -400,18 +400,7 @@ def test_020_post_install(self, mock_import_root_img,
         self.app.expected_calls[
             ('test-vm', 'admin.vm.Start', None, None)] = b'0\0'
         self.app.expected_calls[
-            ('test-vm', 'admin.vm.Shutdown', None, None)] = b'0\0'
-
-        if qubesadmin.tools.qvm_template_postprocess.have_events:
-            patch_domain_shutdown = mock.patch(
-                'qubesadmin.events.utils.wait_for_domain_shutdown')
-            self.addCleanup(patch_domain_shutdown.stop)
-            mock_domain_shutdown = patch_domain_shutdown.start()
-            mock_domain_shutdown.side_effect = self.wait_for_shutdown
-        else:
-            self.app.expected_calls[
-                ('test-vm', 'admin.vm.List', None, None)] = \
-                b'0\0test-vm class=TemplateVM state=Halted\n'
+            ('test-vm', 'admin.vm.Shutdown', 'wait', None)] = b'0\0'
 
         asyncio.set_event_loop(asyncio.new_event_loop())
         ret = qubesadmin.tools.qvm_template_postprocess.main([
@@ -424,9 +413,6 @@ def test_020_post_install(self, mock_import_root_img,
             'test-vm'], self.source_dir.name)
         mock_import_appmenus.assert_called_once_with(self.app.domains[
             'test-vm'], self.source_dir.name, skip_generate=True)
-        if qubesadmin.tools.qvm_template_postprocess.have_events:
-            mock_domain_shutdown.assert_called_once_with([self.app.domains[
-                'test-vm']])
         self.assertEqual(self.app.service_calls, [
             ('test-vm', 'qubes.PostInstall', {
                 'stdin': subprocess.PIPE,
@@ -458,18 +444,7 @@ def test_021_post_install_reinstall(self, mock_reset_private_img,
         self.app.expected_calls[
             ('test-vm', 'admin.vm.Start', None, None)] = b'0\0'
         self.app.expected_calls[
-            ('test-vm', 'admin.vm.Shutdown', None, None)] = b'0\0'
-
-        if qubesadmin.tools.qvm_template_postprocess.have_events:
-            patch_domain_shutdown = mock.patch(
-                'qubesadmin.events.utils.wait_for_domain_shutdown')
-            self.addCleanup(patch_domain_shutdown.stop)
-            mock_domain_shutdown = patch_domain_shutdown.start()
-            mock_domain_shutdown.side_effect = self.wait_for_shutdown
-        else:
-            self.app.expected_calls[
-                ('test-vm', 'admin.vm.List', None, None)] = \
-                b'0\0test-vm class=TemplateVM state=Halted\n'
+            ('test-vm', 'admin.vm.Shutdown', 'wait', None)] = b'0\0'
 
         asyncio.set_event_loop(asyncio.new_event_loop())
         ret = qubesadmin.tools.qvm_template_postprocess.main([
@@ -483,9 +458,6 @@ def test_021_post_install_reinstall(self, mock_reset_private_img,
             'test-vm'])
         mock_import_appmenus.assert_called_once_with(self.app.domains[
             'test-vm'], self.source_dir.name, skip_generate=True)
-        if qubesadmin.tools.qvm_template_postprocess.have_events:
-            mock_domain_shutdown.assert_called_once_with([self.app.domains[
-                'test-vm']])
         self.assertEqual(self.app.service_calls, [
             ('test-vm', 'qubes.PostInstall', {
                 'stdin': subprocess.PIPE,
@@ -508,13 +480,6 @@ def test_022_post_install_skip_start(self, mock_reset_private_img,
             = b'0\0'
         self.app.add_new_vm = mock.Mock()
 
-        if qubesadmin.tools.qvm_template_postprocess.have_events:
-            patch_domain_shutdown = mock.patch(
-                'qubesadmin.events.utils.wait_for_domain_shutdown')
-            self.addCleanup(patch_domain_shutdown.stop)
-            mock_domain_shutdown = patch_domain_shutdown.start()
-            mock_domain_shutdown.side_effect = self.wait_for_shutdown
-
         asyncio.set_event_loop(asyncio.new_event_loop())
         ret = qubesadmin.tools.qvm_template_postprocess.main([
             '--really', '--skip-start', 'post-install', 'test-vm',
@@ -528,8 +493,6 @@ def test_022_post_install_skip_start(self, mock_reset_private_img,
             'test-vm'])
         mock_import_appmenus.assert_called_once_with(self.app.domains[
             'test-vm'], self.source_dir.name, skip_generate=False)
-        if qubesadmin.tools.qvm_template_postprocess.have_events:
-            self.assertFalse(mock_domain_shutdown.called)
         self.assertEqual(self.app.service_calls, [])
         self.assertAllCalled()
 

qubesadmin/tools/qvm_kill.py

@@ -21,36 +21,41 @@
 '''Immediately terminate a qube without a graceful shutdown sequence.'''
 
 
+import asyncio
 import sys
+
 import qubesadmin.exc
 import qubesadmin.tools
+import qubesadmin.tools.qvm_shutdown
 
 parser = qubesadmin.tools.QubesArgumentParser(
     description='immediately terminate a qube without a graceful shutdown'
                 ' sequence',
     vmname_nargs='+')
 
-def main(args=None, app=None):
-    '''Main routine of :program:`qvm-kill`.
-
-    :param list args: Optional arguments to override those delivered from \
-        command line.
-    '''
 
+async def run_async(args=None, app=None):
+    # pylint: disable=missing-docstring
     args = parser.parse_args(args, app=app)
+    remnants = await qubesadmin.tools.qvm_shutdown.kill(domains=args.domains)
+    if not remnants:
+        return 0
+    parser.error_runtime(
+        "Failed to kill: {}".format(
+            ", ".join(qube.name for qube in remnants)
+        ),
+        len(remnants)
+    )
+
 
-    exit_code = 0
-    for domain in args.domains:
-        try:
-            domain.kill()
-        except qubesadmin.exc.QubesVMNotStartedError:
-            pass
-        except (IOError, OSError, qubesadmin.exc.QubesException) as e:
-            exit_code = 1
-            parser.print_error("Failed to kill '{}': {}".format(
-                domain.name, e))
-
-    return exit_code
+async def main_async(args=None, app=None):
+    # pylint: disable=missing-docstring
+    return await run_async(args=args, app=app)
+
+
+def main(args=None, app=None):
+    # pylint: disable=missing-docstring
+    return asyncio.run(run_async(args=args, app=app))
 
 
 if __name__ == '__main__':

qubesadmin/tools/qvm_shutdown.py

@@ -69,12 +69,7 @@
 )
 
 
-def shutdown(
-    args,
-    loop: asyncio.AbstractEventLoop,
-    domains: list[qubesadmin.vm.QubesVM],
-    force: bool,
-):
+async def shutdown(args, domains: list[qubesadmin.vm.QubesVM]):
     """
     Asynchronously shutdown qubes and return qubes that failed to shutdown
     because and the client can't handle, as well as qubes that were in use
@@ -84,14 +79,12 @@ def shutdown(
     unhandled, used, timedout = [], [], []
     tasks = [
         asyncio.wait_for(
-            asyncio.to_thread(qube.shutdown, force=force, wait=args.wait),
+            asyncio.to_thread(qube.shutdown, force=args.force, wait=args.wait),
             timeout=args.timeout,
         )
         for qube in domains
     ]
-    results = loop.run_until_complete(
-        asyncio.gather(*tasks, return_exceptions=True)
-    )
+    results = await asyncio.gather(*tasks, return_exceptions=True)
     for qube, res in zip(domains, results):
         if not isinstance(res, BaseException):
             qube.log.info("Shutdown succeeded")
@@ -124,16 +117,14 @@ def shutdown(
     return unhandled, used, timedout
 
 
-def kill(loop: asyncio.AbstractEventLoop, domains: list[qubesadmin.vm.QubesVM]):
+async def kill(domains: list[qubesadmin.vm.QubesVM]):
     """
     Asynchronously kill qubes and return qubes that failed to shutdown.
     """
     # pylint: disable=missing-docstring
     unhandled = domains.copy()
     tasks = [asyncio.to_thread(qube.kill) for qube in domains]
-    results = loop.run_until_complete(
-        asyncio.gather(*tasks, return_exceptions=True)
-    )
+    results = await asyncio.gather(*tasks, return_exceptions=True)
     for qube, res in zip(domains, results):
         if not isinstance(res, BaseException):
             qube.log.info("Killing succeeded")
@@ -148,18 +139,14 @@ def kill(loop: asyncio.AbstractEventLoop, domains: list[qubesadmin.vm.QubesVM]):
     return unhandled
 
 
-def main(args=None, app=None):
+async def run_async(args=None, app=None):
     # pylint: disable=missing-docstring
     args = parser.parse_args(args, app=app)
-    force = args.force or (args.all_domains and not args.exclude)
     if args.dry_run:
         return
+    args.force = args.force or (args.all_domains and not args.exclude)
 
-    loop = asyncio.new_event_loop()
-    asyncio.set_event_loop(loop)
-    unhandled, used, timedout = shutdown(
-        args=args, force=force, loop=loop, domains=args.domains
-    )
+    unhandled, used, timedout = await shutdown(args=args, domains=args.domains)
     unhandled_retry = []
     timedout_retry = []
     if used:
@@ -174,8 +161,8 @@ def main(args=None, app=None):
                 ", ".join(qube.name for qube in used)
             )
         )
-        unhandled_retry, used, timedout_retry = shutdown(
-            args=args, force=force, loop=loop, domains=used
+        unhandled_retry, used, timedout_retry = await shutdown(
+            args=args, domains=used
         )
     unhandled.extend(qube for qube in unhandled_retry if qube not in unhandled)
     timedout.extend(qube for qube in timedout_retry if qube not in timedout)
@@ -187,17 +174,15 @@ def main(args=None, app=None):
                 ", ".join(qube.name for qube in timedout)
             )
         )
-        unhandled, used, timedout = shutdown(
-            args=args, force=force, loop=loop, domains=timedout
-        )
+        unhandled, used, timedout = await shutdown(args=args, domains=timedout)
 
     if timedout:
         parser.print_error(
             "Killing timed out qubes: {}".format(
                 ", ".join(qube.name for qube in timedout)
             )
         )
-        unhandled = kill(loop=loop, domains=timedout)
+        unhandled = await kill(domains=timedout)
 
     if not unhandled and not used and not timedout:
         return
@@ -218,5 +203,15 @@ def main(args=None, app=None):
     parser.error_runtime(msg, len(unhandled + used + timedout))
 
 
+async def main_async(args=None, app=None):
+    # pylint: disable=missing-docstring
+    await run_async(args=args, app=app)
+
+
+def main(args=None, app=None):
+    # pylint: disable=missing-docstring
+    asyncio.run(run_async(args=args, app=app))
+
+
 if __name__ == "__main__":
     sys.exit(main())