Re: [PATCH v3.1 15/34] check: run tests in a private pid/mount namespace

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Feb 25, 2025 at 11:27:19AM +0000, Shinichiro Kawasaki wrote:
> On Feb 14, 2025 / 13:13, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > As mentioned in the previous patch, trying to isolate processes from
> > separate test instances through the use of distinct Unix process
> > sessions is annoying due to the many complications with signal handling.
> > 
> > Instead, we could just use nsexec to run the test program with a private
> > pid namespace so that each test instance can only see its own processes;
> > and private mount namespace so that tests writing to /tmp cannot clobber
> > other tests or the stuff running on the main system.  Further, the
> > process created by the clone(CLONE_NEWPID) call is considered the init
> > process of that pid namespace, so all processes will be SIGKILL'd when
> > the init process terminates, so we no longer need systemd scopes for
> > externally enforced cleanup.
> > 
> > However, it's not guaranteed that a particular kernel has pid and mount
> > namespaces enabled.  Mount (2.4.19) and pid (2.6.24) namespaces have
> > been around for a long time, but there's no hard requirement for the
> > latter to be enabled in the kernel.  Therefore, this bugfix slips
> > namespace support in alongside the session id thing.
> > 
> > Declaring CONFIG_PID_NS=n a deprecated configuration and removing
> > support should be a separate conversation, not something that I have to
> > do in a bug fix to get mainline QA back up.
> > 
> > Note that the new helper cannot unmount the /proc it inherits before
> > mounting a pidns-specific /proc because generic/504 relies on being able
> > to read the init_pid_ns (aka systemwide) version of /proc/locks to find
> > a file lock that was taken and leaked by a process.
> 
> Hello Darrick,
> 
> I ran fstests for zoned btrfs using the latest fstests tag v2025.02.23, and
> observed all test cases failed with my set up. I bisected and found that this
> commit is the trigger. Let me share my observations.
> 
> For example, btrfs/001.out.bad contents are as follows:
> 
>   QA output created by 001
>   mount: bad usage
>   Try 'mount --help' for more information.
>   common/rc: retrying test device mount with external set
>   mount: bad usage
>   Try 'mount --help' for more information.
>   common/rc: could not mount /dev/sda on common/config: TEST_DIR (/tmp/test) is not a directory
> 
> As the last line above shows, fstests failed to find out TEST_DIR, /tmp/test.
> 
> My set up uses mount point directories in tmpfs, /tmp/*:
> 
>   export TEST_DIR=/tmp/test
>   export SCRATCH_MNT=/tmp/scratch
> 
> I guessed that tmpfs might be a cause. As a trial, I modified these to,
> 
>   export TEST_DIR=/var/test
>   export SCRATCH_MNT=/var/scratch
> 
> then I observed the failures disappeared. I guess this implies that the commit
> for the private pid/mount namespace makes tmpfs unique to each namespace. Then,
> the the mount points in tmpfs were not found in the private namespaces context,
> probably.

Yes, /tmp is now private to the test program (e.g. tests/btrfs/001) so
that tests run in parallel cannot interfere with each other.

> If this guess is correct, in indicates that tmpfs can no longer be used for
> fstests mount points. Is this expected?

Expected, yes.  But still broken for you. :(

I can think of a few solutions:

1. Perhaps run_privatens could detect that TEST_DIR/SCRATCH_MNT start
with "/tmp" and bind mount them into the private /tmp before it starts
the test.

2. fstests could take care of defining and mkdir'ing the
TEST_DIR/SCRATCH_MNT directories and users no longer have to create
them.  It might, however, be useful to have them accessible to someone
who has ssh'd in to look around after a failure.

3. Everyone rewrites their fstests configs to choose something outside
of /tmp (e.g. /var/tmp/{test,scratch})?

Any thoughts?  #3 is the least thinking for me, but #1 is the least
thinking for everyone /else/ :)

--D