Re: [LTP] [PATCH v5] syscalls/mount_setattr01: Add basic functional test

[Petr Vorel <pvorel@suse.cz>]

Hi Chen, Dai,

...
> +static void setup(void)
> +{
> +	fsopen_supported_by_kernel();
> +	struct stat st = {0};
> +
> +	if (stat(OT_MNTPOINT, &st) == -1)
> +		SAFE_MKDIR(OT_MNTPOINT, 0777);
> +}
> +
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	struct mount_attr attr = {
> +		.attr_set = tc->mount_attrs,
> +	};
> +	struct statvfs buf;
> +
> +	TST_EXP_FD_SILENT(open_tree(AT_FDCWD, MNTPOINT, AT_EMPTY_PATH |
> +		AT_SYMLINK_NOFOLLOW | OPEN_TREE_CLOEXEC | OPEN_TREE_CLONE));
> +	if (!TST_PASS)
> +		return;
> +
> +	otfd = (int)TST_RET;
> +
> +	TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
> +		"%s set", tc->name);
> +	if (!TST_PASS)
> +		goto out1;
> +
> +	TST_EXP_PASS_SILENT(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
> +	if (!TST_PASS)
> +		goto out1;
> +	mount_flag = 1;
> +	SAFE_CLOSE(otfd);
> +
> +	TST_EXP_PASS_SILENT(statvfs(OT_MNTPOINT, &buf), "statvfs sucess");
> +	if (!TST_PASS)
> +		goto out2;
> +
> +	if (buf.f_flag & tc->expect_attrs)
> +		tst_res(TPASS, "%s is actually set as expected", tc->name);
> +	else
> +		tst_res(TFAIL, "%s is not actually set as expected", tc->name);
> +
> +	goto out2;
> +
> +out1:
> +	SAFE_CLOSE(otfd);
> +out2:
> +	mount_flag = 0;
> +	SAFE_UMOUNT(OT_MNTPOINT);
mount_flag needs to be checked before otherwise if it fails on:
TST_EXP_PASS_SILENT(mount_setattr(otfd, "", AT_EMPTY_PATH, &attr, sizeof(attr)),
		"%s set", tc->name);

It tries to umount() not yet mounted filesystem:

mount_setattr01.c:109: TPASS: MOUNT_ATTR_RDONLY is actually set as expected
mount_setattr01.c:109: TPASS: MOUNT_ATTR_NOSUID is actually set as expected
mount_setattr01.c:109: TPASS: MOUNT_ATTR_NODEV is actually set as expected
mount_setattr01.c:109: TPASS: MOUNT_ATTR_NOEXEC is actually set as expected
mount_setattr01.c:93: TFAIL: MOUNT_ATTR_NOSYMFOLLOW set failed: EINVAL (22)
tst_device.c:394: TWARN: umount('ot_mntpoint') failed with EINVAL
mount_setattr01.c:119: TBROK: umount(ot_mntpoint) failed: EINVAL (22)

Because we mount later than opening otfd with open_tree() but need to close otfd
before umount() we cannot depend only on labels.

Thus I suggest to have this cleanup code:

out1:
	SAFE_CLOSE(otfd);
out2:
	if (mount_flag)
		SAFE_UMOUNT(OT_MNTPOINT);

	mount_flag = 0;

If Cyril is ok with this I'd merge it before release.
(I tested it on various systems, found only this issue.)

Kind regards,
Petr

+++ testcases/kernel/syscalls/mount_setattr/mount_setattr01.c
@@ -115,8 +115,10 @@ static void run(unsigned int n)
 out1:
 	SAFE_CLOSE(otfd);
 out2:
+	if (mount_flag)
+		SAFE_UMOUNT(OT_MNTPOINT);
+
 	mount_flag = 0;
-	SAFE_UMOUNT(OT_MNTPOINT);
 }
 
 static struct tst_test test = {

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