On 05/01/2018 03:30 PM, Cleber Rosa wrote:
>
>
> On 04/19/2018 12:46 PM, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>> ---
>> 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.
>
Never mind these previous comments. I was looking at a tree with the
next set of patches applied and missed the mktemp() here.
Acked-by: Cleber Rosa <cr...@redhat.com>
