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

* [LTP] [PATCH v2] Fix mountns01/02/03/04 final umounts using more peculiar dir names
@ 2022-06-20 13:37 Cristian Marussi
  2022-06-21  1:43 ` xuyang2018.jy
  2022-06-21  6:51 ` Joerg Vehlow
  0 siblings, 2 replies; 6+ messages in thread
From: Cristian Marussi @ 2022-06-20 13:37 UTC (permalink / raw)
  To: ltp

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>
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread