Bug 13208 - vfs_ceph uses a local statvfs() call to determine FS capabilities
Summary: vfs_ceph uses a local statvfs() call to determine FS capabilities
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-10 00:32 UTC by David Disseldorp
Modified: 2018-01-31 09:45 UTC (History)
1 user (show)

See Also:


Attachments
patchset for 4.6.next (3.54 KB, patch)
2018-01-16 08:58 UTC, David Disseldorp
jra: review-
Details
patchset for 4.7.next (3.53 KB, patch)
2018-01-16 08:59 UTC, David Disseldorp
jra: review+
Details
v2 patchset for 4.6.next (3.55 KB, patch)
2018-01-17 09:44 UTC, David Disseldorp
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2018-01-10 00:32:19 UTC
vfs_ceph doesn't currently implement an fs_capabilities() hook, which sees it fallback to the vfs_default code-path, and subsequently call statvfs() against the share path on the *local* filesystem.
Comment 1 David Disseldorp 2018-01-16 08:58:06 UTC
Created attachment 13908 [details]
patchset for 4.6.next
Comment 2 David Disseldorp 2018-01-16 08:59:46 UTC
Created attachment 13909 [details]
patchset for 4.7.next
Comment 3 David Disseldorp 2018-01-16 09:06:32 UTC
@Jeremy: I've included the vfs_default fs_capabilities() statvfs->VFS_STATVFS change, which could be considered a (minor) API change. I could drop this if you have any concerns.
Comment 4 Jeremy Allison 2018-01-16 22:32:02 UTC
No, I think this is OK. The obvious intent was for all calls to go through the VFS by default so I don't think this is going to cause a problem.
Comment 5 Jeremy Allison 2018-01-16 23:02:23 UTC
Comment on attachment 13908 [details]
patchset for 4.6.next

This needs fixing for 4.6 David I'm afraid. statvfs takes a const char * in 4.6, not a struct smb_filename pointer.

../source3/modules/vfs_default.c: In function ‘vfswrap_fs_capabilities’:
../source3/modules/vfs_default.c:131:2: error: passing argument 2 of ‘smb_vfs_call_statvfs’ from incompatible pointer type [-Werror]
  ret = SMB_VFS_STATVFS(conn, smb_fname_cpath, &statbuf);
  ^
In file included from ../source3/include/smb.h:155:0,
                 from ../source3/include/includes.h:325,
                 from ../source3/modules/vfs_default.c:21:
../source3/include/vfs.h:1012:5: note: expected ‘const char *’ but argument is of type ‘struct smb_filename *’
 int smb_vfs_call_statvfs(struct vfs_handle_struct *handle, const char *path,
Comment 6 David Disseldorp 2018-01-17 09:44:55 UTC
Created attachment 13915 [details]
v2 patchset for 4.6.next

(In reply to Jeremy Allison from comment #5)
> Comment on attachment 13908 [details]
> patchset for 4.6.next
> 
> This needs fixing for 4.6 David I'm afraid. statvfs takes a const char * in
> 4.6, not a struct smb_filename pointer.
> 
> ../source3/modules/vfs_default.c: In function ‘vfswrap_fs_capabilities’:
> ../source3/modules/vfs_default.c:131:2: error: passing argument 2 of
> ‘smb_vfs_call_statvfs’ from incompatible pointer type [-Werror]
>   ret = SMB_VFS_STATVFS(conn, smb_fname_cpath, &statbuf);
>   ^
> In file included from ../source3/include/smb.h:155:0,
>                  from ../source3/include/includes.h:325,
>                  from ../source3/modules/vfs_default.c:21:
> ../source3/include/vfs.h:1012:5: note: expected ‘const char *’ but argument
> is of type ‘struct smb_filename *’
>  int smb_vfs_call_statvfs(struct vfs_handle_struct *handle, const char *path,

Blah, I overlooked this warning in my test build, sorry. See new version attached.
Comment 7 Jeremy Allison 2018-01-18 00:34:05 UTC
Re-assigning to Karolin for inclusion in 4.7.next, 4.6.next.
Comment 8 Karolin Seeger 2018-01-22 07:50:48 UTC
Pushed to autobuild-v4-{7,6}-test.
Comment 9 Karolin Seeger 2018-01-31 09:45:04 UTC
(In reply to Karolin Seeger from comment #8)
Pushed to both branches.
Closing out bug report.

Thanks!