From 2be77f58b3055fd59818e2777d598b3776a50d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marta=20Marczykowska-G=C3=B3recka?= Date: Tue, 21 Jan 2020 22:20:36 +0100 Subject: [PATCH 1/2] Added force shutdown option to vm.shutdown Furthermore makes qvm-shutdown --all use that option to force shutdown and avoid unnecessary errors. requires https://github.com/QubesOS/qubes-core-admin/pull/312 fixes QubesOS/qubes-issues#5591 fixes QubesOS/qubes-issues#4572 --- qubesadmin/tests/tools/qvm_shutdown.py | 101 ++----------------------- qubesadmin/tools/qvm_shutdown.py | 4 +- qubesadmin/vm/__init__.py | 5 +- 3 files changed, 12 insertions(+), 98 deletions(-) diff --git a/qubesadmin/tests/tools/qvm_shutdown.py b/qubesadmin/tests/tools/qvm_shutdown.py index 2201594..c7dc732 100644 --- a/qubesadmin/tests/tools/qvm_shutdown.py +++ b/qubesadmin/tests/tools/qvm_shutdown.py @@ -126,13 +126,13 @@ class TC_00_qvm_shutdown(qubesadmin.tests.QubesTestCase): ]) self.app.expected_calls[ - ('some-vm', 'admin.vm.Shutdown', None, None)] = \ + ('some-vm', 'admin.vm.Shutdown', 'force', None)] = \ b'0\x00' self.app.expected_calls[ - ('other-vm', 'admin.vm.Shutdown', None, None)] = \ + ('other-vm', 'admin.vm.Shutdown', 'force', None)] = \ b'0\x00' self.app.expected_calls[ - ('sys-net', 'admin.vm.Shutdown', None, None)] = \ + ('sys-net', 'admin.vm.Shutdown', 'force', None)] = \ b'0\x00' self.app.expected_calls[ ('dom0', 'admin.vm.List', None, None)] = \ @@ -152,95 +152,6 @@ class TC_00_qvm_shutdown(qubesadmin.tests.QubesTestCase): 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_013_wait_all_order(self): - '''test --wait option, with some VMs requiring specific shutdown - order''' - loop = asyncio.new_event_loop() - asyncio.set_event_loop(loop) - - 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\0sys-net\0domain-shutdown\0\0', - b'1\0some-vm\0domain-shutdown\0\0', - b'1\0other-vm\0domain-shutdown\0\0', - ]) - - self.app.expected_calls[ - ('some-vm', 'admin.vm.Shutdown', None, None)] = \ - b'0\x00' - self.app.expected_calls[ - ('other-vm', 'admin.vm.Shutdown', None, None)] = \ - b'0\x00' - self.app.expected_calls[ - ('sys-net', 'admin.vm.Shutdown', None, None)] = [ - b'2\x00QubesVMError\x00\x00Other Vms connected\x00', - b'0\x00', - ] - self.app.expected_calls[ - ('dom0', 'admin.vm.List', None, None)] = \ - b'0\x00' \ - b'sys-net class=AppVM state=Running\n' \ - b'some-vm class=AppVM state=Running\n' \ - b'other-vm class=AppVM state=Running\n' - self.app.expected_calls[ - ('some-vm', 'admin.vm.CurrentState', None, None)] = \ - b'0\x00power_state=Halted' - self.app.expected_calls[ - ('other-vm', 'admin.vm.CurrentState', None, None)] = \ - b'0\x00power_state=Halted' - self.app.expected_calls[ - ('sys-net', 'admin.vm.CurrentState', None, None)] = \ - b'0\x00power_state=Halted' - qubesadmin.tools.qvm_shutdown.main(['--wait', '--all'], app=self.app) - self.assertAllCalled() - - def test_014_wait_all_order_no_events(self): - '''test --wait option, with some VMs requiring specific shutdown - order''' - - self.app.expected_calls[ - ('some-vm', 'admin.vm.Shutdown', None, None)] = \ - b'0\x00' - self.app.expected_calls[ - ('other-vm', 'admin.vm.Shutdown', None, None)] = \ - b'0\x00' - self.app.expected_calls[ - ('sys-net', 'admin.vm.Shutdown', None, None)] = [ - b'2\x00QubesVMError\x00\x00Other Vms connected\x00', - b'0\x00', - ] - self.app.expected_calls[ - ('dom0', 'admin.vm.List', None, None)] = \ - b'0\x00' \ - b'sys-net class=AppVM state=Running\n' \ - b'some-vm class=AppVM state=Running\n' \ - b'other-vm class=AppVM state=Running\n' - self.app.expected_calls[ - ('some-vm', 'admin.vm.CurrentState', None, None)] = \ - [b'0\x00power_state=Running', - b'0\x00power_state=Halted', - 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', - b'0\x00power_state=Halted'] - self.app.expected_calls[ - ('sys-net', 'admin.vm.CurrentState', None, None)] = \ - [b'0\x00power_state=Running', - b'0\x00power_state=Halted', - b'0\x00power_state=Halted'] - with unittest.mock.patch('qubesadmin.tools.qvm_shutdown.have_events', - False): - 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): @@ -258,19 +169,19 @@ class TC_00_qvm_shutdown(qubesadmin.tests.QubesTestCase): ]) self.app.expected_calls[ - ('some-vm', 'admin.vm.Shutdown', None, None)] = \ + ('some-vm', 'admin.vm.Shutdown', 'force', None)] = \ b'0\x00' self.app.expected_calls[ ('some-vm', 'admin.vm.Kill', None, None)] = \ b'2\x00QubesVMNotStartedError\x00\x00Domain is powered off\x00' self.app.expected_calls[ - ('other-vm', 'admin.vm.Shutdown', None, None)] = \ + ('other-vm', 'admin.vm.Shutdown', 'force', None)] = \ b'0\x00' self.app.expected_calls[ ('other-vm', 'admin.vm.Kill', None, None)] = \ b'0\x00' self.app.expected_calls[ - ('sys-net', 'admin.vm.Shutdown', None, None)] = \ + ('sys-net', 'admin.vm.Shutdown', 'force', None)] = \ b'0\x00' self.app.expected_calls[ ('sys-net', 'admin.vm.Kill', None, None)] = \ diff --git a/qubesadmin/tools/qvm_shutdown.py b/qubesadmin/tools/qvm_shutdown.py index f9b10cf..12f4a95 100644 --- a/qubesadmin/tools/qvm_shutdown.py +++ b/qubesadmin/tools/qvm_shutdown.py @@ -54,6 +54,8 @@ parser.add_argument('--timeout', def main(args=None, app=None): # pylint: disable=missing-docstring args = parser.parse_args(args, app=app) + force = bool(args.all_domains) + if have_events: loop = asyncio.get_event_loop() remaining_domains = args.domains @@ -64,7 +66,7 @@ def main(args=None, app=None): # pylint: disable=missing-docstring remaining_domains = set() for vm in this_round_domains: try: - vm.shutdown() + vm.shutdown(force=force) except qubesadmin.exc.QubesVMNotRunningError: pass except qubesadmin.exc.QubesException as e: diff --git a/qubesadmin/vm/__init__.py b/qubesadmin/vm/__init__.py index c40a4b4..2d74aae 100644 --- a/qubesadmin/vm/__init__.py +++ b/qubesadmin/vm/__init__.py @@ -113,8 +113,9 @@ class QubesVM(qubesadmin.base.PropertyHolder): # TODO: force parameter # TODO: wait parameter (using event?) if force: - raise NotImplementedError - self.qubesd_call(self._method_dest, 'admin.vm.Shutdown') + self.qubesd_call(self._method_dest, 'admin.vm.Shutdown', 'force') + else: + self.qubesd_call(self._method_dest, 'admin.vm.Shutdown') def kill(self): ''' From 5c94c72ecd49be04eb0eec1eb4f98b24051ff479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marta=20Marczykowska-G=C3=B3recka?= Date: Wed, 29 Jan 2020 19:34:57 +0100 Subject: [PATCH 2/2] Added explicit 'force' option to qvm-shutdown --- doc/manpages/qvm-shutdown.rst | 10 ++++++++-- qubesadmin/tools/qvm_shutdown.py | 7 ++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/doc/manpages/qvm-shutdown.rst b/doc/manpages/qvm-shutdown.rst index f9239f0..4d08d99 100644 --- a/doc/manpages/qvm-shutdown.rst +++ b/doc/manpages/qvm-shutdown.rst @@ -25,7 +25,7 @@ Options .. option:: --all - perform the action on all qubes + perform the action on all qubes; implies :option:`--force` .. option:: --exclude=EXCLUDE @@ -35,12 +35,18 @@ Options wait for the VMs to shut down. If some domains are providing network to other domains, wait for those domains to shut down before shutting down their - dependents. + dependents, unless :option:`--all` was specified .. option:: --timeout timeout after which domains are killed when using :option:`--wait` +.. option:: --force + + force qube shutdown, regardless of whether there exist any connected domains + (such as those using it as network VM) + + Authors ------- diff --git a/qubesadmin/tools/qvm_shutdown.py b/qubesadmin/tools/qvm_shutdown.py index 12f4a95..fe0b68d 100644 --- a/qubesadmin/tools/qvm_shutdown.py +++ b/qubesadmin/tools/qvm_shutdown.py @@ -50,11 +50,16 @@ parser.add_argument('--timeout', help='timeout after which domains are killed when using --wait' ' (default: %(default)d)') +parser.add_argument( + '--force', + action='store_true', default=False, + help='force shutdown regardless of connected domains; use with caution') + def main(args=None, app=None): # pylint: disable=missing-docstring args = parser.parse_args(args, app=app) - force = bool(args.all_domains) + force = args.force or bool(args.all_domains) if have_events: loop = asyncio.get_event_loop()