----- Original Message -----
> From: "Eric Sandeen" <sandeen@xxxxxxxxxxx>
> To: "Jan Tulak" <jtulak@xxxxxxxxxx>, "Eric Sandeen" <sandeen@xxxxxxxxxx>
> Cc: "xfs-oss" <xfs@xxxxxxxxxxx>
> 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@xxxxxxxxxx>
> >> To: "xfs-oss" <xfs@xxxxxxxxxxx>
> >> 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@xxxxxxxxxx>
> >> ---
> >>
> >> (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@xxxxxxxxxx
|