Marc-André Lureau <marcandre.lur...@gmail.com> writes:
> Hi Markus
>
> On Fri, Feb 17, 2023 at 1:49 PM Markus Armbruster <arm...@redhat.com> wrote:
>>
>> marcandre.lur...@redhat.com writes:
>>
>> > From: Marc-André Lureau <marcandre.lur...@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.lur...@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"
> (https://patchew.org/QEMU/20230212204942.1905959-1-marcandre.lur...@redhat.com/)
> merged.
Would it make sense to rebase this series on top of that one, so we
can have simpler documentation from the start?
>> What does this command do under Windows before this patch? Fail always?
>
> Without ancillary data support on Windows, you can't make it work.
Yes, but how does it fail? Hmm, you actually answer that below.
>> 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.
I worry about potential consequences of baking Windows ABI into QMP.
What if the memory representation of this struct changes?
Such ABI changes are unpleasant, but they are not impossible.
>> > # 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.
So it fails exactly like it fails on a POSIX host when you execute getfd
without passing along a file descriptor with SCM_RIGHTS. Correct?
