From: Al Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <brauner@kernel.org>
Cc: linux-fsdevel@vger.kernel.org, Miklos Szeredi <miklos@szeredi.hu>,
Jan Kara <jack@suse.cz>, Allison Karlitskaya <lis@redhat.com>
Subject: Re: [PATCH 1/2] mount: fix detached mount regression
Date: Sat, 7 Jun 2025 06:20:48 +0100 [thread overview]
Message-ID: <20250607052048.GZ299672@ZenIV> (raw)
In-Reply-To: <20250606174502.GY299672@ZenIV>
On Fri, Jun 06, 2025 at 06:45:02PM +0100, Al Viro wrote:
> On Fri, Jun 06, 2025 at 09:58:26AM +0200, Christian Brauner wrote:
>
> > Fwiw, check_mnt() is a useless name for this function that's been
> > bothering me forever.
>
> Point, but let's keep the renaming (s/check_mnt/our_mount/, for
> example) separate.
Modified and force-pushed.
It does pass xfstests without regressions. kselftests... AFAICS,
no regressions either, but the damn thing is a mess. Example:
# # set_layers_via_fds.c:711:set_layers_via_detached_mount_fds:Expected layers_found[i] (0) == true (1)
# # set_layers_via_fds.c:39:set_layers_via_detached_mount_fds:Expected rmdir("/set_layers_via_fds") (-1) == 0 (0)
# # set_layers_via_detached_mount_fds: Test terminated by assertion
# # FAIL set_layers_via_fds.set_layers_via_detached_mount_fds
Not a regression, AFAICT; the underlying problem is that mount options
are shown incorrectly in the tested case. Still present after overlayfs
merge. mount does succeed, but... in options we see this:
rw,relatime,lowerdir+=/,lowerdir+=/,lowerdir+=/,lowerdir+=/,datadir+=/,datadir+=/,datadir+=/,upperdir=/upper,workdir=/work,redirect_dir=on,uuid=on,metacopy=on
And it's a perfectly expected result - you are giving fsconfig(2) empty
path on a detached tree, created with OPEN_TREE_CLONE. I.e. it *is*
an empty path in the mount tree the sucker's in. What could d_path()
produce other than "/"?
Note, BTW that it really does create set_layers_via_fds in root (WTF?) and
running that sucker again yields a predictable fun result - mkdir() failing
with EEXIST...
IMO that kind of stuff should be dealt with by creating a temporary directory
somewhere in /tmp, mounting tmpfs on it, then doing all creations, etc.
inside that. Then umount -l /tmp/<whatever>; rmdir /tmp/<whatever> will
clean the things up.
next prev parent reply other threads:[~2025-06-07 5:20 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-05 12:50 [PATCH 0/2] mount: fix detached mount regression Christian Brauner
2025-06-05 12:50 ` [PATCH 1/2] " Christian Brauner
2025-06-06 4:54 ` Al Viro
2025-06-06 5:14 ` Al Viro
2025-06-06 7:01 ` Al Viro
2025-06-06 7:58 ` Christian Brauner
2025-06-06 17:45 ` Al Viro
2025-06-07 5:20 ` Al Viro [this message]
2025-06-11 9:36 ` Christian Brauner
2025-06-11 17:29 ` Al Viro
2025-06-12 12:16 ` Christian Brauner
2025-06-05 12:50 ` [PATCH 2/2] selftests/mount_setattr: adapt detached mount propagation test Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250607052048.GZ299672@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=lis@redhat.com \
--cc=miklos@szeredi.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox