From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Yang Date: Thu, 25 Feb 2021 18:57:39 +0800 Subject: [LTP] [PATCH] unshare01.sh: Setup parent mount flag before unshare testing In-Reply-To: References: Message-ID: <603782A3.7030902@cn.fujitsu.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ltp@lists.linux.it On 2021/2/25 18:02, zhaogongyi wrote: > Hi Yang, > >> I don't like the approach which enforces mountpoint to be shared in >> parent mount namespace. >> I think we can tune expected value by checking propagation flag in parent >> mount namespace because of two reasons: >> 1) Make test cover more cases. >> 2) Don't depend on the fixed tmpfs. > > If we have no fixed parent mount namespace, the test looks like will pass at any cases since we judge result by "grep -w 'dir_B' /proc/self/mountinfo | grep -qw 'shared'". Hi Zhongyi, This issue is caused by the state(e.g. share or private) of parent mount namespace and both of results are expected. > It seems a bit tangential to our test objective as " # 6) If we run with "--mount --propagation shared", mount and unmount events propagate to its parent mount namespace. " We also need to update the description of #6 because it is true only when the parent mount namespace is shared. For example: # 6) If we run with "--mount --propagation shared" and parent mount namespace is shared, mount and unmount events can do propagation. " # 7) If we run with "--mount --propagation shared" and parent mount namespace is not shared, mount and unmount events cannot do propagation. " Best Regards, Xiao Yang > > Thanks! > > ========================================================== > >> Hi Zhongyi, Petr >> >> I don't like the approach which enforces mountpoint to be shared in >> parent mount namespace. >> I think we can tune expected value by checking propagation flag in parent >> mount namespace because of two reasons: >> 1) Make test cover more cases. >> 2) Don't depend on the fixed tmpfs. >> >> Zhongyi, could you test the following patch on your enviorment? >> ------------------------------------------------------------------------------------------------- >> diff --git a/testcases/commands/unshare/unshare01.sh >> b/testcases/commands/unshare/unshare01.sh >> index bf163a7f4..78ea83fc0 100755 >> --- a/testcases/commands/unshare/unshare01.sh >> +++ b/testcases/commands/unshare/unshare01.sh >> @@ -40,6 +40,17 @@ >> max_mntns_path="/proc/sys/user/max_mnt_namespaces" >> default_max_userns=-1 >> default_max_mntns=-1 >> >> +parse_propagation_flag() >> +{ >> + mount --bind dir_A dir_B >> + if grep -w 'dir_B' /proc/self/mountinfo | grep -qw 'shared'; then >> + echo "mounted" >> + else >> + echo "unmounted" >> + fi >> + umount dir_B >> +} >> + >> setup() >> { >> # On some distributions(e.g RHEL7.4), the default value of @@ >> -149,7 +160,8 @@ do_test() >> 4) unshare_test "--user --map-root-user" "id -g" "0";; >> 5) unshare_test "--mount" "mount --bind dir_A dir_B" >> "unmounted";; >> 6) unshare_test "--mount --propagation shared" \ >> - "mount --bind dir_A dir_B" "mounted";; >> + "mount --bind dir_A dir_B" \ >> + "$(parse_propagation_flag)";; >> 7) unshare_test "--user --map-root-user --mount" \ >> "mount --bind dir_A dir_B" "unmounted";; >> 8) unshare_test "--user --map-root-user --mount --propagation >> shared" \ >> -- >> ------------------------------------------------------------------------------------------ >> >> Best Regards, >> Xiao Yang >> On 2021/2/24 9:40, Petr Vorel wrote: >>> Hi, >>> >>>> We need setup parent mount flag to shared before unshare testing, or >>>> it will fail for system which has no systemd service since the >>>> propagation flag is changed by systemd. From man 7 >> mount_namespaces. >>> Do I understand correctly that all distros without systemd are >>> affected, because systemd "automatically remounts all mount points as >>> MS_SHARED on system startup" and test expect it? >>> >>>> Signed-off-by: Zhao Gongyi >>>> --- >>>> testcases/commands/unshare/unshare01.sh | 9 ++++++++- >>>> 1 file changed, 8 insertions(+), 1 deletion(-) diff --git >>>> a/testcases/commands/unshare/unshare01.sh >>>> b/testcases/commands/unshare/unshare01.sh >>>> index bf163a7f4..e1fb15035 100755 >>>> --- a/testcases/commands/unshare/unshare01.sh >>>> +++ b/testcases/commands/unshare/unshare01.sh >>>> @@ -31,7 +31,6 @@ TST_SETUP=setup >>>> TST_CLEANUP=cleanup >>>> TST_TESTFUNC=do_test >>>> TST_NEEDS_ROOT=1 >>>> -TST_NEEDS_TMPDIR=1 >>> You still need TST_NEEDS_TMPDIR=1, because you create files and >> directories. >>> Also your patch breaks bind test on very old systems (kernel 2.6, >>> util-linux 2.17.2, glibc 2.12): >>> unshare01 5 TFAIL: unshare --mount mount --bind dir_A dir_B got bind >>> info >>> >>> Any idea why (how to avoid this regression)? >>> >>>> TST_NEEDS_CMDS="unshare id mount umount" >>>> . tst_test.sh >>>> @@ -39,6 +38,7 @@ >> max_userns_path="/proc/sys/user/max_user_namespaces" >>>> max_mntns_path="/proc/sys/user/max_mnt_namespaces" >>>> default_max_userns=-1 >>>> default_max_mntns=-1 >>>> +CURR=$(pwd) >>> Instead of $CURR, cd - can be used. >>> >>>> setup() >>>> { >>>> @@ -55,6 +55,10 @@ setup() >>>> echo 1024> "${max_mntns_path}" >>>> fi >>>> + mkdir $CURR/dir_C >>> just mkdir dir_C >>>> + mount -t tmpfs none dir_C >>>> + mount --make-shared dir_C >>> FYI We have tst_mount, but it'd not help much here. >>> >>>> + cd dir_C >>>> mkdir -p dir_A dir_B >>>> touch dir_A/A dir_B/B >>>> } >>>> @@ -66,6 +70,9 @@ cleanup() >>>> echo ${default_max_userns}> "${max_userns_path}" >>>> [ ${default_max_mntns} -ne -1 ]&& \ >>>> echo ${default_max_mntns}> "${max_mntns_path}" >>>> + cd $CURR >>>> + umount dir_C >>> tst_umount dir_C >>> >>>> + rm -rf dir_C >>> rm is not needed (cleanup is done automatically). >>>> } >>>> check_id() >>> Full diff of changes I propose below. >>> >>> Kind regards, >>> Petr >>> >>> diff --git testcases/commands/unshare/unshare01.sh >>> testcases/commands/unshare/unshare01.sh >>> index e1fb15035..0b5c56811 100755 >>> --- testcases/commands/unshare/unshare01.sh >>> +++ testcases/commands/unshare/unshare01.sh >>> @@ -31,6 +31,7 @@ TST_SETUP=setup >>> TST_CLEANUP=cleanup >>> TST_TESTFUNC=do_test >>> TST_NEEDS_ROOT=1 >>> +TST_NEEDS_TMPDIR=1 >>> TST_NEEDS_CMDS="unshare id mount umount" >>> . tst_test.sh >>> >>> @@ -38,7 +39,6 @@ >> max_userns_path="/proc/sys/user/max_user_namespaces" >>> max_mntns_path="/proc/sys/user/max_mnt_namespaces" >>> default_max_userns=-1 >>> default_max_mntns=-1 >>> -CURR=$(pwd) >>> >>> setup() >>> { >>> @@ -55,7 +55,7 @@ setup() >>> echo 1024> "${max_mntns_path}" >>> fi >>> >>> - mkdir $CURR/dir_C >>> + mkdir dir_C >>> mount -t tmpfs none dir_C >>> mount --make-shared dir_C >>> cd dir_C >>> @@ -70,9 +70,8 @@ cleanup() >>> echo ${default_max_userns}> "${max_userns_path}" >>> [ ${default_max_mntns} -ne -1 ]&& \ >>> echo ${default_max_mntns}> "${max_mntns_path}" >>> - cd $CURR >>> - umount dir_C >>> - rm -rf dir_C >>> + cd ->/dev/null >>> + tst_umount dir_C >>> } >>> >>> check_id() >>> >> > > > . >