Windows explorer displays error dialogue box(Invalid file handle) while trying to open file/s from a GlusterFS volume shared using vfs_glusterfs module within Samba. Additional info: The following commit changed the way in which O_NONBLOCK is added/removed from file status flags during open. https://git.samba.org/?p=samba.git;a=commit;h=ef582ffcf3a220b73f678d9bce0fd37800f76c54 smbd: Always open files with O_NONBLOCK Since O_NONBLOCK is now internally getting added unconditionally, storage backed by GlusterFS volumes using vfs_glusterfs produces *Invalid file handle* error while opening files(I tested from Windows). set_blocking() from open_file() is now being invoked due to the presence of O_NONBLOCK flag and fails at fcntl() call operating on fsp->fh->fd which is obviously bad and returns EBADF.
Created attachment 15742 [details] patch-for-v4-12 Adding the patch as discussed offline.
Comment on attachment 15742 [details] patch-for-v4-12 Pipeline: https://gitlab.com/samba-team/devel/samba/pipelines/110183222
Comment on attachment 15742 [details] patch-for-v4-12 Lgtm.
Comment on attachment 15742 [details] patch-for-v4-12 LGTM
IIUC, this is also going to affect other VFS modules which provide a dummy fd, e.g. vfs_ceph. I think a proper fix here would be to move the fcntl call into the VFS. Or am I missing something?
(In reply to David Disseldorp from comment #5) The fcntl call did get moved into the VFS layer in master as commit 0abd1189a60eea4501b5279ebc4bff2b5689f888. I think this is a back-port for 4.12 which can't change the VFS interface (correct me if I'm wrong please Anoop).
Oh, this went into master too. Anoop, why can't you do this via adding a gluster-specific SMB_VFS_FCNTL() call that got added in master ? I can see why this is done for the back-port but in master we should have a method of doing it the clean way David mentioned.
(In reply to Jeremy Allison from comment #7) Yes, that is the plan. But as of now GlusterFS doesn't have an API implementation to support SMB_VFS_FCNTL() interface in Samba. I made the proposal(https://lists.gluster.org/pipermail/gluster-devel/2019-October/056629.html) sometime back but didn't get much interest at first pass. Due to time constraints this bug is being raised to get the issue fixed with upcoming(soon approaching) v4.12.
You don't need to have an interface for this inside glusterfs, just filter out the O_NONBLOCK setting on a gluster fd handle. It's only being done for Linux-specific-fd lease code calls anyway, so just add an implementation in glusterfs and ignore the call if it's setting or removing O_NONBLOCK. I understand you might need this for a released version, but not in master. In master you should be doing it right - that's why you added the SMB_VFS_FCNTL call in the first place.
Created attachment 15745 [details] SMB_VFS_FCNTL in vfs_glusterfs Hm..patch itself is already in master. I never noticed it being merged. Shall we leave as it is for v4.12 to get branched from master(till tomorrow)? I am attaching a new patch for SMB_VFS_FCNTL implementation in vfs_glusterfs. Please have a look at it. Let me know your thoughts..
(In reply to Anoop C S from comment #10) > Created attachment 15745 [details] > SMB_VFS_FCNTL in vfs_glusterfs > > Hm..patch itself is already in master. I never noticed it being merged. > > Shall we leave as it is for v4.12 to get branched from master(till > tomorrow)? I am attaching a new patch for SMB_VFS_FCNTL implementation in > vfs_glusterfs. Please have a look at it. > > Let me know your thoughts.. Oops, only just saw this, sorry. I've submitted vfs_ceph and vfs_glusterfs fcntl handlers via https://gitlab.com/samba-team/samba/merge_requests/1076 . I think it's probably worth reverting the fake fd change c9adf47a.
Yep. Can you add the revert into the gitlab merge request please ?
Anoop, can you confirm David's patch fixes this bug for you ? Thanks, Jeremy.
(In reply to Jeremy Allison from comment #13) Patch from https://gitlab.com/samba-team/samba/merge_requests/1076 also works fine.
Comment on attachment 15745 [details] SMB_VFS_FCNTL in vfs_glusterfs Isn't this one already in 4.12rc1 ? I think you only need the fd-hack for 4.11 and prior releases.
(In reply to Jeremy Allison from comment #15) Yes. David added SMB_VFS_FCNTL() implementation in vfs_glusterfs and is part of v4.12rc1. I will attach fake-fd fix backport for v4.11 and v4.10 after fixes for bug #14060 lands in those branches.
(In reply to Anoop C S from comment #16) Fix for bug #14060 is only present from v4.12(and above) and corresponding vfs_glusterfs fixes are present along with it. Therefore closing the bug report.