[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/6] Python: expose QEMUMachine's temporary directory
|
From: |
John Snow |
|
Subject: |
Re: [PATCH 2/6] Python: expose QEMUMachine's temporary directory |
|
Date: |
Mon, 15 Feb 2021 17:25:12 -0500 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 2/11/21 5:01 PM, Cleber Rosa wrote:
Each instance of qemu.machine.QEMUMachine currently has a "test
directory", which may not have any relation to a "test", and it's
really a temporary directory.
Users instantiating the QEMUMachine class will be able to set the
location of the directory that will *contain* the QEMUMachine unique
temporary directory, so that parameter name has been changed from
test_dir to base_temp_dir.
Yeah, makes sense. It's a bad name.
A property has been added to allow users to access it without using
private attributes,
OK
and with that, the directory is created on first
use of the property.
Less sure if I want this.
Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
python/qemu/machine.py | 24 ++++++++++++++++--------
python/qemu/qtest.py | 6 +++---
tests/acceptance/virtio-gpu.py | 2 +-
tests/qemu-iotests/iotests.py | 2 +-
4 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..b379fcbe72 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -84,7 +84,7 @@ class QEMUMachine:
args: Sequence[str] = (),
wrapper: Sequence[str] = (),
name: Optional[str] = None,
- test_dir: str = "/var/tmp",
+ base_temp_dir: str = "/var/tmp",
monitor_address: Optional[SocketAddrT] = None,
socket_scm_helper: Optional[str] = None,
sock_dir: Optional[str] = None,
@@ -97,10 +97,10 @@ class QEMUMachine:
@param args: list of extra arguments
@param wrapper: list of arguments used as prefix to qemu binary
@param name: prefix for socket and log file names (default: qemu-PID)
- @param test_dir: where to create socket and log file
+ @param base_temp_dir: default location where temporary files are
created
@param monitor_address: address for QMP monitor
@param socket_scm_helper: helper program, required for send_fd_scm()
- @param sock_dir: where to create socket (overrides test_dir for sock)
+ @param sock_dir: where to create socket (defaults to base_temp_dir)
@param drain_console: (optional) True to drain console socket to
buffer
@param console_log: (optional) path to console log file
@note: Qemu process is not started until launch() is used.
@@ -112,8 +112,8 @@ class QEMUMachine:
self._wrapper = wrapper
self._name = name or "qemu-%d" % os.getpid()
- self._test_dir = test_dir
- self._sock_dir = sock_dir or self._test_dir
+ self._base_temp_dir = base_temp_dir
+ self._sock_dir = sock_dir or self._base_temp_dir
self._socket_scm_helper = socket_scm_helper
if monitor_address is not None:
@@ -303,9 +303,7 @@ class QEMUMachine:
return args
def _pre_launch(self) -> None:
- self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
- dir=self._test_dir)
- self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
+ self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log")
self._qemu_log_file = open(self._qemu_log_path, 'wb')
if self._console_set:
@@ -744,3 +742,13 @@ class QEMUMachine:
file=self._console_log_path,
drain=self._drain_console)
return self._console_socket
+
+ @property
+ def temp_dir(self) -> str:
+ """
+ Returns a temporary directory to be used for this machine
+ """
+ if self._temp_dir is None:
+ self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
+ dir=self._base_temp_dir)
+ return self._temp_dir
Since this property has a side effect that will outlive the process, I
think it ought to be a function (or ditch the side-effect.) The
docstring should state that the directory will be created when this
property is first accessed.
(I realize you do it here because until we create the directory, we
cannot return any particular value. Internal cleanup routines will need
to be careful NOT to call temp_dir, though!)
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 39a0cf62fe..78b97d13cf 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -112,14 +112,14 @@ class QEMUQtestMachine(QEMUMachine):
binary: str,
args: Sequence[str] = (),
name: Optional[str] = None,
- test_dir: str = "/var/tmp",
+ base_temp_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:
- sock_dir = test_dir
- super().__init__(binary, args, name=name, test_dir=test_dir,
+ sock_dir = base_temp_dir
+ super().__init__(binary, args, name=name, base_temp_dir=base_temp_dir,
socket_scm_helper=socket_scm_helper,
sock_dir=sock_dir)
self._qtest: Optional[QEMUQtestProtocol] = None
diff --git a/tests/acceptance/virtio-gpu.py b/tests/acceptance/virtio-gpu.py
index 211f02932f..8d689eb820 100644
--- a/tests/acceptance/virtio-gpu.py
+++ b/tests/acceptance/virtio-gpu.py
@@ -119,7 +119,7 @@ class VirtioGPUx86(Test):
os.set_inheritable(vug_sock.fileno(), True)
self._vug_log_path = os.path.join(
- self.vm._test_dir, "vhost-user-gpu.log"
+ self.vm.temp_dir, "vhost-user-gpu.log"
)
self._vug_log_file = open(self._vug_log_path, "wb")
print(self._vug_log_path)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 00be68eca3..b02a3dc092 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -562,7 +562,7 @@ class VM(qtest.QEMUQtestMachine):
def __init__(self, path_suffix=''):
name = "qemu%s-%d" % (path_suffix, os.getpid())
super().__init__(qemu_prog, qemu_opts, name=name,
- test_dir=test_dir,
+ base_temp_dir=test_dir,
socket_scm_helper=socket_scm_helper,
sock_dir=sock_dir)
self._num_drives = 0
Seems OK otherwise, at a glance.