<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><span style="font-family:arial,sans-serif">On Fri, Apr 8, 2016 at 5:50 PM, Eric Sandeen </span><span dir="ltr" style="font-family:arial,sans-serif"><<a href="mailto:sandeen@sandeen.net" target="_blank">sandeen@sandeen.net</a>></span><span style="font-family:arial,sans-serif"> wrote:</span><br></div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 4/8/16 9:58 AM, Jan Tulak wrote:<br>
> Ok, platform_findsizes already explicitly handled regular files, and tries to<br>
> find out via an xfs ioctl what the minimum DIO size is, and uses that for<br>
> the sector size for the filesystem in the iamge.<br>
><br>
><br>
> Now you stat & get the blocksize, and use that instead, but it's likely<br>
> to be different:<br>
><br>
> i.e. before:<br>
><br>
> # mkfs/mkfs.xfs -f fsfile<br>
> meta-data=fsfile isize=512 agcount=4, agsize=65536 blks<br>
> = sectsz=512 attr=2, projid32bit=1<br>
><br>
> after:<br>
><br>
> # mkfs/mkfs.xfs -f fsfile<br>
> meta-data=fsfile isize=512 agcount=4, agsize=65536 blks<br>
> = sectsz=4096 attr=2, projid32bit=1<br>
><br>
> and also, now:<br>
><br>
> # mkfs/mkfs.xfs -f -dfile,name=fsfile,size=1g -b size=2048<br>
> block size 2048 cannot be smaller than logical sector size 4096<br>
><br>
> What prompted you to make this change, was there some other problem you<br>
> needed to fix?<br>
><br>
><br>
> But DIO is disabled for the files, per the commit message:<br>
> [...] and turning off<br>
> direct IO. Then ensure that we check the "isfile" options before<br>
> doing things that are specific to block devices. Also, as direct IO<br>
> is disabled for files, use statfs() for getting host FS blocksize,<br>
> not platform_findsizes().<br>
<br>
</div></div>But doing DIO to the file during mkfs isn't the issue I'm talking about;<br>
For example a vm image hosted in a file will have direct IO done to it<br>
when it is in use, and filesystem block size is not the controlling<br>
factor there.<br>
<br>
With your change, we can no longer make i.e. a 2k fs image hosted on a 4k<br>
fs. i.e. your change regresses this commit:<br>
<br>
commit 98dd72d3b239050138cf9eb9373c83743878a7d2<br>
Author: Eric Sandeen <<a href="mailto:sandeen@sandeen.net">sandeen@sandeen.net</a>><br>
Date: Fri Dec 18 12:15:25 2015 +1100<br>
<br>
mkfs: get sector size from host fs d_miniosz when mkfs'ing file<br>
<br>
Now that we allow logical-sector-sized DIOs even if our xfs<br>
filesystem is set to physical-sector-sized "sectors," we can<br>
allow the creation of filesystem images with block and sector<br>
sizes down to the host device's logical sector size, rather<br>
than the filesystem's sector size.<br>
<br>
So in platform_findsizes(), change our query of the filesystem<br>
to an XFS_IOC_DIOINFO query, and use the returned d_miniosz for<br>
sector size in the S_IFREG case.<br>
<br>
This allows the creation of i.e. a 2k block sized image on<br>
an xfs filesystem w/ 4k sector size created on a 4k/512<br>
block device, which would otherwise fail today.<br>
<br>
Signed-off-by: Eric Sandeen <<a href="mailto:sandeen@redhat.com">sandeen@redhat.com</a>><br>
Reviewed-by: Dave Chinner <<a href="mailto:dchinner@redhat.com">dchinner@redhat.com</a>><br>
Signed-off-by: Dave Chinner <<a href="mailto:david@fromorbit.com">david@fromorbit.com</a>><br>
<span class=""><br>
<br>
> So we have to use whatever the underlying fs tells us, not what the physical device has, right?<br>
<br>
</span>Well, tells us about what? XFS can tell us its block size, but also can<br>
tell us about minimum size and alignment required for direct IO, which is<br>
more relevant to a filesystem image than the filesystem's block size.<br>
<span class=""><br>
> Rather, I wonder if there is any reason to keep the<br>
> platform_findsizes part about regular files - it shouldn't get into<br>
> the branch ever.<br>
<br>
</span>In general, having a wrapper which finds sizes of a target, regardless of<br>
platform, device, or file, makes sense to me rather than having stat calls<br>
in various other places...<br>
<span class="HOEnZb"><font color="#888888"><br>
-Eric<br></font></span></blockquote><div><br></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;display:inline">It seems I misunderstood some things about what happens/what should happen with files. :-)</div></div><div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif;display:inline">I will see what happens with the tests, what difference in the results is with this patch in the original version.</div></div></div><div class="gmail_extra"><br></div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Cheers,</div><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Jan</div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Jan Tulak<br></div><a href="mailto:jtulak@redhat.com" target="_blank">jtulak@redhat.com</a> / <a href="mailto:jan@tulak.me" target="_blank">jan@tulak.me</a></div></div></div></div>
</div></div>