[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 07/11] sockets: Fix default of UnixSocketAddress member @tigh
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH 07/11] sockets: Fix default of UnixSocketAddress member @tight |
Date: |
Thu, 29 Oct 2020 19:05:04 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 |
On 29/10/20 18:39, Paolo Bonzini wrote:
>> When @tight was set to false as it should be, absent @tight defaults
>> to false. Wrong, it should default to true. This is what breaks QMP.
> When @has_tight...
Ah, I see what you meant here. Suggested reword:
---------
An optional bool member of a QAPI struct can be false, true, or absent.
The previous commit demonstrated that socket_listen() and
socket_connect() are broken for absent @tight, and indeed QMP chardev-
add also defaults absent member @tight to false instead of true.
In C, QAPI members are represented by two fields, has_MEMBER and MEMBER.
We have:
has_MEMBER MEMBER
false true false
true true false
absent false false/ignore
When has_MEMBER is false, MEMBER should be set to false on write, and
ignored on read.
For QMP, the QAPI visitors handle absent @tight by setting both
@has_tight and @tight to false. unix_listen_saddr() and
unix_connect_saddr() however use @tight only, disregarding @has_tight.
This is wrong and means that absent @tight defaults to false whereas it
should default to true.
The same is true for @has_abstract, though @abstract defaults to
false and therefore has the same behavior for all of QMP, HMP and CLI.
Fix unix_listen_saddr() and unix_connect_saddr() to check
@has_abstract/@has_tight, and to default absent @tight to true.
However, this is only half of the story. HMP chardev-add and CLI
-chardev so far correctly defaulted @tight to true, but defaults to
false again with the above fix for HMP and CLI. In fact, the "tight"
and "abstract" options now break completely.
Digging deeper, we find that qemu_chr_parse_socket() also ignores
@has_tight, leaving it false when it sets @tight. That is also wrong,
but the two wrongs cancelled out. Fix qemu_chr_parse_socket() to set
@has_tight and @has_abstract; writing testcases for HMP and CLI is left
for another day.
---------
Apologies if the last sentence is incorrect. :)
Thanks,
Paolo
- Re: [PATCH 08/11] sockets: Fix socket_sockaddr_to_address_unix() for abstract sockets, (continued)
- [PATCH 02/11] test-util-sockets: Correct to set has_abstract, has_tight, Markus Armbruster, 2020/10/29
- [PATCH 01/11] test-util-sockets: Plug file descriptor leak, Markus Armbruster, 2020/10/29
- [PATCH 05/11] test-util-sockets: Synchronize properly, don't sleep(1), Markus Armbruster, 2020/10/29
- [PATCH 07/11] sockets: Fix default of UnixSocketAddress member @tight, Markus Armbruster, 2020/10/29
- Re: [PATCH 07/11] sockets: Fix default of UnixSocketAddress member @tight, Eric Blake, 2020/10/29
- [PATCH 06/11] test-util-sockets: Test the complete abstract socket matrix, Markus Armbruster, 2020/10/29
- [PATCH 10/11] sockets: Bypass "replace empty @path" for abstract unix sockets, Markus Armbruster, 2020/10/29
- [PATCH 11/11] sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUX, Markus Armbruster, 2020/10/29