Daniel P. Berrangé <berra...@redhat.com> writes: > On Fri, Oct 30, 2020 at 11:11:19AM +0100, Markus Armbruster wrote: >> Marc-André Lureau <marcandre.lur...@gmail.com> writes: >> >> > Hi Markus, >> > >> > On Thu, Oct 29, 2020 at 5:43 PM Markus Armbruster <arm...@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.
Daniel, do you object, yes or no? I need to know to avoid wasting even more time. >> > 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. > > Well if you have an existing application that uses abstract sockets, > and you want to connect QEMU to it, then QEMU needs to support it, > or you are forced to interject a proxy between your app and QEMU > which is an extra point of failure and lantency. I was interested > in whether there was a specific application they were using, but > that is largely just from a curiosity POV. There's enough usage of > abstract sockets in apps in general that is it clearly a desirable > feature to enable. > > Even if no external app is involved and you're just connecting > together 2 QEMU processes' chardevs, abstract sockets are interesting > because of their differing behaviour to non-abstract sockets. > > Most notably non-abstract sockets are tied to the filesystem mount > namespace in Linux, where as abstract sockets are tied to the network > namespace. This is a useful distinction in the container world. Ordinarily > you can't connect QEMUs in 2 separate containers together, because they > always have distinct mount namespaces. There is often the ability to > share network namespaces between containers though, and thus unlock > use of abstract sockets for communications. If this was patch review, I'd now ask the patch submitter to work it into the commit message. Thanks anyway :) [...]