Re: [Qemu-devel] [RFC PATCH 3/7] qemu.py: Check console arch is supported before calling mktemp()

[Cleber Rosa]

On 04/19/2018 12:46 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  scripts/qemu.py | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 0eecc44d09..379767b62f 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -189,6 +189,16 @@ class QEMUMachine(object):
>                  if option in item:
>                      return []
>  
> +        device = '{dev_type},chardev=console'
> +        if '86' in self._arch:
> +            device = device.format(dev_type='isa-serial')
> +        elif 'ppc' in self._arch:
> +            device = device.format(dev_type='spapr-vty')
> +        elif 's390x' in self._arch:
> +            device = device.format(dev_type='sclpconsole')
> +        else:
> +            return []
> +
>          chardev = 'socket,id=console,{address},server,nowait'
>          if console_address is None:
>              console_address = tempfile.mktemp()
> @@ -203,16 +213,6 @@ class QEMUMachine(object):
>  
>          self._console_address = console_address
>  
> -        device = '{dev_type},chardev=console'
> -        if '86' in self._arch:
> -            device = device.format(dev_type='isa-serial')
> -        elif 'ppc' in self._arch:
> -            device = device.format(dev_type='spapr-vty')
> -        elif 's390x' in self._arch:
> -            device = device.format(dev_type='sclpconsole')
> -        else:
> -            return []
> -
>          return ['-chardev', chardev,
>                  '-device', device]
>  
> 

I understand your point here, but I found the commit message to be
misleading.  You're probably referring to this snippet (from
tests/avocado/test_linux-boot-console.py):

   +    def setUp(self):
   +        self.console_path = tempfile.mkstemp()[1]

So I see the following points regarding this patch:

1) Function called is mkstemp(), and not mktemp(), assuming you meant
the one from the pasted snippet above.

2) The commit message should just state that it returns earlier when
architecture is not supported wrt console creation.