Re: [PATCH v2 04/17] python/aqmp: add send_fd_scm
On Thu, Oct 07, 2021 at 12:27:24PM -0400, John Snow wrote: > On Thu, Oct 7, 2021 at 10:52 AM Eric Blake wrote: > > > On Wed, Sep 22, 2021 at 08:49:25PM -0400, John Snow wrote: > > > The single space is indeed required to successfully transmit the file > > > descriptor to QEMU. > > > > Sending fds requires a payload of at least one byte, but I don't think > > that qemu cares which byte. Thus, while your choice of space is fine, > > the commit message may be a bit misleading at implying it must be > > space. > > > > > OK, I'll rephrase. (Space winds up being useful in particular because it > doesn't mess with the parsing for subsequent JSON objects sent over the > wire.) > > (Idle curiosity: Is it possible to make QEMU accept an empty payload here? > I understand that for compatibility reasons it wouldn't change much for the > python lib even if we did, but I'm curious.) No, my understanding is that for SCM_RIGHTS to work reliably, you HAVE to have a payload to avoid a return value of 0 from recvmsg() which would be ambiguous with the peer performing orderly shutdown. 'man 7 unix' confirms: At least one byte of real data should be sent when sending ancillary data. On Linux, this is required to successfully send ancillary data over a UNIX domain stream socket. When sending ancillary data over a UNIX domain datagram socket, it is not necessary on Linux to send any accompanying real data. However, portable applications should also in‐ clude at least one byte of real data when sending ancillary data over a datagram socket. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 04/17] python/aqmp: add send_fd_scm
On Thu, Oct 7, 2021 at 10:52 AM Eric Blake wrote: > On Wed, Sep 22, 2021 at 08:49:25PM -0400, John Snow wrote: > > The single space is indeed required to successfully transmit the file > > descriptor to QEMU. > > Sending fds requires a payload of at least one byte, but I don't think > that qemu cares which byte. Thus, while your choice of space is fine, > the commit message may be a bit misleading at implying it must be > space. > > OK, I'll rephrase. (Space winds up being useful in particular because it doesn't mess with the parsing for subsequent JSON objects sent over the wire.) (Idle curiosity: Is it possible to make QEMU accept an empty payload here? I understand that for compatibility reasons it wouldn't change much for the python lib even if we did, but I'm curious.) > > > > Python 3.11 removes support for calling sendmsg directly from a > > transport's socket. There is no other interface for doing this, our use > > case is, I suspect, "quite unique". > > > > As far as I can tell, this is safe to do -- send_fd_scm is a synchronous > > function and we can be guaranteed that the async coroutines will *not* be > > running when it is invoked. In testing, it works correctly. > > > > I investigated quite thoroughly the possibility of creating my own > > asyncio Transport (The class that ultimately manages the raw socket > > object) so that I could manage the socket myself, but this is so wildly > > invasive and unportable I scrapped the idea. It would involve a lot of > > copy-pasting of various python utilities and classes just to re-create > > the same infrastructure, and for extremely little benefit. Nah. > > > > Just boldly void the warranty instead, while I try to follow up on > > https://bugs.python.org/issue43232 > > Bummer that we have to do that, but at least you are documenting the > problems and pursuing a remedy upstream. > > Yeah. I suspect our use case is so niche that it's not likely to get traction, but I'll try again. This sort of thing might make it harder to use projects like pypy, so it does feel like a defeat. Still, where there's a will, there's a way, right? :) --js
Re: [PATCH v2 04/17] python/aqmp: add send_fd_scm
On Wed, Sep 22, 2021 at 08:49:25PM -0400, John Snow wrote:
> The single space is indeed required to successfully transmit the file
> descriptor to QEMU.
Sending fds requires a payload of at least one byte, but I don't think
that qemu cares which byte. Thus, while your choice of space is fine,
the commit message may be a bit misleading at implying it must be
space.
>
> Python 3.11 removes support for calling sendmsg directly from a
> transport's socket. There is no other interface for doing this, our use
> case is, I suspect, "quite unique".
>
> As far as I can tell, this is safe to do -- send_fd_scm is a synchronous
> function and we can be guaranteed that the async coroutines will *not* be
> running when it is invoked. In testing, it works correctly.
>
> I investigated quite thoroughly the possibility of creating my own
> asyncio Transport (The class that ultimately manages the raw socket
> object) so that I could manage the socket myself, but this is so wildly
> invasive and unportable I scrapped the idea. It would involve a lot of
> copy-pasting of various python utilities and classes just to re-create
> the same infrastructure, and for extremely little benefit. Nah.
>
> Just boldly void the warranty instead, while I try to follow up on
> https://bugs.python.org/issue43232
Bummer that we have to do that, but at least you are documenting the
problems and pursuing a remedy upstream.
>
> Signed-off-by: John Snow
> ---
> python/qemu/aqmp/qmp_client.py | 22 ++
> 1 file changed, 22 insertions(+)
>
> diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
> index d2ad7459f9f..f987da02eb0 100644
> --- a/python/qemu/aqmp/qmp_client.py
> +++ b/python/qemu/aqmp/qmp_client.py
> @@ -9,6 +9,8 @@
>
> import asyncio
> import logging
> +import socket
> +import struct
> from typing import (
> Dict,
> List,
> @@ -624,3 +626,23 @@ async def execute(self, cmd: str,
> """
> msg = self.make_execute_msg(cmd, arguments, oob=oob)
> return await self.execute_msg(msg)
> +
> +@upper_half
> +@require(Runstate.RUNNING)
> +def send_fd_scm(self, fd: int) -> None:
> +"""
> +Send a file descriptor to the remote via SCM_RIGHTS.
> +"""
> +assert self._writer is not None
> +sock = self._writer.transport.get_extra_info('socket')
> +
> +if sock.family != socket.AF_UNIX:
> +raise AQMPError("Sending file descriptors requires a UNIX
> socket.")
> +
> +# Void the warranty sticker.
> +# Access to sendmsg in asyncio is scheduled for removal in Python
> 3.11.
> +sock = sock._sock # pylint: disable=protected-access
> +sock.sendmsg(
> +[b' '],
> +[(socket.SOL_SOCKET, socket.SCM_RIGHTS, struct.pack('@i', fd))]
> +)
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[PATCH v2 04/17] python/aqmp: add send_fd_scm
The single space is indeed required to successfully transmit the file
descriptor to QEMU.
Python 3.11 removes support for calling sendmsg directly from a
transport's socket. There is no other interface for doing this, our use
case is, I suspect, "quite unique".
As far as I can tell, this is safe to do -- send_fd_scm is a synchronous
function and we can be guaranteed that the async coroutines will *not* be
running when it is invoked. In testing, it works correctly.
I investigated quite thoroughly the possibility of creating my own
asyncio Transport (The class that ultimately manages the raw socket
object) so that I could manage the socket myself, but this is so wildly
invasive and unportable I scrapped the idea. It would involve a lot of
copy-pasting of various python utilities and classes just to re-create
the same infrastructure, and for extremely little benefit. Nah.
Just boldly void the warranty instead, while I try to follow up on
https://bugs.python.org/issue43232
Signed-off-by: John Snow
---
python/qemu/aqmp/qmp_client.py | 22 ++
1 file changed, 22 insertions(+)
diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
index d2ad7459f9f..f987da02eb0 100644
--- a/python/qemu/aqmp/qmp_client.py
+++ b/python/qemu/aqmp/qmp_client.py
@@ -9,6 +9,8 @@
import asyncio
import logging
+import socket
+import struct
from typing import (
Dict,
List,
@@ -624,3 +626,23 @@ async def execute(self, cmd: str,
"""
msg = self.make_execute_msg(cmd, arguments, oob=oob)
return await self.execute_msg(msg)
+
+@upper_half
+@require(Runstate.RUNNING)
+def send_fd_scm(self, fd: int) -> None:
+"""
+Send a file descriptor to the remote via SCM_RIGHTS.
+"""
+assert self._writer is not None
+sock = self._writer.transport.get_extra_info('socket')
+
+if sock.family != socket.AF_UNIX:
+raise AQMPError("Sending file descriptors requires a UNIX socket.")
+
+# Void the warranty sticker.
+# Access to sendmsg in asyncio is scheduled for removal in Python 3.11.
+sock = sock._sock # pylint: disable=protected-access
+sock.sendmsg(
+[b' '],
+[(socket.SOL_SOCKET, socket.SCM_RIGHTS, struct.pack('@i', fd))]
+)
--
2.31.1
