On Tue, Nov 12, 2019 at 08:58:00AM -0500, Wainer dos Santos Moschetta wrote:
> The QEMUMachine VM has a monitor setup on which an QMP
> connection is always attempted on _post_launch() (executed
> by launch()). In case the QEMU process immediatly exits
> then the qmp.accept() (used to establish the connection) stalls
> until it reaches timeout and consequently an exception raises.
>
> That behavior is undesirable when, for instance, it needs to
> gather information from the QEMU binary ($ qemu -cpu list) or a
> test which launches the VM expecting its failure.
>
> This patch adds the set_qmp_monitor() method to QEMUMachine that
> allows turn off the creation of the monitor machinery on VM launch.
>
> Signed-off-by: Wainer dos Santos Moschetta <waine...@redhat.com>
> ---
> python/qemu/machine.py | 68 ++++++++++++++++++++++++++++--------------
> 1 file changed, 45 insertions(+), 23 deletions(-)
>
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index a4631d6934..04ee86e1ba 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -104,6 +104,7 @@ class QEMUMachine(object):
> self._events = []
> self._iolog = None
> self._socket_scm_helper = socket_scm_helper
> + self._qmp_set = True # Enable QMP monitor by default.
> self._qmp = None
> self._qemu_full_args = None
> self._test_dir = test_dir
> @@ -228,15 +229,16 @@ class QEMUMachine(object):
> self._iolog = iolog.read()
>
> def _base_args(self):
> - if isinstance(self._monitor_address, tuple):
> - moncdev = "socket,id=mon,host=%s,port=%s" % (
> + args = ['-display', 'none', '-vga', 'none']
> + if self._qmp_set:
> + if isinstance(self._monitor_address, tuple):
> + moncdev = "socket,id=mon,host=%s,port=%s" % (
> self._monitor_address[0],
> self._monitor_address[1])
> - else:
> - moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
> - args = ['-chardev', moncdev,
> - '-mon', 'chardev=mon,mode=control',
> - '-display', 'none', '-vga', 'none']
> + else:
> + moncdev = 'socket,id=mon,path=%s' % self._vm_monitor
> + args.extend(['-chardev', moncdev, '-mon',
> + 'chardev=mon,mode=control'])
> if self._machine is not None:
> args.extend(['-machine', self._machine])
> if self._console_set:
> @@ -255,20 +257,21 @@ class QEMUMachine(object):
>
> def _pre_launch(self):
> self._temp_dir = tempfile.mkdtemp(dir=self._test_dir)
> - if self._monitor_address is not None:
> - self._vm_monitor = self._monitor_address
> - else:
> - self._vm_monitor = os.path.join(self._sock_dir,
> - self._name + "-monitor.sock")
> - self._remove_files.append(self._vm_monitor)
> self._qemu_log_path = os.path.join(self._temp_dir, self._name +
> ".log")
> self._qemu_log_file = open(self._qemu_log_path, 'wb')
>
> - self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
> - server=True)
> + if self._qmp_set:
> + if self._monitor_address is not None:
> + self._vm_monitor = self._monitor_address
> + else:
> + self._vm_monitor = os.path.join(self._sock_dir,
> + self._name + "-monitor.sock")
> + self._remove_files.append(self._vm_monitor)
> + self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
> server=True)
>
> def _post_launch(self):
> - self._qmp.accept()
> + if self._qmp:
> + self._qmp.accept()
>
> def _post_shutdown(self):
> if self._qemu_log_file is not None:
> @@ -330,7 +333,8 @@ class QEMUMachine(object):
> Wait for the VM to power off
> """
> self._popen.wait()
> - self._qmp.close()
> + if self._qmp:
> + self._qmp.close()
> self._load_io_log()
> self._post_shutdown()
>
> @@ -346,12 +350,13 @@ class QEMUMachine(object):
> self._console_socket = None
>
> if self.is_running():
> - try:
> - if not has_quit:
> - self._qmp.cmd('quit')
> - self._qmp.close()
> - except:
> - self._popen.kill()
> + if self._qmp:
> + try:
> + if not has_quit:
> + self._qmp.cmd('quit')
> + self._qmp.close()
> + except:
> + self._popen.kill()
> self._popen.wait()
>
> self._load_io_log()
> @@ -368,6 +373,23 @@ class QEMUMachine(object):
>
> self._launched = False
>
> + def set_qmp_monitor(self, disabled=False, monitor_address=None):
> + """
> + Set the QMP monitor.
> +
> + @param disabled: if True, qmp monitor options will be removed from
> the
> + base arguments of the resulting QEMU command line.I personally tend avoid double negatives as long as I'm aware of them. Wouldn't "enabled=True" be more straightforward? Just my personal preference though. > + @param monitor_address: address for the QMP monitor. Do you have a use case for passing the monitor address here too, in addition to also being available as a parameter to __init__()? In the next patch, I don't see it being used (or here for that matter). > + @note: call this function before launch(). > + """ > + if disabled: > + self._qmp_set = False > + self._qmp = None > + else: > + self._qmp_set = True > + if monitor_address: > + self._monitor_address = monitor_address > + > def qmp(self, cmd, conv_keys=True, **args): > """ > Invoke a QMP command and return the response dict > -- > 2.18.1 > Other than those comments, it LGTM. - Cleber.
signature.asc
Description: PGP signature
