[PATCH 0/2, RFC] xfsprogs: try to handle mkfs of a file on 4k sector device

[PATCH 0/2, RFC] xfsprogs: try to handle mkfs of a file on 4k sector device

[Eric Sandeen]

Today if you mkfs.xfs <filename> where the file resides on a
hard-4k filesystem, we fail because it tries to do 512 direct
IO when 4k is required; this is a bit cryptic:

# mkfs.xfs -f mnt/fsfile
meta-data=mnt/fsfile             isize=256    agcount=4, agsize=8192 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=0
data     =                       bsize=4096   blocks=32768, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=0
log      =internal log           bsize=4096   blocks=853, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
existing superblock read failed: Invalid argument
mkfs.xfs: pwrite64 failed: Invalid argument

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

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

[PATCH 1/2, RFC] xfsprogs: check fs sector size in platform_findsizes()

[Eric Sandeen]

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 <sandeen@redhat.com>
---

diff --git a/libxfs/linux.c b/libxfs/linux.c
index 2e07d54..d66a90f 100644
--- a/libxfs/linux.c
+++ b/libxfs/linux.c
@@ -141,10 +141,19 @@ 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) {
+			fprintf(stderr, _("Cannot get host filesystem geometry.\n"
+	"mkfs may fail if there is a sector size mismatch between\n"
+	"the image and the host filesystem.\n"));
+			*bsz = BBSIZE;
+		} else
+			*bsz = geom.sectsize;
+
+		if (*bsz > max_block_alignment)
+			max_block_alignment = *bsz;
 		return;
 	}
 


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

[PATCH 2/2, RFC] mkfs.xfs: don't call blkid_get_topology on regular files

[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 <sandeen@redhat.com>
---

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 37c05a9..74180c9 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -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;
 
+	if (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))
+		is_a_file = 1;
+
+	if (!xi->disfile && !is_a_file) {
 		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
 				   &ft->lsectorsize, &ft->psectorsize,
 				   force_overwrite);
+	} else {
+		int fd;
+		long long dummy;
+
+		fd = open(dfile, O_RDONLY);
+		if (fd >= 0) {
+			platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize);
+			close(fd);
+		}
 	}
 
 	if (xi->rtname && !xi->risfile) {

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

Re: [PATCH 1/2, RFC] xfsprogs: check fs sector size in platform_findsizes()

[Christoph Hellwig]

On Tue, May 27, 2014 at 07:14:25PM -0500, Eric Sandeen wrote:
> 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.

You could simple not print the warning in case this bothers you..

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

Otherwise looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Re: [PATCH 2/2, RFC] mkfs.xfs: don't call blkid_get_topology on regular files

[Christoph Hellwig]

On Tue, May 27, 2014 at 07:15:20PM -0500, Eric Sandeen wrote:
> 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 <sandeen@redhat.com>
> ---
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 37c05a9..74180c9 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -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;
>  
> +	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?

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

Re: [PATCH 0/2, RFC] xfsprogs: try to handle mkfs of a file on 4k sector device

[Christoph Hellwig]

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

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

Re: [PATCH 2/2, RFC] mkfs.xfs: don't call blkid_get_topology on regular files

[Eric Sandeen]

On 5/28/14, 12:40 AM, Christoph Hellwig wrote:
> On Tue, May 27, 2014 at 07:15:20PM -0500, Eric Sandeen wrote:
>> 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 <sandeen@redhat.com>
>> ---
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index 37c05a9..74180c9 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -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;
>>  
>> +	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?

Good point that we don't need to do the stat if the old disfile flag
is already set.

But "disfile" implies a lot of other things, and comes from specifying
"-d file,..." on the cmdline.  If you point mkfs straight at a file,
disfile is NOT set, so I check it here explicitly.

The dichotomy between "mkfs.xfs -dfile,name=$FOO" and "mkfs.xfs $FOO"
is a weird one, and should probably just go away in the long run...

-Eric

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

Re: [PATCH 0/2, RFC] xfsprogs: try to handle mkfs of a file on 4k sector device

[Eric Sandeen]

On 5/28/14, 12:41 AM, Christoph Hellwig wrote:
>> 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? :)

Roses from you, Christoph.  ;)

Yeah, we do desperately need this.  Not sure why nobody's done it yet,
I'll see if I can get the time & motivation going, and launch into
the bike shed building.  :)

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

Hm yeah fair enough; I tested it w/ scsi-debug...

-Eric

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

[PATCH 1/2 V2] xfsprogs: try to handle mkfs of a file on 4k sector device

[Eric Sandeen]

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 <sandeen@redhat.com>
---

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

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

[PATCH 2/2 V2] mkfs.xfs: don't call blkid_get_topology on regular files

[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 <sandeen@redhat.com>
---

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

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 188b6b3..3b8bf67 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -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.
+	 */
+	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);
+		}
+	} else {
 		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
 				   &ft->lsectorsize, &ft->psectorsize,
 				   force_overwrite);

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

Re: [PATCH 1/2 V2] xfsprogs: try to handle mkfs of a file on 4k sector device

[Brian Foster]

On Thu, Jun 05, 2014 at 02:13:02PM -0500, Eric Sandeen wrote:
> 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 <sandeen@redhat.com>
> ---

Looks Ok...

Reviewed-by: Brian Foster <bfoster@redhat.com>

> 
> V2: remove warning when FSGEOMETRY fails (i.e. on non-xfs
> filesystems) for now
> 
> diff --git a/libxfs/linux.c b/libxfs/linux.c
> index 2e07d54..3796fdd 100644
> --- a/libxfs/linux.c
> +++ b/libxfs/linux.c
> @@ -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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 2/2 V2] mkfs.xfs: don't call blkid_get_topology on regular files

[Brian Foster]

On Thu, Jun 05, 2014 at 02:15:03PM -0500, Eric Sandeen wrote:
> 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 <sandeen@redhat.com>
> ---
> 
> V2: Lose local "isa_file" flag, just switch based on 
> (xi->disfile) or (stat works & S_ISREG)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 188b6b3..3b8bf67 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -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.
> +	 */
> +	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 <bfoster@redhat.com>

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

Brian

> +	} else {
>  		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
>  				   &ft->lsectorsize, &ft->psectorsize,
>  				   force_overwrite);
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 2/2 V2] mkfs.xfs: don't call blkid_get_topology on regular files

[Eric Sandeen]

On 6/6/14, 10:21 AM, Brian Foster wrote:
> On Thu, Jun 05, 2014 at 02:15:03PM -0500, Eric Sandeen wrote:
>> 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 <sandeen@redhat.com>
>> ---
>>
>> V2: Lose local "isa_file" flag, just switch based on 
>> (xi->disfile) or (stat works & S_ISREG)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index 188b6b3..3b8bf67 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -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.
>> +	 */
>> +	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 <bfoster@redhat.com>
> 
> 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.

-Eric

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

Re: [PATCH 2/2 V2] mkfs.xfs: don't call blkid_get_topology on regular files

[Eric Sandeen]

On 6/6/14, 10:27 AM, Eric Sandeen wrote:
> On 6/6/14, 10:21 AM, Brian Foster wrote:
>> On Thu, Jun 05, 2014 at 02:15:03PM -0500, Eric Sandeen wrote:
>>> 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 <sandeen@redhat.com>
>>> ---
>>>
>>> V2: Lose local "isa_file" flag, just switch based on 
>>> (xi->disfile) or (stat works & S_ISREG)
>>>
>>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>>> index 188b6b3..3b8bf67 100644
>>> --- a/mkfs/xfs_mkfs.c
>>> +++ b/mkfs/xfs_mkfs.c
>>> @@ -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.
>>> +	 */
>>> +	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 <bfoster@redhat.com>
>>
>> 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.

Oh, I see, sorry.  Crud, you're right.

What prompted all this was that making a fs-in-a-file on a filesystem hosted
on a hard 4k sector device fails, because it tries to do 512-byte DIO.

If we use the "-d file" invocation, it works, because it doesn't do direct IO.

Sigh.  It's complicated.  Let me think about the 2nd patch just a bit more.

Perhaps only doing the platform_findsizes if stat succeeds, not if xi->disfile.

-Eric

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

[PATCH 2/2 V3] mkfs.xfs: don't call blkid_get_topology on existing regular files

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

Otherwise mkfs.xfs will try to do direct IO with a default
512 sector size, and if the underlying file has different
DIO requirements, mkfs will fail.

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

V3: only call platform_findsizes if xi->disfile isn't set,
and our target is a regular file: i.e. only in the 
"mkfs.xfs <existing regular file>" case.

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 188b6b3..c85258a 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -457,11 +457,30 @@ static void get_topology(
 	int			force_overwrite)
 {
 	if (!xi->disfile) {
-		const char *dfile = xi->volname ? xi->volname : xi->dname;
+		char *dfile = xi->volname ? xi->volname : xi->dname;
+		struct stat statbuf;
 
-		blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
-				   &ft->lsectorsize, &ft->psectorsize,
-				   force_overwrite);
+		/*
+		 * 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 (xi->rtname && !xi->risfile) {


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