Re: [PATCH 15/19] mkfs: don't treat files as though they are block devices

[Eric Sandeen <sandeen@sandeen.net>]

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@sandeen.net>
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@redhat.com>
    Reviewed-by: Dave Chinner <dchinner@redhat.com>
    Signed-off-by: Dave Chinner <david@fromorbit.com>


> 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@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs