Re: [LTP] [PATCH] Fix mountns01/02/03/04 removing unneeded final umounts

[Cristian Marussi <cristian.marussi@arm.com>]

On Mon, Jun 20, 2022 at 03:55:53AM +0000, xuyang2018.jy@fujitsu.com wrote:
> Hi  Cristian
> 

Hi Yang Xu

thanks for the feedback.

> > Running LTP20220527 release it appears that the recently re-written tests
> > mountns02/03/04 now throw a warning on their final umount attempt:
> >
> > <<<test_output>>>
> > tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
> > mountns04.c:38: TPASS: unbindable mount passed
> > tst_device.c:395: TWARN: umount('A') failed with EINVAL
> > mountns.h:36: TWARN: umount(A) failed: EINVAL (22)
> > tst_device.c:434: TINFO: No device is mounted at B
> 
> I guess this failure because /proc/mounts still has "A", so 
> tst_is_mounted returns true and umount failed.
> 
> we can reproduce it by using a /mnt/A mntpoint in /proc/mounts.

Yes indeed, reasoning about your feedback on this I think the issue
really is different from what I thought (my bad): it is the final
SAFE_UMOUNT in the cleanup function that fails really because the
tst_is_mounted() that protects it is based on a simnple strstr() on
/proc/mounts/ and the directory names used are too much simple ("A")
so it is sufficient to have some unrelated mountpoint like
"/mnt/this_is_Another_mnt" to fool tst_is_mounted and make it think
our "A" still needs to be removed.

This also would explin why this bug has not been noticed elsewhere...
...it depends on your final running environment. (i.e. mountpoints)

Using a bit more peculiar names like to __DIR_A __DIR_B solves for me
indeed:


root@debian-arm64-bullseye:~# ./mountns01
tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
mountns01.c:42: TPASS: shared mount in parent passed
mountns01.c:83: TPASS: shared mount in child passed
tst_device.c:434: TINFO: No device is mounted at __DIR_B

Summary:
passed   2
failed   0
broken   0
skipped  0
warnings 0
root@debian-arm64-bullseye:~# ./mountns02
tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
mountns02.c:45: TPASS: private mount in parent passed
mountns02.c:86: TPASS: private mount in child passed
tst_device.c:434: TINFO: No device is mounted at __DIR_A
tst_device.c:434: TINFO: No device is mounted at __DIR_B

Summary:
passed   2
failed   0
broken   0
skipped  0
warnings 0
root@debian-arm64-bullseye:~# ./mountns03
tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
mountns03.c:51: TPASS: propagation to slave mount passed
mountns03.c:94: TPASS: propagation from slave mount passed
tst_device.c:434: TINFO: No device is mounted at __DIR_A
tst_device.c:434: TINFO: No device is mounted at __DIR_B

Summary:
passed   2
failed   0
broken   0
skipped  0
warnings 0
root@debian-arm64-bullseye:~# ./mountns04
tst_test.c:1526: TINFO: Timeout per run is 0h 00m 30s
mountns04.c:38: TPASS: unbindable mount passed
tst_device.c:434: TINFO: No device is mounted at __DIR_A
tst_device.c:434: TINFO: No device is mounted at __DIR_B


> >
> > Moreover, the underlying safe_umount() then upgrades the TWARN emitted
> > from tst_umount to a TBROK, so causing the test to completely fail:
> >
> > Summary:
> > passed   1
> > failed   0
> > broken   0
> > skipped  0
> > warnings 2
> > <<<execution_status>>>
> > initiation_status="ok"
> > duration=0 termination_type=exited termination_id=4 corefile=no
> >
> > In fact, the final umounts on DIRA seem not needed in mountns02/03/04
> > looking at the previous chain of umounts calls and the tests logic and,
> > in any case, the .cleanup functions of all these tests take care to
> > finally unmount both DIRA/DIRB after having checked if they were still
> > mounted at all.
> >
> > Remove all the final SAFE_UMOUNT calls (even for mountns01) since all the
> > possibly needed umounts are already eventually performed by .cleanup.
> 
> Yes, it can fix this problem but these case still will fail when 
> /proc/mounts has a B mntpoint,
> 
> ie
>   mount /dev/sda11 /mnt/B
> ./mountns04
> tst_test.c:1528: TINFO: Timeout per run is 0h 00m 30s
> mountns04.c:38: TPASS: unbindable mount passed
> tst_device.c:395: TWARN: umount('B') failed with EINVAL
> mountns.h:39: TWARN: umount(B) failed: EINVAL (22)
> 
> So I think a right fix should fix the tst_is_mounted function or 
> umount_folders instead of removing final SAFE_UMOUNT.
> 

Agreed that the failure was elsewhere as above mentioned, so I will
keep the final SAFE_UMOUNT and just change the dir names and let the
cleanups remove what remains to be removed (e.g. in case of error)

Thanks,
Cristian

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp