Re: [PATCH 4/4] python/machine: remove unused console socket configuration arguments

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Jul 20, 2023 at 09:04:48AM -0400, John Snow wrote:
> By using a socketpair for the console, we don't need the sock_dir
> argument for the base class anymore, remove it.
> 
> The qtest subclass still uses the argument for the qtest socket for now.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/qemu/machine/machine.py             | 18 ------------------
>  python/qemu/machine/qtest.py               |  6 +++---
>  tests/qemu-iotests/tests/copy-before-write |  3 +--
>  3 files changed, 4 insertions(+), 23 deletions(-)

A couple of callers to be updated to remove 'sock_dir=':

$ git grep 'sock_dir=' tests
tests/avocado/acpi-bits.py:                         sock_dir=sock_dir, qmp_timer=qmp_timer)
tests/avocado/avocado_qemu/__init__.py:                         sock_dir=self._sd.name, log_dir=self.logdir)
tests/qemu-iotests/iotests.py:                         sock_dir=sock_dir, qmp_timer=timer)
tests/qemu-iotests/tests/copy-before-write:                              sock_dir=iotests.sock_dir)

presume the avocado_qemu/__init__.py one is what caused the
failure Peter reported.

> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index ef9b2dc02e..350aa8bb26 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -127,7 +127,6 @@ def __init__(self,
>                   name: Optional[str] = None,
>                   base_temp_dir: str = "/var/tmp",
>                   monitor_address: Optional[SocketAddrT] = None,
> -                 sock_dir: Optional[str] = None,
>                   drain_console: bool = False,
>                   console_log: Optional[str] = None,
>                   log_dir: Optional[str] = None,
> @@ -141,7 +140,6 @@ def __init__(self,
>          @param name: prefix for socket and log file names (default: qemu-PID)
>          @param base_temp_dir: default location where temp files are created
>          @param monitor_address: address for QMP monitor
> -        @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
>          @param log_dir: where to create and keep log files
> @@ -163,7 +161,6 @@ def __init__(self,
>              Tuple[socket.socket, socket.socket]] = None
>          self._temp_dir: Optional[str] = None
>          self._base_temp_dir = base_temp_dir
> -        self._sock_dir = sock_dir
>          self._log_dir = log_dir
>  
>          self._monitor_address = monitor_address
> @@ -189,9 +186,6 @@ def __init__(self,
>          self._console_index = 0
>          self._console_set = False
>          self._console_device_type: Optional[str] = None
> -        self._console_address = os.path.join(
> -            self.sock_dir, f"{self._name}.con"
> -        )
>          self._console_socket: Optional[socket.socket] = None
>          self._remove_files: List[str] = []
>          self._user_killed = False
> @@ -334,9 +328,6 @@ def args(self) -> List[str]:
>          return self._args
>  
>      def _pre_launch(self) -> None:
> -        if self._console_set:
> -            self._remove_files.append(self._console_address)
> -
>          if self._qmp_set:
>              if self._monitor_address is None:
>                  self._sock_pair = socket.socketpair()
> @@ -900,15 +891,6 @@ def temp_dir(self) -> str:
>                                                dir=self._base_temp_dir)
>          return self._temp_dir
>  
> -    @property
> -    def sock_dir(self) -> str:
> -        """
> -        Returns the directory used for sockfiles by this machine.
> -        """
> -        if self._sock_dir:
> -            return self._sock_dir
> -        return self.temp_dir
> -
>      @property
>      def log_dir(self) -> str:
>          """
> diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
> index 1c46138bd0..22f8045ef6 100644
> --- a/python/qemu/machine/qtest.py
> +++ b/python/qemu/machine/qtest.py
> @@ -115,8 +115,8 @@ def __init__(self,
>                   wrapper: Sequence[str] = (),
>                   name: Optional[str] = None,
>                   base_temp_dir: str = "/var/tmp",
> -                 sock_dir: Optional[str] = None,
> -                 qmp_timer: Optional[float] = None):
> +                 qmp_timer: Optional[float] = None,
> +                 sock_dir: Optional[str] = None):
>          # pylint: disable=too-many-arguments
>  
>          if name is None:
> @@ -125,7 +125,7 @@ def __init__(self,
>              sock_dir = base_temp_dir
>          super().__init__(binary, args, wrapper=wrapper, name=name,
>                           base_temp_dir=base_temp_dir,
> -                         sock_dir=sock_dir, qmp_timer=qmp_timer)
> +                         qmp_timer=qmp_timer)
>          self._qtest: Optional[QEMUQtestProtocol] = None
>          self._qtest_path = os.path.join(sock_dir, name + "-qtest.sock")
>  
> diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write
> index 2ffe092b31..d3987db942 100755
> --- a/tests/qemu-iotests/tests/copy-before-write
> +++ b/tests/qemu-iotests/tests/copy-before-write
> @@ -44,8 +44,7 @@ class TestCbwError(iotests.QMPTestCase):
>  
>          opts = ['-nodefaults', '-display', 'none', '-machine', 'none']
>          self.vm = QEMUMachine(iotests.qemu_prog, opts,
> -                              base_temp_dir=iotests.test_dir,
> -                              sock_dir=iotests.sock_dir)
> +                              base_temp_dir=iotests.test_dir)
>          self.vm.launch()
>  
>      def do_cbw_error(self, on_cbw_error):
> -- 
> 2.41.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|