| Message ID | 20180419164642.9536-3-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: > QEMUMachine arguments member is called '_args'. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > scripts/qemu.py | 14 +++++++------- > tests/avocado/avocado_qemu/test.py | 12 ++++++------ > tests/qemu-iotests/iotests.py | 28 ++++++++++++++-------------- > 3 files changed, 27 insertions(+), 27 deletions(-) > > diff --git a/scripts/qemu.py b/scripts/qemu.py > index bd66620f45..0eecc44d09 100644 > --- a/scripts/qemu.py > +++ b/scripts/qemu.py > @@ -81,7 +81,7 @@ class QEMUMachine(object): > self._qemu_log_file = None > self._popen = None > self._binary = binary > - self.args = list(args) # Force copy args in case we modify them > + self._args = list(args) # Force copy args in case we modify them > self._wrapper = wrapper > self._events = [] > self._iolog = None > @@ -109,8 +109,8 @@ class QEMUMachine(object): > # This can be used to add an unused monitor instance. > def add_monitor_telnet(self, ip, port): > args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port) > - self.args.append('-monitor') > - self.args.append(args) > + self._args.append('-monitor') > + self._args.append(args) > > def add_fd(self, fd, fdset, opaque, opts=''): > '''Pass a file descriptor to the VM''' > @@ -120,8 +120,8 @@ class QEMUMachine(object): > if opts: > options.append(opts) > > - self.args.append('-add-fd') > - self.args.append(','.join(options)) > + self._args.append('-add-fd') > + self._args.append(','.join(options)) > return self > > def send_fd_scm(self, fd_file_path): > @@ -184,7 +184,7 @@ class QEMUMachine(object): > '-display', 'none', '-vga', 'none'] > > def _create_console(self, console_address): > - for item in self.args: > + for item in self._args: > for option in ['isa-serial', 'spapr-vty', 'sclpconsole']: > if option in item: > return [] > @@ -274,7 +274,7 @@ class QEMUMachine(object): > bargs = self._base_args() > bargs.extend(self._create_console(console_address)) > self._qemu_full_args = (self._wrapper + [self._binary] + > - bargs + self.args) > + bargs + self._args) > self._popen = subprocess.Popen(self._qemu_full_args, > stdin=devnull, > stdout=self._qemu_log_file, > diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py > index 5a08dace45..1ead917014 100644 > --- a/tests/avocado/avocado_qemu/test.py > +++ b/tests/avocado/avocado_qemu/test.py > @@ -297,8 +297,8 @@ class _VM(qemu.QEMUMachine): > port = self.ports.find_free_port() > newvm = _VM(self.qemu_dst_bin, self._arch, username=self.username, > password=self.password) > - newvm.args = self.args > - newvm.args.extend(['-incoming', 'tcp:0:%s' % port]) > + newvm._args = self._args > + newvm._args.extend(['-incoming', 'tcp:0:%s' % port]) > newvm.username = self.username > newvm.password = self.password > > @@ -329,7 +329,7 @@ class _VM(qemu.QEMUMachine): > :param extra: Extra parameters to the -drive option > """ > file_option = 'file=%s' % path > - for item in self.args: > + for item in self._args: > if file_option in item: > logging.error('Image %s already present', path) > return > @@ -340,7 +340,7 @@ class _VM(qemu.QEMUMachine): > if snapshot: > file_option += ',snapshot=on' > > - self.args.extend(['-drive', file_option]) > + self._args.extend(['-drive', file_option]) > > if username is not None: > self.username = username > @@ -387,7 +387,7 @@ class _VM(qemu.QEMUMachine): > process.run("%s -output %s -volid cidata -joliet -rock %s %s" % > (geniso_bin, iso_path, metadata_path, userdata_path)) > > - self.args.extend(['-cdrom', iso_path]) > + self._args.extend(['-cdrom', iso_path]) > > class QemuTest(Test): > > @@ -415,4 +415,4 @@ class QemuTest(Test): > if machine_kvm_type is not None: > machine += "kvm-type=%s," % machine_kvm_type > if machine: > - self.vm.args.extend(['-machine', machine]) > + self.vm._args.extend(['-machine', machine]) > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index a2e4f03743..b25d48a91b 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -293,18 +293,18 @@ class VM(qtest.QEMUQtestMachine): > self._num_drives = 0 > > def add_object(self, opts): > - self.args.append('-object') > - self.args.append(opts) > + self._args.append('-object') > + self._args.append(opts) > return self > > def add_device(self, opts): > - self.args.append('-device') > - self.args.append(opts) > + self._args.append('-device') > + self._args.append(opts) > return self > > def add_drive_raw(self, opts): > - self.args.append('-drive') > - self.args.append(opts) > + self._args.append('-drive') > + self._args.append(opts) > return self > > def add_drive(self, path, opts='', interface='virtio', format=imgfmt): > @@ -322,27 +322,27 @@ class VM(qtest.QEMUQtestMachine): > > if format == 'luks' and 'key-secret' not in opts: > # default luks support > - if luks_default_secret_object not in self.args: > + if luks_default_secret_object not in self._args: > self.add_object(luks_default_secret_object) > > options.append(luks_default_key_secret_opt) > > - self.args.append('-drive') > - self.args.append(','.join(options)) > + self._args.append('-drive') > + self._args.append(','.join(options)) > self._num_drives += 1 > return self > > def add_blockdev(self, opts): > - self.args.append('-blockdev') > + self._args.append('-blockdev') > if isinstance(opts, str): > - self.args.append(opts) > + self._args.append(opts) > else: > - self.args.append(','.join(opts)) > + self._args.append(','.join(opts)) > return self > > def add_incoming(self, addr): > - self.args.append('-incoming') > - self.args.append(addr) > + self._args.append('-incoming') > + self._args.append(addr) > return self > > def pause_drive(self, drive, event=None): > Well, this is basically a revert of parts of this patch: https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg03446.html There was some previous discussions on this topic, basically regarding two conflicting points: 1) The fact that other instances of scripts.qemu.QEMUMachine (such as the _VM instances created by avocado_qemu.tests.QemuTest) need access to the QEMU args; 2) That attributes prefixed with (single) underscore should only be accessed by code in the class itself. The discussion then orbited around the usefulness (or not) of a "dumb" wrapper for a list. For the record, I'm in favor of exposing the list directly, until/if some extra functionality is needed.
diff --git a/scripts/qemu.py b/scripts/qemu.py index bd66620f45..0eecc44d09 100644 --- a/scripts/qemu.py +++ b/scripts/qemu.py @@ -81,7 +81,7 @@ class QEMUMachine(object): self._qemu_log_file = None self._popen = None self._binary = binary - self.args = list(args) # Force copy args in case we modify them + self._args = list(args) # Force copy args in case we modify them self._wrapper = wrapper self._events = [] self._iolog = None @@ -109,8 +109,8 @@ class QEMUMachine(object): # This can be used to add an unused monitor instance. def add_monitor_telnet(self, ip, port): args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port) - self.args.append('-monitor') - self.args.append(args) + self._args.append('-monitor') + self._args.append(args) def add_fd(self, fd, fdset, opaque, opts=''): '''Pass a file descriptor to the VM''' @@ -120,8 +120,8 @@ class QEMUMachine(object): if opts: options.append(opts) - self.args.append('-add-fd') - self.args.append(','.join(options)) + self._args.append('-add-fd') + self._args.append(','.join(options)) return self def send_fd_scm(self, fd_file_path): @@ -184,7 +184,7 @@ class QEMUMachine(object): '-display', 'none', '-vga', 'none'] def _create_console(self, console_address): - for item in self.args: + for item in self._args: for option in ['isa-serial', 'spapr-vty', 'sclpconsole']: if option in item: return [] @@ -274,7 +274,7 @@ class QEMUMachine(object): bargs = self._base_args() bargs.extend(self._create_console(console_address)) self._qemu_full_args = (self._wrapper + [self._binary] + - bargs + self.args) + bargs + self._args) self._popen = subprocess.Popen(self._qemu_full_args, stdin=devnull, stdout=self._qemu_log_file, diff --git a/tests/avocado/avocado_qemu/test.py b/tests/avocado/avocado_qemu/test.py index 5a08dace45..1ead917014 100644 --- a/tests/avocado/avocado_qemu/test.py +++ b/tests/avocado/avocado_qemu/test.py @@ -297,8 +297,8 @@ class _VM(qemu.QEMUMachine): port = self.ports.find_free_port() newvm = _VM(self.qemu_dst_bin, self._arch, username=self.username, password=self.password) - newvm.args = self.args - newvm.args.extend(['-incoming', 'tcp:0:%s' % port]) + newvm._args = self._args + newvm._args.extend(['-incoming', 'tcp:0:%s' % port]) newvm.username = self.username newvm.password = self.password @@ -329,7 +329,7 @@ class _VM(qemu.QEMUMachine): :param extra: Extra parameters to the -drive option """ file_option = 'file=%s' % path - for item in self.args: + for item in self._args: if file_option in item: logging.error('Image %s already present', path) return @@ -340,7 +340,7 @@ class _VM(qemu.QEMUMachine): if snapshot: file_option += ',snapshot=on' - self.args.extend(['-drive', file_option]) + self._args.extend(['-drive', file_option]) if username is not None: self.username = username @@ -387,7 +387,7 @@ class _VM(qemu.QEMUMachine): process.run("%s -output %s -volid cidata -joliet -rock %s %s" % (geniso_bin, iso_path, metadata_path, userdata_path)) - self.args.extend(['-cdrom', iso_path]) + self._args.extend(['-cdrom', iso_path]) class QemuTest(Test): @@ -415,4 +415,4 @@ class QemuTest(Test): if machine_kvm_type is not None: machine += "kvm-type=%s," % machine_kvm_type if machine: - self.vm.args.extend(['-machine', machine]) + self.vm._args.extend(['-machine', machine]) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index a2e4f03743..b25d48a91b 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -293,18 +293,18 @@ class VM(qtest.QEMUQtestMachine): self._num_drives = 0 def add_object(self, opts): - self.args.append('-object') - self.args.append(opts) + self._args.append('-object') + self._args.append(opts) return self def add_device(self, opts): - self.args.append('-device') - self.args.append(opts) + self._args.append('-device') + self._args.append(opts) return self def add_drive_raw(self, opts): - self.args.append('-drive') - self.args.append(opts) + self._args.append('-drive') + self._args.append(opts) return self def add_drive(self, path, opts='', interface='virtio', format=imgfmt): @@ -322,27 +322,27 @@ class VM(qtest.QEMUQtestMachine): if format == 'luks' and 'key-secret' not in opts: # default luks support - if luks_default_secret_object not in self.args: + if luks_default_secret_object not in self._args: self.add_object(luks_default_secret_object) options.append(luks_default_key_secret_opt) - self.args.append('-drive') - self.args.append(','.join(options)) + self._args.append('-drive') + self._args.append(','.join(options)) self._num_drives += 1 return self def add_blockdev(self, opts): - self.args.append('-blockdev') + self._args.append('-blockdev') if isinstance(opts, str): - self.args.append(opts) + self._args.append(opts) else: - self.args.append(','.join(opts)) + self._args.append(','.join(opts)) return self def add_incoming(self, addr): - self.args.append('-incoming') - self.args.append(addr) + self._args.append('-incoming') + self._args.append(addr) return self def pause_drive(self, drive, event=None):
QEMUMachine arguments member is called '_args'. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- scripts/qemu.py | 14 +++++++------- tests/avocado/avocado_qemu/test.py | 12 ++++++------ tests/qemu-iotests/iotests.py | 28 ++++++++++++++-------------- 3 files changed, 27 insertions(+), 27 deletions(-)