Skip to contentSkip to conversation

mkfs: get sector size from host fs dev when mkfs'ing file

RFC PATCH In review Eric Sandeen 3 participants 4 messages Dec 3, 2015 – Dec 14, 2015

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 mkfs: get sector size from host fs dev when mkfs'ing file

Now that we allow logical-sector-sized DIOs even if our xfs filesystem is set to physical-sector-sized "sectors," we can allow the creation of filesystem images with block and sector sizes down to the host device's logical sector size, rather than the filesystem's sector size.

So in platform_findsizes(), change our query of the filesystem to a query of the device, and use that for sector size in the S_IFREG case.

This allows the creation of i.e. a 2k block sized image on an xfs filesystem w/ 4k sector size created on a 4k/512 block device, which would otherwise fail today.

---

This feels like about my 5th stab at this; it seems like the correct thing to do but by now my brain is full. Seem sane?

Still needs a run through xfstests but wanted to see if this was obviously bonkers or not...?

(I don't know why copy/Makefile needs a $(LIBBLKID), either...)

libxfs/Makefile +1 -1
index ecf1921..87f9d93 100644
@@ -100,7 +100,7 @@ LSRCFILES += gen_crc32table.c
 
 FCFLAGS = -I.
 
-LTLIBS = $(LIBPTHREAD) $(LIBRT)
+LTLIBS = $(LIBPTHREAD) $(LIBRT) $(LIBBLKID)
 
 # don't try linking xfs_repair with a debug libxfs.
 DEBUG = -DNDEBUG
libxfs/linux.c +21 -9
index 885016a..3a8dc12 100644
@@ -24,6 +24,7 @@
 #include <sys/mount.h>
 #include <sys/ioctl.h>
 #include <sys/sysinfo.h>
+#include <blkid/blkid.h>
 
 #include "libxfs_priv.h"
 #include "xfs_fs.h"
@@ -142,18 +143,29 @@ platform_findsizes(char *path, int fd, long long *sz, int *bsz)
 			progname, path, strerror(errno));
 		exit(1);
 	}
+
 	if ((st.st_mode & S_IFMT) == S_IFREG) {
-		struct xfs_fsop_geom_v1 geom = { 0 };
+		int	fd;
+		char	*hostfs_dev_path;	/* path to host fs device */
 
 		*sz = (long long)(st.st_size >> 9);
-		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;
+
+		/*
+		 * Default to BBSIZE; mkfs might fail if there's a
+		 * size mismatch between the image & the host fs...
+		 */
+		*bsz = BBSIZE;
+
+		/* Get logical sector size of host device */
+		hostfs_dev_path = blkid_devno_to_devname(st.st_dev);
+		if (hostfs_dev_path) {
+			fd = open(hostfs_dev_path, O_RDONLY);
+			if (fd >= 0)
+				if (ioctl(fd, BLKSSZGET, bsz) < 0)
+					*bsz = BBSIZE;
+			close(fd);
+			free(hostfs_dev_path);
+		}
 
 		if (*bsz > max_block_alignment)
 			max_block_alignment = *bsz;
copy/Makefile +0 -0
index e630b17..4b45a66 100644
@@ -9,7 +9,7 @@ LTCOMMAND = xfs_copy

CFILES = xfs_copy.c
HFILES = xfs_copy.h

-LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBPTHREAD) $(LIBRT) +LLDLIBS = $(LIBXFS) $(LIBXLOG) $(LIBUUID) $(LIBPTHREAD) $(LIBRT) $(LIBBLKID) LTDEPENDENCIES = $(LIBXFS) $(LIBXLOG)
LLDFLAGS = -static-libtool-libs

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

#1 lore raw
Quoted 14 lines · Eric Sandeen

Now that we allow logical-sector-sized DIOs even if our xfs filesystem is set to physical-sector-sized "sectors," we can allow the creation of filesystem images with block and sector sizes down to the host device's logical sector size, rather than the filesystem's sector size.

So in platform_findsizes(), change our query of the filesystem to a query of the device, and use that for sector size in the S_IFREG case.

This allows the creation of i.e. a 2k block sized image on an xfs filesystem w/ 4k sector size created on a 4k/512 block device, which would otherwise fail today.

Quoted 6 lines

---

This feels like about my 5th stab at this; it seems like the correct thing to do but by now my brain is full. Seem sane?

I'm not sure I like the idea of encoding blkid functionality into libxfs, especially as blkid support is optional:

commit 6635d6ab510c58996f5ed3f212148db5e3c299ba

Author: Jan Tulak <[email protected]>
Date:   Wed Oct 14 10:57:39 2015 +1100

build: make libblkid usage optional

Quoted 4 lines

Still needs a run through xfstests but wanted to see if this was obviously bonkers or not...?

(I don't know why copy/Makefile needs a $(LIBBLKID), either...)

Because libxfs now has a dependency on libblkid, and libxfs is a static library so it doesn't resolve undefined extern objects as the library is built. That is only done when linking binaries....

Quoted 37 lines

index 885016a..3a8dc12 100644

libxfs/linux.c +11 -9
@@ -24,6 +24,7 @@
 #include <sys/mount.h>
 #include <sys/ioctl.h>
 #include <sys/sysinfo.h>
+#include <blkid/blkid.h>
 
 #include "libxfs_priv.h"
 #include "xfs_fs.h"
@@ -142,18 +143,29 @@ platform_findsizes(char *path, int fd, long long *sz, int *bsz)
 			progname, path, strerror(errno));
 		exit(1);
 	}
+
 	if ((st.st_mode & S_IFMT) == S_IFREG) {
-		struct xfs_fsop_geom_v1 geom = { 0 };
+		int	fd;
+		char	*hostfs_dev_path;	/* path to host fs device */
 
 		*sz = (long long)(st.st_size >> 9);
-		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;
+
+		/*
+		 * Default to BBSIZE; mkfs might fail if there's a
+		 * size mismatch between the image & the host fs...
+		 */
+		*bsz = BBSIZE;
+

libxfs/linux.c platform_findsizes(char *path, int fd, long long *sz, int *bsz) · line 168

Quoted 10 lines
libxfs/linux.c +10 -0
+		/* Get logical sector size of host device */
+		hostfs_dev_path = blkid_devno_to_devname(st.st_dev);
+		if (hostfs_dev_path) {
+			fd = open(hostfs_dev_path, O_RDONLY);
+			if (fd >= 0)
+				if (ioctl(fd, BLKSSZGET, bsz) < 0)
+					*bsz = BBSIZE;
+			close(fd);
+			free(hostfs_dev_path);
+		}

So the current behaviour is to use the underlying filesystem sector size, but we might have a 4k fs sector on a 512 physical sector device, and you want to detect this, right? The logical sector size is exposed by the filesystem in the XFS_IOC_DIOINFO information. i.e:

        case XFS_IOC_DIOINFO: {
                struct dioattr  da;
                xfs_buftarg_t   *target =
                        XFS_IS_REALTIME_INODE(ip) ?
                        mp->m_rtdev_targp : mp->m_ddev_targp;

                da.d_mem =  da.d_miniosz = target->bt_logical_sectorsize;
                da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);

                if (copy_to_user(arg, &da, sizeof(da)))
                        return -EFAULT;
                return 0;
        }

Isn't this exactly what XFS_IOC_DIOINFO is for?

(Oh, and "old kernels don't support...." simply means the distros will have a kernel patch to backport, too....)

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

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

On 12/13/15 7:27 PM, Dave Chinner wrote:
...

Quoted 20 lines

So the current behaviour is to use the underlying filesystem sector size, but we might have a 4k fs sector on a 512 physical sector device, and you want to detect this, right? The logical sector size is exposed by the filesystem in the XFS_IOC_DIOINFO information. i.e:

        case XFS_IOC_DIOINFO: {
                struct dioattr  da;
                xfs_buftarg_t   *target =
                        XFS_IS_REALTIME_INODE(ip) ?
                        mp->m_rtdev_targp : mp->m_ddev_targp;

                da.d_mem =  da.d_miniosz = target->bt_logical_sectorsize;
                da.d_maxiosz = INT_MAX & ~(da.d_miniosz - 1);

                if (copy_to_user(arg, &da, sizeof(da)))
                        return -EFAULT;
                return 0;
        }
Quoted 1 line

Isn't this exactly what XFS_IOC_DIOINFO is for?

Oh, right. SO MANY INTERFACES. ;)

But sure, that makes more sense, thanks for the reminder.

-Eric

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

#3 lore raw

PATCH v2 1/1 mkfs: get sector size from host fs d_miniosz when mkfs'ing file

In reply to Eric Sandeen

Now that we allow logical-sector-sized DIOs even if our xfs filesystem is set to physical-sector-sized "sectors," we can allow the creation of filesystem images with block and sector sizes down to the host device's logical sector size, rather than the filesystem's sector size.

So in platform_findsizes(), change our query of the filesystem to an XFS_IOC_DIOINFO query, and use the returned d_miniosz for sector size in the S_IFREG case.

This allows the creation of i.e. a 2k block sized image on an xfs filesystem w/ 4k sector size created on a 4k/512 block device, which would otherwise fail today.

---

V2: Just use XFS_IOC_DIOINFO to get the minimum DIO size, thanks Dave.

libxfs/linux.c +5 -3
index 885016a..f6ea1b2 100644
@@ -142,18 +142,20 @@ platform_findsizes(char *path, int fd, long long *sz, int *bsz)
 			progname, path, strerror(errno));
 		exit(1);
 	}
+
 	if ((st.st_mode & S_IFMT) == S_IFREG) {
-		struct xfs_fsop_geom_v1 geom = { 0 };
+		struct dioattr	da;
 
 		*sz = (long long)(st.st_size >> 9);
-		if (ioctl(fd, XFS_IOC_FSGEOMETRY_V1, &geom) < 0) {
+
+		if (ioctl(fd, XFS_IOC_DIOINFO, &da) < 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;
+			*bsz = da.d_miniosz;
 
 		if (*bsz > max_block_alignment)
 			max_block_alignment = *bsz;

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