Skip to content

Commit

Permalink
Better error reporting on backup errors
Browse files Browse the repository at this point in the history
qvm-backup-restore will now show output from the rpc command
if backup restore fails at the very start (e.g. if file was not
found)

fixes QubesOS/qubes-issues#3446
  • Loading branch information
marmarta committed Sep 16, 2021
1 parent f186f7a commit a55b4c5
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
7 changes: 4 additions & 3 deletions qubesadmin/backup/restore.py
Original file line number Diff line number Diff line change
Expand Up @@ -1195,15 +1195,16 @@ def _retrieve_backup_header_files(self, files, allow_none=False):
retrieve_proc_returncode = retrieve_proc.wait()
if retrieve_proc in self.processes_to_kill_on_cancel:
self.processes_to_kill_on_cancel.remove(retrieve_proc)
extract_stderr = error_pipe.read(MAX_STDERR_BYTES)
extract_stderr = error_pipe.read(MAX_STDERR_BYTES).decode(
'ascii', 'ignore')
error_pipe.close()

# wait for other processes (if any)
for proc in self.processes_to_kill_on_cancel:
if proc.wait() != 0:
raise QubesException(
"Backup header retrieval failed (exit code {})".format(
proc.wait())
"Backup header retrieval failed (exit code {}): {}".format(
proc.wait(), extract_stderr)
)

if retrieve_proc_returncode != 0:
Expand Down
16 changes: 14 additions & 2 deletions qubesadmin/tests/tools/qvm_backup_restore.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ def tearDown(self):
@mock.patch('qubesadmin.tools.qvm_backup_restore.input', create=True)
@mock.patch('getpass.getpass')
@mock.patch('qubesadmin.tools.qvm_backup_restore.BackupRestore')
def test_000_simple(self, mock_backup, mock_getpass, mock_input):
@mock.patch('os.path.exists', return_value=True)
def test_000_simple(self, _mock_exists,
mock_backup, mock_getpass, mock_input):
mock_getpass.return_value = 'testpass'
mock_input.return_value = 'Y'
vm1 = BackupVM()
Expand Down Expand Up @@ -68,7 +70,9 @@ def test_000_simple(self, mock_backup, mock_getpass, mock_input):
@mock.patch('qubesadmin.tools.qvm_backup_restore.input', create=True)
@mock.patch('getpass.getpass')
@mock.patch('qubesadmin.tools.qvm_backup_restore.BackupRestore')
def test_001_selected_vms(self, mock_backup, mock_getpass, mock_input):
@mock.patch('os.path.exists', return_value=True)
def test_001_selected_vms(self, _mock_exist,
mock_backup, mock_getpass, mock_input):
mock_getpass.return_value = 'testpass'
mock_input.return_value = 'Y'
vm1 = BackupVM()
Expand Down Expand Up @@ -235,6 +239,14 @@ def test_012_handle_broken_missing_netvm(self):
self.app, mock_args, mock_restore_info)
self.assertAppropriateLogging('NetVM', 'error')

@mock.patch('argparse.ArgumentParser.error')
def test_013_missing_file(self, mock_argparse):
mock_argparse.side_effect = qubesadmin.exc.QubesException('Error')
with self.assertRaises(qubesadmin.exc.QubesException):
qubesadmin.tools.qvm_backup_restore.main(['/some/path'],
app=self.app)
mock_argparse.assert_called_once_with("File not found: '/some/path'")

def test_100_restore_in_dispvm_parser(self):
"""Verify if qvm-backup-restore tool options matches un-parser
for paranoid restore mode"""
Expand Down

0 comments on commit a55b4c5

Please sign in to comment.