Re: [PATCH 00/11] sockets: Attempt to drain the abstract socket swamp

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi Markus,
>
> On Thu, Oct 29, 2020 at 5:43 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> 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
>> *hint* *hint*.
>>
>
> well, fwiw, I would also take that approach too, to the risk of upsetting
> the users.

Reverting the feature requires rough consensus and a patch.

I can provide a patch, but let's give everybody a chance to object
first.

>            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)

I just did, and found next to nothing.

This is the final cover letter:

    qemu-sockets: add abstract UNIX domain socket support

    qemu does not support abstract UNIX domain
    socket address. Add this ability to make qemu handy
    when abstract address is needed.

Boils down to "$feature is needed because it's handy when it's needed",
which is less than helpful.

Patch history:

v1: https://lists.nongnu.org/archive/html/qemu-devel/2020-04/msg03799.html

    This version repurposes @path starting with '@' for abstract
    sockets, always tight.  Only connect, no tests, no documentation.

    R-by Marc-André.

v2: https://lists.nongnu.org/archive/html/qemu-devel/2020-04/msg03803.html

    Minor cleanup.

    Daniel asks why it's needed, points out listen is missing, and
    suggests the two boolean flags abstract, tight.

v3: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg02291.html

    Implement interface proposed by Daniel, default of @tight broken,
    tests (which don't catch the broken default), documentation.

    Eric suggests QAPI schema doc improvements (but doesn't challenge
    the interface).

    R-by Daniel for the code.  He asks for randomized @path in tests.

v4: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg04036.html

    Daniel points out style nits in tests.

    Eric suggests a few more QAPI schema doc improvements.

v5: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg04144.html

    R-by Daniel for the tests.

v6: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg04508.html

    No further review comments.

PR: https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg05747.html

    Pull request catches my eye.  The interface looks odd, and I
    challenge @tight.  I silently accept Daniel's defense of it without
    digging deeper.

This is a review failure.  I do not blame the patch submitter.