qvm-shutdown: report errors, don't crash on DispVMs

qvm-shutdown with the --wait option checks if the machine
state is 'Halted', but a disposable VM is usually deleted by
the time of the final check, resulting in a non-zero exit code.

This change handles properly disposable VMs, and makes sure
we always output an error message when finishing with a non-zero
exit code.

Fixes QubesOS/qubes-issues#5245.

qubesadmin/tests/tools/qvm_shutdown.py

@@ -293,6 +293,7 @@ class TC_00_qvm_shutdown(qubesadmin.tests.QubesTestCase):
         self.app.expected_calls[
             ('sys-net', 'admin.vm.List', None, None)] = \
             b'0\x00sys-net class=AppVM state=Halted\n'
-        qubesadmin.tools.qvm_shutdown.main(
-            ['--wait', '--all', '--timeout=1'], app=self.app)
+        with self.assertRaisesRegexp(SystemExit, '2'):
+            qubesadmin.tools.qvm_shutdown.main(
+                ['--wait', '--all', '--timeout=1'], app=self.app)
         self.assertAllCalled()

qubesadmin/tools/__init__.py

@@ -408,12 +408,12 @@ class QubesArgumentParser(argparse.ArgumentParser):
         return namespace
 
 
-    def error_runtime(self, message):
+    def error_runtime(self, message, exit_code=1):
         '''Runtime error, without showing usage.
 
         :param str message: message to show
         '''
-        self.exit(1, '{}: error: {}\n'.format(self.prog, message))
+        self.exit(exit_code, '{}: error: {}\n'.format(self.prog, message))
 
 
     @staticmethod

qubesadmin/tools/qvm_shutdown.py

@@ -73,7 +73,12 @@ def main(args=None, app=None):  # pylint: disable=missing-docstring
                 else:
                     remaining_domains.add(vm)
         if not args.wait:
-            return len(remaining_domains)
+            if remaining_domains:
+                parser.error_runtime(
+                    'Failed to shut down: ' +
+                    ', '.join(vm.name for vm in remaining_domains),
+                    len(remaining_domains))
+            return
         this_round_domains.difference_update(remaining_domains)
         if not this_round_domains:
             # no VM shutdown request succeed, no sense to try again
@@ -122,8 +127,20 @@ def main(args=None, app=None):  # pylint: disable=missing-docstring
     if args.wait:
         if have_events:
             loop.close()
-        return len([vm for vm in args.domains
-            if vm.get_power_state() != 'Halted'])
+        failed = []
+        for vm in args.domains:
+            power_state = vm.get_power_state()
+            # DispVM might have been deleted before we check them,
+            # so NA is acceptable.
+            if not (power_state == 'Halted' or
+                    (vm.klass == 'DispVM' and power_state == 'NA')):
+                failed.append(vm)
+        if failed:
+            parser.error_runtime(
+                'Failed to shut down: ' +
+                ', '.join(vm.name for vm in failed),
+                len(failed))
+
 
 if __name__ == '__main__':
     sys.exit(main())