Re: [Qemu-devel] [PATCH v3 13/20] scripts/qemu.py: support adding a console with the default serial device

2019-03-11 Thread Cleber Rosa
On Fri, Mar 01, 2019 at 11:55:13AM +0100, Cornelia Huck wrote:
> On Wed, 20 Feb 2019 19:57:46 -0500
> Cleber Rosa  wrote:
> 
> > The set_console() utility function traditionally adds a device either
> > based on the explicitly given device type, or based on the machine type,
> > a known good type of device.
> 
> Hm, I find this sentence hard to parse... maybe it should be "either
> adds a device..." and "adds a known good type of device"?
>

OK, sounds good, but I may change it a little bit given that you
following feedback made me change (positively) a lot here.

> > 
> > But, for a number of machine types, it may be impossible or
> > inconvenient to add the devices my means of "-device" command line
> 
> s/my/by/
>

Same here.

> > options, and then it may better to just use the "-serial" option and
> > let QEMU itself, based on the machine type, set the device
> > accordingly.
> > 
> > To achieve that, the behavior of set_console() now flags the intention
> > to add a console device on launch(), and if no explicit device type is
> > given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial"
> > is going to be added to the QEMU command line, instead of raising
> > exceptions.
> > 
> > Based on testing with different machine types, the CONSOLE_DEV_TYPES
> > is now being set to the bare essential entries (one entry to be
> > honest), for machine types that can not easily give us a working
> > console with "-serial".
> > 
> > Signed-off-by: Cleber Rosa 
> > ---
> >  scripts/qemu.py | 39 +++
> >  1 file changed, 19 insertions(+), 20 deletions(-)
> > 
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index ee85309923..bd1d2e2b9a 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -42,11 +42,6 @@ def kvm_available(target_arch=None):
> >  
> >  #: Maps machine types to the preferred console device types
> >  CONSOLE_DEV_TYPES = {
> > -r'^clipper$': 'isa-serial',
> > -r'^malta': 'isa-serial',
> > -r'^(pc.*|q35.*|isapc)$': 'isa-serial',
> > -r'^(40p|powernv|prep)$': 'isa-serial',
> > -r'^pseries.*': 'spapr-vty',
> >  r'^s390-ccw-virtio.*': 'sclpconsole',
> 
> FWIW, s390x has supported the '-serial' parameter since 3.0 (commit
> 052888f043bacb18231046045b1f9cd946703170). Maybe you can drop this now?

Yes, thanks for pointing that out!  I'm simplifying the set_console()
behavior and dropping the CONSOLE_DEV_TYPES dict completely.

> If not, what error are you getting?

No errors, it seems to work great now.

- Cleber.



Re: [Qemu-devel] [PATCH v3 13/20] scripts/qemu.py: support adding a console with the default serial device

2019-03-01 Thread Cornelia Huck
On Wed, 20 Feb 2019 19:57:46 -0500
Cleber Rosa  wrote:

> The set_console() utility function traditionally adds a device either
> based on the explicitly given device type, or based on the machine type,
> a known good type of device.

Hm, I find this sentence hard to parse... maybe it should be "either
adds a device..." and "adds a known good type of device"?

> 
> But, for a number of machine types, it may be impossible or
> inconvenient to add the devices my means of "-device" command line

s/my/by/

> options, and then it may better to just use the "-serial" option and
> let QEMU itself, based on the machine type, set the device
> accordingly.
> 
> To achieve that, the behavior of set_console() now flags the intention
> to add a console device on launch(), and if no explicit device type is
> given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial"
> is going to be added to the QEMU command line, instead of raising
> exceptions.
> 
> Based on testing with different machine types, the CONSOLE_DEV_TYPES
> is now being set to the bare essential entries (one entry to be
> honest), for machine types that can not easily give us a working
> console with "-serial".
> 
> Signed-off-by: Cleber Rosa 
> ---
>  scripts/qemu.py | 39 +++
>  1 file changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index ee85309923..bd1d2e2b9a 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -42,11 +42,6 @@ def kvm_available(target_arch=None):
>  
>  #: Maps machine types to the preferred console device types
>  CONSOLE_DEV_TYPES = {
> -r'^clipper$': 'isa-serial',
> -r'^malta': 'isa-serial',
> -r'^(pc.*|q35.*|isapc)$': 'isa-serial',
> -r'^(40p|powernv|prep)$': 'isa-serial',
> -r'^pseries.*': 'spapr-vty',
>  r'^s390-ccw-virtio.*': 'sclpconsole',

FWIW, s390x has supported the '-serial' parameter since 3.0 (commit
052888f043bacb18231046045b1f9cd946703170). Maybe you can drop this now?
If not, what error are you getting?



[Qemu-devel] [PATCH v3 13/20] scripts/qemu.py: support adding a console with the default serial device

2019-02-20 Thread Cleber Rosa
The set_console() utility function traditionally adds a device either
based on the explicitly given device type, or based on the machine type,
a known good type of device.

But, for a number of machine types, it may be impossible or
inconvenient to add the devices my means of "-device" command line
options, and then it may better to just use the "-serial" option and
let QEMU itself, based on the machine type, set the device
accordingly.

To achieve that, the behavior of set_console() now flags the intention
to add a console device on launch(), and if no explicit device type is
given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial"
is going to be added to the QEMU command line, instead of raising
exceptions.

Based on testing with different machine types, the CONSOLE_DEV_TYPES
is now being set to the bare essential entries (one entry to be
honest), for machine types that can not easily give us a working
console with "-serial".

Signed-off-by: Cleber Rosa 
---
 scripts/qemu.py | 39 +++
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index ee85309923..bd1d2e2b9a 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -42,11 +42,6 @@ def kvm_available(target_arch=None):
 
 #: Maps machine types to the preferred console device types
 CONSOLE_DEV_TYPES = {
-r'^clipper$': 'isa-serial',
-r'^malta': 'isa-serial',
-r'^(pc.*|q35.*|isapc)$': 'isa-serial',
-r'^(40p|powernv|prep)$': 'isa-serial',
-r'^pseries.*': 'spapr-vty',
 r'^s390-ccw-virtio.*': 'sclpconsole',
 }
 
@@ -129,6 +124,7 @@ class QEMUMachine(object):
 self._temp_dir = None
 self._launched = False
 self._machine = None
+self._console_set = False
 self._console_device_type = None
 self._console_address = None
 self._console_socket = None
@@ -248,13 +244,17 @@ class QEMUMachine(object):
 '-display', 'none', '-vga', 'none']
 if self._machine is not None:
 args.extend(['-machine', self._machine])
-if self._console_device_type is not None:
+if self._console_set:
 self._console_address = os.path.join(self._temp_dir,
  self._name + "-console.sock")
 chardev = ('socket,id=console,path=%s,server,nowait' %
self._console_address)
-device = '%s,chardev=console' % self._console_device_type
-args.extend(['-chardev', chardev, '-device', device])
+args.extend(['-chardev', chardev])
+if self._console_device_type is None:
+args.extend(['-serial', 'chardev:console'])
+else:
+device = '%s,chardev=console' % self._console_device_type
+args.extend(['-device', device])
 return args
 
 def _pre_launch(self):
@@ -480,30 +480,29 @@ class QEMUMachine(object):
 line.
 
 This is a convenience method that will either use the provided
-device type, of if not given, it will used the device type set
-on CONSOLE_DEV_TYPES.
+device type, of if not given, it will use the device type set
+on CONSOLE_DEV_TYPES if a machine type is set, and a matching
+entry exists on CONSOLE_DEV_TYPES.
 
 The actual setting of command line arguments will be be done at
 machine launch time, as it depends on the temporary directory
 to be created.
 
-@param device_type: the device type, such as "isa-serial"
+@param device_type: the device type, such as "isa-serial".  If
+None is given (the default value) a "-serial
+chardev:console" command line argument will
+be used instead, resorting to the machine's
+default device type, if a machine type is set,
+and a matching entry exists on CONSOLE_DEV_TYPES.
 @raises: QEMUMachineAddDeviceError if the device type is not given
  and can not be determined.
 """
-if device_type is None:
-if self._machine is None:
-raise QEMUMachineAddDeviceError("Can not add a console device:"
-" QEMU instance without a "
-"defined machine type")
+self._console_set = True
+if device_type is None and self._machine is not None:
 for regex, device in CONSOLE_DEV_TYPES.items():
 if re.match(regex, self._machine):
 device_type = device
 break
-if device_type is None:
-raise QEMUMachineAddDeviceError("Can not add a console device:"
-" no matching console device "
-