[PATCH, RFC] mkfs: get sector size from host fs dev when mkfs'ing file

[PATCH, RFC] mkfs: get sector size from host fs dev when mkfs'ing file

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

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


diff --git a/libxfs/Makefile b/libxfs/Makefile
index ecf1921..87f9d93 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -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
diff --git a/libxfs/linux.c b/libxfs/linux.c
index 885016a..3a8dc12 100644
--- a/libxfs/linux.c
+++ b/libxfs/linux.c
@@ -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;

diff --git a/copy/Makefile b/copy/Makefile
index e630b17..4b45a66 100644
--- a/copy/Makefile
+++ b/copy/Makefile
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, RFC] mkfs: get sector size from host fs dev when mkfs'ing file

[Dave Chinner]

On Thu, Dec 03, 2015 at 05:16:46PM -0600, Eric Sandeen wrote:
> 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.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> 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 <jtulak@redhat.com>
Date:   Wed Oct 14 10:57:39 2015 +1100

    build: make libblkid usage optional


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

> index 885016a..3a8dc12 100644
> --- a/libxfs/linux.c
> +++ b/libxfs/linux.c
> @@ -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);
> +		}

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.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH, RFC] mkfs: get sector size from host fs dev when mkfs'ing file

[Eric Sandeen]

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

> 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, right.  SO MANY INTERFACES.  ;)

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

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH V2] mkfs: get sector size from host fs d_miniosz when mkfs'ing file

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

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

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

diff --git a/libxfs/linux.c b/libxfs/linux.c
index 885016a..f6ea1b2 100644
--- a/libxfs/linux.c
+++ b/libxfs/linux.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs