Re: [LTP] [PATCH v2] Fix mountns01/02/03/04 final umounts using more peculiar dir names

[Joerg Vehlow <lkml@jv-coder.de>]

Hi,

Am 6/20/2022 um 3:37 PM schrieb Cristian Marussi:
> 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"
This is the only non-comment change. How does renaming the directories
change anything? Am I missing something?

Joerg

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