[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 08/10] qmp: teach 'getfd' to import sockets on win32
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH v3 08/10] qmp: teach 'getfd' to import sockets on win32 |
Date: |
Sat, 18 Feb 2023 14:15:11 +0400 |
Hi Markus
On Fri, Feb 17, 2023 at 1:49 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > A process with enough capabilities can duplicate a socket to QEMU.
> > Modify 'getfd' to import it and add it to the monitor fd list, so it can
> > be later used by other commands.
> >
> > Note that we actually store the SOCKET in the FD list, appropriate care
> > must now be taken to use the correct socket functions (similar approach
> > is taken by our io/ code and in glib, this is internal and shouldn't
> > affect the QEMU/QMP users)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > qapi/misc.json | 16 ++++++++--
> > monitor/fds.c | 79 ++++++++++++++++++++++++++++++++++++----------
> > monitor/hmp-cmds.c | 6 +++-
> > 3 files changed, 81 insertions(+), 20 deletions(-)
> >
> > diff --git a/qapi/misc.json b/qapi/misc.json
> > index 27ef5a2b20..cd36d8befb 100644
> > --- a/qapi/misc.json
> > +++ b/qapi/misc.json
> > @@ -249,10 +249,18 @@
> > ##
> > # @getfd:
> > #
> > -# Receive a file descriptor via SCM rights and assign it a name
> > +# On UNIX, receive a file descriptor via SCM rights and assign it a name.
> > +#
> > +# On Windows, (where ancillary socket fd-passing isn't an option yet), add
> > a
> > +# socket that was duplicated to QEMU process with WSADuplicateSocketW() via
> > +# WSASocket() & WSAPROTOCOL_INFOW structure and assign it a name. A SOCKET
> > is
> > +# considered as a kind of "file descriptor" in QMP context, for historical
> > +# reasons and simplicity. QEMU takes care to use socket functions
> > appropriately.
>
> The Windows part explains things in terms of the C socket API. Less
> than ideal for the QEMU QMP Reference Manual, isn't it? I don't know
> nearly enough about this stuff to suggest concrete improvements...
We don't have to, after all we don't explain how to use sendmsg/cmsg
stuff to pass FDs.
I will drop the part about "A SOCKET is considered as a kind of "file
descriptor" in QMP context", after we get "[PATCH 0/4] win32: do not
mix SOCKET and fd space"
(20230212204942.1905959-1-marcandre.lureau@redhat.com/">https://patchew.org/QEMU/20230212204942.1905959-1-marcandre.lureau@redhat.com/)
merged.
>
> What does this command do under Windows before this patch? Fail always?
Without ancillary data support on Windows, you can't make it work.
>
> Wrap your lines a bit earlier, please.
>
> > #
> > # @fdname: file descriptor name
> > #
> > +# @wsa-info: a WSAPROTOCOL_INFOW structure (encoded in base64). Since 8.0.
> > +#
>
> No way around passing a binary blob?
WSAPROTOCOL_INFOW is a fairly big structure, with private/reserved fields,
it contains another structure (WSAPROTOCOLCHAIN), has fixed-length arrays,
GUID, and utf16 string.
QAPI'fying that structure back and forth would be tedious and
error-prone. Better to treat it as an opaque blob imho.
>
> > # Returns: Nothing on success
> > #
> > # Since: 0.14
> > @@ -270,7 +278,11 @@
> > # <- { "return": {} }
> > #
> > ##
> > -{ 'command': 'getfd', 'data': {'fdname': 'str'} }
> > +{ 'command': 'getfd', 'data': {
> > + 'fdname': 'str',
> > + '*wsa-info': {'type': 'str', 'if': 'CONFIG_WIN32'}
> > + }
> > +}
>
> What happens when QEMU runs on a Windows host and the client doesn't
> pass @wsa-info?
It attempts to get the fd from the last recv, but it will fail on
Windows, this is not available.
--
Marc-André Lureau
- Re: [PATCH v3 07/10] qapi: implement conditional command arguments, (continued)
[PATCH v3 08/10] qmp: teach 'getfd' to import sockets on win32, marcandre . lureau, 2023/02/07
[PATCH v3 10/10] qtest: enable vnc-display test on win32, marcandre . lureau, 2023/02/07
[PATCH v3 09/10] libqtest: make qtest_qmp_add_client work on win32, marcandre . lureau, 2023/02/07