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

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

Hi Dai

Sorry, it seems keep umount2_02 and umount2_03 is more clean.
> Hi Dai
>
> Since umount2_02 and umount2_03 is similar, can we merge them into a
> single case?
>
> Best Regards
> Yang Xu
>> 1. use TST_EXP_FAIL and TST_EXP_PASS macro
>> 2. use SAFE macro
>> 3. simplify verify operations
>>
>> Signed-off-by: Dai Shili<daisl.fnst@fujitsu.com>
>> ---
>>    testcases/kernel/syscalls/umount2/umount2.h    |   5 +-
>>    testcases/kernel/syscalls/umount2/umount2_02.c | 194 +++++++------------------
>>    testcases/kernel/syscalls/umount2/umount2_03.c | 167 +++++----------------
>>    3 files changed, 94 insertions(+), 272 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/umount2/umount2.h b/testcases/kernel/syscalls/umount2/umount2.h
>> index 65e4c24..d1e486e 100644
>> --- a/testcases/kernel/syscalls/umount2/umount2.h
>> +++ b/testcases/kernel/syscalls/umount2/umount2.h
>> @@ -1,3 +1,4 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>    /*
>>     * Copyright (c) 2016 Cyril Hrubis<chrubis@suse.cz>
>>     *
>> @@ -28,13 +29,13 @@ static inline int umount2_retry(const char *target, int flags)
>>    		if (ret == 0 || errno != EBUSY)
>>    			return ret;
>>
>> -		tst_resm(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
>> +		tst_res(TINFO, "umount('%s', %i) failed with EBUSY, try %2i...",
>>    			 target, flags, i);
>>
>>    		usleep(100000);
>>    	}
>>
>> -	tst_resm(TWARN, "Failed to umount('%s', %i) after 50 retries",
>> +	tst_res(TWARN, "Failed to umount('%s', %i) after 50 retries",
>>    	         target, flags);
>>
>>    	errno = EBUSY;
>> diff --git a/testcases/kernel/syscalls/umount2/umount2_02.c b/testcases/kernel/syscalls/umount2/umount2_02.c
>> index 7d558fa..39f1608 100644
>> --- a/testcases/kernel/syscalls/umount2/umount2_02.c
>> +++ b/testcases/kernel/syscalls/umount2/umount2_02.c
>> @@ -1,182 +1,90 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>    /*
>> - * Copyright (c) 2015 Fujitsu Ltd.
>> + * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
>>     * Author: Guangwen Feng<fenggw-fnst@cn.fujitsu.com>
>> - *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms of version 2 of the GNU General Public License as
>> - * published by the Free Software Foundation.
>> - *
>> - * This program is distributed in the hope that it would be useful, but
>> - * WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * alone with this program.
>>     */
>>
>>    /*
typo, /* => /*\
>> - * 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
>>     */
>>
>> -#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"
>>
>>    static int mount_flag;
>>
>> -static struct test_case_t {
>> +static struct tcase {
>>    	int flag;
>>    	int exp_errno;
>>    	int do_access;
>>    	const char *desc;
>> -} test_cases[] = {
>> +} tcases[] = {
>>    	{MNT_EXPIRE | MNT_FORCE, EINVAL, 0,
>> -		"umount2(2) with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
>> +		"umount2() with MNT_EXPIRE | MNT_FORCE expected EINVAL"},
>> +
>>    	{MNT_EXPIRE | MNT_DETACH, EINVAL, 0,
>> -		"umount2(2) with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
>> +		"umount2() with MNT_EXPIRE | MNT_DETACH expected EINVAL"},
>> +
>>    	{MNT_EXPIRE, EAGAIN, 0,
>> -		"initial call to umount2(2) with MNT_EXPIRE expected EAGAIN"},
>> +		"initial call to umount2() with MNT_EXPIRE expected EAGAIN"},
>> +
>>    	{MNT_EXPIRE, EAGAIN, 1,
>> -		"umount2(2) with MNT_EXPIRE after access(2) expected EAGAIN"},
>> +		"umount2() with MNT_EXPIRE after access() expected EAGAIN"},
>> +
>>    	{MNT_EXPIRE, 0, 0,
>> -		"second call to umount2(2) with MNT_EXPIRE expected success"},
>> +		"second call to umount2() with MNT_EXPIRE expected success"},
>>    };
>>
>> -char *TCID = "umount2_02";
>> -int TST_TOTAL = ARRAY_SIZE(test_cases);
>> -
>> -int main(int ac, char **av)
>> +static void test_umount2(unsigned int n)
>>    {
>> -	int lc;
>> -	int tc;
>> +	struct tcase *tc =&tcases[n];
>>
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> -
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		tst_count = 0;
>> -
>> -		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
>> +	if (!mount_flag) {
>> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
>>    		mount_flag = 1;
>> -
>> -		for (tc = 0; tc<   TST_TOTAL; tc++)
>> -			test_umount2(tc);
>> -
>> -		if (mount_flag) {
>> -			if (tst_umount(MNTPOINT))
>> -				tst_brkm(TBROK, cleanup, "umount() failed");
>> -			mount_flag = 0;
>> -		}
>>    	}
>>
>> -	cleanup();
>> -	tst_exit();
>> -}
>> -
>> -static void setup(void)
>> -{
>> -	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");
>> -	}
>> -
>> -	tst_sig(NOFORK, DEF_HANDLER, NULL);
>> -
>> -	tst_tmpdir();
>> +	tst_res(TINFO, "Testing %s", tc->desc);
>>
>> -	fs_type = tst_dev_fs_type();
>> -	device = tst_acquire_device(cleanup);
>> +	if (tc->do_access)
>> +		SAFE_ACCESS(MNTPOINT, F_OK);
>>
>> -	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");
>> -	}
>> -
>> -	TEST(umount2_retry(MNTPOINT, test_cases[i].flag));
>> -
>> -	if (test_cases[i].exp_errno != 0)
>> -		verify_failure(i);
>> -	else
>> -		verify_success(i);
>> -}
>> -
>> -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 (TEST_ERRNO != test_cases[i].exp_errno) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> -	}
>> -
>> -	tst_resm(TPASS | TTERRNO, "umount2(2) failed as expected");
>> -}
>> -
>> -static void verify_success(int i)
>> -{
>> -	if (TEST_RETURN != 0) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> +	if (tc->exp_errno) {
>> +		TST_EXP_FAIL(umount2_retry(MNTPOINT, tc->flag), tc->exp_errno,
>> +			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
>> +		if (!TST_PASS)
>> +			mount_flag = 0;
>> +	} else {
>> +		TST_EXP_PASS(umount2_retry(MNTPOINT, tc->flag),
>> +			"umount2_retry(%s, %d)", MNTPOINT, tc->flag);
>> +		if (TST_PASS)
>> +			mount_flag = 0;
>>    	}
>> -
>> -	tst_resm(TPASS, "umount2(2) succeeded as expected");
>> -	mount_flag = 0;
>>    }
>>
>>    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,
>> +	.needs_root = 1,
>> +	.needs_device = 1,
>> +	.format_device = 1,
If we specify format_device, it wil assign value for needs_device a 
utomatically. So we can remove redundant needs_device.
>> +	.mntpoint = MNTPOINT,
>> +	.test = test_umount2,
>> +};
>> diff --git a/testcases/kernel/syscalls/umount2/umount2_03.c b/testcases/kernel/syscalls/umount2/umount2_03.c
>> index a8fddf6..f44ff79 100644
>> --- a/testcases/kernel/syscalls/umount2/umount2_03.c
>> +++ b/testcases/kernel/syscalls/umount2/umount2_03.c
>> @@ -1,167 +1,80 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>    /*
>> - * Copyright (c) 2015 Fujitsu Ltd.
>> + * Copyright (c) 2015-2022 FUJITSU LIMITED. All rights reserved
>>     * Author: Guangwen Feng<fenggw-fnst@cn.fujitsu.com>
>> - *
>> - * This program is free software; you can redistribute it and/or modify it
>> - * under the terms of version 2 of the GNU General Public License as
>> - * published by the Free Software Foundation.
>> - *
>> - * This program is distributed in the hope that it would be useful, but
>> - * WITHOUT ANY WARRANTY; without even the implied warranty of
>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> - *
>> - * You should have received a copy of the GNU General Public License
>> - * alone with this program.
>>     */
>>
>>    /*
Here as well.

Best Regards
Yang Xu
>> - * DESCRIPTION
>> + * [Description]
>> + *
>>     *  Test for feature UMOUNT_NOFOLLOW of umount2().
>> - *  "Don't dereference target if it is a symbolic link,
>> - *   and fails with the error EINVAL."
>> + *
>> + * - 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 "tst_test.h"
>>    #include "lapi/mount.h"
>> -
>>    #include "umount2.h"
>>
>> -#define DIR_MODE	(S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH)
>>    #define MNTPOINT	"mntpoint"
>>    #define SYMLINK	"symlink"
>>
>> -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;
>> -
>>    static int mount_flag;
>>
>>    static struct test_case_t {
>>    	const char *mntpoint;
>>    	int exp_errno;
>>    	const char *desc;
>> -} test_cases[] = {
>> +} tcase[] = {
>>    	{SYMLINK, EINVAL,
>>    		"umount2('symlink', UMOUNT_NOFOLLOW) expected EINVAL"},
>>    	{MNTPOINT, 0,
>>    		"umount2('mntpoint', UMOUNT_NOFOLLOW) expected success"},
>>    };
>>
>> -char *TCID = "umount2_03";
>> -int TST_TOTAL = ARRAY_SIZE(test_cases);
>> -
>> -int main(int ac, char **av)
>> +static void test_umount2(unsigned int n)
>>    {
>> -	int lc;
>> -	int tc;
>> -
>> -	tst_parse_opts(ac, av, NULL, NULL);
>> -
>> -	setup();
>> +	struct test_case_t *tc =&tcase[n];
>>
>> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -		tst_count = 0;
>> -
>> -		for (tc = 0; tc<   TST_TOTAL; tc++)
>> -			test_umount2(tc);
>> +	if (!mount_flag) {
>> +		SAFE_MOUNT(tst_device->dev, MNTPOINT, tst_device->fs_type, 0, NULL);
>> +		mount_flag = 1;
>>    	}
>>
>> -	cleanup();
>> -	tst_exit();
>> -}
>> -
>> -static void setup(void)
>> -{
>> -	tst_require_root();
>> -
>> -	if ((tst_kvercmp(2, 6, 34))<   0) {
>> -		tst_brkm(TCONF, NULL, "This test can only run on kernels "
>> -			 "that are 2.6.34 or higher");
>> -	}
>> -
>> -	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);
>> -
>> -	SAFE_SYMLINK(cleanup, MNTPOINT, SYMLINK);
>> -
>> -	TEST_PAUSE;
>> -}
>> -
>> -static void test_umount2(int i)
>> -{
>> -	SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
>> -	mount_flag = 1;
>> -
>> -	TEST(umount2_retry(test_cases[i].mntpoint, UMOUNT_NOFOLLOW));
>> -
>> -	if (test_cases[i].exp_errno != 0)
>> -		verify_failure(i);
>> -	else
>> -		verify_success(i);
>> -
>> -	if (mount_flag) {
>> -		if (tst_umount(MNTPOINT))
>> -			tst_brkm(TBROK, cleanup, "umount() failed");
>> -		mount_flag = 0;
>> +	tst_res(TINFO, "Testing %s", tc->desc);
>> +
>> +	if (tc->exp_errno) {
>> +		TST_EXP_FAIL(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW), tc->exp_errno,
>> +			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
>> +		if (!TST_PASS)
>> +			mount_flag = 0;
>> +	} else {
>> +		TST_EXP_PASS(umount2_retry(tc->mntpoint, UMOUNT_NOFOLLOW),
>> +			"umount2_retry(%s, %d)", tc->mntpoint, UMOUNT_NOFOLLOW);
>> +		if (TST_PASS)
>> +			mount_flag = 0;
>>    	}
>>    }
>>
>> -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 (TEST_ERRNO != test_cases[i].exp_errno) {
>> -		tst_resm(TFAIL | TTERRNO, "%s failed unexpectedly",
>> -			 test_cases[i].desc);
>> -		return;
>> -	}
>> -
>> -	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(tcase),
>> +	.needs_tmpdir = 1,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.format_device = 1,
>> +	.mntpoint = MNTPOINT,
>> +	.test = test_umount2,
>> +};
>

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