Skip to content

Commit

Permalink
python/machine: raise VMLaunchFailure exception from launch()
Browse files Browse the repository at this point in the history
This allows us to pack in some extra information about the failure,
which guarantees that if the caller did not *intentionally* cause a
failure (by capturing this Exception), some pretty good clues will be
printed at the bottom of the traceback information.

This will help make failures in the event of a non-negative return code
more obvious when they go unhandled; the current behavior in
_post_shutdown() is to print a warning message only in the event of
signal-based terminations (for negative return codes).

(Note: In Python, catching BaseException instead of Exception catches a
broader array of Exception events, including SystemExit and
KeyboardInterrupt. We do not want to "wrap" such exceptions as a
VMLaunchFailure, because that will 'downgrade' the exception from a
BaseException to a regular Exception. We do, however, want to perform
cleanup in either case, so catch on the broadest scope and
wrap-and-re-raise only in the more targeted scope.)

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20220201041134.1237016-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
  • Loading branch information
jnsnow committed Feb 2, 2022
1 parent fa73e6e commit 50465f9
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 8 deletions.
45 changes: 39 additions & 6 deletions python/qemu/machine/machine.py
Expand Up @@ -74,6 +74,35 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
"""


class VMLaunchFailure(QEMUMachineError):
"""
Exception raised when a VM launch was attempted, but failed.
"""
def __init__(self, exitcode: Optional[int],
command: str, output: Optional[str]):
super().__init__(exitcode, command, output)
self.exitcode = exitcode
self.command = command
self.output = output

def __str__(self) -> str:
ret = ''
if self.__cause__ is not None:
name = type(self.__cause__).__name__
reason = str(self.__cause__)
if reason:
ret += f"{name}: {reason}"
else:
ret += f"{name}"
ret += '\n'

if self.exitcode is not None:
ret += f"\tExit code: {self.exitcode}\n"
ret += f"\tCommand: {self.command}\n"
ret += f"\tOutput: {self.output}\n"
return ret


class AbnormalShutdown(QEMUMachineError):
"""
Exception raised when a graceful shutdown was requested, but not performed.
Expand Down Expand Up @@ -397,7 +426,7 @@ def launch(self) -> None:

try:
self._launch()
except:
except BaseException as exc:
# We may have launched the process but it may
# have exited before we could connect via QMP.
# Assume the VM didn't launch or is exiting.
Expand All @@ -408,11 +437,15 @@ def launch(self) -> None:
else:
self._post_shutdown()

LOG.debug('Error launching VM')
if self._qemu_full_args:
LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
if self._iolog:
LOG.debug('Output: %r', self._iolog)
if isinstance(exc, Exception):
raise VMLaunchFailure(
exitcode=self.exitcode(),
command=' '.join(self._qemu_full_args),
output=self._iolog
) from exc

# Don't wrap 'BaseException'; doing so would downgrade
# that exception. However, we still want to clean up.
raise

def _launch(self) -> None:
Expand Down
3 changes: 1 addition & 2 deletions tests/qemu-iotests/tests/mirror-top-perms
Expand Up @@ -21,7 +21,6 @@

import os

from qemu.aqmp import ConnectError
from qemu.machine import machine
from qemu.qmp import QMPConnectError

Expand Down Expand Up @@ -107,7 +106,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
self.vm_b.launch()
print('ERROR: VM B launched successfully, '
'this should not have happened')
except (QMPConnectError, ConnectError):
except (QMPConnectError, machine.VMLaunchFailure):
assert 'Is another process using the image' in self.vm_b.get_log()

result = self.vm.qmp('block-job-cancel',
Expand Down

0 comments on commit 50465f9

Please sign in to comment.