Re: [PATCH 2/6] Python: expose QEMUMachine's temporary directory

[Wainer dos Santos Moschetta <wainersm@redhat.com>]

Hi,

On 2/11/21 7: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.
>
> A property has been added to allow users to access it without using
> private attributes, and with that, the directory is created on first
> use of the property.
>
> 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
> 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",


In qtest.QEMUQtestMachine.__init__(), the argument named 'test_dir' 
still make sense, right?

- Wainer

>                    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