* [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
@ 2015-06-17 23:26 Eric Sandeen
2015-06-18 11:03 ` Jan Tulak
2015-06-19 17:09 ` PATCH V2] " Eric Sandeen
0 siblings, 2 replies; 9+ messages in thread
From: Eric Sandeen @ 2015-06-17 23:26 UTC (permalink / raw)
To: xfs-oss
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>
---
(This does it for 512e as well as hard 4k drives, but I think that's
probably ok? If not, perhaps we should go further and attempt to
discern logical and physical sectors for the device under the
filesystem. Is it worth it? Not sure it is.)
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index e2a052d..e44c390 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -462,31 +462,34 @@ 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);
}
+
+ } 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
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 15:09 ` Christoph Hellwig
2015-06-19 17:09 ` PATCH V2] " Eric Sandeen
1 sibling, 2 replies; 9+ messages in thread
From: Jan Tulak @ 2015-06-18 11:03 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
----- Original Message -----
> From: "Eric Sandeen" <sandeen@redhat.com>
> To: "xfs-oss" <xfs@oss.sgi.com>
> Sent: Thursday, June 18, 2015 1:26:33 AM
> Subject: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
>
> 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>
> ---
>
> (This does it for 512e as well as hard 4k drives, but I think that's
> probably ok? If not, perhaps we should go further and attempt to
> discern logical and physical sectors for the device under the
> filesystem. Is it worth it? Not sure it is.)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index e2a052d..e44c390 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -462,31 +462,34 @@ 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);
> }
> +
> + } else {
> + blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
> + &ft->lsectorsize, &ft->psectorsize,
> + force_overwrite);
> }
>
> if (xi->rtname && !xi->risfile) {
>
This changes get_topology only for ENABLE_BLKID branch of #ifdef. Is that intentional, i.e. we don't expect anyone not using ENABLE_BLKID? Because otherwise, if mkfs is compiled without ENABLE_BLKID, then all we get is:
int bsz = BBSIZE;
if (!xi->disfile) {
int fd;
long long dummy;
get_subvol_stripe_wrapper(dfile, SVTYPE_DATA,
&ft->dsunit, &ft->dswidth, &ft->sectoralign);
fd = open(dfile, O_RDONLY);
/* If this fails we just fall back to BBSIZE */
if (fd >= 0) {
platform_findsizes(dfile, fd, &dummy, &bsz);
close(fd);
}
}
ft->lsectorsize = bsz;
ft->psectorsize = bsz;
Two definitions of get_topology looks really unfortunate - this is something I have on my radar to change.
--
Jan Tulak
jtulak@redhat.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
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
1 sibling, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2015-06-18 14:57 UTC (permalink / raw)
To: Jan Tulak, Eric Sandeen; +Cc: xfs-oss
On 6/18/15 6:03 AM, Jan Tulak wrote:
>
>
> ----- Original Message -----
>> From: "Eric Sandeen" <sandeen@redhat.com>
>> To: "xfs-oss" <xfs@oss.sgi.com>
>> Sent: Thursday, June 18, 2015 1:26:33 AM
>> Subject: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
>>
>> 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>
>> ---
>>
>> (This does it for 512e as well as hard 4k drives, but I think that's
>> probably ok? If not, perhaps we should go further and attempt to
>> discern logical and physical sectors for the device under the
>> filesystem. Is it worth it? Not sure it is.)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index e2a052d..e44c390 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -462,31 +462,34 @@ 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);
>> }
>> +
>> + } else {
>> + blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
>> + &ft->lsectorsize, &ft->psectorsize,
>> + force_overwrite);
>> }
>>
>> if (xi->rtname && !xi->risfile) {
>>
>
> This changes get_topology only for ENABLE_BLKID branch of #ifdef. Is
> that intentional, i.e. we don't expect anyone not using ENABLE_BLKID?
> Because otherwise, if mkfs is compiled without ENABLE_BLKID, then all
> we get is:
Hm, yeah, good point. I always forget about this. :( I can send V2.
And sorry if this overlaps w/ your changes- I got a bug report about
xfstests failing when testing hard 4k devices, and it was due to image
files created on a filesystem on that hard 4k device, and xfsprogs tools
fail when they try to do 512-byte direct IO to the image.
Thanks,
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
2015-06-18 14:57 ` Eric Sandeen
@ 2015-06-19 7:01 ` Jan Tulak
0 siblings, 0 replies; 9+ messages in thread
From: Jan Tulak @ 2015-06-19 7:01 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss
----- Original Message -----
> From: "Eric Sandeen" <sandeen@sandeen.net>
> To: "Jan Tulak" <jtulak@redhat.com>, "Eric Sandeen" <sandeen@redhat.com>
> Cc: "xfs-oss" <xfs@oss.sgi.com>
> Sent: Thursday, June 18, 2015 4:57:09 PM
> Subject: Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
>
> On 6/18/15 6:03 AM, Jan Tulak wrote:
> >
> >
> > ----- Original Message -----
> >> From: "Eric Sandeen" <sandeen@redhat.com>
> >> To: "xfs-oss" <xfs@oss.sgi.com>
> >> Sent: Thursday, June 18, 2015 1:26:33 AM
> >> Subject: [PATCH] mkfs.xfs: always use underlying fs sector size when
> >> mkfs'ing a file
> >>
> >> 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>
> >> ---
> >>
> >> (This does it for 512e as well as hard 4k drives, but I think that's
> >> probably ok? If not, perhaps we should go further and attempt to
> >> discern logical and physical sectors for the device under the
> >> filesystem. Is it worth it? Not sure it is.)
> >>
> >> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> >> index e2a052d..e44c390 100644
> >> --- a/mkfs/xfs_mkfs.c
> >> +++ b/mkfs/xfs_mkfs.c
> >> @@ -462,31 +462,34 @@ 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);
> >> }
> >> +
> >> + } else {
> >> + blkid_get_topology(dfile, &ft->dsunit, &ft->dswidth,
> >> + &ft->lsectorsize, &ft->psectorsize,
> >> + force_overwrite);
> >> }
> >>
> >> if (xi->rtname && !xi->risfile) {
> >>
> >
> > This changes get_topology only for ENABLE_BLKID branch of #ifdef. Is
> > that intentional, i.e. we don't expect anyone not using ENABLE_BLKID?
> > Because otherwise, if mkfs is compiled without ENABLE_BLKID, then all
> > we get is:
>
> Hm, yeah, good point. I always forget about this. :( I can send V2.
>
> And sorry if this overlaps w/ your changes-
No problem. :-)
Cheers,
Jan
--
Jan Tulak
jtulak@redhat.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
2015-06-18 11:03 ` Jan Tulak
2015-06-18 14:57 ` Eric Sandeen
@ 2015-06-19 15:09 ` Christoph Hellwig
2015-06-19 15:17 ` Eric Sandeen
1 sibling, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2015-06-19 15:09 UTC (permalink / raw)
To: Jan Tulak; +Cc: Eric Sandeen, xfs-oss
On Thu, Jun 18, 2015 at 07:03:39AM -0400, Jan Tulak wrote:
> This changes get_topology only for ENABLE_BLKID branch of #ifdef. Is that intentional, i.e. we don't expect anyone not using ENABLE_BLKID? Because otherwise, if mkfs is compiled without ENABLE_BLKID, then all we get is:
I'm in favor of killing the !ENABLE_BLKID entirely. The proper
support in blkid has been around long enough to require it.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
2015-06-19 15:09 ` Christoph Hellwig
@ 2015-06-19 15:17 ` Eric Sandeen
2015-06-19 15:21 ` Christoph Hellwig
0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2015-06-19 15:17 UTC (permalink / raw)
To: Christoph Hellwig, Jan Tulak; +Cc: xfs-oss
On 6/19/15 10:09 AM, Christoph Hellwig wrote:
> On Thu, Jun 18, 2015 at 07:03:39AM -0400, Jan Tulak wrote:
>> This changes get_topology only for ENABLE_BLKID branch of #ifdef. Is that intentional, i.e. we don't expect anyone not using ENABLE_BLKID? Because otherwise, if mkfs is compiled without ENABLE_BLKID, then all we get is:
>
> I'm in favor of killing the !ENABLE_BLKID entirely. The proper
> support in blkid has been around long enough to require it.
>
though we also claim to nominally support building on osx, irix, etc, which doesn't have that library, right?
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
2015-06-19 15:17 ` Eric Sandeen
@ 2015-06-19 15:21 ` Christoph Hellwig
2015-06-19 15:24 ` Eric Sandeen
0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2015-06-19 15:21 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, xfs-oss, Jan Tulak
On Fri, Jun 19, 2015 at 10:17:10AM -0500, Eric Sandeen wrote:
> though we also claim to nominally support building on osx, irix, etc,
> which doesn't have that library, right?
Debian supports it for kfreebsd and hurd, and
https://trac.macports.org/ticket/15546 suggest an older version did
build on OS X. So I don't think it's a real problem even if IRIX
might require some minor porting efforts.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
2015-06-19 15:21 ` Christoph Hellwig
@ 2015-06-19 15:24 ` Eric Sandeen
0 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2015-06-19 15:24 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs-oss, Jan Tulak
On 6/19/15 10:21 AM, Christoph Hellwig wrote:
> On Fri, Jun 19, 2015 at 10:17:10AM -0500, Eric Sandeen wrote:
>> though we also claim to nominally support building on osx, irix, etc,
>> which doesn't have that library, right?
>
> Debian supports it for kfreebsd and hurd, and
> https://trac.macports.org/ticket/15546 suggest an older version did
> build on OS X. So I don't think it's a real problem even if IRIX
> might require some minor porting efforts.
>
Ok. And honestly, building xfsprogs on OSX seems like nothing
more than a curiosity ...
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread
* PATCH V2] mkfs.xfs: always use underlying fs sector size when mkfs'ing a file
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-19 17:09 ` Eric Sandeen
1 sibling, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2015-06-19 17:09 UTC (permalink / raw)
To: Eric Sandeen, xfs-oss
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-06-19 17:20 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` PATCH V2] " Eric Sandeen
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.