Parcourir la source

backup/restore: better error detection for --paranoid-mode

Xterm doesn't preserve exit code of the process running inside. This
means, the whole xterm always exits with 0, even if qvm-backup-restore
failed.
Fix this by printing the exit code at the end to the log and then extract
that last line from the log on the calling side. This way we can also
distinguish whether qvm-backup-restore or xterm failed.
Marek Marczykowski-Górecki il y a 3 ans
Parent
commit
1660a1cbf6
2 fichiers modifiés avec 29 ajouts et 8 suppressions
  1. 26 6
      qubesadmin/backup/dispvm.py
  2. 3 2
      qubesadmin/tests/backup/dispvm.py

+ 26 - 6
qubesadmin/backup/dispvm.py

@@ -135,8 +135,8 @@ class RestoreInDisposableVM:
         self.backup_log_path = '/var/tmp/backup-restore.log'
         self.terminal_app = ('xterm', '-hold', '-title', 'Backup restore', '-e',
                              '/bin/sh', '-c',
-                             'exec "$0" "$@" 2>&1 | tee {}'.format(
-                                 self.backup_log_path))
+                             '("$0" "$@" 2>&1; echo exit code: $?) | tee {}'.
+                             format(self.backup_log_path))
         if args.auto_close:
             # filter-out '-hold'
             self.terminal_app = tuple(a for a in self.terminal_app
@@ -285,17 +285,37 @@ class RestoreInDisposableVM:
                     self.args.pass_file)
             args = self.prepare_inner_args()
             self.dispvm.tags.add(self.dispvm_tag)
-            # TODO: better error detection
             self.dispvm.run_with_args(*self.terminal_app,
                                       'qvm-backup-restore', *args,
                                       stdout=subprocess.DEVNULL,
                                       stderr=subprocess.DEVNULL)
-            return self.extract_log()
+            backup_log = self.extract_log()
+            last_line = backup_log.splitlines()[-1]
+            if not last_line.startswith(b'exit code:'):
+                raise qubesadmin.exc.BackupRestoreError(
+                    'qvm-backup-restore did not reported exit code',
+                    backup_log=backup_log)
+            try:
+                exit_code = int(last_line.split()[-1])
+            except ValueError:
+                raise qubesadmin.exc.BackupRestoreError(
+                    'qvm-backup-restore reported unexpected exit code',
+                    backup_log=backup_log)
+            if exit_code == 127:
+                raise qubesadmin.exc.QubesException(
+                    'qvm-backup-restore tool '
+                    'missing in {} template, install qubes-core-admin-client '
+                    'package there'.format(self.dispvm.template.template.name)
+                )
+            if exit_code != 0:
+                raise qubesadmin.exc.BackupRestoreError(
+                    'qvm-backup-restore failed with {}'.format(exit_code),
+                    backup_log=backup_log)
+            return backup_log
         except subprocess.CalledProcessError as e:
             if e.returncode == 127:
                 raise qubesadmin.exc.QubesException(
-                    'qvm-backup-restore tool or {} '
-                    'missing in {} template, install qubes-core-admin-client '
+                    '{} missing in {} template, install it there '
                     'package there'.format(self.terminal_app[0],
                                            self.dispvm.template.template.name)
                 )

+ 3 - 2
qubesadmin/tests/backup/dispvm.py

@@ -311,6 +311,7 @@ class TC_00_RestoreInDispVM(qubesadmin.tests.QubesTestCase):
                    'prepare_inner_args', 'extract_log', 'finalize_tags']
         for m in methods:
             setattr(obj, m, unittest.mock.Mock())
+        obj.extract_log.return_value = b'Some logs\nexit code: 0\n'
         obj.transfer_pass_file = unittest.mock.Mock()
         obj.prepare_inner_args.return_value = ['args']
         obj.terminal_app = ('terminal',)
@@ -348,6 +349,7 @@ class TC_00_RestoreInDispVM(qubesadmin.tests.QubesTestCase):
                    'transfer_pass_file']
         for m in methods:
             setattr(obj, m, unittest.mock.Mock())
+        obj.extract_log.return_value = b'Some logs\nexit code: 0\n'
         obj.prepare_inner_args.return_value = ['args']
         obj.terminal_app = ('terminal',)
         obj.dispvm = unittest.mock.Mock()
@@ -382,12 +384,11 @@ class TC_00_RestoreInDispVM(qubesadmin.tests.QubesTestCase):
                    'prepare_inner_args', 'extract_log', 'finalize_tags']
         for m in methods:
             setattr(obj, m, unittest.mock.Mock())
+        obj.extract_log.return_value = b'Some error\nexit code: 1\n'
         obj.transfer_pass_file = unittest.mock.Mock()
         obj.prepare_inner_args.return_value = ['args']
         obj.terminal_app = ('terminal',)
         obj.dispvm = unittest.mock.Mock()
-        obj.dispvm.run_with_args.side_effect = subprocess.CalledProcessError(
-            1, cmd='cmd')
         with tempfile.NamedTemporaryFile() as tmp:
             with unittest.mock.patch('qubesadmin.backup.dispvm.LOCKFILE',
                     tmp.name):