syscalls/fallocate04: add new fallocate() test

From: "Alexey Kodanev" <[email protected]>
To: [email protected]
Cc: "vasily isaenko" <[email protected]>
Sent: Tuesday, 21 April, 2015 3:05:07 PM
Subject: [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test

This is a new functional test of fallocate() syscall with the focus on FALLOC_FL_ZERO_RANGE (since Linux 3.15) and FALLOC_FL_COLLAPSE_RANGE (since Linux 3.15) modes.

Steps of test-cases:
* allocate a file with specified size;
* make a hole in the middle of the file with FALLOC_FL_PUNCH_HOLE;
* fill the hole and adjacent space with FALLOC_FL_ZERO_RANGE;
* remove a block from a file with FALLOC_FL_COLLAPSE_RANGE

Signed-off-by Alexey Kodanev <[email protected]>

---
v4: corrected Linux version for FALLOC_FL_ZERO_RANGE
removed second Linux version check in test04()
added 'EOPNOTSUPP' check for initial fallocate()
v3: correctly indented second line in if blocks and tst_resm
v2: replaced lseek, read, write, etc. with LTP safe macros
moved FALLOC_FL_* macros to fallocate.h
removed FALLOC_FL_KEEP_SIZE from fallocate03 (it is now in fallocate.h) made one more test-case (split setup())

 runtest/syscalls                                  |    1 +
 testcases/kernel/syscalls/.gitignore              |    1 +
 testcases/kernel/syscalls/fallocate/fallocate.h   |   20 ++
 testcases/kernel/syscalls/fallocate/fallocate03.c |    1 -
 testcases/kernel/syscalls/fallocate/fallocate04.c |  273
 +++++++++++++++++++++
 5 files changed, 295 insertions(+), 1 deletions(-)
 create mode 100644 testcases/kernel/syscalls/fallocate/fallocate04.c

testcases/kernel/syscalls/fallocate/fallocate04.c new file +8 -0

new file mode 100644
index 0000000..f99492d
@@ -0,0 +1,273 @@
+/*
+ * Copyright (c) 2015 Oracle and/or its affiliates. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *

Quoted 3 lines

The rest looks fine. I may be tempted not to use tst_brkm(TCONF, ...) in the test functions and do tst_resm(TCONF, ...); return; instead, but that is minor thing.

It could be better to do so if each test-case (step) was independent, but each one (except the first) relies on the work done by the previous one.

SEEK_HOLE is only supported since version 3.1. Check the specified range blocks are zeroed while the kernel does not supported SEEK_HOLE.

Signed-off-by Zeng Linggang <[email protected]>

---
  testcases/kernel/syscalls/fallocate/fallocate04.c | 49 ++++++++++++++++++-----
  1 file changed, 39 insertions(+), 10 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -5

index 911bbe8..9d9587b 100644
@@ -50,6 +50,8 @@ static const option_t options[] = {
  	{NULL, NULL, NULL}
  };
 
+static off_t check_file_hole(int, off_t, off_t);
+
  static void help(void)
  {
  	printf("  -v      Verbose\n");

@@ -150,17 +152,12 @@ static void test02(void)
  	}
 
  	tst_resm(TINFO, "check that file has a hole with lseek(,,SEEK_HOLE)");
-	off_t ret = lseek(fd, 0, SEEK_HOLE);
 
-	if (ret != (ssize_t)block_size) {
-		/* exclude error when kernel doesn't have SEEK_HOLE support */
-		if (errno != EINVAL) {
-			tst_brkm(TFAIL | TERRNO, cleanup,

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: "vasily isaenko" <[email protected]>, [email protected]
Sent: Tuesday, 19 May, 2015 10:50:22 AM
Subject: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

SEEK_HOLE is only supported since version 3.1. Check the specified range blocks are zeroed while the kernel does not supported SEEK_HOLE.

Signed-off-by Zhang Jin <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -2

index 911bbe8..e94c572 100644
@@ -98,13 +98,13 @@ static void setup(void)
 	get_blocksize();
 }
 
-static void check_file_data(const char exp_buf[], size_t size)
+static void check_file_data_(const char exp_buf[], size_t size, off_t
 offset)
 {
 	char rbuf[size];
 
 	tst_resm(TINFO, "reading the file, compare with expected buffer");
 
-	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
+	SAFE_LSEEK(cleanup, fd, offset, SEEK_SET);
 	SAFE_READ(cleanup, 1, fd, rbuf, size);
 

testcases/kernel/syscalls/fallocate/fallocate04.c +5 -0

 	if (memcmp(exp_buf, rbuf, size)) {

@@ -116,6 +116,11 @@ static void check_file_data(const char exp_buf[], size_t
 size)
 	}
 }
 
+static inline void check_file_data(const char exp_buf[], size_t size)
+{
+	check_file_data_(exp_buf, size, 0);
+}
+
 static void test01(void)
 {

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

 	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);

----- Original Message -----

Quoted 50 lines

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: "vasily isaenko" <[email protected]>, [email protected]
Sent: Tuesday, 19 May, 2015 10:50:22 AM
Subject: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

SEEK_HOLE is only supported since version 3.1. Check the specified range blocks are zeroed while the kernel does not supported SEEK_HOLE.

Signed-off-by Zhang Jin <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -2

index 911bbe8..e94c572 100644
@@ -98,13 +98,13 @@ static void setup(void)
 	get_blocksize();
 }
 
-static void check_file_data(const char exp_buf[], size_t size)
+static void check_file_data_(const char exp_buf[], size_t size, off_t
 offset)
 {
 	char rbuf[size];
 
 	tst_resm(TINFO, "reading the file, compare with expected buffer");
 
-	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
+	SAFE_LSEEK(cleanup, fd, offset, SEEK_SET);
 	SAFE_READ(cleanup, 1, fd, rbuf, size);
 

testcases/kernel/syscalls/fallocate/fallocate04.c +5 -0

 	if (memcmp(exp_buf, rbuf, size)) {

@@ -116,6 +116,11 @@ static void check_file_data(const char exp_buf[], size_t
 size)
 	}
 }
 
+static inline void check_file_data(const char exp_buf[], size_t size)
+{
+	check_file_data_(exp_buf, size, 0);
+}
+
 static void test01(void)
 {

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

 	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 168

Quoted 7 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -1

@@ -158,7 +163,9 @@ static void test02(void)
 			tst_brkm(TFAIL | TERRNO, cleanup,
 				 "fallocate() or lseek() failed");
 		}
-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+		char zeros[block_size];
+		memset(zeros, 0, block_size);
+		check_file_data_(zeros, block_size, block_size);

Hi,

Isn't this redundant?

Couple lines below is this check, which also checks that range <block_size, block_size*2> is zeroed.

        char exp_buf[buf_size];

        fill_tst_buf(exp_buf);
        memset(exp_buf + block_size, 0, block_size);
        check_file_data(exp_buf, buf_size);

Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

index 911bbe8..45d9827 100644

testcases/kernel/syscalls/fallocate/fallocate04.c +4 -1

+                       tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
+                               "this is expected for < 3.1 kernels");
+       } else {
+               tst_resm(TINFO, "found a hole at '%ld' offset", ret);
        }
-       tst_resm(TINFO, "found a hole at '%ld' offset", ret);
 
        size_t alloc_size1 = get_allocsize();
 

Regards,
Jan

From: "Zeng Linggang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "Alexey Kodanev" <[email protected]>, "vasily isaenko" <[email protected]>, [email protected]
Sent: Wednesday, 20 May, 2015 3:47:22 AM
Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

Hi!

Quoted 67 lines · Jan Stancek

----- Original Message -----

Quoted 53 lines

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: "vasily isaenko" <[email protected]>, [email protected]
Sent: Tuesday, 19 May, 2015 10:50:22 AM
Subject: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not
supported

SEEK_HOLE is only supported since version 3.1. Check the specified range blocks are zeroed while the kernel does not supported SEEK_HOLE.

Signed-off-by Zhang Jin <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -2

index 911bbe8..e94c572 100644
@@ -98,13 +98,13 @@ static void setup(void)
 	get_blocksize();
 }
 
-static void check_file_data(const char exp_buf[], size_t size)
+static void check_file_data_(const char exp_buf[], size_t size, off_t
 offset)
 {
 	char rbuf[size];
 
 	tst_resm(TINFO, "reading the file, compare with expected buffer");
 
-	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
+	SAFE_LSEEK(cleanup, fd, offset, SEEK_SET);
 	SAFE_READ(cleanup, 1, fd, rbuf, size);
 

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

 	if (memcmp(exp_buf, rbuf, size)) {

@@ -116,6 +116,11 @@ static void check_file_data(const char exp_buf[],

size_t
size)

testcases/kernel/syscalls/fallocate/fallocate04.c +5 -0

 	}
 }
 
+static inline void check_file_data(const char exp_buf[], size_t size)
+{
+	check_file_data_(exp_buf, size, 0);
+}
+
 static void test01(void)
 {
 	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 168

Quoted 7 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -1

@@ -158,7 +163,9 @@ static void test02(void)
 			tst_brkm(TFAIL | TERRNO, cleanup,
 				 "fallocate() or lseek() failed");
 		}
-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+		char zeros[block_size];
+		memset(zeros, 0, block_size);
+		check_file_data_(zeros, block_size, block_size);

Hi,

Quoted 1 line

Isn't this redundant?

To be honest, I do not think it is redundant.

Quoted 4 lines

Couple lines below is this check, which also checks that range <block_size, block_size*2> is zeroed.

Yep, this looks simple.
This maybe cost more runtime because of 'fill_tst_buf'. But I do not reject it.

Quoted 10 lines

        char exp_buf[buf_size];

        fill_tst_buf(exp_buf);
        memset(exp_buf + block_size, 0, block_size);
        check_file_data(exp_buf, buf_size);

Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 162

Quoted 12 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

index 911bbe8..45d9827 100644
@@ -158,9 +158,12 @@ static void test02(void)
                        tst_brkm(TFAIL | TERRNO, cleanup,
                                 "fallocate() or lseek() failed");
                }
-               tst_resm(TWARN | TERRNO, "lseek() doesn't support
 SEEK_HOLE");
+               if (tst_kvercmp(3, 1, 0) < 0)

How about put them before lseek(SEEK_HOLE) ?

if (tst_kvercmp(3, 1, 0) < 0) {
	fill_tst_buf();
	memset();
	check_file_data();
} else {
	lseek(SEEK_HOLE);
	...
}

Thank you very much.

Best regards,
Zeng

Quoted 12 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -0

+                       tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE,

"

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -1

+                               "this is expected for < 3.1 kernels");
+       } else {
+               tst_resm(TINFO, "found a hole at '%ld' offset", ret);
        }
-       tst_resm(TINFO, "found a hole at '%ld' offset", ret);
 
        size_t alloc_size1 = get_allocsize();

Regards,
Jan

----- Original Message -----

Quoted 77 lines

From: "Zeng Linggang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "Alexey Kodanev" <[email protected]>, "vasily isaenko" <[email protected]>, [email protected]
Sent: Wednesday, 20 May, 2015 3:47:22 AM
Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

Hi!

Quoted 67 lines · Jan Stancek

----- Original Message -----

Quoted 53 lines

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: "vasily isaenko" <[email protected]>, [email protected]
Sent: Tuesday, 19 May, 2015 10:50:22 AM
Subject: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not
supported

SEEK_HOLE is only supported since version 3.1. Check the specified range blocks are zeroed while the kernel does not supported SEEK_HOLE.

Signed-off-by Zhang Jin <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -2

index 911bbe8..e94c572 100644
@@ -98,13 +98,13 @@ static void setup(void)
 	get_blocksize();
 }
 
-static void check_file_data(const char exp_buf[], size_t size)
+static void check_file_data_(const char exp_buf[], size_t size, off_t
 offset)
 {
 	char rbuf[size];
 
 	tst_resm(TINFO, "reading the file, compare with expected buffer");
 
-	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
+	SAFE_LSEEK(cleanup, fd, offset, SEEK_SET);
 	SAFE_READ(cleanup, 1, fd, rbuf, size);
 

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

 	if (memcmp(exp_buf, rbuf, size)) {

@@ -116,6 +116,11 @@ static void check_file_data(const char exp_buf[],

size_t
size)

testcases/kernel/syscalls/fallocate/fallocate04.c +5 -0

 	}
 }
 
+static inline void check_file_data(const char exp_buf[], size_t size)
+{
+	check_file_data_(exp_buf, size, 0);
+}
+
 static void test01(void)
 {
 	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 168

Quoted 7 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -1

@@ -158,7 +163,9 @@ static void test02(void)
 			tst_brkm(TFAIL | TERRNO, cleanup,
 				 "fallocate() or lseek() failed");
 		}
-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+		char zeros[block_size];
+		memset(zeros, 0, block_size);
+		check_file_data_(zeros, block_size, block_size);

Hi,

Quoted 3 lines

Quoted 1 line

Isn't this redundant?

To be honest, I do not think it is redundant.

Can you explain why?

Quoted 8 lines

Quoted 4 lines

Couple lines below is this check, which also checks that range <block_size, block_size*2> is zeroed.

Yep, this looks simple.
This maybe cost more runtime because of 'fill_tst_buf'. But I do not reject it.

That code is already there, your patch is not removing it.

Quoted 10 lines

Quoted 10 lines

        char exp_buf[buf_size];

        fill_tst_buf(exp_buf);
        memset(exp_buf + block_size, 0, block_size);
        check_file_data(exp_buf, buf_size);

Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?

Quoted 14 lines

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 162

Quoted 12 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

index 911bbe8..45d9827 100644
@@ -158,9 +158,12 @@ static void test02(void)
                        tst_brkm(TFAIL | TERRNO, cleanup,
                                 "fallocate() or lseek() failed");
                }
-               tst_resm(TWARN | TERRNO, "lseek() doesn't support
 SEEK_HOLE");
+               if (tst_kvercmp(3, 1, 0) < 0)

How about put them before lseek(SEEK_HOLE) ?

I think we should do the check on all kernels. lseek() can only check that the hole exists, but current check also verifies that hole has correct size and content.

Regards,
Jan

From: "Zeng Linggang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "Alexey Kodanev" <[email protected]>, "vasily isaenko" <[email protected]>, [email protected]
Sent: Wednesday, 20 May, 2015 11:58:01 AM
Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

Hi,

Quoted 131 lines · Jan Stancek

----- Original Message -----

Quoted 84 lines

From: "Zeng Linggang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "Alexey Kodanev" <[email protected]>, "vasily isaenko" <[email protected]>,
[email protected]
Sent: Wednesday, 20 May, 2015 3:47:22 AM
Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

Hi!

Quoted 72 lines · Jan Stancek

----- Original Message -----

Quoted 58 lines

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: "vasily isaenko" <[email protected]>, [email protected]
Sent: Tuesday, 19 May, 2015 10:50:22 AM
Subject: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not
supported

SEEK_HOLE is only supported since version 3.1. Check the specified range blocks are zeroed while the kernel does not supported SEEK_HOLE.

Signed-off-by Zhang Jin <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

index 911bbe8..e94c572 100644
@@ -98,13 +98,13 @@ static void setup(void)
 	get_blocksize();
 }
 
-static void check_file_data(const char exp_buf[], size_t size)
+static void check_file_data_(const char exp_buf[], size_t size,

off_t
offset)
{

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

 	char rbuf[size];
 
 	tst_resm(TINFO, "reading the file, compare with expected buffer");
 
-	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
+	SAFE_LSEEK(cleanup, fd, offset, SEEK_SET);
 	SAFE_READ(cleanup, 1, fd, rbuf, size);
 
 	if (memcmp(exp_buf, rbuf, size)) {

@@ -116,6 +116,11 @@ static void check_file_data(const char
 exp_buf[],

size_t
size)

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -0

 	}
 }
 
+static inline void check_file_data(const char exp_buf[], size_t

size)

testcases/kernel/syscalls/fallocate/fallocate04.c +4 -0

+{
+	check_file_data_(exp_buf, size, 0);
+}
+
 static void test01(void)
 {
 	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 168

Quoted 7 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +3 -1

@@ -158,7 +163,9 @@ static void test02(void)
 			tst_brkm(TFAIL | TERRNO, cleanup,
 				 "fallocate() or lseek() failed");
 		}
-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+		char zeros[block_size];
+		memset(zeros, 0, block_size);
+		check_file_data_(zeros, block_size, block_size);

Hi,

Quoted 3 lines

Quoted 1 line

Isn't this redundant?

To be honest, I do not think it is redundant.

Can you explain why?

Quoted 8 lines

Quoted 4 lines

Couple lines below is this check, which also checks that range <block_size, block_size*2> is zeroed.

Yep, this looks simple.
This maybe cost more runtime because of 'fill_tst_buf'. But I do not reject it.

That code is already there, your patch is not removing it.

Quoted 25 lines

Quoted 17 lines

        char exp_buf[buf_size];

        fill_tst_buf(exp_buf);
        memset(exp_buf + block_size, 0, block_size);
        check_file_data(exp_buf, buf_size);

Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?

testcases/kernel/syscalls/fallocate/fallocate04.c +0 -0

index 911bbe8..45d9827 100644

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 162

Quoted 6 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

@@ -158,9 +158,12 @@ static void test02(void)
                        tst_brkm(TFAIL | TERRNO, cleanup,
                                 "fallocate() or lseek() failed");
                }
-               tst_resm(TWARN | TERRNO, "lseek() doesn't support
 SEEK_HOLE");
+               if (tst_kvercmp(3, 1, 0) < 0)

How about put them before lseek(SEEK_HOLE) ?

Quoted 5 lines

I think we should do the check on all kernels. lseek() can only check that the
hole exists, but current check also verifies that hole has correct size and content.

Right. It is the strict way we need to do. But it would make the test complex.

Now, "turn that warning into TINFO" looks simple but effective. If nobody reject, I will send new patch like:

- tst_resm(TWARN | TERRNO, "lseek() doesn't support
SEEK_HOLE");

testcases/kernel/syscalls/fallocate/fallocate04.c +5 -1

+               if (tst_kvercmp(3, 1, 0) < 0)
+                       tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
+                               "this is expected for < 3.1 kernels");
+       } else {
+               tst_resm(TINFO, "found a hole at '%ld' offset", ret);
        }
-       tst_resm(TINFO, "found a hole at '%ld' offset", ret);

Thank you.

Best regards,
Zeng

Quoted 2 lines

Regards,
Jan

[...]

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

How about put them before lseek(SEEK_HOLE) ?

I think we should do the check on all kernels. lseek() can only check that the
hole exists, but current check also verifies that hole has correct size and content.

Quoted 2 lines

Right. It is the strict way we need to do. But it would make the test complex.

I think we already check it strictly, see line 172-177. This covers blocks 1,2,3. The patch you proposed _adds_ one more check only for zero-ed block 2. My objection was that this seems redundant, because we already have a check (line 172-177), which covers all 3 blocks. Am I missing something?

Quoted 2 lines

Now, "turn that warning into TINFO" looks simple but effective. If nobody reject, I will send new patch like:

Looks OK to me, though maybe it should TBROK on kernels > 3.1 even when errno is EINVAL.

Regards,
Jan

Quoted 22 lines

- tst_resm(TWARN | TERRNO, "lseek() doesn't support
SEEK_HOLE");

testcases/kernel/syscalls/fallocate/fallocate04.c +5 -1

+               if (tst_kvercmp(3, 1, 0) < 0)
+                       tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
+                               "this is expected for < 3.1 kernels");
+       } else {
+               tst_resm(TINFO, "found a hole at '%ld' offset", ret);
        }
-       tst_resm(TINFO, "found a hole at '%ld' offset", ret);

Thank you.

Best regards,
Zeng

Quoted 2 lines

Regards,
Jan

[...]

Hi!

On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 161

Quoted 12 lines

Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

index 911bbe8..45d9827 100644
@@ -158,9 +158,12 @@ static void test02(void)
                         tst_brkm(TFAIL | TERRNO, cleanup,
                                  "fallocate() or lseek() failed");
                 }
-               tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+               if (tst_kvercmp(3, 1, 0) < 0)

How about put them before lseek(SEEK_HOLE) ?

if (tst_kvercmp(3, 1, 0) < 0) {
	fill_tst_buf();
	memset();
	check_file_data();
} else {
	lseek(SEEK_HOLE);
	...
}

Hi,

Quoted 3 lines · Zeng Linggang

Hi!

On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:

[ ... ]

Quoted 14 lines

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void) · line 161

Quoted 12 lines

Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?

testcases/kernel/syscalls/fallocate/fallocate04.c +1 -1

index 911bbe8..45d9827 100644
@@ -158,9 +158,12 @@ static void test02(void)
                         tst_brkm(TFAIL | TERRNO, cleanup,
                                  "fallocate() or lseek() failed");
                 }
-               tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+               if (tst_kvercmp(3, 1, 0) < 0)

How about put them before lseek(SEEK_HOLE) ?

Quoted 8 lines

if (tst_kvercmp(3, 1, 0) < 0) {
	fill_tst_buf();
	memset();
	check_file_data();
} else {
	lseek(SEEK_HOLE);
	...
}

Please don't complicate the test-case with this, I agree with Jan, it is enough to add the kernel version check. It can be as follows:

testcases/kernel/syscalls/fallocate/fallocate04.c +2 -1

@@ -158,7 +158,8 @@ static void test02(void)
                         tst_brkm(TFAIL | TERRNO, cleanup,
                                  "fallocate() or lseek() failed");
                 }
-               tst_resm(TWARN | TERRNO, "lseek() doesn't support 
 SEEK_HOLE");
+               tst_resm((tst_kvercmp(3, 1, 0) < 0) ? TINFO : (TWARN | 
 TERRNO),
+                        "lseek() doesn't support SEEK_HOLE");
         }

         tst_resm(TINFO, "found a hole at '%ld' offset", ret);

Best regards,
Alexey

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: [email protected], "vasily isaenko" <[email protected]>
Sent: Thursday, 21 May, 2015 7:39:37 AM
Subject: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL

SEEK_HOLE is only supported since version 3.1. Just print some information to remind users if kernel is before 3.1, if not print 'TBROK' and quit, instead of 'TWARN'.

Signed-off-by Zeng Linggang <[email protected]>

Signed-off-by Jan Stancek <[email protected]>

Signed-off-by Alexey Kodanev <[email protected]>

---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

testcases/kernel/syscalls/fallocate/fallocate04.c +8 -3

index 911bbe8..0c904a0 100644
@@ -158,9 +158,15 @@ static void test02(void)
 			tst_brkm(TFAIL | TERRNO, cleanup,
 				 "fallocate() or lseek() failed");
 		}
-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+		if (tst_kvercmp(3, 1, 0) < 0)
+			tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
+				 "this is expected for < 3.1 kernels");
+		else
+			tst_brkm(TBROK | TERRNO, cleanup,
+				 "lseek() doesn't support SEEK_HOLE");
+	} else {
+		tst_resm(TINFO, "found a hole at '%ld' offset", ret);
 	}
-	tst_resm(TINFO, "found a hole at '%ld' offset", ret);
 
 	size_t alloc_size1 = get_allocsize();
 
--

1.9.3

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

----- Original Message -----

Quoted 9 lines

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: [email protected], "vasily isaenko" <[email protected]>
Sent: Thursday, 21 May, 2015 7:39:37 AM
Subject: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL

SEEK_HOLE is only supported since version 3.1. Just print some information to remind users if kernel is before 3.1, if not print 'TBROK' and quit, instead of 'TWARN'.

Looks OK to me. Alexey, any objections from your side?

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void)

Quoted 7 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +6 -1

-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+		if (tst_kvercmp(3, 1, 0) < 0)
+			tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
+				 "this is expected for < 3.1 kernels");
+		else
+			tst_brkm(TBROK | TERRNO, cleanup,
+		

From: "Alexey Kodanev" <[email protected]>
To: "Jan Stancek" <[email protected]>, "Zeng Linggang" <[email protected]>
Cc: [email protected], "vasily isaenko" <[email protected]>
Sent: Thursday, 4 June, 2015 1:35:50 PM
Subject: Re: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL

Hi,

Quoted 9 lines · Jan Stancek

----- Original Message -----

Quoted 8 lines

From: "Zeng Linggang" <[email protected]>
To: "Alexey Kodanev" <[email protected]>
Cc: [email protected], "vasily isaenko" <[email protected]>
Sent: Thursday, 21 May, 2015 7:39:37 AM
Subject: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL

Quoted 4 lines

Quoted 3 lines

SEEK_HOLE is only supported since version 3.1. Just print some information to remind users if kernel is before 3.1, if not print 'TBROK' and quit, instead of 'TWARN'.

Looks OK to me. Alexey, any objections from your side?

Sorry for the delay, looks good to me. There is stylistic comment below, but it is minor:

Quoted 7 lines

testcases/kernel/syscalls/fallocate/fallocate04.c static void test02(void)

Quoted 7 lines

testcases/kernel/syscalls/fallocate/fallocate04.c +6 -1

-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+		if (tst_kvercmp(3, 1, 0) < 0)
+			tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
+				 "this is expected for < 3.1 kernels");
+		else
+			tst_brkm(TBROK | TERRNO, cleanup,
+

It would be better to add curly braces around multiline 'if else' block.

Thanks,
Alexey