From 277b5fe300b832b657e8258b268759f9ea711373 Mon Sep 17 00:00:00 2001 From: anon <@> Date: Thu, 12 Mar 2026 15:37:35 +0900 Subject: [PATCH 1/3] Harmonize behavior between has/hasnt_event, and remove deprecated hasnt_event branch --- qubesadmin/tests/tools/qvm_shutdown.py | 12 +++--- qubesadmin/tools/qvm_shutdown.py | 57 +++++++------------------- 2 files changed, 22 insertions(+), 47 deletions(-) diff --git a/qubesadmin/tests/tools/qvm_shutdown.py b/qubesadmin/tests/tools/qvm_shutdown.py index c1926fad..8bef7be1 100644 --- a/qubesadmin/tests/tools/qvm_shutdown.py +++ b/qubesadmin/tests/tools/qvm_shutdown.py @@ -194,9 +194,6 @@ def test_015_wait_all_kill_timeout(self): self.app.expected_calls[ ('sys-net', 'admin.vm.Shutdown', 'force', None)] = \ b'0\x00' - self.app.expected_calls[ - ('sys-net', 'admin.vm.Kill', None, None)] = \ - b'2\x00QubesVMNotStartedError\x00\x00Domain is powered off\x00' self.app.expected_calls[ ('dom0', 'admin.vm.List', None, None)] = \ b'0\x00' \ @@ -207,15 +204,20 @@ def test_015_wait_all_kill_timeout(self): ('some-vm', 'admin.vm.CurrentState', None, None)] = [ b'0\x00power_state=Running', b'0\x00power_state=Running', + b'0\x00power_state=Running', ] self.app.expected_calls[ ('other-vm', 'admin.vm.CurrentState', None, None)] = [ b'0\x00power_state=Running', b'0\x00power_state=Running', + b'0\x00power_state=Running', ] self.app.expected_calls[ - ('sys-net', 'admin.vm.CurrentState', None, None)] = \ - b'0\x00power_state=Halted' + ('sys-net', 'admin.vm.CurrentState', None, None)] = [ + b'0\x00power_state=Halted', + b'0\x00power_state=Halted', + b'0\x00power_state=Halted', + ] with self.assertRaisesRegex(SystemExit, '2'): qubesadmin.tools.qvm_shutdown.main( ['--wait', '--all', '--timeout=1'], app=self.app) diff --git a/qubesadmin/tools/qvm_shutdown.py b/qubesadmin/tools/qvm_shutdown.py index a00d9509..8a9f4451 100644 --- a/qubesadmin/tools/qvm_shutdown.py +++ b/qubesadmin/tools/qvm_shutdown.py @@ -25,15 +25,10 @@ from __future__ import print_function import sys -import time import asyncio -try: - import qubesadmin.events.utils - have_events = True -except ImportError: - have_events = False +import qubesadmin.events.utils import qubesadmin.tools import qubesadmin.exc @@ -74,9 +69,8 @@ def main(args=None, app=None): # pylint: disable=missing-docstring force = args.force or (args.all_domains and not args.exclude) - if have_events: - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) remaining_domains = args.domains for _ in range(len(args.domains)): this_round_domains = set(remaining_domains) @@ -95,45 +89,25 @@ def main(args=None, app=None): # pylint: disable=missing-docstring else: remaining_domains.add(vm) if not args.wait: - if remaining_domains: + if shutdown_failed: parser.error_runtime( 'Failed to shut down: ' + - ', '.join(vm.name for vm in remaining_domains), - len(remaining_domains)) + ', '.join(vm.name for vm in shutdown_failed), + len(shutdown_failed)) return this_round_domains.difference_update(remaining_domains) if not this_round_domains: # no VM shutdown request succeed, no sense to try again break - if have_events: - try: - # pylint: disable=no-member - loop.run_until_complete(asyncio.wait_for( - qubesadmin.events.utils.wait_for_domain_shutdown( - this_round_domains), - args.timeout)) - except asyncio.TimeoutError: - if not args.dry_run: - for vm in this_round_domains: - try: - vm.kill() - except qubesadmin.exc.QubesVMNotStartedError: - # already shut down - pass - except qubesadmin.exc.QubesException as e: - parser.error_runtime(e) - else: - timeout = args.timeout - current_vms = list(sorted(this_round_domains)) - while timeout >= 0: - current_vms = failed_domains(current_vms) - if not current_vms: - break - args.app.log.info('Waiting for shutdown ({}): {}'.format( - timeout, ', '.join([str(vm) for vm in current_vms]))) - time.sleep(1) - timeout -= 1 + + try: + # pylint: disable=no-member + loop.run_until_complete(asyncio.wait_for( + qubesadmin.events.utils.wait_for_domain_shutdown( + awaiting), args.timeout)) + except (TimeoutError, asyncio.TimeoutError): if not args.dry_run: + current_vms = failed_domains(this_round_domains) if current_vms: args.app.log.info( 'Killing remaining qubes: {}' @@ -148,8 +122,7 @@ def main(args=None, app=None): # pylint: disable=missing-docstring parser.error_runtime(e) if args.wait: - if have_events: - loop.close() + loop.close() failed = failed_domains(args.domains) if failed: parser.error_runtime( From b3f15f5edaaf4d8739055793492223cdce28ecaa Mon Sep 17 00:00:00 2001 From: anon <@> Date: Thu, 12 Mar 2026 17:01:00 +0900 Subject: [PATCH 2/3] remove dead code around args.wait --- qubesadmin/tools/qvm_shutdown.py | 36 +++++++++++++++----------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/qubesadmin/tools/qvm_shutdown.py b/qubesadmin/tools/qvm_shutdown.py index 8a9f4451..eeecb8c0 100644 --- a/qubesadmin/tools/qvm_shutdown.py +++ b/qubesadmin/tools/qvm_shutdown.py @@ -71,14 +71,13 @@ def main(args=None, app=None): # pylint: disable=missing-docstring loop = asyncio.new_event_loop() asyncio.set_event_loop(loop) - remaining_domains = args.domains + remaining_domains = set(args.domains) for _ in range(len(args.domains)): - this_round_domains = set(remaining_domains) - if not this_round_domains: + if not remaining_domains: break - remaining_domains = set() + shutdown_failed = set() if not args.dry_run: - for vm in this_round_domains: + for vm in remaining_domains: try: vm.shutdown(force=force) except qubesadmin.exc.QubesVMNotStartedError: @@ -86,8 +85,7 @@ def main(args=None, app=None): # pylint: disable=missing-docstring except qubesadmin.exc.QubesException as e: if not args.wait: vm.log.error('Shutdown error: {}'.format(e)) - else: - remaining_domains.add(vm) + shutdown_failed.add(vm) if not args.wait: if shutdown_failed: parser.error_runtime( @@ -95,9 +93,10 @@ def main(args=None, app=None): # pylint: disable=missing-docstring ', '.join(vm.name for vm in shutdown_failed), len(shutdown_failed)) return - this_round_domains.difference_update(remaining_domains) - if not this_round_domains: - # no VM shutdown request succeed, no sense to try again + awaiting = remaining_domains - shutdown_failed + remaining_domains = shutdown_failed + if not awaiting: + # no VM shutdown request succeeded, no sense to try again break try: @@ -107,7 +106,7 @@ def main(args=None, app=None): # pylint: disable=missing-docstring awaiting), args.timeout)) except (TimeoutError, asyncio.TimeoutError): if not args.dry_run: - current_vms = failed_domains(this_round_domains) + current_vms = failed_domains(awaiting) if current_vms: args.app.log.info( 'Killing remaining qubes: {}' @@ -121,14 +120,13 @@ def main(args=None, app=None): # pylint: disable=missing-docstring except qubesadmin.exc.QubesException as e: parser.error_runtime(e) - if args.wait: - loop.close() - failed = failed_domains(args.domains) - if failed: - parser.error_runtime( - 'Failed to shut down: ' + - ', '.join(vm.name for vm in failed), - len(failed)) + loop.close() + failed = failed_domains(args.domains) + if failed: + parser.error_runtime( + 'Failed to shut down: ' + + ', '.join(vm.name for vm in failed), + len(failed)) if __name__ == '__main__': From 17c938006a8699758beb0c6d2e4cf6b991e42ba4 Mon Sep 17 00:00:00 2001 From: anon <@> Date: Thu, 12 Mar 2026 17:16:33 +0900 Subject: [PATCH 3/3] add utests --- qubesadmin/tests/tools/qvm_shutdown.py | 158 ++++++++++++++++++++++++- 1 file changed, 152 insertions(+), 6 deletions(-) diff --git a/qubesadmin/tests/tools/qvm_shutdown.py b/qubesadmin/tests/tools/qvm_shutdown.py index 8bef7be1..a43808c4 100644 --- a/qubesadmin/tests/tools/qvm_shutdown.py +++ b/qubesadmin/tests/tools/qvm_shutdown.py @@ -83,8 +83,6 @@ def test_004_multiple_vms(self): app=self.app) self.assertAllCalled() - @unittest.skipUnless(qubesadmin.tools.qvm_shutdown.have_events, - 'Events not present') def test_010_wait(self): '''test --wait option''' loop = asyncio.new_event_loop() @@ -114,8 +112,6 @@ def test_010_wait(self): qubesadmin.tools.qvm_shutdown.main(['--wait', 'some-vm'], app=self.app) self.assertAllCalled() - @unittest.skipUnless(qubesadmin.tools.qvm_shutdown.have_events, - 'Events not present') def test_012_wait_all(self): '''test --wait option, with multiple VMs''' loop = asyncio.new_event_loop() @@ -161,8 +157,6 @@ def test_012_wait_all(self): qubesadmin.tools.qvm_shutdown.main(['--wait', '--all'], app=self.app) self.assertAllCalled() - @unittest.skipUnless(qubesadmin.tools.qvm_shutdown.have_events, - 'Events not present') def test_015_wait_all_kill_timeout(self): '''test --wait option, with multiple VMs and killing on timeout''' loop = asyncio.new_event_loop() @@ -251,3 +245,155 @@ def test_017_all_exclude_force_explicit(self): '--force'], app=self.app) self.assertAllCalled() + + def test_005_force(self): + '''test --force sends force flag to shutdown call''' + 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.Shutdown', 'force', None)] = b'0\x00' + qubesadmin.tools.qvm_shutdown.main( + ['--force', 'some-vm'], app=self.app) + self.assertAllCalled() + + def test_006_dry_run(self): + '''test --dry-run skips shutdown calls''' + self.app.expected_calls[ + ('dom0', 'admin.vm.List', None, None)] = \ + b'0\x00some-vm class=AppVM state=Running\n' + qubesadmin.tools.qvm_shutdown.main( + ['--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() + asyncio.set_event_loop(loop) + + mock_events = unittest.mock.AsyncMock() + patch = unittest.mock.patch( + 'qubesadmin.events.EventsDispatcher._get_events_reader', + mock_events) + patch.start() + self.addCleanup(patch.stop) + mock_events.side_effect = qubesadmin.tests.tools.MockEventsReader([ + # round 1: wait for some-vm + b'1\0\0connection-established\0\0', + b'1\0some-vm\0domain-shutdown\0\0', + # round 2: wait for other-vm + b'1\0\0connection-established\0\0', + b'1\0other-vm\0domain-shutdown\0\0', + ]) + + self.app.expected_calls[ + ('dom0', 'admin.vm.List', None, None)] = \ + b'0\x00' \ + b'some-vm class=AppVM state=Running\n' \ + b'other-vm class=AppVM state=Running\n' + self.app.expected_calls[ + ('some-vm', 'admin.vm.Shutdown', None, None)] = \ + b'0\x00' + # other-vm fails first attempt, succeeds on retry + self.app.expected_calls[ + ('other-vm', 'admin.vm.Shutdown', None, None)] = [ + b'2\x00QubesException\x00\x00Shutdown refused\x00', + b'0\x00', + ] + self.app.expected_calls[ + ('some-vm', 'admin.vm.CurrentState', None, None)] = [ + b'0\x00power_state=Running', + b'0\x00power_state=Halted', + ] + self.app.expected_calls[ + ('other-vm', 'admin.vm.CurrentState', None, None)] = [ + b'0\x00power_state=Running', + b'0\x00power_state=Halted', + ] + qubesadmin.tools.qvm_shutdown.main( + ['--wait', 'some-vm', 'other-vm'], app=self.app) + self.assertAllCalled() + + def test_013_wait_all_shutdown_fail(self): + '''test --wait exits with error when all shutdown requests fail''' + 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.Shutdown', None, None)] = \ + b'2\x00QubesException\x00\x00Shutdown refused\x00' + self.app.expected_calls[ + ('some-vm', 'admin.vm.CurrentState', None, None)] = \ + b'0\x00power_state=Running' + with self.assertRaises(SystemExit): + qubesadmin.tools.qvm_shutdown.main( + ['--wait', 'some-vm'], app=self.app) + self.assertAllCalled() + + def test_016_wait_kill_exception(self): + '''test --wait timeout where kill raises QubesException''' + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + + mock_events = unittest.mock.AsyncMock() + patch = unittest.mock.patch( + 'qubesadmin.events.EventsDispatcher._get_events_reader', + mock_events) + patch.start() + self.addCleanup(patch.stop) + mock_events.side_effect = qubesadmin.tests.tools.MockEventsReader([ + b'1\0\0connection-established\0\0', + ]) + + 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.Shutdown', None, None)] = \ + b'0\x00' + self.app.expected_calls[ + ('some-vm', 'admin.vm.Kill', None, None)] = \ + b'2\x00QubesException\x00\x00Kill failed\x00' + self.app.expected_calls[ + ('some-vm', 'admin.vm.CurrentState', None, None)] = [ + b'0\x00power_state=Running', + b'0\x00power_state=Running', + ] + with self.assertRaises(SystemExit): + qubesadmin.tools.qvm_shutdown.main( + ['--wait', '--timeout=1', 'some-vm'], app=self.app) + self.assertAllCalled() + + def test_017_wait_dispvm_na(self): + '''test --wait treats DispVM with NA power state as shut down''' + loop = asyncio.new_event_loop() + asyncio.set_event_loop(loop) + + mock_events = unittest.mock.AsyncMock() + patch = unittest.mock.patch( + 'qubesadmin.events.EventsDispatcher._get_events_reader', + mock_events) + patch.start() + self.addCleanup(patch.stop) + mock_events.side_effect = qubesadmin.tests.tools.MockEventsReader([ + b'1\0\0connection-established\0\0', + b'1\0disp123\0domain-shutdown\0\0', + ]) + + self.app.expected_calls[ + ('dom0', 'admin.vm.List', None, None)] = \ + b'0\x00disp123 class=DispVM state=Running\n' + self.app.expected_calls[ + ('disp123', 'admin.vm.Shutdown', None, None)] = \ + b'0\x00' + self.app.expected_calls[ + ('disp123', 'admin.vm.CurrentState', None, None)] = [ + b'0\x00power_state=Running', + # failed_domains: first get_power_state() != 'Halted', + # then klass == 'DispVM' triggers second get_power_state() + b'0\x00power_state=NA', + b'0\x00power_state=NA', + ] + qubesadmin.tools.qvm_shutdown.main( + ['--wait', 'disp123'], app=self.app) + self.assertAllCalled()