sys_statvfs - Samba on illumos/solaris, linux df showing the wrong size

[Jeremy Allison]

On Wed, Dec 30, 2015 at 03:02:27PM +0100, Alexandre Lécuyer wrote:
> Hello,
> 
> I have come across a situation where "df" on a linux client will show the
> wrong filesystem size.
> 
> Server side :
> Filesystem             Size   Used  Available Capacity  Mounted on
> testpool/test           10G    96K        10G     1% /testpool/test
> 
> Client side :
> Filesystem                   Size  Used Avail Use% Mounted on
> //10.6.2.4/testpool_test  2.5T   24M  2.5T   1% /cifs
> 
> The discrepancy is caused by this difference in the statvfs() syscall result
> :
> Server side :
> f_blocks: 20971520
> f_frsize: 512
> f_bsize: 131072
> 
> Client side :
> f_blocks: 20971520
> f_frsize: 131072
> f_bsize: 131072
> 
> 
> The server is running illumos (né opensolaris, I believe solaris will have
> the same behavior), and the filesystem is ZFS.
> The manpage for statvfs() has this to say :
> 
>          u_long      f_bsize;             /* preferred file system block
> size */
>          u_long      f_frsize;            /* fundamental filesystem block
>                                              (size if supported) */
> 
> f_bsize is the "preferred file system block size". It is populated with the
> value of ZFS's recordsize, which in this example is 128k (default value).
> f_frsize is the supported block size, 512 bytes.
> 
> Now on the samba side, sys_statvfs will be either linux_statvfs or
> bsd_statvfs, detected at ./configure time. On solaris, samba will use
> linux_statvfs because solaris statvfs struct lacks f_iosize.
> linux_statvfs does this :
>         statbuf->OptimalTransferSize = statvfs_buf.f_frsize;
>         statbuf->BlockSize = statvfs_buf.f_bsize;
> 
> Which does not match the manpage definition. BlockSize should be made equal
> to f_bsize, not f_frsize.
> Indeed, recompiling with this change fixes the problem.
> 
> A proper fix might be to detect illumos/solaris statvfs, and introduce a
> solaris_statvfs() function, along with existing linux_statvfs and
> bsd_statvfs().
> Before I attempt to write a clean patch, does this seem sensible or am I off
> base here ?

Always test for features, not platforms. So add a test for a statvfs
without f_iosize, and then select that.