All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Eric Sandeen <sandeen@redhat.com>, xfs-oss <xfs@oss.sgi.com>
Subject: PATCH V2] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
Date: Fri, 19 Jun 2015 12:09:23 -0500	[thread overview]
Message-ID: <55844CC3.3080007@sandeen.net> (raw)
In-Reply-To: <55820229.1010701@redhat.com>

If we are mkfs'ing a file, and that file is on a 4k sector filesystem,
we should make the fs image file with the same sector size, or things
may fail when they try to do direct IO in 512 byte chunks (depending
on whether it is a 512e or "hard" 4k device).

Earlier commits attempted this to some degree:

5a7d59 xfsprogs: try to handle mkfs of a file on 4k sector device
3800a2 mkfs.xfs: don't call blkid_get_topology on existing regular files

but inexplicably missed the case where mkfs.xfs with "-d file" was
specified.

One more try; in get_topology(), try to get the underlying fs sector
size in *all* cases where we are mkfs'ing a file, and set the sector size
accordingly.

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

V2: Handle the !ENABLE_BLKID case as well

Jan, if this collides with your work, let's talk - I would like to 
try to fix things up short term even if there's a big rework in
progress, but we should probably coordinate ...

Christoph, yes, we should probably eliminate HAVE_BLKID, but for
now it's easy enough to fix both spots, and that's separate work -
I can send a patch for it as well.

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index e2a052d..c184a63 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -462,31 +462,35 @@ static void get_topology(
 	struct fs_topology	*ft,
 	int			force_overwrite)
 {
-	if (!xi->disfile) {
-		char *dfile = xi->volname ? xi->volname : xi->dname;
-		struct stat statbuf;
+	struct stat statbuf;
+	char *dfile = xi->volname ? xi->volname : xi->dname;
 
-		/*
-		 * If our target is a regular file, and xi->disfile isn't
-		 * set (i.e. no "-d file" invocation), use platform_findsizes
-		 * to try to obtain the underlying filesystem's requirements
-		 * for direct IO; we'll set our sector size to that if possible.
-		 */
-		if (!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);
-			}
-		} else {
-			blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
-					   &ft->lsectorsize, &ft->psectorsize,
-					   force_overwrite);
-		}
+	/*
+	 * If our target is a regular file, use platform_findsizes
+	 * to try to obtain the underlying filesystem's requirements
+	 * for direct IO; we'll set our sector size to that if possible.
+	 */
+	if (xi->disfile ||
+	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
+		int fd;
+		int flags = O_RDONLY;
+		long long dummy;
+
+		/* with xi->disfile we may not have the file yet! */
+		if (xi->disfile)
+			flags |= O_CREAT;
+
+		fd = open(dfile, flags, 0666);
+		if (fd >= 0) {
+			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
+			close(fd);
+			ft->psectorsize = ft->lsectorsize;
+		} else
+			ft->psectorsize = ft->lsectorsize = BBSIZE;
+	} else {
+		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
+				   &ft->lsectorsize, &ft->psectorsize,
+				   force_overwrite);
 	}
 
 	if (xi->rtname && !xi->risfile) {
@@ -525,11 +529,32 @@ static void get_topology(
 	struct fs_topology	*ft,
 	int			force_overwrite)
 {
-
+	struct stat statbuf;
 	char *dfile = xi->volname ? xi->volname : xi->dname;
 	int bsz = BBSIZE;
 
-	if (!xi->disfile) {
+        /*
+	 * If our target is a regular file, use platform_findsizes
+	 * to try to obtain the underlying filesystem's requirements
+	 * for direct IO; we'll set our sector size to that if possible.
+	 */
+	if (xi->disfile ||
+	    (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) {
+		int fd;
+		int flags = O_RDONLY;
+		long long dummy;
+
+		/* with xi->disfile we may not have the file yet! */
+		if (xi->disfile)
+			flags |= O_CREAT;
+
+		fd = open(dfile, flags, 0666);
+		/* If this fails we just fall back to BBSIZE */
+		if (fd >= 0) {
+			platform_findsizes(dfile, fd, &dummy, &bsz);
+			close(fd);
+		}
+	} else {
 		int fd;
 		long long dummy;
 

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

      parent reply	other threads:[~2015-06-19 17:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 23:26 [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file Eric Sandeen
2015-06-18 11:03 ` Jan Tulak
2015-06-18 14:57   ` Eric Sandeen
2015-06-19  7:01     ` Jan Tulak
2015-06-19 15:09   ` Christoph Hellwig
2015-06-19 15:17     ` Eric Sandeen
2015-06-19 15:21       ` Christoph Hellwig
2015-06-19 15:24         ` Eric Sandeen
2015-06-19 17:09 ` Eric Sandeen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55844CC3.3080007@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=sandeen@redhat.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.