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

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

Hi Petr
> 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.)
This fix is obviously correct. Since I have reviewed this patch in 
internal, you can also add my reviewed-by for this patch.

ps: I think we should also check functionality like mount03 did instead 
of just check flag.

Best Regards
Yang Xu
>
> 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