[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/4] python/console_socket: accept existing FD in initializer
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH 2/4] python/console_socket: accept existing FD in initializer |
Date: |
Thu, 20 Jul 2023 15:01:22 +0100 |
User-agent: |
Mutt/2.2.9 (2022-11-12) |
On Thu, Jul 20, 2023 at 09:04:46AM -0400, John Snow wrote:
> Useful if we want to use ConsoleSocket() for a socket created by
> socketpair().
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> python/qemu/machine/console_socket.py | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/python/qemu/machine/console_socket.py
> b/python/qemu/machine/console_socket.py
> index 4e28ba9bb2..42bfa12411 100644
> --- a/python/qemu/machine/console_socket.py
> +++ b/python/qemu/machine/console_socket.py
> @@ -17,7 +17,7 @@
> import socket
> import threading
> import time
> -from typing import Deque, Optional
> +from typing import Deque, Optional, Union
>
>
> class ConsoleSocket(socket.socket):
> @@ -30,13 +30,16 @@ class ConsoleSocket(socket.socket):
> Optionally a file path can be passed in and we will also
> dump the characters to this file for debugging purposes.
> """
> - def __init__(self, address: str, file: Optional[str] = None,
> + def __init__(self, address: Union[str, int], file: Optional[str] = None,
> drain: bool = False):
IMHO calling the pre-opened FD an "address" is pushing the
interpretation a bit.
It also makes the behaviour non-obvious from a caller. Seeing a
caller, you have to work backwards to find out what type it has,
to figure out the semantics of the method call.
IOW, I'd prefer to see
address: Optional[str], sock_fd: Optional[int]
and then just do a check
if (address is not None and sock_fd is not None) or
(address is None and sock_fd is None):
raise Exception("either 'address' or 'sock_fd' is required")
thus when you see
ConsoleSocket(sock_fd=xxx)
it is now obvious it has different behaviour to
ConsoleSocket(address=yyy)
> self._recv_timeout_sec = 300.0
> self._sleep_time = 0.5
> self._buffer: Deque[int] = deque()
> - socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM)
> - self.connect(address)
> + if isinstance(address, str):
> + socket.socket.__init__(self, socket.AF_UNIX, socket.SOCK_STREAM)
> + self.connect(address)
> + else:
> + socket.socket.__init__(self, fileno=address)
> self._logfile = None
> if file:
> # pylint: disable=consider-using-with
> --
> 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 :|
- [PATCH 0/4] python/machine: use socketpair() for console socket, John Snow, 2023/07/20
- [PATCH 2/4] python/console_socket: accept existing FD in initializer, John Snow, 2023/07/20
- Re: [PATCH 2/4] python/console_socket: accept existing FD in initializer,
Daniel P . Berrangé <=
- [PATCH 3/4] python/machine: use socketpair() for console connections, John Snow, 2023/07/20
- [PATCH 1/4] python/machine: move socket setup out of _base_args property, John Snow, 2023/07/20
- [PATCH 4/4] python/machine: remove unused console socket configuration arguments, John Snow, 2023/07/20
- Re: [PATCH 0/4] python/machine: use socketpair() for console socket, Peter Maydell, 2023/07/20