All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-block@nongnu.org,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>, "Cleber Rosa" <crosa@redhat.com>,
	"John Snow" <jsnow@redhat.com>
Subject: [PULL 10/21] python/qemu: make 'args' style arguments immutable
Date: Tue, 20 Oct 2020 13:27:31 -0400	[thread overview]
Message-ID: <20201020172742.1483258-11-jsnow@redhat.com> (raw)
In-Reply-To: <20201020172742.1483258-1-jsnow@redhat.com>

These arguments don't need to be mutable and aren't really used as
such. Clarify their types as immutable and adjust code to match where
necessary.

In general, It's probably best not to accept a user-defined mutable
object and store it as internal object state unless there's a strong
justification for doing so. Instead, try to use generic types as input
with empty tuples as the default, and coerce to list where necessary.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201006235817.3280413-10-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/machine.py | 30 +++++++++++++++++-------------
 python/qemu/qtest.py   | 22 +++++++++++++++++-----
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 4e762fcd52..e599cb7439 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -18,6 +18,7 @@
 #
 
 import errno
+from itertools import chain
 import logging
 import os
 import shutil
@@ -30,6 +31,8 @@
     Dict,
     List,
     Optional,
+    Sequence,
+    Tuple,
     Type,
 )
 
@@ -74,8 +77,12 @@ class QEMUMachine:
         # vm is guaranteed to be shut down here
     """
 
-    def __init__(self, binary, args=None, wrapper=None, name=None,
-                 test_dir="/var/tmp",
+    def __init__(self,
+                 binary: str,
+                 args: Sequence[str] = (),
+                 wrapper: Sequence[str] = (),
+                 name: Optional[str] = None,
+                 test_dir: str = "/var/tmp",
                  monitor_address: Optional[SocketAddrT] = None,
                  socket_scm_helper=None, sock_dir=None,
                  drain_console=False, console_log=None):
@@ -97,14 +104,7 @@ def __init__(self, binary, args=None, wrapper=None, name=None,
         # Direct user configuration
 
         self._binary = binary
-
-        if args is None:
-            args = []
-        # Copy mutable input: we will be modifying our copy
         self._args = list(args)
-
-        if wrapper is None:
-            wrapper = []
         self._wrapper = wrapper
 
         self._name = name or "qemu-%d" % os.getpid()
@@ -136,7 +136,7 @@ def __init__(self, binary, args=None, wrapper=None, name=None,
         self._iolog = None
         self._qmp_set = True   # Enable QMP monitor by default.
         self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None
-        self._qemu_full_args = None
+        self._qemu_full_args: Tuple[str, ...] = ()
         self._temp_dir = None
         self._launched = False
         self._machine = None
@@ -368,7 +368,7 @@ def launch(self):
             raise QEMUMachineError('VM already launched')
 
         self._iolog = None
-        self._qemu_full_args = None
+        self._qemu_full_args = ()
         try:
             self._launch()
             self._launched = True
@@ -388,8 +388,12 @@ def _launch(self):
         """
         devnull = open(os.path.devnull, 'rb')
         self._pre_launch()
-        self._qemu_full_args = (self._wrapper + [self._binary] +
-                                self._base_args + self._args)
+        self._qemu_full_args = tuple(
+            chain(self._wrapper,
+                  [self._binary],
+                  self._base_args,
+                  self._args)
+        )
         LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
         self._popen = subprocess.Popen(self._qemu_full_args,
                                        stdin=devnull,
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 7fde2565a0..675310d8df 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -19,7 +19,12 @@
 
 import os
 import socket
-from typing import List, Optional, TextIO
+from typing import (
+    List,
+    Optional,
+    Sequence,
+    TextIO,
+)
 
 from .machine import QEMUMachine
 
@@ -99,8 +104,13 @@ class QEMUQtestMachine(QEMUMachine):
     A QEMU VM, with a qtest socket available.
     """
 
-    def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
-                 socket_scm_helper=None, sock_dir=None):
+    def __init__(self,
+                 binary: str,
+                 args: Sequence[str] = (),
+                 name: Optional[str] = None,
+                 test_dir: str = "/var/tmp",
+                 socket_scm_helper: Optional[str] = None,
+                 sock_dir: Optional[str] = None):
         if name is None:
             name = "qemu-%d" % os.getpid()
         if sock_dir is None:
@@ -114,8 +124,10 @@ def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
     @property
     def _base_args(self) -> List[str]:
         args = super()._base_args
-        args.extend(['-qtest', 'unix:path=' + self._qtest_path,
-                     '-accel', 'qtest'])
+        args.extend([
+            '-qtest', f"unix:path={self._qtest_path}",
+            '-accel', 'qtest'
+        ])
         return args
 
     def _pre_launch(self):
-- 
2.26.2



  parent reply	other threads:[~2020-10-20 17:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20 17:27 [PULL 00/21] Python patches John Snow
2020-10-20 17:27 ` [PULL 01/21] MAINTAINERS: Add Python library stanza John Snow
2020-10-20 17:27 ` [PULL 02/21] python/qemu: use isort to lay out imports John Snow
2020-10-20 17:27 ` [PULL 03/21] python/machine.py: Fix monitor address typing John Snow
2020-10-20 17:27 ` [PULL 04/21] python/machine.py: reorder __init__ John Snow
2020-10-20 17:27 ` [PULL 05/21] python/machine.py: Don't modify state in _base_args() John Snow
2020-10-20 17:27 ` [PULL 06/21] python/machine.py: Handle None events in events_wait John Snow
2020-10-20 17:27 ` [PULL 07/21] python/machine.py: use qmp.command John Snow
2020-10-20 17:27 ` [PULL 08/21] python/machine.py: Add _qmp access shim John Snow
2020-10-20 17:27 ` [PULL 09/21] python/machine.py: fix _popen access John Snow
2020-10-20 17:27 ` John Snow [this message]
2020-10-20 17:27 ` [PULL 11/21] iotests.py: Adjust HMP kwargs typing John Snow
2020-10-20 17:27 ` [PULL 12/21] python/qemu: Add mypy type annotations John Snow
2020-10-20 17:27 ` [PULL 13/21] python/qemu/console_socket.py: Correct type of recv() John Snow
2020-10-20 17:27 ` [PULL 14/21] python/qemu/console_socket.py: fix typing of settimeout John Snow
2020-10-20 17:27 ` [PULL 15/21] python/qemu/console_socket.py: Clarify type of drain_thread John Snow
2020-10-20 17:27 ` [PULL 16/21] python/qemu/console_socket.py: Add type hint annotations John Snow
2020-10-20 17:27 ` [PULL 17/21] python/qemu/console_socket.py: avoid encoding to/from string John Snow
2020-10-20 17:27 ` [PULL 18/21] python/qemu/qmp.py: Preserve error context on re-raise John Snow
2020-10-20 17:27 ` [PULL 19/21] python: add mypy config John Snow
2020-10-20 17:27 ` [PULL 20/21] python/qemu/qmp.py: re-raise OSError when encountered John Snow
2020-10-20 18:15   ` Nir Soffer
2020-10-20 19:06     ` John Snow
2020-10-20 17:27 ` [PULL 21/21] python/qemu/qmp.py: Fix settimeout operation John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201020172742.1483258-11-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=crosa@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.