On 4/8/16 9:58 AM, Jan Tulak wrote:
> Ok, platform_findsizes already explicitly handled regular files, and tries to
> find out via an xfs ioctl what the minimum DIO size is, and uses that for
> the sector size for the filesystem in the iamge.
>
>
> Now you stat & get the blocksize, and use that instead, but it's likely
> to be different:
>
> i.e. before:
>
> # mkfs/mkfs.xfs -f fsfile
> meta-data=fsfile isize=512 agcount=4, agsize=65536 blks
> = sectsz=512 attr=2, projid32bit=1
>
> after:
>
> # mkfs/mkfs.xfs -f fsfile
> meta-data=fsfile isize=512 agcount=4, agsize=65536 blks
> = sectsz=4096 attr=2, projid32bit=1
>
> and also, now:
>
> # mkfs/mkfs.xfs -f -dfile,name=fsfile,size=1g -b size=2048
> block size 2048 cannot be smaller than logical sector size 4096
>
> What prompted you to make this change, was there some other problem you
> needed to fix?
>
>
> But DIO is disabled for the files, per the commit message:
> [...] and turning off
> direct IO. Then ensure that we check the "isfile" options before
> doing things that are specific to block devices. Also, as direct IO
> is disabled for files, use statfs() for getting host FS blocksize,
> not platform_findsizes().
But doing DIO to the file during mkfs isn't the issue I'm talking about;
For example a vm image hosted in a file will have direct IO done to it
when it is in use, and filesystem block size is not the controlling
factor there.
With your change, we can no longer make i.e. a 2k fs image hosted on a 4k
fs. i.e. your change regresses this commit:
commit 98dd72d3b239050138cf9eb9373c83743878a7d2
Author: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri Dec 18 12:15:25 2015 +1100
mkfs: get sector size from host fs d_miniosz when mkfs'ing file
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@xxxxxxxxxx>
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
> So we have to use whatever the underlying fs tells us, not what the physical device has, right?
Well, tells us about what? XFS can tell us its block size, but also can
tell us about minimum size and alignment required for direct IO, which is
more relevant to a filesystem image than the filesystem's block size.
> Rather, I wonder if there is any reason to keep the
> platform_findsizes part about regular files - it shouldn't get into
> the branch ever.
In general, having a wrapper which finds sizes of a target, regardless of
platform, device, or file, makes sense to me rather than having stat calls
in various other places...
-Eric
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
