On 04/19/2018 12:46 PM, Philippe Mathieu-Daudé wrote:
> QEMUMachine arguments member is called '_args'.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4...@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.
