On Tue, May 20, 2025 at 01:10:24PM +0200, Christian Brauner wrote:
> > +It is convenient to define several properties of sets of mounts:
> > +
> > +1) A set S of mounts is non-shifting if for any mount X belonging
> > +to S all subtrees mounted strictly inside of X (i.e. not overmounting
> > +the root of X) contain only elements of S.
>
> I think "shifting" is misleading. I would suggest either "isolated" or
> "contained" or ideally "closed" which would mean...
Umm... I'm not sure. "Shifting" in a sense that pulling that set out
and reparenting everything that remains to the nearest surviving ancestor
won't change the pathnames. "Contained" or "isolated"... what would
that be about?
> > +of that set, but only on top of stacks of root-overmounting elements
> > +of set. They can be reparented to the place where the bottom of
> > +stack is attached to a mount that will survive. NOTE: doing that
> > +will violate a constraint on having no more than one mount with
> > +the same parent/mountpoint pair; however, the caller (umount_tree())
>
> I would prefer if this would insert the term "shadow mounts" since
> that's what we've traditionally used for that.
There's a bit of ambiguity - if we have done
mount -t tmpfs none /tmp/foo
touch /tmp/foo/A
mount -t tmpfs none /tmp/foo
touch /tmp/foo/B
we have two mounts, one overmounting the root of another. Does "shadow"
apply to the lower (with A on it) or the upper (with B on it)?
> > +{
> > + while (1) {
> > + struct mount *master = m->mnt_master;
> > +
> > + if (master == origin->mnt_master) {
> > + struct mount *next = next_peer(m);
> > + return (next == origin) ? NULL : next;
> > + } else if (m->mnt_slave.next != &master->mnt_slave_list)
> > + return next_slave(m);
>
> Please add a comment to that helper that explains how it walks the
> propagation tree. I remember having to fix bugs in that code and the
> lack of comments was noticable.
Ugh... Let's separate that - it's not specific to propagate_umount()
and the helper is the "we hadn't gone into ->mnt_slave_list" half of
propagation_next(), verbatim.
I agree that comments there would be a good thing, but it (and next_group())
belong to different layer - how do we walk the propagation graph.
FWIW, the current variant of that thing (which seems to survive the tests
so far) already has a plenty in it; let's try to keep at least some parts
in separate commits...
