[LTP] [PATCH v2] move_mount03: check allow to mount beneath top mount
Petr Vorel
pvorel@suse.cz
Wed Dec 27 15:26:00 CET 2023
Hi Wei,
I suppose there was no v1, right?
-i2 fails, please fix it.
# move_mount03 -i2
move_mount03.c:79: TINFO: Mounting none to /tmp/LTP_movcovMfK/LTP_DIR_A fstyp=tmpfs flags=0
move_mount03.c:80: TINFO: Mounting none to /tmp/LTP_movcovMfK/LTP_DIR_B fstyp=tmpfs flags=0
move_mount03.c:92: TPASS: move_mount(fda, "", fdb, "", MOVE_MOUNT_BENEATH | MOVE_MOUNT_F_EMPTY_PATH | MOVE_MOUNT_T_EMPTY_PATH) passed
move_mount03.c:98: TPASS: access(DIRB "/B", F_OK) passed
move_mount03.c:99: TINFO: Umounting /tmp/LTP_movcovMfK/LTP_DIR_B
move_mount03.c:100: TPASS: access(DIRB "/A", F_OK) passed
move_mount03.c:102: TINFO: Umounting /tmp/LTP_movcovMfK/LTP_DIR_B
move_mount03.c:103: TINFO: Umounting /tmp/LTP_movcovMfK/LTP_DIR_A
move_mount03.c:77: TBROK: mkdir(LTP_DIR_A, 0777) failed: EEXIST (17)
NOTE: you can speed up the review process, if you check list of common errors
before sending patch to ML
https://github.com/linux-test-project/ltp/wiki/Maintainer-Patch-Review-Checklist#how-to-find-clear-errors
make check-move_mount03
CHECK testcases/kernel/syscalls/move_mount/move_mount03.c
move_mount03.c:92:9: error: undefined identifier 'MOVE_MOUNT_BENEATH'
I see false positive from checkpatch.pl, it would be interesting to check if it
can be fixed.
> diff --git a/testcases/kernel/syscalls/move_mount/move_mount03.c b/testcases/kernel/syscalls/move_mount/move_mount03.c
> new file mode 100644
> index 000000000..dadb19178
> --- /dev/null
> +++ b/testcases/kernel/syscalls/move_mount/move_mount03.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 Christian Brauner <brauner@kernel.org>
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify allow to mount beneath top mount base following commit:
This will be very badly formatted in docs.
> + * commit 6ac392815628f317fcfdca1a39df00b9cc4ebc8b
> + * Author: Christian Brauner <brauner@kernel.org>
> + * Date: Wed May 3 13:18:42 2023 +0200
> + * fs: allow to mount beneath top mount
> + *
> + * Above commit has heavily commented but i found following commit
> + * contain simple summary of this feature for easy understanding:
> + *
> + * commit c0a572d9d32fe1e95672f24e860776dba0750a38
> + * Author: Linus Torvalds <torvalds@linux-foundation.org>
> + * TL;DR:
> + *
Please generate metadata and have look at result. The output below looks ugly.
Either you find a way to preformat it as generated output (put extra space after
* in the comment - equivalent of <pre>... </pre> HTML output) or put it into /*
* */, so that it's not in docs (good for some info useful in C source but more
related to the test implementation so that it does not have to be in generated docs).
> + * > mount -t ext4 /dev/sda /mnt
> + * |
> + * --/mnt /dev/sda ext4
> + *
> + * > mount --beneath -t xfs /dev/sdb /mnt
> + * |
> + * --/mnt /dev/sdb xfs
> + * --/mnt /dev/sda ext4
> + *
> + * > umount /mnt
> + * |
> + * --/mnt /dev/sdb xfs
> + *
> + * So base above scenario design following scenario for LTP check:
> + *
> + * > mount -t tmpfs /DIRA
> + * |
> + * --/DIRA(create A file within DIRA)
> + *
> + * > mount -t tmpfs /DIRB
> + * |
> + * --/DIRA(create B file within DIRB)
> + *
> + * > move_mount --beneath /DIRA /DIRB
> + * |
> + * --/mnt /DIRA /DIRB
> + * --/mnt /DIRB
> + *
> + * If you check content of /DIRB, you can see file B
> + *
> + * > umount /DIRB
> + * |
> + * --/mnt /DIRA /DIRB
> + * Check content of /DIRB, you can see file A exist since
> + * current /DIRB mount source is already become /DIRA
> + *
> + * More detail can be found in following link:
> + * Link: https://lwn.net/Articles/930591/
> + * Link: https://github.com/brauner/move-mount-beneath
Link is bogus (we use it only as tag in the git commit message, not in docs).
* See also:
* https://lwn.net/Articles/930591/
* https://github.com/brauner/move-mount-beneath
*/
> + */
> +
> +#include <stdio.h>
> +
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +#include "lapi/sched.h"
> +
> +#define DIRA "LTP_DIR_A"
> +#define DIRB "LTP_DIR_B"
> +
> +static void run(void)
> +{
> + int fda, fdb;
> +
> + SAFE_MKDIR(DIRA, 0777);
> + SAFE_MKDIR(DIRB, 0777);
> + SAFE_MOUNT("none", DIRA, "tmpfs", 0, 0);
> + SAFE_MOUNT("none", DIRB, "tmpfs", 0, 0);
> + SAFE_TOUCH(DIRA "/A", 0, NULL);
> + SAFE_TOUCH(DIRB "/B", 0, NULL);
Maybe whole code in above could be in setup() function, it can be created only
once even we run test more times with -iN, right? Setup function is for test
speedup (imagine somebody run test with -i10000, why to create files all the
time?
> +
> + TEST(fda = open_tree(AT_FDCWD, DIRA, OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
> +
> + if (fda == -1) {
> + tst_res(TFAIL | TTERRNO, "open_tree() failed");
> + return;
> + }
Maybe this should also be in the setup function. also not using TEST() for it?
fda = open_tree(AT_FDCWD, DIRA, OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE);
if (fda > 0)
tst_brk(TBROK | TERRNO, "open_tree() failed");
> +
> + fdb = SAFE_OPEN(DIRB, O_PATH | O_NOFOLLOW, 0666);
> + TST_EXP_PASS(move_mount(fda, "", fdb, "",
> + MOVE_MOUNT_BENEATH | MOVE_MOUNT_F_EMPTY_PATH |
> + MOVE_MOUNT_T_EMPTY_PATH));
I wonder what happen if any of the following SAFE_*() functions fail.
There will be unrelated errors and left mount directories. IMHO at least
SAFE_UMOUNT() should be in cleanup functions and guarded by flags (e.g. not
trying to umount() when nothing was mounted because tst_brk() earlier).
Kind regards,
Petr
> + SAFE_CLOSE(fda);
> + SAFE_CLOSE(fdb);
> +
> + TST_EXP_PASS(access(DIRB "/B", F_OK));
> + SAFE_UMOUNT(DIRB);
> + TST_EXP_PASS(access(DIRB "/A", F_OK));
> +
> + SAFE_UMOUNT(DIRB);
> + SAFE_UMOUNT(DIRA);
> +}
> +
> +static struct tst_test test = {
> + .test_all = run,
> + .needs_root = 1,
> + .min_kver = "6.5.0",
> + .needs_tmpdir = 1,
> +};
More information about the ltp
mailing list