Skip to contentSkip to conversation

xfstests 225: add additional fiemap-tester run without sync

PATCH In review Eric Sandeen 4 participants 4 messages Feb 24, 2011 – Feb 25, 2011

lore
Share & tools
lore

Eric Sandeen

author
#0 lore raw

PATCH 2/2 xfstests 225: add additional fiemap-tester run without sync

Chris Mason pointed out that some filesystems were not doing the right thing on fiemap, in the face of delalloc extents.

Because test 225 ran with FIEMAP_FLAG_SYNC only, this didn't get caught. Add a runtime option, and run it both ways.

Signed-off-by Eric Sandeen <[email protected]>

---

225 +4 -1
index 6e879a4..6b203e6 100755
@@ -60,7 +60,10 @@ _cleanup()
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
-echo "fiemap run without preallocation"
+echo "fiemap run without preallocation, with sync"
+$here/src/fiemap-tester -q -p 0 -r 200 $fiemapfile 2>&1 | tee $fiemaplog
+
+echo "fiemap run without preallocation or sync"
 $here/src/fiemap-tester -q -p 0 -r 200 $fiemapfile 2>&1 | tee $fiemaplog
 
 if grep -q "Operation not supported" $fiemaplog; then
225.out +2 -1
index 7bc9312..b4f6477 100644
@@ -1,2 +1,3 @@
 QA output created by 225
-fiemap run without preallocation
+fiemap run without preallocation, with sync
+fiemap run without preallocation or sync
src/fiemap-tester.c +9 -5
index 53d7a64..6320580 100644
@@ -418,7 +418,7 @@ static int query_fiemap_count(int fd, int blocks, int blocksize)
 }
 
 static int
-compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize)
+compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize, int syncfile)
 {
 	struct fiemap *fiemap;
 	char *fiebuf;

@@ -446,7 +446,7 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize)
 			last_data = i;
 	}
 
-	fiemap->fm_flags = FIEMAP_FLAG_SYNC;
+	fiemap->fm_flags = syncfile ? FIEMAP_FLAG_SYNC : 0;
 	fiemap->fm_extent_count = blocks_to_map;
 	fiemap->fm_mapped_extents = 0;
 

@@ -519,9 +519,10 @@ main(int argc, char **argv)
 	int	blocks = 0;	/* the number of blocks to generate */
 	int	maxblocks = 0;	/* max # of blocks to create */
 	int	prealloc = 1;	/* whether or not to do preallocation */
+	int	syncfile = 0;	/* whether fiemap should  sync file first */
 	int	seed = 1;
 
-	while ((opt = getopt(argc, argv, "m:r:s:p:q")) != -1) {
+	while ((opt = getopt(argc, argv, "m:r:s:p:qS")) != -1) {
 		switch(opt) {
 		case 'm':
 			map = strdup(optarg);

@@ -538,13 +539,16 @@ main(int argc, char **argv)
 		case 'q':
 			quiet = 1;
 			break;
-		/* sync file before mapping */
 		case 'r':
 			runs = atoi(optarg);
 			break;
 		case 's':
 			seed = atoi(optarg);
 			break;
+		/* sync file before mapping */
+		case 'S':
+			syncfile = 1;
+			break;
 		default:
 			usage();
 		}

@@ -623,7 +627,7 @@ main(int argc, char **argv)
 			exit(1);
 		}
 
-		rc = compare_fiemap_and_map(fd, map, blocks, blocksize);
+		rc = compare_fiemap_and_map(fd, map, blocks, blocksize, syncfile);
 		if (rc) {
 			printf("Problem comparing fiemap and map\n");
 			free(map);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw

PATCH v2 2/2 xfstests 225: add additional fiemap-tester run without sync

Chris Mason pointed out that some filesystems were not doing the right thing on fiemap, in the face of delalloc extents.

Because test 225 ran with FIEMAP_FLAG_SYNC only, this didn't get caught. Add a runtime option, and run it both ways.

Signed-off-by Eric Sandeen <[email protected]>

---

Whoops, V2: actually call w/ the sync flag.

225 +4 -1
index 6e879a4..6b203e6 100755
@@ -60,7 +60,10 @@ _cleanup()
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
-echo "fiemap run without preallocation"
+echo "fiemap run without preallocation, with sync"
+$here/src/fiemap-tester -q -S -p 0 -r 200 $fiemapfile 2>&1 | tee $fiemaplog
+
+echo "fiemap run without preallocation or sync"
 $here/src/fiemap-tester -q -p 0 -r 200 $fiemapfile 2>&1 | tee $fiemaplog
 
 if grep -q "Operation not supported" $fiemaplog; then
225.out +2 -1
index 7bc9312..b4f6477 100644
@@ -1,2 +1,3 @@
 QA output created by 225
-fiemap run without preallocation
+fiemap run without preallocation, with sync
+fiemap run without preallocation or sync
src/fiemap-tester.c +9 -5
index 53d7a64..6320580 100644
@@ -418,7 +418,7 @@ static int query_fiemap_count(int fd, int blocks, int blocksize)
 }
 
 static int
-compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize)
+compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize, int syncfile)
 {
 	struct fiemap *fiemap;
 	char *fiebuf;

@@ -446,7 +446,7 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize)
 			last_data = i;
 	}
 
-	fiemap->fm_flags = FIEMAP_FLAG_SYNC;
+	fiemap->fm_flags = syncfile ? FIEMAP_FLAG_SYNC : 0;
 	fiemap->fm_extent_count = blocks_to_map;
 	fiemap->fm_mapped_extents = 0;
 

@@ -519,9 +519,10 @@ main(int argc, char **argv)
 	int	blocks = 0;	/* the number of blocks to generate */
 	int	maxblocks = 0;	/* max # of blocks to create */
 	int	prealloc = 1;	/* whether or not to do preallocation */
+	int	syncfile = 0;	/* whether fiemap should  sync file first */
 	int	seed = 1;
 
-	while ((opt = getopt(argc, argv, "m:r:s:p:q")) != -1) {
+	while ((opt = getopt(argc, argv, "m:r:s:p:qS")) != -1) {
 		switch(opt) {
 		case 'm':
 			map = strdup(optarg);

@@ -538,13 +539,16 @@ main(int argc, char **argv)
 		case 'q':
 			quiet = 1;
 			break;
-		/* sync file before mapping */
 		case 'r':
 			runs = atoi(optarg);
 			break;
 		case 's':
 			seed = atoi(optarg);
 			break;
+		/* sync file before mapping */
+		case 'S':
+			syncfile = 1;
+			break;
 		default:
 			usage();
 		}

@@ -623,7 +627,7 @@ main(int argc, char **argv)
 			exit(1);
 		}
 
-		rc = compare_fiemap_and_map(fd, map, blocks, blocksize);
+		rc = compare_fiemap_and_map(fd, map, blocks, blocksize, syncfile);
 		if (rc) {
 			printf("Problem comparing fiemap and map\n");
 			free(map);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw
Quoted 91 lines · Eric Sandeen

Chris Mason pointed out that some filesystems were not doing the right thing on fiemap, in the face of delalloc extents.

Because test 225 ran with FIEMAP_FLAG_SYNC only, this didn't get caught. Add a runtime option, and run it both ways.

Signed-off-by Eric Sandeen <[email protected]>

---

Whoops, V2: actually call w/ the sync flag.

225 +4 -1
index 6e879a4..6b203e6 100755
@@ -60,7 +60,10 @@ _cleanup()
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
-echo "fiemap run without preallocation"
+echo "fiemap run without preallocation, with sync"
+$here/src/fiemap-tester -q -S -p 0 -r 200 $fiemapfile 2>&1 | tee $fiemaplog
+
+echo "fiemap run without preallocation or sync"
 $here/src/fiemap-tester -q -p 0 -r 200 $fiemapfile 2>&1 | tee $fiemaplog
 
 if grep -q "Operation not supported" $fiemaplog; then
225.out +2 -1
index 7bc9312..b4f6477 100644
@@ -1,2 +1,3 @@
 QA output created by 225
-fiemap run without preallocation
+fiemap run without preallocation, with sync
+fiemap run without preallocation or sync
src/fiemap-tester.c +8 -4
index 53d7a64..6320580 100644
@@ -418,7 +418,7 @@ static int query_fiemap_count(int fd, int blocks, int blocksize)
 }
 
 static int
-compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize)
+compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize, int syncfile)
 {
 	struct fiemap *fiemap;
 	char *fiebuf;

@@ -446,7 +446,7 @@ compare_fiemap_and_map(int fd, char *map, int blocks, int blocksize)
 			last_data = i;
 	}
 
-	fiemap->fm_flags = FIEMAP_FLAG_SYNC;
+	fiemap->fm_flags = syncfile ? FIEMAP_FLAG_SYNC : 0;
 	fiemap->fm_extent_count = blocks_to_map;
 	fiemap->fm_mapped_extents = 0;
 

@@ -519,9 +519,10 @@ main(int argc, char **argv)
 	int	blocks = 0;	/* the number of blocks to generate */
 	int	maxblocks = 0;	/* max # of blocks to create */
 	int	prealloc = 1;	/* whether or not to do preallocation */
+	int	syncfile = 0;	/* whether fiemap should  sync file first */
 	int	seed = 1;
 
-	while ((opt = getopt(argc, argv, "m:r:s:p:q")) != -1) {
+	while ((opt = getopt(argc, argv, "m:r:s:p:qS")) != -1) {
 		switch(opt) {
 		case 'm':
 			map = strdup(optarg);

@@ -538,13 +539,16 @@ main(int argc, char **argv)
 		case 'q':
 			quiet = 1;
 			break;
-		/* sync file before mapping */
 		case 'r':
 			runs = atoi(optarg);
 			break;
 		case 's':
 			seed = atoi(optarg);
 			break;
+		/* sync file before mapping */
+		case 'S':
+			syncfile = 1;
+			break;
 		default:
 			usage();
 		}

src/fiemap-tester.c main(int argc, char **argv) · line 633

Quoted 8 lines
src/fiemap-tester.c +1 -1
@@ -623,7 +627,7 @@ main(int argc, char **argv)
 			exit(1);
 		}
 
-		rc = compare_fiemap_and_map(fd, map, blocks, blocksize);
+		rc = compare_fiemap_and_map(fd, map, blocks, blocksize, syncfile);
 		if (rc) {
 			printf("Problem comparing fiemap and map\n");
 			free(map);

How about something in the usage() function to show we have this handy option :). Thanks,

Josef

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v2 2/2
#3 lore raw
In reply to Eric Sandeen
Quoted 7 lines · Eric Sandeen

Chris Mason pointed out that some filesystems were not doing the right thing on fiemap, in the face of delalloc extents.

Because test 225 ran with FIEMAP_FLAG_SYNC only, this didn't get caught. Add a runtime option, and run it both ways.

Signed-off-by Eric Sandeen <[email protected]>

Looks good. Please add a line to document the option in the usage() function as suggested by Josef before committing.

Reviewed-by Alex Elder <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs