[LTP] [PATCH v2 3/3] syscalls/umount2: Convert to new API and use SAFE_ACCESS
xuyang2018.jy@fujitsu.com
xuyang2018.jy@fujitsu.com
Fri Mar 18 06:11:39 CET 2022
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,
>> +};
>
More information about the ltp
mailing list