Re: [PATCH 1/2] fstests: remove privatens test way from check

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

On Fri, Mar 21, 2025 at 03:28:11AM +0800, Zorro Lang wrote:
> This patch totally revert:
>   ce7f796ad check: remove the deprecation of sessionid
>   336784e3d check: disable HAVE_PRIVATENS by default
>   949bdf8ea check: deprecate using process sessions to isolate test instances
> and partially revert:
>   247ab01fa check: run tests in a private pid/mount namespace
> 
> So it does:
> 1. Remove "run_privatens" related things from xfstests/check, due to
>    check doesn't need that test way.
> 2. Keep run_privatens script and related changes for check-parallel.
> 
> Signed-off-by: Zorro Lang <zlang@kernel.org>

Seems to run fine here, so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  check     | 37 +++++++++++--------------------------
>  common/rc | 12 ++----------
>  2 files changed, 13 insertions(+), 36 deletions(-)
> 
> diff --git a/check b/check
> index 33eb3e085..ba853ae85 100755
> --- a/check
> +++ b/check
> @@ -674,14 +674,6 @@ _stash_test_status() {
>  	esac
>  }
>  
> -# Don't try "privatens" by default, it's experimental for now.
> -if [ "$TRY_PRIVATENS" = "yes" ];then
> -	# Can we run in a private pid/mount namespace?
> -	HAVE_PRIVATENS=
> -	./tools/run_privatens bash -c "exit 77"
> -	test $? -eq 77 && HAVE_PRIVATENS=yes
> -fi
> -
>  # Can we run systemd scopes?
>  HAVE_SYSTEMD_SCOPES=
>  systemctl reset-failed "fstests-check" &>/dev/null
> @@ -699,29 +691,22 @@ _adjust_oom_score -500
>  # the system runs out of memory it'll be the test that gets killed and not the
>  # test framework.  The test is run in a separate process without any of our
>  # functions, so we open-code adjusting the OOM score.
> +#
> +# If systemd is available, run the entire test script in a scope so that we can
> +# kill all subprocesses of the test if it fails to clean up after itself.  This
> +# is essential for ensuring that the post-test unmount succeeds.  Note that
> +# systemd doesn't automatically remove transient scopes that fail to terminate
> +# when systemd tells them to terminate (e.g. programs stuck in D state when
> +# systemd sends SIGKILL), so we use reset-failed to tear down the scope.
> +#
> +# Use setsid to run the test program with a separate session id so that we
> +# can pkill only the processes started by this test.
>  _run_seq() {
>  	local res
>  	unset CHILDPID
>  	unset FSTESTS_ISOL	# set by tools/run_seq_*
>  
> -	if [ -n "${HAVE_PRIVATENS}" ]; then
> -		# If pid and mount namespaces are available, run the whole test
> -		# inside them so that the test cannot access any process or
> -		# /tmp contents that it does not itself create.  The ./$seq
> -		# process is considered the "init" process of the pid
> -		# namespace, so all subprocesses will be sent SIGKILL when it
> -		# terminates.
> -		./tools/run_privatens "./$seq"
> -		res=$?
> -	elif [ -n "${HAVE_SYSTEMD_SCOPES}" ]; then
> -		# If systemd is available, run the entire test script in a
> -		# scope so that we can kill all subprocesses of the test if it
> -		# fails to clean up after itself.  This is essential for
> -		# ensuring that the post-test unmount succeeds.  Note that
> -		# systemd doesn't automatically remove transient scopes that
> -		# fail to terminate when systemd tells them to terminate (e.g.
> -		# programs stuck in D state when systemd sends SIGKILL), so we
> -		# use reset-failed to tear down the scope.
> +	if [ -n "${HAVE_SYSTEMD_SCOPES}" ]; then
>  		local unit="$(systemd-escape "fs$seq").scope"
>  		systemctl reset-failed "${unit}" &> /dev/null
>  		systemd-run --quiet --unit "${unit}" --scope \
> diff --git a/common/rc b/common/rc
> index e51686389..55c384a63 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -33,11 +33,7 @@ _test_sync()
>  # Kill only the processes started by this test.
>  _pkill()
>  {
> -	if [ "$FSTESTS_ISOL" = "setsid" ]; then
> -		pkill --session 0 "$@"
> -	else
> -		pkill "$@"
> -	fi
> +	pkill --session 0 "$@"
>  }
>  
>  # Find only the test processes started by this test
> @@ -2796,11 +2792,7 @@ _require_user_exists()
>  # not, passing $SHELL in this manner works both for "su" and "su -c cmd".
>  _su()
>  {
> -	if [ "$FSTESTS_ISOL" = "setsid" ]; then
> -		su --session-command $SHELL "$@"
> -	else
> -		su "$@"
> -	fi
> +	su --session-command $SHELL "$@"
>  }
>  
>  # check if a user exists and is able to execute commands.
> -- 
> 2.47.1
> 
>