public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [PATCH] unlink: Add error tests for EPERM and EROFS
@ 2024-04-01  2:49 Yang Xu via ltp
  2024-04-08 15:14 ` Avinesh Kumar
  0 siblings, 1 reply; 4+ messages in thread
From: Yang Xu via ltp @ 2024-04-01  2:49 UTC (permalink / raw)
  To: ltp

Add negative cases for unlink(), when errno is EPERM or EROFS.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/unlink/.gitignore |  1 +
 testcases/kernel/syscalls/unlink/unlink09.c | 86 +++++++++++++++++++++
 3 files changed, 88 insertions(+)
 create mode 100644 testcases/kernel/syscalls/unlink/unlink09.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 4ed2b5602..b99ce7170 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1651,6 +1651,7 @@ unlink01 symlink01 -T unlink01
 unlink05 unlink05
 unlink07 unlink07
 unlink08 unlink08
+unlink09 unlink09
 
 #unlinkat test cases
 unlinkat01 unlinkat01
diff --git a/testcases/kernel/syscalls/unlink/.gitignore b/testcases/kernel/syscalls/unlink/.gitignore
index 2e783580d..6038cc29d 100644
--- a/testcases/kernel/syscalls/unlink/.gitignore
+++ b/testcases/kernel/syscalls/unlink/.gitignore
@@ -1,3 +1,4 @@
 /unlink05
 /unlink07
 /unlink08
+/unlink09
diff --git a/testcases/kernel/syscalls/unlink/unlink09.c b/testcases/kernel/syscalls/unlink/unlink09.c
new file mode 100644
index 000000000..b7ff94ee6
--- /dev/null
+++ b/testcases/kernel/syscalls/unlink/unlink09.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * Verify that unlink(2) fails with
+ *
+ * - EPERM when target file is marked as immutable or append-only
+ * - EROFS when target file is on a read-only filesystem.
+ */
+
+#include <errno.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include "tst_test.h"
+#include "tst_safe_macros.h"
+#include "lapi/fs.h"
+
+#define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
+#define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
+#define DIR_EROFS "erofs"
+#define TEST_EROFS "erofs/test_erofs"
+
+static int fd_immutable;
+static int fd_append_only;
+
+static struct test_case_t {
+	char *filename;
+	int expected_errno;
+	char *desc;
+} tcases[] = {
+	{TEST_EPERM_IMMUTABLE, EPERM, "target file is immutable"},
+	{TEST_EPERM_APPEND_ONLY, EPERM, "target file is append-only"},
+	{TEST_EROFS, EROFS, "target file in read-only filesystem"},
+};
+
+static void setup(void)
+{
+	int attr;
+
+	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
+	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
+	attr |= FS_IMMUTABLE_FL;
+	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
+
+	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
+	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
+	attr |= FS_APPEND_FL;
+	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
+}
+
+static void cleanup(void)
+{
+	int attr;
+
+	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
+	attr &= ~FS_IMMUTABLE_FL;
+	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
+	SAFE_CLOSE(fd_immutable);
+
+	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
+	attr &= ~FS_APPEND_FL;
+	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
+	SAFE_CLOSE(fd_append_only);
+}
+
+static void verify_unlink(unsigned int i)
+{
+	struct test_case_t *tc = &tcases[i];
+
+	TST_EXP_FAIL(unlink(tc->filename), tc->expected_errno, "%s", tc->desc);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.tcnt = ARRAY_SIZE(tcases),
+	.cleanup = cleanup,
+	.test = verify_unlink,
+	.needs_rofs = 1,
+	.mntpoint = DIR_EROFS,
+	.needs_root = 1,
+};
-- 
2.39.3


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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] unlink: Add error tests for EPERM and EROFS
  2024-04-01  2:49 [LTP] [PATCH] unlink: Add error tests for EPERM and EROFS Yang Xu via ltp
@ 2024-04-08 15:14 ` Avinesh Kumar
  2024-04-10  9:03   ` Yang Xu (Fujitsu) via ltp
  0 siblings, 1 reply; 4+ messages in thread
From: Avinesh Kumar @ 2024-04-08 15:14 UTC (permalink / raw)
  To: Yang Xu; +Cc: ltp

Hi Yang Xu,
some comments below

On Monday, April 1, 2024 4:49:48 AM CEST Yang Xu via ltp wrote:
> Add negative cases for unlink(), when errno is EPERM or EROFS.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  runtest/syscalls                            |  1 +
>  testcases/kernel/syscalls/unlink/.gitignore |  1 +
>  testcases/kernel/syscalls/unlink/unlink09.c | 86 +++++++++++++++++++++
>  3 files changed, 88 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/unlink/unlink09.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 4ed2b5602..b99ce7170 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1651,6 +1651,7 @@ unlink01 symlink01 -T unlink01
>  unlink05 unlink05
>  unlink07 unlink07
>  unlink08 unlink08
> +unlink09 unlink09
> 
>  #unlinkat test cases
>  unlinkat01 unlinkat01
> diff --git a/testcases/kernel/syscalls/unlink/.gitignore
> b/testcases/kernel/syscalls/unlink/.gitignore index 2e783580d..6038cc29d
> 100644
> --- a/testcases/kernel/syscalls/unlink/.gitignore
> +++ b/testcases/kernel/syscalls/unlink/.gitignore
> @@ -1,3 +1,4 @@
>  /unlink05
>  /unlink07
>  /unlink08
> +/unlink09
> diff --git a/testcases/kernel/syscalls/unlink/unlink09.c
> b/testcases/kernel/syscalls/unlink/unlink09.c new file mode 100644
> index 000000000..b7ff94ee6
> --- /dev/null
> +++ b/testcases/kernel/syscalls/unlink/unlink09.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that unlink(2) fails with
> + *
> + * - EPERM when target file is marked as immutable or append-only
> + * - EROFS when target file is on a read-only filesystem.
> + */
> +
> +#include <errno.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include "tst_test.h"
> +#include "tst_safe_macros.h"
> +#include "lapi/fs.h"
errno.h, unistd.h, ioctl.h and tst_safe_macros.h headers are already included
via tst_test.h
 
> +
> +#define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
> +#define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
> +#define DIR_EROFS "erofs"
> +#define TEST_EROFS "erofs/test_erofs"
> +
> +static int fd_immutable;
> +static int fd_append_only;
> +
> +static struct test_case_t {
> +	char *filename;
> +	int expected_errno;
> +	char *desc;
> +} tcases[] = {
> +	{TEST_EPERM_IMMUTABLE, EPERM, "target file is immutable"},
> +	{TEST_EPERM_APPEND_ONLY, EPERM, "target file is append-only"},
> +	{TEST_EROFS, EROFS, "target file in read-only filesystem"},
> +};
> +
> +static void setup(void)
> +{
> +	int attr;
> +
> +	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_IMMUTABLE_FL;
> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
> +
> +	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr |= FS_APPEND_FL;
> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
We can use SAFE_IOCTL() in all places.
> +}
> +
> +static void cleanup(void)
> +{
> +	int attr;
> +
> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_IMMUTABLE_FL;
> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_immutable);
> +
> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> +	attr &= ~FS_APPEND_FL;
> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
> +	SAFE_CLOSE(fd_append_only);
> +}
> +
> +static void verify_unlink(unsigned int i)
> +{
> +	struct test_case_t *tc = &tcases[i];
> +
> +	TST_EXP_FAIL(unlink(tc->filename), tc->expected_errno, "%s", tc->desc);
If for whatever reason, unlink() call passes here, further test iterations
will not work correctly as test file no longer exist. I guess we need to handle
the restoring in this main test function. And cleanup() does not need to reset
any flags as it is called only once after all the iterations.

> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.cleanup = cleanup,
> +	.test = verify_unlink,
> +	.needs_rofs = 1,
> +	.mntpoint = DIR_EROFS,
> +	.needs_root = 1,
> +};


Regards,
Avinesh




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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] unlink: Add error tests for EPERM and EROFS
  2024-04-08 15:14 ` Avinesh Kumar
@ 2024-04-10  9:03   ` Yang Xu (Fujitsu) via ltp
  2024-04-10 11:31     ` Avinesh Kumar
  0 siblings, 1 reply; 4+ messages in thread
From: Yang Xu (Fujitsu) via ltp @ 2024-04-10  9:03 UTC (permalink / raw)
  To: Avinesh Kumar; +Cc: ltp@lists.linux.it

Hi Avinesh.

Thanks for your comments.

> Hi Yang Xu,
> some comments below
> 
> On Monday, April 1, 2024 4:49:48 AM CEST Yang Xu via ltp wrote:
>> Add negative cases for unlink(), when errno is EPERM or EROFS.
>>
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>   runtest/syscalls                            |  1 +
>>   testcases/kernel/syscalls/unlink/.gitignore |  1 +
>>   testcases/kernel/syscalls/unlink/unlink09.c | 86 +++++++++++++++++++++
>>   3 files changed, 88 insertions(+)
>>   create mode 100644 testcases/kernel/syscalls/unlink/unlink09.c
>>
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 4ed2b5602..b99ce7170 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -1651,6 +1651,7 @@ unlink01 symlink01 -T unlink01
>>   unlink05 unlink05
>>   unlink07 unlink07
>>   unlink08 unlink08
>> +unlink09 unlink09
>>
>>   #unlinkat test cases
>>   unlinkat01 unlinkat01
>> diff --git a/testcases/kernel/syscalls/unlink/.gitignore
>> b/testcases/kernel/syscalls/unlink/.gitignore index 2e783580d..6038cc29d
>> 100644
>> --- a/testcases/kernel/syscalls/unlink/.gitignore
>> +++ b/testcases/kernel/syscalls/unlink/.gitignore
>> @@ -1,3 +1,4 @@
>>   /unlink05
>>   /unlink07
>>   /unlink08
>> +/unlink09
>> diff --git a/testcases/kernel/syscalls/unlink/unlink09.c
>> b/testcases/kernel/syscalls/unlink/unlink09.c new file mode 100644
>> index 000000000..b7ff94ee6
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/unlink/unlink09.c
>> @@ -0,0 +1,86 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
>> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Verify that unlink(2) fails with
>> + *
>> + * - EPERM when target file is marked as immutable or append-only
>> + * - EROFS when target file is on a read-only filesystem.
>> + */
>> +
>> +#include <errno.h>
>> +#include <unistd.h>
>> +#include <sys/ioctl.h>
>> +#include "tst_test.h"
>> +#include "tst_safe_macros.h"
>> +#include "lapi/fs.h"
> errno.h, unistd.h, ioctl.h and tst_safe_macros.h headers are already included
> via tst_test.h
>   
>> +
>> +#define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
>> +#define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
>> +#define DIR_EROFS "erofs"
>> +#define TEST_EROFS "erofs/test_erofs"
>> +
>> +static int fd_immutable;
>> +static int fd_append_only;
>> +
>> +static struct test_case_t {
>> +	char *filename;
>> +	int expected_errno;
>> +	char *desc;
>> +} tcases[] = {
>> +	{TEST_EPERM_IMMUTABLE, EPERM, "target file is immutable"},
>> +	{TEST_EPERM_APPEND_ONLY, EPERM, "target file is append-only"},
>> +	{TEST_EROFS, EROFS, "target file in read-only filesystem"},
>> +};
>> +
>> +static void setup(void)
>> +{
>> +	int attr;
>> +
>> +	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
>> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
>> +	attr |= FS_IMMUTABLE_FL;
>> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
>> +
>> +	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
>> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
>> +	attr |= FS_APPEND_FL;
>> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
> We can use SAFE_IOCTL() in all places.
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	int attr;
>> +
>> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
>> +	attr &= ~FS_IMMUTABLE_FL;
>> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
>> +	SAFE_CLOSE(fd_immutable);
>> +
>> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
>> +	attr &= ~FS_APPEND_FL;
>> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
>> +	SAFE_CLOSE(fd_append_only);
>> +}
>> +
>> +static void verify_unlink(unsigned int i)
>> +{
>> +	struct test_case_t *tc = &tcases[i];
>> +
>> +	TST_EXP_FAIL(unlink(tc->filename), tc->expected_errno, "%s", tc->desc);
> If for whatever reason, unlink() call passes here, further test iterations
> will not work correctly as test file no longer exist. I guess we need to handle
> the restoring in this main test function. And cleanup() does not need to reset
> any flags as it is called only once after all the iterations.
> 
In this case, every iteration uses a separate file. Even if unlink() 
passes unexpectedly, other iteration will not be affected.
So I think there is no need to restore the file.

The reset of flag is for the test file cleanup.

If flag is not reset, the test framework will throw a warning as following:
tst_tmpdir.c:343: TWARN: tst_rmdir: rmobj(/tmp/LTP_unlYn0Rtu) failed: 
unlink(/tmp/LTP_unlYn0Rtu/test_eperm_append_only) failed; errno=1: EPERM

In tst_tmpdir.c, unlink is called to remove the temp file and dir. So if 
flag is not reset, the temp file and dir will not be removed.

Soon I will send out V2 patch.

Best regards
Yang Xu

>> +}
>> +
>> +static struct tst_test test = {
>> +	.setup = setup,
>> +	.tcnt = ARRAY_SIZE(tcases),
>> +	.cleanup = cleanup,
>> +	.test = verify_unlink,
>> +	.needs_rofs = 1,
>> +	.mntpoint = DIR_EROFS,
>> +	.needs_root = 1,
>> +};
> 
> 
> Regards,
> Avinesh
> 
> 
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH] unlink: Add error tests for EPERM and EROFS
  2024-04-10  9:03   ` Yang Xu (Fujitsu) via ltp
@ 2024-04-10 11:31     ` Avinesh Kumar
  0 siblings, 0 replies; 4+ messages in thread
From: Avinesh Kumar @ 2024-04-10 11:31 UTC (permalink / raw)
  To: Yang Xu (Fujitsu); +Cc: ltp@lists.linux.it

Hi,

On Wednesday, April 10, 2024 11:03:19 AM CEST Yang Xu (Fujitsu) wrote:
> Hi Avinesh.
> 
> Thanks for your comments.
> 
> 
> > Hi Yang Xu,
> > some comments below
> > 
> > On Monday, April 1, 2024 4:49:48 AM CEST Yang Xu via ltp wrote:
> > 
> >> Add negative cases for unlink(), when errno is EPERM or EROFS.
> >>
> >>
> >>
> >> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> >> ---
> >> 
> >>   runtest/syscalls                            |  1 +
> >>   testcases/kernel/syscalls/unlink/.gitignore |  1 +
> >>   testcases/kernel/syscalls/unlink/unlink09.c | 86 +++++++++++++++++++++
> >>   3 files changed, 88 insertions(+)
> >>   create mode 100644 testcases/kernel/syscalls/unlink/unlink09.c
> >>
> >>
> >>
> >> diff --git a/runtest/syscalls b/runtest/syscalls
> >> index 4ed2b5602..b99ce7170 100644
> >> --- a/runtest/syscalls
> >> +++ b/runtest/syscalls
> >> @@ -1651,6 +1651,7 @@ unlink01 symlink01 -T unlink01
> >> 
> >>   unlink05 unlink05
> >>   unlink07 unlink07
> >>   unlink08 unlink08
> >> 
> >> +unlink09 unlink09
> >>
> >>
> >>
> >>   #unlinkat test cases
> >>   unlinkat01 unlinkat01
> >> 
> >> diff --git a/testcases/kernel/syscalls/unlink/.gitignore
> >> b/testcases/kernel/syscalls/unlink/.gitignore index 2e783580d..6038cc29d
> >> 100644
> >> --- a/testcases/kernel/syscalls/unlink/.gitignore
> >> +++ b/testcases/kernel/syscalls/unlink/.gitignore
> >> @@ -1,3 +1,4 @@
> >> 
> >>   /unlink05
> >>   /unlink07
> >>   /unlink08
> >> 
> >> +/unlink09
> >> diff --git a/testcases/kernel/syscalls/unlink/unlink09.c
> >> b/testcases/kernel/syscalls/unlink/unlink09.c new file mode 100644
> >> index 000000000..b7ff94ee6
> >> --- /dev/null
> >> +++ b/testcases/kernel/syscalls/unlink/unlink09.c
> >> @@ -0,0 +1,86 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved.
> >> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> >> + */
> >> +
> >> +/*\
> >> + * [Description]
> >> + *
> >> + * Verify that unlink(2) fails with
> >> + *
> >> + * - EPERM when target file is marked as immutable or append-only
> >> + * - EROFS when target file is on a read-only filesystem.
> >> + */
> >> +
> >> +#include <errno.h>
> >> +#include <unistd.h>
> >> +#include <sys/ioctl.h>
> >> +#include "tst_test.h"
> >> +#include "tst_safe_macros.h"
> >> +#include "lapi/fs.h"
> > 
> > errno.h, unistd.h, ioctl.h and tst_safe_macros.h headers are already
> > included  via tst_test.h
> > 
> >   
> >> 
> >> +
> >> +#define TEST_EPERM_IMMUTABLE "test_eperm_immutable"
> >> +#define TEST_EPERM_APPEND_ONLY "test_eperm_append_only"
> >> +#define DIR_EROFS "erofs"
> >> +#define TEST_EROFS "erofs/test_erofs"
> >> +
> >> +static int fd_immutable;
> >> +static int fd_append_only;
> >> +
> >> +static struct test_case_t {
> >> +	char *filename;
> >> +	int expected_errno;
> >> +	char *desc;
> >> +} tcases[] = {
> >> +	{TEST_EPERM_IMMUTABLE, EPERM, "target file is immutable"},
> >> +	{TEST_EPERM_APPEND_ONLY, EPERM, "target file is append-only"},
> >> +	{TEST_EROFS, EROFS, "target file in read-only filesystem"},
> >> +};
> >> +
> >> +static void setup(void)
> >> +{
> >> +	int attr;
> >> +
> >> +	fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
> >> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> >> +	attr |= FS_IMMUTABLE_FL;
> >> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
> >> +
> >> +	fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
> >> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> >> +	attr |= FS_APPEND_FL;
> >> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
> > 
> > We can use SAFE_IOCTL() in all places.
> > 
> >> +}
> >> +
> >> +static void cleanup(void)
> >> +{
> >> +	int attr;
> >> +
> >> +	ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
> >> +	attr &= ~FS_IMMUTABLE_FL;
> >> +	ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
> >> +	SAFE_CLOSE(fd_immutable);
> >> +
> >> +	ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);
> >> +	attr &= ~FS_APPEND_FL;
> >> +	ioctl(fd_append_only, FS_IOC_SETFLAGS, &attr);
> >> +	SAFE_CLOSE(fd_append_only);
> >> +}
> >> +
> >> +static void verify_unlink(unsigned int i)
> >> +{
> >> +	struct test_case_t *tc = &tcases[i];
> >> +
> >> +	TST_EXP_FAIL(unlink(tc->filename), tc->expected_errno, "%s",
> >> tc->desc);
> > 
> > If for whatever reason, unlink() call passes here, further test
> > iterations
> > will not work correctly as test file no longer exist. I guess we need to
> > handle  the restoring in this main test function. And cleanup() does not
> > need to reset any flags as it is called only once after all the
> > iterations.
> > 
> 
> In this case, every iteration uses a separate file. Even if unlink() 
> passes unexpectedly, other iteration will not be affected.
> So I think there is no need to restore the file.
> 
@@ -43,9 +44,9 @@ static void setup(void)
        int attr;
 
        fd_immutable = SAFE_OPEN(TEST_EPERM_IMMUTABLE, O_CREAT, 0600);
-       ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
-       attr |= FS_IMMUTABLE_FL;
-       ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
+       // ioctl(fd_immutable, FS_IOC_GETFLAGS, &attr);
+       // attr |= FS_IMMUTABLE_FL;
+       // ioctl(fd_immutable, FS_IOC_SETFLAGS, &attr);
 
        fd_append_only = SAFE_OPEN(TEST_EPERM_APPEND_ONLY, O_CREAT, 0600);
        ioctl(fd_append_only, FS_IOC_GETFLAGS, &attr);

just to cause the unlink() call pass and the file gets deleted.
If you run with this for i>=2, you will see the test failing with
ENOENT as the test file no longer available.

unlink09.c:82: TFAIL: target file is immutable succeeded
unlink09.c:82: TPASS: target file is append-only : EPERM (1)
unlink09.c:82: TPASS: target file in read-only filesystem : EROFS (30)
unlink09.c:82: TFAIL: target file is immutable expected EPERM: ENOENT (2)
unlink09.c:82: TPASS: target file is append-only : EPERM (1)
unlink09.c:82: TPASS: target file in read-only filesystem : EROFS (30)
unlink09.c:82: TFAIL: target file is immutable expected EPERM: ENOENT (2)
unlink09.c:82: TPASS: target file is append-only : EPERM (1)
unlink09.c:82: TPASS: target file in read-only filesystem : EROFS (30)


> The reset of flag is for the test file cleanup.
> 
> If flag is not reset, the test framework will throw a warning as following:
> tst_tmpdir.c:343: TWARN: tst_rmdir: rmobj(/tmp/LTP_unlYn0Rtu) failed:
> unlink(/tmp/LTP_unlYn0Rtu/test_eperm_append_only) failed; errno=1: EPERM 
> In tst_tmpdir.c, unlink is called to remove the temp file and dir. So if 
> flag is not reset, the temp file and dir will not be removed.
> 
okay, I agree, flag resetting is needed for deletion in the cleanup().

> Soon I will send out V2 patch.
> 
> Best regards
> Yang Xu
> 
> 
> >> +}
> >> +
> >> +static struct tst_test test = {
> >> +	.setup = setup,
> >> +	.tcnt = ARRAY_SIZE(tcases),
> >> +	.cleanup = cleanup,
> >> +	.test = verify_unlink,
> >> +	.needs_rofs = 1,
> >> +	.mntpoint = DIR_EROFS,
> >> +	.needs_root = 1,
> >> +};
> > 
> > 
> > 
> > Regards,
> > Avinesh
> > 
> > 

Regards,
Avinesh




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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-04-10 11:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-01  2:49 [LTP] [PATCH] unlink: Add error tests for EPERM and EROFS Yang Xu via ltp
2024-04-08 15:14 ` Avinesh Kumar
2024-04-10  9:03   ` Yang Xu (Fujitsu) via ltp
2024-04-10 11:31     ` Avinesh Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox