All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test
@ 2015-04-21 13:05 Alexey Kodanev
  2015-04-23  7:44 ` Jan Stancek
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Alexey Kodanev @ 2015-04-21 13:05 UTC (permalink / raw)
  To: ltp-list; +Cc: vasily.isaenko

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 <alexey.kodanev@oracle.com>
---
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

diff --git a/runtest/syscalls b/runtest/syscalls
index 7703825..126f46f 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -154,6 +154,7 @@ faccessat01 faccessat01
 fallocate01 fallocate01
 fallocate02 fallocate02
 fallocate03 fallocate03
+fallocate04 fallocate04
 
 #posix_fadvise test cases
 posix_fadvise01                      posix_fadvise01
diff --git a/testcases/kernel/syscalls/.gitignore b/testcases/kernel/syscalls/.gitignore
index f10e85e..afe7f3b 100644
--- a/testcases/kernel/syscalls/.gitignore
+++ b/testcases/kernel/syscalls/.gitignore
@@ -135,6 +135,7 @@
 /fallocate/fallocate01
 /fallocate/fallocate02
 /fallocate/fallocate03
+/fallocate/fallocate04
 /fchdir/fchdir01
 /fchdir/fchdir02
 /fchdir/fchdir03
diff --git a/testcases/kernel/syscalls/fallocate/fallocate.h b/testcases/kernel/syscalls/fallocate/fallocate.h
index b6c2203..1900aa0 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate.h
+++ b/testcases/kernel/syscalls/fallocate/fallocate.h
@@ -27,6 +27,26 @@
 #include "lapi/abisize.h"
 #include "linux_syscall_numbers.h"
 
+#ifndef SEEK_HOLE
+#define SEEK_HOLE 4
+#endif
+
+#ifndef FALLOC_FL_KEEP_SIZE
+#define FALLOC_FL_KEEP_SIZE 0x01
+#endif
+
+#ifndef FALLOC_FL_PUNCH_HOLE
+#define FALLOC_FL_PUNCH_HOLE 0x02
+#endif
+
+#ifndef FALLOC_FL_COLLAPSE_RANGE
+#define FALLOC_FL_COLLAPSE_RANGE 0x08
+#endif
+
+#ifndef FALLOC_FL_ZERO_RANGE
+#define FALLOC_FL_ZERO_RANGE 0x10
+#endif
+
 #if !defined(HAVE_FALLOCATE)
 static inline long fallocate(int fd, int mode, loff_t offset, loff_t len)
 {
diff --git a/testcases/kernel/syscalls/fallocate/fallocate03.c b/testcases/kernel/syscalls/fallocate/fallocate03.c
index ae2476a..092d2bb 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate03.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate03.c
@@ -101,7 +101,6 @@
 #define BLOCKS_WRITTEN 12
 #define HOLE_SIZE_IN_BLOCKS 12
 #define DEFAULT_MODE 0
-#define FALLOC_FL_KEEP_SIZE 1	//Need to be removed once the glibce support is provided
 #define TRUE 0
 
 void get_blocksize(int);
diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
new file mode 100644
index 0000000..f99492d
--- /dev/null
+++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
@@ -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.
+ *
+ * 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.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Author: Alexey Kodanev <alexey.kodanev@oracle.com>
+ */
+
+#define _GNU_SOURCE
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "test.h"
+#include "safe_macros.h"
+#include "fallocate.h"
+
+char *TCID = "fallocate04";
+int TST_TOTAL = 4;
+
+static int fd;
+static const char fname[] = "fallocate04.txt";
+static size_t block_size;
+static size_t buf_size;
+
+#define NUM_OF_BLOCKS	3
+
+static int verbose;
+static const option_t options[] = {
+	{"v", &verbose, NULL},
+	{NULL, NULL, NULL}
+};
+
+static void help(void)
+{
+	printf("  -v      Verbose\n");
+}
+
+static void cleanup(void)
+{
+	close(fd);
+	tst_rmdir();
+}
+
+static void get_blocksize(void)
+{
+	struct stat file_stat;
+
+	SAFE_FSTAT(cleanup, fd, &file_stat);
+
+	block_size = file_stat.st_blksize;
+	buf_size = NUM_OF_BLOCKS * block_size;
+}
+
+static size_t get_allocsize(void)
+{
+	struct stat file_stat;
+
+	SAFE_FSTAT(cleanup, fd, &file_stat);
+
+	return file_stat.st_blocks * 512;
+}
+
+static void fill_tst_buf(char buf[])
+{
+	/* fill the buffer with a, b, c, ... letters on each block */
+	int i;
+
+	for (i = 0; i < NUM_OF_BLOCKS; ++i)
+		memset(buf + i * block_size, 'a' + i, block_size);
+}
+
+static void setup(void)
+{
+	tst_tmpdir();
+
+	fd = SAFE_OPEN(cleanup, fname, O_RDWR | O_CREAT, 0700);
+
+	get_blocksize();
+}
+
+static void check_file_data(const char exp_buf[])
+{
+	size_t size = sizeof(exp_buf);
+	char rbuf[size];
+
+	tst_resm(TINFO, "reading the file, compare with expected buffer");
+
+	SAFE_LSEEK(cleanup, fd, 0, SEEK_SET);
+	SAFE_READ(cleanup, 1, fd, rbuf, size);
+
+	if (memcmp(exp_buf, rbuf, size)) {
+		if (verbose) {
+			tst_resm_hexd(TINFO, exp_buf, size, "expected:");
+			tst_resm_hexd(TINFO, rbuf, size, "but read:");
+		}
+		tst_brkm(TFAIL, cleanup, "not expected file data");
+	}
+}
+
+static void test01(void)
+{
+	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
+
+	if (fallocate(fd, 0, 0, buf_size) == -1) {
+		if (errno == ENOSYS || errno == EOPNOTSUPP)
+			tst_brkm(TCONF, cleanup, "fallocate() not supported");
+		tst_brkm(TFAIL | TERRNO, cleanup, "fallocate() failed");
+	}
+
+	char buf[buf_size];
+
+	fill_tst_buf(buf);
+
+	SAFE_WRITE(cleanup, 1, fd, buf, buf_size);
+
+	tst_resm(TPASS, "test-case succeeded");
+}
+
+static void test02(void)
+{
+	size_t alloc_size0 = get_allocsize();
+
+	tst_resm(TINFO, "read allocated file size '%zu'", alloc_size0);
+	tst_resm(TINFO, "make a hole with FALLOC_FL_PUNCH_HOLE");
+
+	if (fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+	    block_size, block_size) == -1) {
+		if (errno == EOPNOTSUPP)
+			tst_brkm(TCONF, cleanup, "operation not supported");
+		tst_brkm(TFAIL | TERRNO, cleanup, "fallocate() failed");
+	}
+
+	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,
+				 "fallocate() or lseek() failed");
+		}
+		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
+	}
+	tst_resm(TINFO, "found a hole at '%ld' offset", ret);
+
+	size_t alloc_size1 = get_allocsize();
+
+	tst_resm(TINFO, "allocated file size before '%zu' and after '%zu'",
+		 alloc_size0, alloc_size1);
+	if ((alloc_size0 - block_size) != alloc_size1)
+		tst_brkm(TFAIL, cleanup, "not expected allocated size");
+
+	char exp_buf[buf_size];
+
+	fill_tst_buf(exp_buf);
+	memset(exp_buf + block_size, 0, block_size);
+
+	check_file_data(exp_buf);
+
+	tst_resm(TPASS, "test-case succeeded");
+}
+
+static void test03(void)
+{
+	tst_resm(TINFO, "zeroing file space with FALLOC_FL_ZERO_RANGE");
+
+	if (tst_kvercmp(3, 15, 0) < 0) {
+		tst_brkm(TCONF, cleanup,
+			 "Test must be run with kernel 3.15 or newer");
+	}
+
+	size_t alloc_size0 = get_allocsize();
+
+	tst_resm(TINFO, "read current allocated file size '%zu'", alloc_size0);
+
+	if (fallocate(fd, FALLOC_FL_ZERO_RANGE, block_size - 1,
+	    block_size + 2) == -1) {
+		if (errno == EOPNOTSUPP)
+			tst_brkm(TCONF, cleanup, "operation not supported");
+		tst_brkm(TFAIL | TERRNO, cleanup, "fallocate failed");
+	}
+
+	/* The file hole in the specified range must be allocated and
+	 * filled with zeros. Check it.
+	 */
+	size_t alloc_size1 = get_allocsize();
+
+	tst_resm(TINFO, "allocated file size before '%zu' and after '%zu'",
+		 alloc_size0, alloc_size1);
+	if ((alloc_size0 + block_size) != alloc_size1)
+		tst_brkm(TFAIL, cleanup, "not expected allocated size");
+
+	char exp_buf[buf_size];
+
+	fill_tst_buf(exp_buf);
+	memset(exp_buf + block_size - 1, 0, block_size + 2);
+
+	check_file_data(exp_buf);
+
+	tst_resm(TPASS, "test-case succeeded");
+}
+
+static void test04(void)
+{
+	tst_resm(TINFO, "collapsing file space with FALLOC_FL_COLLAPSE_RANGE");
+
+	size_t alloc_size0 = get_allocsize();
+
+	tst_resm(TINFO, "read current allocated file size '%zu'", alloc_size0);
+
+	if (fallocate(fd, FALLOC_FL_COLLAPSE_RANGE, block_size,
+	    block_size) == -1) {
+		if (errno == EOPNOTSUPP)
+			tst_brkm(TCONF, cleanup, "operation not supported");
+		tst_brkm(TFAIL | TERRNO, cleanup, "fallocate failed");
+	}
+
+	size_t alloc_size1 = get_allocsize();
+
+	tst_resm(TINFO, "allocated file size before '%zu' and after '%zu'",
+		 alloc_size0, alloc_size1);
+	if ((alloc_size0 - block_size) != alloc_size1)
+		tst_brkm(TFAIL, cleanup, "not expected allocated size");
+
+	size_t size = buf_size - block_size;
+	char tmp_buf[buf_size];
+	char exp_buf[size];
+
+	fill_tst_buf(tmp_buf);
+
+	memcpy(exp_buf, tmp_buf, block_size);
+	memcpy(exp_buf + block_size, tmp_buf + size, block_size);
+
+	check_file_data(exp_buf);
+
+	tst_resm(TPASS, "test-case succeeded");
+}
+
+int main(int argc, char *argv[])
+{
+	int lc;
+
+	tst_parse_opts(argc, argv, options, help);
+
+	setup();
+
+	for (lc = 0; TEST_LOOPING(lc); ++lc) {
+		test01();
+		test02();
+		test03();
+		test04();
+	}
+
+	cleanup();
+
+	tst_exit();
+}
-- 
1.7.1


------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test
  2015-04-21 13:05 [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test Alexey Kodanev
@ 2015-04-23  7:44 ` Jan Stancek
  2015-04-27 14:07 ` Cyril Hrubis
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Jan Stancek @ 2015-04-23  7:44 UTC (permalink / raw)
  To: Alexey Kodanev; +Cc: vasily isaenko, ltp-list





----- Original Message -----
> From: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> To: ltp-list@lists.sourceforge.net
> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>
> 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 <alexey.kodanev@oracle.com>
> ---
> 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
> 

v4 looks good, older kernels now end with TCONF. ACK

Regards,
Jan

------------------------------------------------------------------------------
BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
Develop your own process in accordance with the BPMN 2 standard
Learn Process modeling best practices with Bonita BPM through live exercises
http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test
  2015-04-21 13:05 [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test Alexey Kodanev
  2015-04-23  7:44 ` Jan Stancek
@ 2015-04-27 14:07 ` Cyril Hrubis
       [not found]   ` <553E55D3.7070001@oracle.com>
  2015-05-18 10:14 ` [LTP] [PATCH] fallocate04: another check if SEEK_HOLE is not supported Zeng Linggang
  2015-05-21  5:39 ` [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL Zeng Linggang
  3 siblings, 1 reply; 19+ messages in thread
From: Cyril Hrubis @ 2015-04-27 14:07 UTC (permalink / raw)
  To: Alexey Kodanev; +Cc: vasily.isaenko, ltp-list

Hi!
> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
> new file mode 100644
> index 0000000..f99492d
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -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.
> + *
> + * 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.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Author: Alexey Kodanev <alexey.kodanev@oracle.com>
> + */

What about adding a short comment here, with high level description of
the testcase? It's a bit easier to dive in the code if you know the big
picture beforehand.

> +#define _GNU_SOURCE
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>

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.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test
       [not found]   ` <553E55D3.7070001@oracle.com>
@ 2015-04-27 15:27     ` Cyril Hrubis
  0 siblings, 0 replies; 19+ messages in thread
From: Cyril Hrubis @ 2015-04-27 15:27 UTC (permalink / raw)
  To: Alexey Kodanev; +Cc: vasily.isaenko, ltp-list

Hi!
> > 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.

Ah, right. Then we have no choice but to exit the test.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* [LTP] [PATCH] fallocate04: another check if SEEK_HOLE is not supported
  2015-04-21 13:05 [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test Alexey Kodanev
  2015-04-23  7:44 ` Jan Stancek
  2015-04-27 14:07 ` Cyril Hrubis
@ 2015-05-18 10:14 ` Zeng Linggang
  2015-05-18 14:03   ` Alexey Kodanev
  2015-05-21  5:39 ` [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL Zeng Linggang
  3 siblings, 1 reply; 19+ messages in thread
From: Zeng Linggang @ 2015-05-18 10:14 UTC (permalink / raw)
  To: Alexey Kodanev; +Cc: vasily.isaenko, ltp-list

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 <zenglg.jy@cn.fujitsu.com>
---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 49 ++++++++++++++++++-----
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
index 911bbe8..9d9587b 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate04.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
@@ -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,
-				 "fallocate() or lseek() failed");
-		}
-		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
-	}
-	tst_resm(TINFO, "found a hole at '%ld' offset", ret);
+	off_t ret = check_file_hole(fd, block_size, block_size);
+	if (ret != (ssize_t)block_size)
+		tst_resm(TWARN | TERRNO, "No file hole found");
+	else
+		tst_resm(TINFO, "found a hole at '%ld' offset", ret);
 
 	size_t alloc_size1 = get_allocsize();
 
@@ -275,3 +272,35 @@ int main(int argc, char *argv[])
 
 	tst_exit();
 }
+
+static off_t check_file_hole(int fd, off_t offset, off_t len)
+{
+	off_t ret;
+
+	ret = lseek(fd, 0, SEEK_HOLE);
+	if (ret < 0) {
+		/* exclude error when kernel doesn't have SEEK_HOLE support */
+		if (errno != EINVAL)
+			tst_brkm(TFAIL | TERRNO, cleanup, "lseek() failed");
+
+		int i;
+		char *buf;
+
+		SAFE_LSEEK(cleanup, fd, offset, SEEK_SET);
+
+		buf = SAFE_MALLOC(cleanup, len);
+
+		SAFE_READ(cleanup, 1, fd, buf, len);
+
+		for (i = 0; i < len; i++) {
+			if (buf[i] != '\0')
+				break;
+		}
+
+		free(buf);
+
+		return i;
+	}
+
+	return ret;
+}
-- 
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-18 10:14 ` [LTP] [PATCH] fallocate04: another check if SEEK_HOLE is not supported Zeng Linggang
@ 2015-05-18 14:03   ` Alexey Kodanev
  2015-05-19  8:50     ` [LTP] [PATCH v2] " Zeng Linggang
  0 siblings, 1 reply; 19+ messages in thread
From: Alexey Kodanev @ 2015-05-18 14:03 UTC (permalink / raw)
  To: Zeng Linggang; +Cc: vasily.isaenko, ltp-list

Hi!
On 05/18/2015 01:14 PM, Zeng Linggang wrote:
> 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 <zenglg.jy@cn.fujitsu.com>
> ---
>   testcases/kernel/syscalls/fallocate/fallocate04.c | 49 ++++++++++++++++++-----
>   1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
> index 911bbe8..9d9587b 100644
> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -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,
> -				 "fallocate() or lseek() failed");
> -		}
> -		tst_resm(TWARN | TERRNO, "lseek() doesn't support SEEK_HOLE");
> -	}
> -	tst_resm(TINFO, "found a hole at '%ld' offset", ret);
> +	off_t ret = check_file_hole(fd, block_size, block_size);
> +	if (ret != (ssize_t)block_size)
> +		tst_resm(TWARN | TERRNO, "No file hole found");
> +	else
> +		tst_resm(TINFO, "found a hole at '%ld' offset", ret);

'check_file_data' function read the file and check for zeroes (a few 
lines after lseek), so 'check_file_hole' seems pointless here.

Best regards,
Alexey


------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-18 14:03   ` Alexey Kodanev
@ 2015-05-19  8:50     ` Zeng Linggang
  2015-05-19 11:18       ` Jan Stancek
  0 siblings, 1 reply; 19+ messages in thread
From: Zeng Linggang @ 2015-05-19  8:50 UTC (permalink / raw)
  To: Alexey Kodanev; +Cc: vasily.isaenko, ltp-list

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 <jy_zhangjin@cn.fujitsu.com>
---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
index 911bbe8..e94c572 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate04.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
@@ -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);
 
 	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)
 {
 	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
@@ -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);
 	}
 	tst_resm(TINFO, "found a hole at '%ld' offset", ret);
 
-- 
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-19  8:50     ` [LTP] [PATCH v2] " Zeng Linggang
@ 2015-05-19 11:18       ` Jan Stancek
  2015-05-20  1:47         ` Zeng Linggang
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Stancek @ 2015-05-19 11:18 UTC (permalink / raw)
  To: Zeng Linggang; +Cc: vasily isaenko, ltp-list





----- Original Message -----
> From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> Cc: "vasily isaenko" <vasily.isaenko@oracle.com>, ltp-list@lists.sourceforge.net
> 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 <jy_zhangjin@cn.fujitsu.com>
> ---
>  testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> b/testcases/kernel/syscalls/fallocate/fallocate04.c
> index 911bbe8..e94c572 100644
> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -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);
>  
>  	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)
>  {
>  	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
> @@ -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?

diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
index 911bbe8..45d9827 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate04.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
@@ -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)
+                       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

------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-19 11:18       ` Jan Stancek
@ 2015-05-20  1:47         ` Zeng Linggang
  2015-05-20  6:52           ` Jan Stancek
  2015-05-20  8:13           ` Alexey Kodanev
  0 siblings, 2 replies; 19+ messages in thread
From: Zeng Linggang @ 2015-05-20  1:47 UTC (permalink / raw)
  To: Jan Stancek; +Cc: vasily isaenko, ltp-list

Hi!

On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> > To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> > Cc: "vasily isaenko" <vasily.isaenko@oracle.com>, ltp-list@lists.sourceforge.net
> > 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 <jy_zhangjin@cn.fujitsu.com>
> > ---
> >  testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > index 911bbe8..e94c572 100644
> > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > @@ -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);
> >  
> >  	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)
> >  {
> >  	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
> > @@ -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?

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

> 
> 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.

> 
>         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?
> 
> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
> index 911bbe8..45d9827 100644
> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -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

> +                       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



------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-20  1:47         ` Zeng Linggang
@ 2015-05-20  6:52           ` Jan Stancek
  2015-05-20  9:58             ` Zeng Linggang
  2015-05-20  8:13           ` Alexey Kodanev
  1 sibling, 1 reply; 19+ messages in thread
From: Jan Stancek @ 2015-05-20  6:52 UTC (permalink / raw)
  To: Zeng Linggang; +Cc: vasily isaenko, ltp-list





----- Original Message -----
> From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: "Alexey Kodanev" <alexey.kodanev@oracle.com>, "vasily isaenko" <vasily.isaenko@oracle.com>,
> ltp-list@lists.sourceforge.net
> Sent: Wednesday, 20 May, 2015 3:47:22 AM
> Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
> 
> Hi!
> 
> On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> > > To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> > > Cc: "vasily isaenko" <vasily.isaenko@oracle.com>,
> > > ltp-list@lists.sourceforge.net
> > > 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 <jy_zhangjin@cn.fujitsu.com>
> > > ---
> > >  testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
> > >  1 file changed, 10 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > index 911bbe8..e94c572 100644
> > > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > @@ -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);
> > >  
> > >  	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)
> > >  {
> > >  	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
> > > @@ -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?
> 
> To be honest, I do not think it is redundant.

Can you explain why?

> 
> > 
> > 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.

> 
> > 
> >         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?
> > 
> > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > index 911bbe8..45d9827 100644
> > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > @@ -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

> 
> 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
> 
> > +                       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
> 
> 
> 

------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-20  1:47         ` Zeng Linggang
  2015-05-20  6:52           ` Jan Stancek
@ 2015-05-20  8:13           ` Alexey Kodanev
  2015-05-20  9:35             ` Zeng Linggang
  1 sibling, 1 reply; 19+ messages in thread
From: Alexey Kodanev @ 2015-05-20  8:13 UTC (permalink / raw)
  To: Zeng Linggang, Jan Stancek; +Cc: vasily isaenko, ltp-list

Hi,
On 05/20/2015 04:47 AM, Zeng Linggang wrote:
> Hi!
>
> On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
[ ... ]
>> Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?
>>
>> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
>> index 911bbe8..45d9827 100644
>> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
>> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
>> @@ -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);
> 	...
> }

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:

--- a/testcases/kernel/syscalls/fallocate/fallocate04.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
@@ -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


------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-20  8:13           ` Alexey Kodanev
@ 2015-05-20  9:35             ` Zeng Linggang
  0 siblings, 0 replies; 19+ messages in thread
From: Zeng Linggang @ 2015-05-20  9:35 UTC (permalink / raw)
  To: Alexey Kodanev; +Cc: vasily isaenko, ltp-list

Hi, Alexey and Jan

On Wed, 2015-05-20 at 11:13 +0300, Alexey Kodanev wrote:
> Hi,
> On 05/20/2015 04:47 AM, Zeng Linggang wrote:
> > Hi!
> >
> > On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
> [ ... ]
> >> Wouldn't it be enough to turn that warning into TINFO, for kernels < 3.1?
> >>
> >> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
> >> index 911bbe8..45d9827 100644
> >> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> >> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> >> @@ -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);
> > 	...
> > }
> 
> 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:

OK. I will do as that.

Thank both of you.

Best regards,
Zeng

> 
> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -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
> 



------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-20  6:52           ` Jan Stancek
@ 2015-05-20  9:58             ` Zeng Linggang
  2015-05-20 11:32               ` Jan Stancek
  0 siblings, 1 reply; 19+ messages in thread
From: Zeng Linggang @ 2015-05-20  9:58 UTC (permalink / raw)
  To: Jan Stancek; +Cc: vasily isaenko, ltp-list

Hi,

On Wed, 2015-05-20 at 02:52 -0400, Jan Stancek wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> > To: "Jan Stancek" <jstancek@redhat.com>
> > Cc: "Alexey Kodanev" <alexey.kodanev@oracle.com>, "vasily isaenko" <vasily.isaenko@oracle.com>,
> > ltp-list@lists.sourceforge.net
> > Sent: Wednesday, 20 May, 2015 3:47:22 AM
> > Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
> > 
> > Hi!
> > 
> > On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
> > > 
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> > > > To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> > > > Cc: "vasily isaenko" <vasily.isaenko@oracle.com>,
> > > > ltp-list@lists.sourceforge.net
> > > > 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 <jy_zhangjin@cn.fujitsu.com>
> > > > ---
> > > >  testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
> > > >  1 file changed, 10 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > index 911bbe8..e94c572 100644
> > > > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > @@ -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);
> > > >  
> > > >  	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)
> > > >  {
> > > >  	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
> > > > @@ -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?
> > 
> > To be honest, I do not think it is redundant.
> 
> Can you explain why?
> 
> > 
> > > 
> > > 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.
> 
> > 
> > > 
> > >         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?
> > > 
> > > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > index 911bbe8..45d9827 100644
> > > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > @@ -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.
> 

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");
+               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


> Regards,
> Jan
[...]


------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-20  9:58             ` Zeng Linggang
@ 2015-05-20 11:32               ` Jan Stancek
  2015-05-21  1:26                 ` Zeng Linggang
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Stancek @ 2015-05-20 11:32 UTC (permalink / raw)
  To: Zeng Linggang; +Cc: vasily isaenko, ltp-list





----- Original Message -----
> From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: "Alexey Kodanev" <alexey.kodanev@oracle.com>, "vasily isaenko" <vasily.isaenko@oracle.com>,
> ltp-list@lists.sourceforge.net
> Sent: Wednesday, 20 May, 2015 11:58:01 AM
> Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
> 
> Hi,
> 
> On Wed, 2015-05-20 at 02:52 -0400, Jan Stancek wrote:
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> > > To: "Jan Stancek" <jstancek@redhat.com>
> > > Cc: "Alexey Kodanev" <alexey.kodanev@oracle.com>, "vasily isaenko"
> > > <vasily.isaenko@oracle.com>,
> > > ltp-list@lists.sourceforge.net
> > > Sent: Wednesday, 20 May, 2015 3:47:22 AM
> > > Subject: Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is
> > > not supported
> > > 
> > > Hi!
> > > 
> > > On Tue, 2015-05-19 at 07:18 -0400, Jan Stancek wrote:
> > > > 
> > > > 
> > > > 
> > > > ----- Original Message -----
> > > > > From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> > > > > To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> > > > > Cc: "vasily isaenko" <vasily.isaenko@oracle.com>,
> > > > > ltp-list@lists.sourceforge.net
> > > > > 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 <jy_zhangjin@cn.fujitsu.com>
> > > > > ---
> > > > >  testcases/kernel/syscalls/fallocate/fallocate04.c | 13 ++++++++++---
> > > > >  1 file changed, 10 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > > index 911bbe8..e94c572 100644
> > > > > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > > @@ -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);
> > > > >  
> > > > >  	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)
> > > > >  {
> > > > >  	tst_resm(TINFO, "allocate '%zu' bytes", buf_size);
> > > > > @@ -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?
> > > 
> > > To be honest, I do not think it is redundant.
> > 
> > Can you explain why?
> > 
> > > 
> > > > 
> > > > 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.
> > 
> > > 
> > > > 
> > > >         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?
> > > > 
> > > > diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > index 911bbe8..45d9827 100644
> > > > --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> > > > @@ -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.
> > 
> 
> 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?

> 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

> 
> -               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_resm(TINFO, "found a hole at '%ld' offset", ret);
>         }
> -       tst_resm(TINFO, "found a hole at '%ld' offset", ret);
> 
> Thank you.
> 
> Best regards,
> Zeng
> 
> 
> > Regards,
> > Jan
> [...]
> 
> 

------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported
  2015-05-20 11:32               ` Jan Stancek
@ 2015-05-21  1:26                 ` Zeng Linggang
  0 siblings, 0 replies; 19+ messages in thread
From: Zeng Linggang @ 2015-05-21  1:26 UTC (permalink / raw)
  To: Jan Stancek; +Cc: vasily isaenko, ltp-list

On Wed, 2015-05-20 at 07:32 -0400, Jan Stancek wrote:

[...]
> > > > 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.
> > > 
> > 
> > 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?
> 

Oh, I see. I misunderstand your reply previous. Right, the patch is
redundant.
Thank you.

> > 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.

OK. I will fix it at the same time.

Thanks.

Best regards,
Zeng

> 
> Regards,
> Jan
> 
> > 
> > -               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_resm(TINFO, "found a hole at '%ld' offset", ret);
> >         }
> > -       tst_resm(TINFO, "found a hole at '%ld' offset", ret);
> > 
> > Thank you.
> > 
> > Best regards,
> > Zeng
> > 
> > 
> > > Regards,
> > > Jan
> > [...]
> > 
> > 



------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL
  2015-04-21 13:05 [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test Alexey Kodanev
                   ` (2 preceding siblings ...)
  2015-05-18 10:14 ` [LTP] [PATCH] fallocate04: another check if SEEK_HOLE is not supported Zeng Linggang
@ 2015-05-21  5:39 ` Zeng Linggang
  2015-05-22 14:21   ` Jan Stancek
  3 siblings, 1 reply; 19+ messages in thread
From: Zeng Linggang @ 2015-05-21  5:39 UTC (permalink / raw)
  To: Alexey Kodanev; +Cc: ltp-list, vasily.isaenko

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 <zenglg.jy@cn.fujitsu.com>
Signed-off-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
 testcases/kernel/syscalls/fallocate/fallocate04.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c b/testcases/kernel/syscalls/fallocate/fallocate04.c
index 911bbe8..0c904a0 100644
--- a/testcases/kernel/syscalls/fallocate/fallocate04.c
+++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
@@ -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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL
  2015-05-21  5:39 ` [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL Zeng Linggang
@ 2015-05-22 14:21   ` Jan Stancek
  2015-06-04 11:35     ` Alexey Kodanev
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Stancek @ 2015-05-22 14:21 UTC (permalink / raw)
  To: Zeng Linggang, Alexey Kodanev; +Cc: vasily isaenko, ltp-list





----- Original Message -----
> From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> Cc: ltp-list@lists.sourceforge.net, "vasily isaenko" <vasily.isaenko@oracle.com>
> 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?

Regards,
Jan

> 
> Signed-off-by: Zeng Linggang <zenglg.jy@cn.fujitsu.com>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
> ---
>  testcases/kernel/syscalls/fallocate/fallocate04.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fallocate/fallocate04.c
> b/testcases/kernel/syscalls/fallocate/fallocate04.c
> index 911bbe8..0c904a0 100644
> --- a/testcases/kernel/syscalls/fallocate/fallocate04.c
> +++ b/testcases/kernel/syscalls/fallocate/fallocate04.c
> @@ -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
> Ltp-list@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL
  2015-05-22 14:21   ` Jan Stancek
@ 2015-06-04 11:35     ` Alexey Kodanev
  2015-06-04 11:52       ` Jan Stancek
  0 siblings, 1 reply; 19+ messages in thread
From: Alexey Kodanev @ 2015-06-04 11:35 UTC (permalink / raw)
  To: Jan Stancek, Zeng Linggang; +Cc: vasily isaenko, ltp-list

Hi,
On 05/22/2015 05:21 PM, Jan Stancek wrote:
> ----- Original Message -----
>> From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
>> To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
>> Cc: ltp-list@lists.sourceforge.net, "vasily isaenko" <vasily.isaenko@oracle.com>
>> 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?

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

>> -		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


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

* Re: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL
  2015-06-04 11:35     ` Alexey Kodanev
@ 2015-06-04 11:52       ` Jan Stancek
  0 siblings, 0 replies; 19+ messages in thread
From: Jan Stancek @ 2015-06-04 11:52 UTC (permalink / raw)
  To: Alexey Kodanev, Zeng Linggang; +Cc: vasily isaenko, ltp-list





----- Original Message -----
> From: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> To: "Jan Stancek" <jstancek@redhat.com>, "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> Cc: ltp-list@lists.sourceforge.net, "vasily isaenko" <vasily.isaenko@oracle.com>
> Sent: Thursday, 4 June, 2015 1:35:50 PM
> Subject: Re: [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL
> 
> Hi,
> On 05/22/2015 05:21 PM, Jan Stancek wrote:
> > ----- Original Message -----
> >> From: "Zeng Linggang" <zenglg.jy@cn.fujitsu.com>
> >> To: "Alexey Kodanev" <alexey.kodanev@oracle.com>
> >> Cc: ltp-list@lists.sourceforge.net, "vasily isaenko"
> >> <vasily.isaenko@oracle.com>
> >> 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?
> 
> Sorry for the delay, looks good to me. There is stylistic comment below,
> but it is minor:
> 
> >> -		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.

Pushed with curly braces.

Regards,
Jan

> 
> Thanks,
> Alexey
> 
> 

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

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

end of thread, other threads:[~2015-06-04 11:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-21 13:05 [LTP] [PATCH v4] syscalls/fallocate04: add new fallocate() test Alexey Kodanev
2015-04-23  7:44 ` Jan Stancek
2015-04-27 14:07 ` Cyril Hrubis
     [not found]   ` <553E55D3.7070001@oracle.com>
2015-04-27 15:27     ` Cyril Hrubis
2015-05-18 10:14 ` [LTP] [PATCH] fallocate04: another check if SEEK_HOLE is not supported Zeng Linggang
2015-05-18 14:03   ` Alexey Kodanev
2015-05-19  8:50     ` [LTP] [PATCH v2] " Zeng Linggang
2015-05-19 11:18       ` Jan Stancek
2015-05-20  1:47         ` Zeng Linggang
2015-05-20  6:52           ` Jan Stancek
2015-05-20  9:58             ` Zeng Linggang
2015-05-20 11:32               ` Jan Stancek
2015-05-21  1:26                 ` Zeng Linggang
2015-05-20  8:13           ` Alexey Kodanev
2015-05-20  9:35             ` Zeng Linggang
2015-05-21  5:39 ` [LTP] [PATCH] fallocate04: Use tst_kvercmp after lseek(SEEK_HOLE) return EINVAL Zeng Linggang
2015-05-22 14:21   ` Jan Stancek
2015-06-04 11:35     ` Alexey Kodanev
2015-06-04 11:52       ` Jan Stancek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.