diff mbox series

[07/15] python/aqmp: add send_fd_scm

Message ID 20210917054047.2042843-8-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series Switch iotests to using Async QMP | expand

Commit Message

John Snow Sept. 17, 2021, 5:40 a.m. UTC
The single space is indeed required to successfully transmit the file
descriptor to QEMU.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/aqmp/qmp_client.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Hanna Czenczek Sept. 17, 2021, 1:34 p.m. UTC | #1
On 17.09.21 07:40, John Snow wrote:
> The single space is indeed required to successfully transmit the file
> descriptor to QEMU.

Yeah, socket_scm_helper.c said “Send a blank to notify qemu”.

> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   python/qemu/aqmp/qmp_client.py | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
> index d2ad7459f9..58f85990bc 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,18 @@ 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')
> +
> +        # Python 3.9+ adds socket.send_fds(...)
> +        sock.sendmsg(
> +            [b' '],
> +            [(socket.SOL_SOCKET, socket.SCM_RIGHTS, struct.pack('@i', fd))]
> +        )

AFAIU the socket can be either TCP or a UNIX socket 
(AsyncProtocol._do_accept’s docstring sounds this way), so should we 
check that this is a UNIX socket?  (Or is it fine to just run into the 
error that I suspect we would get with a TCP socket?)

Hanna
John Snow Sept. 17, 2021, 6:05 p.m. UTC | #2
On Fri, Sep 17, 2021 at 9:34 AM Hanna Reitz <hreitz@redhat.com> wrote:

> On 17.09.21 07:40, John Snow wrote:
> > The single space is indeed required to successfully transmit the file
> > descriptor to QEMU.
>
> Yeah, socket_scm_helper.c said “Send a blank to notify qemu”.
>
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/qemu/aqmp/qmp_client.py | 17 +++++++++++++++++
> >   1 file changed, 17 insertions(+)
> >
> > diff --git a/python/qemu/aqmp/qmp_client.py
> b/python/qemu/aqmp/qmp_client.py
> > index d2ad7459f9..58f85990bc 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,18 @@ 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')
> > +
> > +        # Python 3.9+ adds socket.send_fds(...)
> > +        sock.sendmsg(
> > +            [b' '],
> > +            [(socket.SOL_SOCKET, socket.SCM_RIGHTS, struct.pack('@i',
> fd))]
> > +        )
>
> AFAIU the socket can be either TCP or a UNIX socket
> (AsyncProtocol._do_accept’s docstring sounds this way), so should we
> check that this is a UNIX socket?  (Or is it fine to just run into the
> error that I suspect we would get with a TCP socket?)
>
> Hanna
>
>
Uhh, hm. I was going to say "Yeah, just let it fail!" but ... upon going to
document what error to expect in this case, I am realizing it fails
silently. So, uh, that's not ideal.

I'll fix this to make it bark.

--js
diff mbox series

Patch

diff --git a/python/qemu/aqmp/qmp_client.py b/python/qemu/aqmp/qmp_client.py
index d2ad7459f9..58f85990bc 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,18 @@  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')
+
+        # Python 3.9+ adds socket.send_fds(...)
+        sock.sendmsg(
+            [b' '],
+            [(socket.SOL_SOCKET, socket.SCM_RIGHTS, struct.pack('@i', fd))]
+        )