On Thu, Oct 01, 2020 at 11:23:53PM +0300, Arseny Maslennikov wrote:
> > > Could you please explain what you're trying to do with this patch?
> > > Even if it's obvious from the source itself, we still must have an
> > > opportunity to discuss, and a decent explanation should stay in the
> > > project history.
> > 
> > I think this patch is simple enough.
> 
> There's a misunderstanding here. I'm not asking to explain the
> semantics (what this patch does) — I repeat, it's rather obvious from
> the source itself, the patch is indeed simple. I'm trying to get how the
> patch's author would describe the pragmatic value of this patch. IOW:
> we see this patch does XXX. What, in Alexey's view, are we trying to
> achieve by implementing XXX?

I remember that this patch was the result of a discussion with ldv. I
didn't want to add complex support for different versions of cgroups. The
idea was that the admin would prepare the system for use of cgroups by the
hasher-privd daemon.

I'm not considering the hasher-privd as an end user server. This is a
low-level server on which you can build different solutions. I don't mean
just hasher. With this in mind, I don't think that this server should do
everything out of the box without configuration.

Does this make sense to you?

> Descriptive commit messages are done (and are enforced in successful
> communities, e. g. LKML) for a reason.
> 
> The above essentially is my previous comment here, reworded and clarified.
> 
> If for some reason you believe it's shameful or rude to the community to
> "waste time" on textual explanations, fair enough — I'll maybe write a commit
> message myself (with my take on why this might be useful) and then most
> likely ACK the same patch, with authorship reattributed to you via From:
> in the patch body and the new commit message. Or else NAK this
> particular revision with an empty commit message and leave it up to
> ldv@.
> If it were up to me, I would not approve of empty commit messages in a
> lasting, crucial project like hasher-privd. People are forgetful, and
> commit messages exist to help.

Ok.

> > > Do we only support cgroup2 and ignore cgroup1? If yes, great, but
> > > perhaps then we might want to have a setting to not fiddle with cgroup
> > > trees, to support the unfortunate users that have to run Docker and
> > > other garbage.
> > 
> > Yeah, I didn't plan on supporting legacy version of cgroups. Docker
> > already can work with cgroupsv2.
> 
> Oh, I heard they were just recently working on cgroup2 support.

https://github.com/opencontainers/runc/blob/master/docs/cgroup-v2.md

-- 
Rgrds, legion