Re: [LTP] [PATCH v3 05/11] Add listmount02 test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> This test verifies that listmount() is properly reading groups of
> mount IDs.
> ---
>  runtest/syscalls                                  |   1 +
>  testcases/kernel/syscalls/listmount/.gitignore    |   1 +
>  testcases/kernel/syscalls/listmount/listmount02.c | 106 ++++++++++++++++++++++
>  3 files changed, 108 insertions(+)
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index bce04c0cf..8831c0aec 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -707,6 +707,7 @@ linkat02 linkat02
>  listen01 listen01
>  
>  listmount01 listmount01
> +listmount02 listmount02
>  
>  listxattr01 listxattr01
>  listxattr02 listxattr02
> diff --git a/testcases/kernel/syscalls/listmount/.gitignore b/testcases/kernel/syscalls/listmount/.gitignore
> index 5257b298c..30bbf9f02 100644
> --- a/testcases/kernel/syscalls/listmount/.gitignore
> +++ b/testcases/kernel/syscalls/listmount/.gitignore
> @@ -1 +1,2 @@
>  listmount01
> +listmount02
> diff --git a/testcases/kernel/syscalls/listmount/listmount02.c b/testcases/kernel/syscalls/listmount/listmount02.c
> new file mode 100644
> index 000000000..cd742bd64
> --- /dev/null
> +++ b/testcases/kernel/syscalls/listmount/listmount02.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that listmount() is properly reading groups of mount IDs.

This description could be way better. The code below tests that both the
oneshoot and the iterator API for listmount() return the same array.
I.e. by passing non-zero as the second argument of the listmount() you
ask only for the ids that are listed only after that id.

> + * [Algorithm]
> + *
> + * * move into a new unshared namespace
> + * * mount() our new root inside temporary folder
> + * * generate a full mounts tree inside root folder
> + * * read the full list of mounted IDs using listmount(LSMT_ROOT, ..)
> + * * read the list of mounted IDs using groups of fixed size
> + * * compare the first mount list with the second mount list

Here as well, dashes please.

> + */
> +
> +#include "listmount.h"
> +#include "lapi/sched.h"
> +
> +#define MNTPOINT "mntpoint"
> +#define BIND_MOUNTS 7
> +#define GROUPS_SIZE 3
> +#define LISTSIZE (1 << BIND_MOUNTS)

So each bind mount doubles the number of mounted filesystems? I suppose
that we copy the whole hierarchy below the current mount point. I guess
that this should be explained in the Algorithm section.

> +static void run(void)
> +{
> +	ssize_t ret;
> +	size_t id, tot_ids, count = 0;
> +	uint64_t mount_ids[LISTSIZE];
> +	uint64_t list[LISTSIZE];
> +
> +	for (int i = 0; i < BIND_MOUNTS; i++)
> +		SAFE_MOUNT("/", "/", NULL, MS_BIND, NULL);
> +
> +	tst_res(TINFO, "Reading all %d mount IDs in once", LISTSIZE);
> +
> +	TST_EXP_POSITIVE(listmount(LSMT_ROOT, 0, mount_ids, LISTSIZE, 0));
> +	if (TST_RET == -1)
> +		goto end;
> +
> +	tot_ids = (size_t)TST_RET;
> +
> +	if (tot_ids != LISTSIZE) {
> +		tst_res(TFAIL, "listmount() returned %lu but %d was expected",
> +			tot_ids, LISTSIZE);
> +		goto end;
> +	}
> +
> +	tst_res(TINFO, "Reading groups of %d mount IDs", GROUPS_SIZE);
> +
> +	while (count < LISTSIZE) {
> +		id = count ? list[count - 1] : 0;
> +		ret = listmount(LSMT_ROOT, id, list + count, GROUPS_SIZE, 0);
> +
> +		tst_res(TDEBUG, "listmount(LSMT_ROOT, %lu, list + %lu, %d, 0)",
> +			id, count, GROUPS_SIZE);
> +
> +		if (ret == -1) {
> +			tst_res(TFAIL, "listmount() failed with %s", tst_strerrno(errno));
> +			goto end;
> +		}
> +
> +		count += ret;
> +
> +		if (TST_RET < GROUPS_SIZE)
> +			break;
> +	}
> +
> +	for (size_t i = 0; i < LISTSIZE; i++) {
> +		if (mount_ids[i] != list[i]) {
> +			tst_res(TFAIL, "Mount ID differs at %ld index", i);
> +			goto end;
> +		}
> +	}
> +
> +	tst_res(TPASS, "All mount IDs have been correctly read");
> +
> +end:
> +	SAFE_UMOUNT("/");

Hmm, shouldn't we umount BIND_MOUNT times here?

I suppose that if we fail to umount everything that we did mount
previously, the test will fail quite soon with just a few (-i)
iterations, since the amount of mounts doubles with each bind mount.

> +}
> +
> +static void setup(void)
> +{
> +	SAFE_UNSHARE(CLONE_NEWNS);
> +	SAFE_CHROOT(MNTPOINT);
> +
> +	SAFE_MOUNT("", "/", NULL, MS_REC | MS_SHARED, NULL);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.forks_child = 1,
> +	.min_kver = "6.8",
> +	.mount_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.all_filesystems = 1,

Again I do not think that it makes much sense to run this for all
filesystems here.

> +	.skip_filesystems = (const char *const []) {
> +		"fuse",
> +		NULL
> +	},
> +};
> 
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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