In my opinion, the Linux-specific abstract UNIX domain socket feature
introduced in 5.1 should have been rejected. The feature is niche,
the interface clumsy, the implementation buggy and incomplete, and the
test coverage insufficient. Review fail.
I also failed (as chardev maintainer..) to not only review but weigh in and discuss the merits or motivations behind it.
I agree with you. Also the commit lacks motivation behind this "feature".
Fixing the parts we can still fix now is regrettably expensive. If I
had the power to decide, I'd unceremoniously revert the feature,
compatibility to 5.1 be damned. But I don't, so here we go.
I'm not sure this set of fixes is complete. However, I already spent
too much time on this, so out it goes. Lightly tested.
Regardless, I *will* make time for ripping the feature out if we
decide to do that. Quick & easy way to avoid reviewing this series
well, fwiw, I would also take that approach too, to the risk of upsetting the users. But maybe we can get a clear reason behind it before that happens. (sorry, I didn't dig the ML archive is such evidence is there, it should have been in the commit message too)
For additional information, see
Subject: Our abstract UNIX domain socket support is a mess
Date: Wed, 28 Oct 2020 13:41:06 +0100
Message-ID: <firstname.lastname@example.org" target="_blank">email@example.com>
Markus Armbruster (11):
test-util-sockets: Plug file descriptor leak
test-util-sockets: Correct to set has_abstract, has_tight
test-util-sockets: Clean up SocketAddress construction
test-util-sockets: Factor out test_socket_unix_abstract_one()
test-util-sockets: Synchronize properly, don't sleep(1)
test-util-sockets: Test the complete abstract socket matrix
sockets: Fix default of UnixSocketAddress member @tight
sockets: Fix socket_sockaddr_to_address_unix() for abstract sockets
char-socket: Fix qemu_chr_socket_address() for abstract sockets
sockets: Bypass "replace empty @path" for abstract unix sockets
sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUX
qapi/sockets.json | 14 ++--
chardev/char-socket.c | 22 +++++-
chardev/char.c | 2 +
tests/test-util-sockets.c | 155 ++++++++++++++++++++------------------
util/qemu-sockets.c | 54 ++++++++++---
5 files changed, 156 insertions(+), 91 deletions(-)