| Message ID | 20180419164642.9536-4-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | avocado: Add acceptance tests parsing the Linux boot console | expand |
On 04/19/2018 12:46 PM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@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.
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é <f4bug@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 <crosa@redhat.com>
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]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- scripts/qemu.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)