Re: [PATCH v3 3/3] python/qemu/machine: use socketpair() for QMP by default

[Vladimir Sementsov-Ogievskiy]

Hi!

By my investigation, this commit (bd4c0ef409140bd1be393407c04005ac077d4574) 
breaks long qmp output again.

Simple test:

$ cd python
$ cat test.py
#!/usr/bin/env python3

import sys
from qemu.machine import QEMUMachine

monitor_address = sys.argv[2] if len(sys.argv) > 2 else None
vm = QEMUMachine('../build/qemu-system-x86_64',
                 monitor_address=monitor_address)
vm.launch()

for x in range(int(sys.argv[1])):
    vm.qmp('blockdev-add', {'driver': 'null-co', 'node-name': f'x{x}'})

vm.qmp('query-named-block-nodes')
vm.shutdown()


./test.py 1000 /tmp/sock

  - works, but if use default behavior (socketpair) we get:

$ ./test.py 1000
Traceback (most recent call last):
  File "/home/vsementsov/work/src/qemu/master/python/./test.py", line 14, in 
<module>
    vm.qmp('query-named-block-nodes')
  File "/home/vsementsov/work/src/qemu/master/python/qemu/machine/machine.py", 
line 686, in qmp
    ret = self._qmp.cmd(cmd, args=qmp_args)
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 
216, in cmd
    return self.cmd_obj(qmp_cmd)
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 
190, in cmd_obj
    self._sync(
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 
105, in _sync
    return self._aloop.run_until_complete(
  File "/usr/lib/python3.10/asyncio/base_events.py", line 646, in 
run_until_complete
    return future.result()
  File "/usr/lib/python3.10/asyncio/tasks.py", line 408, in wait_for
    return await fut
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", 
line 547, in _raw
    return await self._execute(msg, assign_id=assign_id)
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", 
line 496, in _execute
    return await self._reply(exec_id)
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", 
line 463, in _reply
    raise result
qemu.qmp.qmp_client.ExecInterruptedError: Disconnected
Exception ignored in: <function QEMUMonitorProtocol.__del__ at 0x7f8708283eb0>
Traceback (most recent call last):
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 
318, in __del__
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 
289, in close
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/legacy.py", line 
105, in _sync
  File "/usr/lib/python3.10/asyncio/base_events.py", line 646, in 
run_until_complete
  File "/usr/lib/python3.10/asyncio/tasks.py", line 408, in wait_for
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", 
line 413, in disconnect
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", 
line 725, in _wait_disconnect
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", 
line 876, in _bh_loop_forever
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", 
line 914, in _bh_recv_message
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", 
line 1015, in _recv
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/qmp_client.py", 
line 402, in _do_recv
  File "/home/vsementsov/work/src/qemu/master/python/qemu/qmp/protocol.py", 
line 979, in _readline
  File "/usr/lib/python3.10/asyncio/streams.py", line 534, in readline
ValueError: Separator is not found, and chunk exceed the limit



./test.py 100
  - works well.


On 11.01.23 11:01, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> When no monitor address is given, establish the QMP communication through
> a socketpair() (API is also supported on Windows since Python 3.5)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   python/qemu/machine/machine.py | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 748a0d807c..5b2e499e68 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -158,17 +158,13 @@ def __init__(self,
>           self._qmp_timer = qmp_timer
>   
>           self._name = name or f"qemu-{os.getpid()}-{id(self):02x}"
> +        self._sock_pair: Optional[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
>   
> -        if monitor_address is not None:
> -            self._monitor_address = monitor_address
> -        else:
> -            self._monitor_address = os.path.join(
> -                self.sock_dir, f"{self._name}-monitor.sock"
> -            )
> +        self._monitor_address = monitor_address
>   
>           self._console_log_path = console_log
>           if self._console_log_path:
> @@ -303,7 +299,11 @@ def _base_args(self) -> List[str]:
>           args = ['-display', 'none', '-vga', 'none']
>   
>           if self._qmp_set:
> -            if isinstance(self._monitor_address, tuple):
> +            if self._sock_pair:
> +                fd = self._sock_pair[0].fileno()
> +                os.set_inheritable(fd, True)
> +                moncdev = f"socket,id=mon,fd={fd}"
> +            elif isinstance(self._monitor_address, tuple):
>                   moncdev = "socket,id=mon,host={},port={}".format(
>                       *self._monitor_address
>                   )
> @@ -337,10 +337,17 @@ def _pre_launch(self) -> None:
>               self._remove_files.append(self._console_address)
>   
>           if self._qmp_set:
> +            monitor_address = None
> +            sock = None
> +            if self._monitor_address is None:
> +                self._sock_pair = socket.socketpair()
> +                sock = self._sock_pair[1]
>               if isinstance(self._monitor_address, str):
>                   self._remove_files.append(self._monitor_address)
> +                monitor_address = self._monitor_address
>               self._qmp_connection = QEMUMonitorProtocol(
> -                self._monitor_address,
> +                address=monitor_address,
> +                sock=sock,
>                   server=True,
>                   nickname=self._name
>               )
> @@ -360,6 +367,7 @@ def _pre_launch(self) -> None:
>           ))
>   
>       def _post_launch(self) -> None:
> +        self._sock_pair[0].close()
>           if self._qmp_connection:
>               self._qmp.accept(self._qmp_timer)
>   

--
Best regards,
Vladimir