xfsprogs: try to handle mkfs of a file on 4k sector device

Try the xfs geometry ioctl if the mkfs target resides in a file; this gives us the equivalent of a device sector size.

This does, however, emit a warning if the target file exists on a non-XFS filesystem, and that might not be super.

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

If we encounter a target that's really a regular file, even without "-d file..." on the cmdline, call platform_findsizes() instead of blkid_get_topology to try to discover the "sector size" via the fsgeom() call.

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

---

mkfs/xfs_mkfs.c +0 -0

index 37c05a9..74180c9 100644
@@ -454,12 +454,26 @@ static void get_topology(
 	struct fs_topology	*ft,
 	int			force_overwrite)
 {

Quoted 22 lines · Eric Sandeen

If we encounter a target that's really a regular file, even without "-d file..." on the cmdline, call platform_findsizes() instead of blkid_get_topology to try to discover the "sector size" via the fsgeom() call.

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

---

mkfs/xfs_mkfs.c +3 -2

index 37c05a9..74180c9 100644
@@ -454,12 +454,26 @@ static void get_topology(
 	struct fs_topology	*ft,
 	int			force_overwrite)
 {
-	if (!xi->disfile) {
-		const char *dfile = xi->volname ? xi->volname : xi->dname;
+	int is_a_file = 0;
+	struct stat statbuf;
+	char *dfile = xi->volname ? xi->volname : xi->dname;
 

mkfs/xfs_mkfs.c static void get_topology( · line 464

Quoted 4 lines

mkfs/xfs_mkfs.c +4 -0

+	if (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))
+		is_a_file = 1;
+
+	if (!xi->disfile && !is_a_file) {

Why do need both xi->disfile and a local flag/ Why do we do the that even if xi->disfile is set?

We can modify platform_findsizes() to use the fsgeom call to get the "sector size" which should be used here, and warn that mismatches might exist if it fails.

This does mean there'll be a new warning emitted on fs images hosted on non-xfs filesystems; I'm not really quite sure it's worth it, hence the RFC nature of this lightly tested 2-patch series...

Quoted 7 lines

We can modify platform_findsizes() to use the fsgeom call to get the "sector size" which should be used here, and warn that mismatches might exist if it fails.

This does mean there'll be a new warning emitted on fs images hosted on non-xfs filesystems; I'm not really quite sure it's worth it, hence the RFC nature of this lightly tested 2-patch series...

Might be time to introduce some generic VFS-level ioctl for the sector size and dio alignment. Any beer or chocolate that could motivate you to get this done? :)

Also it would be nice to have a test case that exercises this new code.

Try the xfs geometry ioctl if the mkfs target resides in a file; this gives us the equivalent of a device sector size.

If this fails, and there's a sector size mismatch between the host FS and the filesystem, then mkfs might fail - but that's no worse than it's been before.

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

---

V2: remove warning when FSGEOMETRY fails (i.e. on non-xfs filesystems) for now

libxfs/linux.c +13 -3

index 2e07d54..3796fdd 100644
@@ -141,10 +141,20 @@ platform_findsizes(char *path, int fd, long long *sz, int *bsz)
 		exit(1);
 	}
 	if ((st.st_mode & S_IFMT) == S_IFREG) {
+		struct xfs_fsop_geom_v1 geom = { 0 };
+
 		*sz = (long long)(st.st_size >> 9);
-		*bsz = BBSIZE;
-		if (BBSIZE > max_block_alignment)
-			max_block_alignment = BBSIZE;
+		if (ioctl(fd, XFS_IOC_FSGEOMETRY_V1, &geom) < 0) {
+			/*
+			 * fall back to BBSIZE; mkfs might fail if there's a
+			 * size mismatch between the image & the host fs...
+			 */
+			*bsz = BBSIZE;
+		} else
+			*bsz = geom.sectsize;
+
+		if (*bsz > max_block_alignment)
+			max_block_alignment = *bsz;
 		return;
 	}
 

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

If we encounter a target that's really a regular file, even without "-d file..." on the cmdline, call platform_findsizes() instead of blkid_get_topology to try to discover the "sector size" via the fsgeom() call.

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

---

V2: Lose local "isa_file" flag, just switch based on (xi->disfile) or (stat works & S_ISREG)

mkfs/xfs_mkfs.c +7 -2

index 188b6b3..3b8bf67 100644
@@ -456,9 +456,25 @@ static void get_topology(
 	struct fs_topology	*ft,
 	int			force_overwrite)
 {
-	if (!xi->disfile) {
-		const char *dfile = xi->volname ? xi->volname : xi->dname;
+	struct stat statbuf;
+	char *dfile = xi->volname ? xi->volname : xi->dname;
 
+	/*
+	 * Don't call blkid for topology if this is a "-d file" target, or
+	 * if we've simply been pointed at a regular file.  platform_findsizes
+	 * will attempt to find the underlying sector size of the host fs.
+	 */

mkfs/xfs_mkfs.c +1 -0

+	} else {
 		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
 				   &ft->lsectorsize, &ft->psectorsize,
 				   force_overwrite);

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

Quoted 29 lines · Eric Sandeen

If we encounter a target that's really a regular file, even without "-d file..." on the cmdline, call platform_findsizes() instead of blkid_get_topology to try to discover the "sector size" via the fsgeom() call.

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

---

V2: Lose local "isa_file" flag, just switch based on (xi->disfile) or (stat works & S_ISREG)

mkfs/xfs_mkfs.c +7 -2

index 188b6b3..3b8bf67 100644
@@ -456,9 +456,25 @@ static void get_topology(
 	struct fs_topology	*ft,
 	int			force_overwrite)
 {
-	if (!xi->disfile) {
-		const char *dfile = xi->volname ? xi->volname : xi->dname;
+	struct stat statbuf;
+	char *dfile = xi->volname ? xi->volname : xi->dname;
 
+	/*
+	 * Don't call blkid for topology if this is a "-d file" target, or
+	 * if we've simply been pointed at a regular file.  platform_findsizes
+	 * will attempt to find the underlying sector size of the host fs.
+	 */

mkfs/xfs_mkfs.c static void get_topology( · line 476

Quoted 10 lines

mkfs/xfs_mkfs.c +10 -0

+	if (xi->disfile ||
+	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
+		int fd;
+		long long dummy;
+
+		fd = open(dfile, O_RDONLY);
+		if (fd >= 0) {
+			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
+			close(fd);
+		}

The patch looks fine:

Reviewed-by Brian Foster <[email protected]>

It does look like we'd still be susceptible to error in a situation where the file hasn't been created yet (which would occur in libxfs_init()).

Quoted 43 lines · Brian Foster

Quoted 29 lines · Eric Sandeen

If we encounter a target that's really a regular file, even without "-d file..." on the cmdline, call platform_findsizes() instead of blkid_get_topology to try to discover the "sector size" via the fsgeom() call.

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

---

V2: Lose local "isa_file" flag, just switch based on (xi->disfile) or (stat works & S_ISREG)

mkfs/xfs_mkfs.c +7 -2

index 188b6b3..3b8bf67 100644
@@ -456,9 +456,25 @@ static void get_topology(
 	struct fs_topology	*ft,
 	int			force_overwrite)
 {
-	if (!xi->disfile) {
-		const char *dfile = xi->volname ? xi->volname : xi->dname;
+	struct stat statbuf;
+	char *dfile = xi->volname ? xi->volname : xi->dname;
 
+	/*
+	 * Don't call blkid for topology if this is a "-d file" target, or
+	 * if we've simply been pointed at a regular file.  platform_findsizes
+	 * will attempt to find the underlying sector size of the host fs.
+	 */

mkfs/xfs_mkfs.c static void get_topology( · line 476

Quoted 10 lines

mkfs/xfs_mkfs.c +10 -0

+	if (xi->disfile ||
+	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
+		int fd;
+		long long dummy;
+
+		fd = open(dfile, O_RDONLY);
+		if (fd >= 0) {
+			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
+			close(fd);
+		}

The patch looks fine:

Quoted 5 lines

Reviewed-by Brian Foster <[email protected]>

It does look like we'd still be susceptible to error in a situation where the file hasn't been created yet (which would occur in libxfs_init()).

Hm, let's see - the only way to have mkfs create the file is with the -d file,name=foo,size=bar invocation.

Sprinkling some printf's into the if/else above, I see:

# rm -f testfile
# mkfs/mkfs.xfs -dfile,name=testfile,size=1g
** doing platform_findsizes **
meta-data=testfile               isize=256    agcount=4, agsize=65536 blks
...

so it looks like the file does get created before we get here.