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

[Alexandre Lécuyer]

On 12/30/2015 07:52 PM, Jeremy Allison wrote:
> 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.
Hello,

Linux also lacks f_iosize, I have added a test for f_fstr, which I only 
find in solaris's statvfs.
I have compiled samba with the attached patch on linux, freebsd, and 
solaris and checked that only the correct function is compiled and 
linked in libsmbd-base-samba4.so.

Regards,
Alex
-------------- next part --------------
A non-text attachment was scrubbed...
Name: solaris_statvfs.patch
Type: text/x-patch
Size: 3580 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160104/a3b04800/solaris_statvfs.bin>