Hello Wainer and Eduardo,
On Tue, Nov 20, 2018 at 05:02:38PM -0200, Eduardo Habkost wrote:
On Tue, Nov 20, 2018 at 11:53:00AM -0500, Wainer dos Santos Moschetta wrote:
QEMUMachine launches the VM with a monitor enabled, afterwards
a qmp connection is attempted on _post_launch(). In case
the QEMU process exits with an error, qmp.accept() reaches
timeout and raises an exception.
But sometimes you don't need that monitor. As an example,
when a test launches the VM expects its immediate crash,
and only intend to check the process's return code. In this
case the fact that launch() tries to establish the qmp
connection (ending up in an exception) is troublesome.
So this patch adds the disable_qmp() that allow to
launch the VM without creating the monitor machinery.
{...}
+
+ self._args.extend(['-chardev', moncdev, '-mon',
'chardev=mon,mode=control'])
This will extend self._args multiple times if making a
.shutdown()/.launch() cycle:
{...}
Why did you move this code outside _base_args(), where it already
worked without relying on method side-effects and required no
extra state to be kept inside self._args?
Eduardo, I think the purpose was to get a way to set up the monitor
conditionally. So the arguments related with monitor was moved out
_base_args() method and put into the _setup_qmp(). However, as you
showed, this implementation has undesired side-effects.
Wainer, probably the most straightforward way to add this capability to
QEMUMachine is to change the _base_args() method to only include monitor
arguments when necessary. Something like this:
```
# create a proper method to get moncdev_args
def _get_moncdev_args():
if isinstance(self._monitor_address, tuple):
return "socket,id=mon,host=%s,port=%s" % (
self._monitor_address[0],
self._monitor_address[1])
else:
return 'socket,id=mon,path=%s' % self._vm_monitor
# update _base_args method to use new attribute _with_qmp
def _base_args(self):
args = ['-display', 'none', '-vga', 'none']
if self._with_qmp:
args.extend(['-chardev', self._get_moncdev_args(),
'-mon', 'chardev=mon,mode=control'])
if self._machine is not None:
```
Does it make sense?