Re: [Qemu-devel] [PATCH v7 07/11] qemu.py: include debug information on launch error
Dne 18.8.2017 v 19:05 Amador Pahim napsal(a): > When launching a VM, if an exception happens and the VM is not > initiated, it might be useful to see the qemu command line and > the qemu command output. > > This patch creates that message. Notice that self._iolog needs to be > cleaned up in the beginning of the launch() to make sure we will not > expose the qemu log from a previous launch if the current one fails. > > Signed-off-by: Amador Pahim > --- > scripts/qemu.py | 8 > 1 file changed, 8 insertions(+) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index 0bcec4b3b1..29fd2469f9 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -147,6 +147,7 @@ class QEMUMachine(object): > > def launch(self): > '''Launch the VM and establish a QMP connection''' > +self._iolog = None > self._qemu_full_args = None > devnull = open(os.path.devnull, 'rb') > qemulog = open(self._qemu_log_path, 'wb') > @@ -162,6 +163,13 @@ class QEMUMachine(object): > self._post_launch() > except: > self.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) > + > raise > I don't want to open the cave, but anybody else would actually prefer custom Exception with the qemu_full_args, iolog and details about the original exception to this? Users who don't care would see/log all the details from the `exception.__str__` and those who are interested in the original exception would query for it eg. via `exception.original_exception`. Anyway even this change is IMO step into the right direction and we can (and should) refine the use of exceptions in following patches (as Markus spotted, pure Exceptions and other ugliness lives in those waters). As for the debug vs. error I'd actually stick to "debug", because in negative testing the logged message is only an expected debug and in positive testing it expands the knowledge of the flow, but the actual error message should come from the exception. Not a strong opinion, though... Regards, Lukáš > def shutdown(self): > signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v7 07/11] qemu.py: include debug information on launch error
On Wed, 08/30 11:55, Cleber Rosa wrote: > > > On 08/18/2017 01:05 PM, Amador Pahim wrote: > > When launching a VM, if an exception happens and the VM is not > > initiated, it might be useful to see the qemu command line and > > the qemu command output. > > > > This patch creates that message. Notice that self._iolog needs to be > > cleaned up in the beginning of the launch() to make sure we will not > > expose the qemu log from a previous launch if the current one fails. > > > > Signed-off-by: Amador Pahim > > --- > > scripts/qemu.py | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/scripts/qemu.py b/scripts/qemu.py > > index 0bcec4b3b1..29fd2469f9 100644 > > --- a/scripts/qemu.py > > +++ b/scripts/qemu.py > > @@ -147,6 +147,7 @@ class QEMUMachine(object): > > > > def launch(self): > > '''Launch the VM and establish a QMP connection''' > > +self._iolog = None > > self._qemu_full_args = None > > devnull = open(os.path.devnull, 'rb') > > qemulog = open(self._qemu_log_path, 'wb') > > @@ -162,6 +163,13 @@ class QEMUMachine(object): > > self._post_launch() > > except: > > self.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) > > + > > Based on Fam's comment about the signal message being a warning (worth > showing by default), I also think this deserves more than a "debug" > classification. Yes, LOG.error, please. Fam
Re: [Qemu-devel] [PATCH v7 07/11] qemu.py: include debug information on launch error
On 08/18/2017 01:05 PM, Amador Pahim wrote: > When launching a VM, if an exception happens and the VM is not > initiated, it might be useful to see the qemu command line and > the qemu command output. > > This patch creates that message. Notice that self._iolog needs to be > cleaned up in the beginning of the launch() to make sure we will not > expose the qemu log from a previous launch if the current one fails. > > Signed-off-by: Amador Pahim > --- > scripts/qemu.py | 8 > 1 file changed, 8 insertions(+) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index 0bcec4b3b1..29fd2469f9 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -147,6 +147,7 @@ class QEMUMachine(object): > > def launch(self): > '''Launch the VM and establish a QMP connection''' > +self._iolog = None > self._qemu_full_args = None > devnull = open(os.path.devnull, 'rb') > qemulog = open(self._qemu_log_path, 'wb') > @@ -162,6 +163,13 @@ class QEMUMachine(object): > self._post_launch() > except: > self.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) > + Based on Fam's comment about the signal message being a warning (worth showing by default), I also think this deserves more than a "debug" classification. - Cleber. > raise > > def shutdown(self): > signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v7 07/11] qemu.py: include debug information on launch error
When launching a VM, if an exception happens and the VM is not initiated, it might be useful to see the qemu command line and the qemu command output. This patch creates that message. Notice that self._iolog needs to be cleaned up in the beginning of the launch() to make sure we will not expose the qemu log from a previous launch if the current one fails. Signed-off-by: Amador Pahim --- scripts/qemu.py | 8 1 file changed, 8 insertions(+) diff --git a/scripts/qemu.py b/scripts/qemu.py index 0bcec4b3b1..29fd2469f9 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -147,6 +147,7 @@ class QEMUMachine(object): def launch(self): '''Launch the VM and establish a QMP connection''' +self._iolog = None self._qemu_full_args = None devnull = open(os.path.devnull, 'rb') qemulog = open(self._qemu_log_path, 'wb') @@ -162,6 +163,13 @@ class QEMUMachine(object): self._post_launch() except: self.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) + raise def shutdown(self): -- 2.13.5