Re: [LTP] [PATCH v3 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS

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

Hi Dai

It seems you don't fix some problems thant I comment on your v2 patch.
so again

>   /*

Typo  /* =>/*\

So the description can be used in docparse.

ps:you can cd ltp/docparse directory, then run make command, it will
generate metadata.html, then put this html file into your nginx or
appache directory, then you can see this html whether works well for
docparse from other host.
> - * DESCRIPTION
> + * [Description]
> + *
>    *  Test for feature MNT_EXPIRE of umount2().
> - *  "Mark the mount point as expired.If a mount point is not currently
> - *   in use, then an initial call to umount2() with this flag fails with
> - *   the error EAGAIN, but marks the mount point as expired. The mount
> - *   point remains expired as long as it isn't accessed by any process.
> - *   A second umount2() call specifying MNT_EXPIRE unmounts an expired
> - *   mount point. This flag cannot be specified with either MNT_FORCE or
> - *   MNT_DETACH. (fails with the error EINVAL)"
> + *
> + * - EINVAL when flag is specified with either MNT_FORCE or MNT_DETACH
> + * - EAGAIN when initial call to umount2(2) with MNT_EXPIRE
> + * - EAGAIN when umount2(2) with MNT_EXPIRE after access(2)
> + * - succeed when second call to umount2(2) with MNT_EXPIRE
> + *
> + *  Test for feature UMOUNT_NOFOLLOW of umount2().
> + *
> + * - EINVAL when target is a symbolic link
> + * - succeed when target is a mount point
>    */
> 
> -#include<errno.h>
>   #include<sys/mount.h>
> -
> -#include "test.h"
> -#include "safe_macros.h"
>   #include "lapi/mount.h"
> -
> +#include "tst_test.h"
>   #include "umount2.h"
> 
> -#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
> -#define MNTPOINT	"mntpoint"
> -
> -static void setup(void);
> -static void test_umount2(int i);
> -static void verify_failure(int i);
> -static void verify_success(int i);
> -static void cleanup(void);
> -
> -static const char *device;
> -static const char *fs_type;
> +#define MNTPOINT        "mntpoint"
> +#define SYMLINK	"symlink"
> 
>   static int mount_flag;
> 
> -static struct test_case_t {
> +static struct tcase {
> +	const char *mntpoint;
>   	int flag;
>   	int exp_errno;
>   	int do_access;
>   	const char *desc;
> -} test_cases[] = {
> -	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
> -		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
> -	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
> -		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
> -	{MNT_EXPIRE, EAGAIN, 0,
> -		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
> -	{MNT_EXPIRE, EAGAIN, 1,
> -		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
> -	{MNT_EXPIRE, 0, 0,
> -		"second call to umount2(2) with MNT_EXPIRE expected success"},
> -};
> +} tcases[] = {
> +	{MNTPOINT, MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
> +		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
> 
> -char *TCID = "umount2_02";
> -int TST_TOTAL = ARRAY_SIZE(test_cases);
> +	{MNTPOINT, MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
> +		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
> 
> -int main(int ac, char **av)
> -{
> -	int lc;
> -	int tc;
> -
> -	tst_parse_opts(ac, av, NULL, NULL);
> -
> -	setup();
> +	{MNTPOINT, MNT_EXPIRE, EAGAIN, 0,
> +		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
> 
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		tst_count = 0;
> +	{MNTPOINT, MNT_EXPIRE, EAGAIN, 1,
> +		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
> 
> -		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
> -		mount_flag = 1;
> -
> -		for (tc = 0; tc<  TST_TOTAL; tc++)
> -			test_umount2(tc);
> +	{MNTPOINT, MNT_EXPIRE, 0, 0,
> +		"second call to umount2() with MNT_EXPIRE expected success"},
> 
> -		if (mount_flag) {
> -			if (tst_umount(MNTPOINT))
> -				tst_brkm(TBROK, cleanup, "umount() failed");
> -			mount_flag = 0;
> -		}
> -	}
> +	{SYMLINK, UMOUNT_NOFOLLOW, EINVAL, 0,
> +		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
> 
> -	cleanup();
> -	tst_exit();
> -}
> +	{MNTPOINT, UMOUNT_NOFOLLOW, 0, 0,
> +		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
> +};
> 
> -static void setup(void)
> +static void test_umount2(unsigned int n)
>   {
> -	tst_require_root();
> -
> -	if ((tst_kvercmp(2, 6, 8))<  0) {
> -		tst_brkm(TCONF, NULL, "This test can only run on kernels "
> -			 "that are 2.6.8 or higher");
> -	}
> +	struct tcase *tc =&tcases[n];
> 
> -	tst_sig(NOFORK, DEF_HANDLER, NULL);
> -
> -	tst_tmpdir();
> -
> -	fs_type = tst_dev_fs_type();
> -	device = tst_acquire_device(cleanup);
> -
> -	if (!device)
> -		tst_brkm(TCONF, cleanup, "Failed to obtain block device");
> -
> -	tst_mkfs(cleanup, device, fs_type, NULL, NULL);
> -
> -	SAFE_MKDIR(cleanup, MNTPOINT, DIR_MODE);
> -
> -	TEST_PAUSE;
> -}
> -
> -static void test_umount2(int i)
> -{
> -	/* a new access removes the expired mark of the mount point */
> -	if (test_cases[i].do_access) {
> -		if (access(MNTPOINT, F_OK) == -1)
> -			tst_brkm(TBROK | TERRNO, cleanup, "access(2) failed");
> +	if (!mount_flag) {
> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
> +		mount_flag = 1;
>   	}
> 
> -	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
> -
> -	if (test_cases[i].exp_errno != 0)
> -		verify_failure(i);
> -	else
> -		verify_success(i);
> -}
> +	tst_res(TINFO, "Testing %s", tc->desc);
> 
> -static void verify_failure(int i)
> -{
> -	if (TEST_RETURN == 0) {
> -		tst_resm(TFAIL, "%s passed unexpectedly", test_cases[i].desc);
> -		mount_flag = 0;
> -		return;
> -	}
> +	if (tc->do_access)
> +		SAFE_ACCESS(MNTPOINT, F_OK);
> 
> -	if (TEST_ERRNO != test_cases[i].exp_errno) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> +	if (tc->exp_errno) {
> +		TST_EXP_FAIL(umount2_retry(tc->mntpoint, tc->flag), tc->exp_errno,
> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
> +		if (!TST_PASS)
> +			mount_flag = 0;
> +	} else {
> +		TST_EXP_PASS(umount2_retry(tc->mntpoint, tc->flag),
> +			"umount2_retry(%s, %d)", tc->mntpoint, tc->flag);
> +		if (TST_PASS)
> +			mount_flag = 0;
>   	}
> -
> -	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
>   }
> 
> -static void verify_success(int i)
> +static void setup(void)
>   {
> -	if (TEST_RETURN != 0) {
> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
> -			 test_cases[i].desc);
> -		return;
> -	}
> -
> -	tst_resm(TPASS, "umount2(2) succeeded as expected");
> -	mount_flag = 0;
> +	SAFE_SYMLINK(MNTPOINT, SYMLINK);
>   }
> 
>   static void cleanup(void)
>   {
> -	if (mount_flag&&  tst_umount(MNTPOINT))
> -		tst_resm(TWARN | TERRNO, "Failed to unmount");
> -
> -	if (device)
> -		tst_release_device(device);
> -
> -	tst_rmdir();
> +	if (mount_flag)
> +		SAFE_UMOUNT(MNTPOINT);
>   }
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.cleanup = cleanup,
> +	.setup = setup,
> +	.needs_root = 1,
> +	.needs_device = 1,
> +	.format_device = 1,
If we specify format_device, we don't need .needs_device because
format_device will enable needs_device automatically.

Best Regards
Yang Xu
> +	.mntpoint = MNTPOINT,
> +	.test = test_umount2,
> +};

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