[LTP] [PATCH v2] Fix mountns01/02/03/04 final umounts using more peculiar dir names
xuyang2018.jy@fujitsu.com
xuyang2018.jy@fujitsu.com
Tue Jun 21 03:43:16 CEST 2022
Hi Cristian
Looks good to me,
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
Best Regards
Yang Xu
> Running LTP20220527 release it appears that the recently re-written tests
> mountns02/03/04 can now throw a warning on their final umount attempt in
> some setup:
>
> <<<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
>
> 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
>
> Even though the final SAFE_UMOUNTs in the test body properly unmount the
> test created mountpoints, the final cleanup functions, that finally check
> to see if those mountpoints are still mounted, can be fooled into falsely
> thinking that test-chosen mountpoints "A" or "B" are still there: this is
> due to the fact that the internal helper tst_is_mounted() uses a simple
> strstr() on /proc/mounts to check if a directory is still mounted and
> clearly the currently test-chosen names are far too much simple, being
> one-letter, and they can be easily matched by other unrelated mountpoints
> that happen to exist on a specific setup.
>
> Use a more peculiar naming for the test chosen mountpoints and generalize
> accordingly all the comments.
>
> Cc: Andrea Cervesato<andrea.cervesato@suse.de>
> Cc: Cyril Hrubis<chrubis@suse.cz>
> Signed-off-by: Cristian Marussi<cristian.marussi@arm.com>
> ---
> v1 --> v2
> - using more peculiar naming for mountpoints
> - fix comments
> - dropped previous SAFE_UMONUT removal
>
> A better, more long term fix should be to fix/harden tst_is_mounted logic,
> but looking at mountpoint(1) implementation this is far from trivial to
> be done (especially for bind mounts) and it would require a bit of
> 're-inventing the wheel' to bring all the mountpoint/libmount helpers and
> logic inside LTP; on the other side a dirty and ugly solution based on
> something like tst_system("mountpoint -q<dir>") would be less portable
> since would add the new mountpoint application as an LTP pre-requisite.
> (and so just breaking a few CI probably without having a 'mountpoint-less'
> failover mechanism)...so I just generalized the chosen names for now...
> ---
> testcases/kernel/containers/mountns/mountns.h | 4 ++--
> .../kernel/containers/mountns/mountns01.c | 18 +++++++++---------
> .../kernel/containers/mountns/mountns02.c | 18 +++++++++---------
> .../kernel/containers/mountns/mountns03.c | 18 +++++++++---------
> .../kernel/containers/mountns/mountns04.c | 8 ++++----
> 5 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/testcases/kernel/containers/mountns/mountns.h b/testcases/kernel/containers/mountns/mountns.h
> index ad8befa71..347f0783a 100644
> --- a/testcases/kernel/containers/mountns/mountns.h
> +++ b/testcases/kernel/containers/mountns/mountns.h
> @@ -10,8 +10,8 @@
> #include "tst_test.h"
> #include "lapi/namespaces_constants.h"
>
> -#define DIRA "A"
> -#define DIRB "B"
> +#define DIRA "__DIR_A"
> +#define DIRB "__DIR_B"
>
> static int dummy_child(void *v)
> {
> diff --git a/testcases/kernel/containers/mountns/mountns01.c b/testcases/kernel/containers/mountns/mountns01.c
> index 452fe1d10..e99134aba 100644
> --- a/testcases/kernel/containers/mountns/mountns01.c
> +++ b/testcases/kernel/containers/mountns/mountns01.c
> @@ -12,21 +12,21 @@
> *
> * [Algorithm]
> *
> - * - Creates directories "A", "B" and files "A/A", "B/B"
> + * - Creates directories DIR_A, DIR_B and files DIR_A/"A", DIR_B/"B"
> * - Unshares mount namespace and makes it private (so mounts/umounts have no
> * effect on a real system)
> - * - Bind mounts directory "A" to "A"
> - * - Makes directory "A" shared
> + * - Bind mounts directory DIR_A to DIR_A
> + * - Makes directory DIR_A shared
> * - Clones a new child process with CLONE_NEWNS flag
> * - There are two test cases (where X is parent namespace and Y child namespace):
> * 1. First test case
> - * .. X: bind mounts "B" to "A"
> - * .. Y: must see "A/B"
> - * .. X: umounts "A"
> + * .. X: bind mounts DIR_B to DIR_A
> + * .. Y: must see DIR_A/"B"
> + * .. X: umounts DIR_A
> * 2. Second test case
> - * .. Y: bind mounts "B" to "A"
> - * .. X: must see "A/B"
> - * .. Y: umounts "A"
> + * .. Y: bind mounts DIR_B to DIR_A
> + * .. X: must see DIR_A/"B"
> + * .. Y: umounts DIR_A
> */
>
> #include<sys/wait.h>
> diff --git a/testcases/kernel/containers/mountns/mountns02.c b/testcases/kernel/containers/mountns/mountns02.c
> index cbd435958..258b61217 100644
> --- a/testcases/kernel/containers/mountns/mountns02.c
> +++ b/testcases/kernel/containers/mountns/mountns02.c
> @@ -12,22 +12,22 @@
> *
> * [Algorithm]
> *
> - * - Creates directories "A", "B" and files "A/A", "B/B"
> + * - Creates directories DIR_A, DIR_B and files DIR_A/"A", DIR_B/"B"
> * - Unshares mount namespace and makes it private (so mounts/umounts have no
> * effect on a real system)
> - * - Bind mounts directory "A" to "A"
> - * - Makes directory "A" private
> + * - Bind mounts directory DIR_A to DIR_A
> + * - Makes directory DIR_A private
> * - Clones a new child process with CLONE_NEWNS flag
> * - There are two test cases (where X is parent namespace and Y child
> * namespace):
> * 1. First test case
> - * .. X: bind mounts "B" to "A"
> - * .. Y: must see "A/A" and must not see "A/B"
> - * .. X: umounts "A"
> + * .. X: bind mounts DIR_B to DIR_A
> + * .. Y: must see DIR_A/"A" and must not see DIR_A/"B"
> + * .. X: umounts DIR_A
> * 2. Second test case
> - * .. Y: bind mounts "B" to "A"
> - * .. X: must see "A/A" and must not see "A/B"
> - * .. Y: umounts A
> + * .. Y: bind mounts DIR_B to DIR_A
> + * .. X: must see DIR_A/"A" and must not see DIR_A/"B"
> + * .. Y: umounts DIRA
> */
>
> #include<sys/wait.h>
> diff --git a/testcases/kernel/containers/mountns/mountns03.c b/testcases/kernel/containers/mountns/mountns03.c
> index 5c19a96a9..f37ae7902 100644
> --- a/testcases/kernel/containers/mountns/mountns03.c
> +++ b/testcases/kernel/containers/mountns/mountns03.c
> @@ -12,24 +12,24 @@
> *
> * [Algorithm]
> *
> - * - Creates directories "A", "B" and files "A/A", "B/B"
> + * - Creates directories DIRA, DIRB and files DIRA/"A", DIRB/"B"
> * - Unshares mount namespace and makes it private (so mounts/umounts have no
> * effect on a real system)
> - * - Bind mounts directory "A" to itself
> - * - Makes directory "A" shared
> + * - Bind mounts directory DIRA to itself
> + * - Makes directory DIRA shared
> * - Clones a new child process with CLONE_NEWNS flag and makes "A" a slave
> * mount
> * - There are two testcases (where X is parent namespace and Y child
> * namespace):
> * 1. First test case
> - * .. X: bind mounts "B" to "A"
> - * .. Y: must see the file "A/B"
> - * .. X: umounts "A"
> + * .. X: bind mounts DIRB to DIRA
> + * .. Y: must see the file DIRA/"B"
> + * .. X: umounts DIRA
> * 2. Second test case
> - * .. Y: bind mounts "B" to "A"
> - * .. X: must see only the "A/A" and must not see "A/B" (as slave mount does
> + * .. Y: bind mounts DIRB to DIRA
> + * .. X: must see only the DIRA/"A" and must not see DIRA/"B" (as slave mount does
> * not forward propagation)
> - * .. Y: umounts "A"
> + * .. Y: umounts DIRA
> */
>
> #include<sys/wait.h>
> diff --git a/testcases/kernel/containers/mountns/mountns04.c b/testcases/kernel/containers/mountns/mountns04.c
> index cc63a03d9..46937ddcd 100644
> --- a/testcases/kernel/containers/mountns/mountns04.c
> +++ b/testcases/kernel/containers/mountns/mountns04.c
> @@ -10,12 +10,12 @@
> * Tests an unbindable mount: unbindable mount is an unbindable
> * private mount.
> *
> - * - Creates directories "A", "B" and files "A/A", "B/B"
> + * - Creates directories DIRA, DIRB and files DIRA/"A", DIRB/"B"
> * - Unshares mount namespace and makes it private (so mounts/umounts have no
> * effect on a real system)
> - * - Bind mounts directory "A" to "A"
> - * - Makes directory "A" unbindable
> - * - Check if bind mount unbindable "A" to "B" fails as expected
> + * - Bind mounts directory DIRA to DIRA
> + * - Makes directory DIRA unbindable
> + * - Check if bind mount unbindable DIRA to DIRB fails as expected
> */
>
> #include<sys/wait.h>
More information about the ltp
mailing list