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(-)
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.0On 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 :|
On Thu, Jul 20, 2023 at 10:05 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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.
>
Yep, missed a spot, sorry :( I tested avocado after patch 3 but not here.
While I'm at it, though, I am testing the same treatment for the qtest
extension and just removing sock_dir from *everything*, since we don't
need that workaround anymore if we aren't creating named sockets.
...And if I get rid of *that*, I can get rid of some other temp
directory management stuff too.
> >
> > 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 :|
>
© 2016 - 2026 Red Hat, Inc.