Re: [LTP] [PATCH v1] mount_setattr02.c: Check mount_setattr attr.propagation

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

nit: I guess you want to replace dot with space in subject.

> +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Basic mount_setattr() test.
> + * Test basic propagation mount attributes are set correctly.
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <sys/statvfs.h>
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +#include "tst_safe_stdio.h"
> +
> +#define DIRA "/DIRA_PROPAGATION_CHECK"

Is it necessary to to use directory under root?

...
> +static void cleanup(void)
> +{

I guess this is due result of:
SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
There should be either a proper detection whether this works or 

../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)


> +	SAFE_RMDIR(DIRA);

When running on old kernel (e.g. SLES based 4.12) it fails due TCONF:

../../../../include/lapi/fsmount.h:197: TCONF: Test not supported on kernel version < v5.2
mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: ENOENT (2)

There should be a flag to remove dir only when it was created.

> +
nit: please remove this new line (I have to keep asking this :( ).
> +}
> +
> +static void setup(void)
> +{
> +	fsopen_supported_by_kernel();
I wonder if this needed for detecting new mount API support. Because second
SAFE_MOUNT also runs code which detects code unsupported:

../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)

But I have no idea what would be needed to be done to cleanup result of the
first SAFE_MOUNT().

> +
> +	SAFE_MKDIR(DIRA, 0777);
> +}
> +
> +static void run(void)
> +{
> +
and here new line.
> +	SAFE_UNSHARE(CLONE_NEWNS);
> +	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> +	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
Do these 2 needs to be in the run()? How about move them to setup()?

static int dir_created, mounted;

static void setup(void)
{
	fsopen_supported_by_kernel();

	SAFE_MKDIR(DIRA, 0777);
	dir_created = 1;
	SAFE_UNSHARE(CLONE_NEWNS);
	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
	mounted = 1;
}

static void cleanup(void)
{
	if (mounted)
		SAFE_UMOUNT(DIRA);

	if (dir_created)
		SAFE_RMDIR(DIRA);
}

+ I later create generic helper from is_shared_mount().

Kind regards,
Petr

> +
> +	struct mount_attr attr = {
> +		.attr_set       = MOUNT_ATTR_RDONLY | MOUNT_ATTR_NOEXEC | MOUNT_ATTR_RELATIME,
> +		.attr_clr       = MOUNT_ATTR__ATIME,
> +	};
> +
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	attr.propagation = -1;
> +	TST_EXP_FAIL_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)), EINVAL);
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	attr.propagation = MS_SHARED;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 1);
> +
> +	attr.propagation = MS_PRIVATE;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	attr.propagation = MS_SLAVE;
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +
> +	SAFE_UMOUNT(DIRA);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +};

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