Created attachment 15367 [details] Patch for 4.11 https://gitlab.com/samba-team/devel/samba/pipelines/74335669
Created attachment 15368 [details] Patch for 4.10 https://gitlab.com/samba-team/devel/samba/pipelines/74335691
Created attachment 15369 [details] Patch for 4.9 https://gitlab.com/samba-team/devel/samba/pipelines/74335906
Re-assigning to Karolin for inclusion in 4.11.RCnext, 4.10.next, 4.9.next
(In reply to Jeremy Allison from comment #4) We have an issue with above patches where file open resulting in "Invalid file handle" error on Windows. See the following thread on samba-technical: https://lists.samba.org/archive/samba-technical/2019-August/134039.html
(In reply to Anoop C S from comment #5) Hi Anoop, can you fix this by removing the O_NONBLOCK from the open call inside vfs_glusterfs ? Adding the O_NONBLOCK is correct for the local filesystem side of things, so if glusterfs needs something different it'll need to cope. Adding a test environment for gluster into Samba would also be a really good way to ensure we don't cause problems like this in the future - but currently our base test environment is the local filesystem, which I think is the correct decision.
(In reply to Jeremy Allison from comment #6) I don't think that helps with the problem. We're making a call to fcntl() via set_blocking() called from open_file() which of course fails with fake fds returned from vfs_glusterfs open. Lacking aSMB_VFS_FCNTL() we may want to somehow funnel this through SMB_VFS_FSCTL().
Oh, I see the problem now. We're opening with O_NONBLOCK (which is OK for gluster), then we run into: if (local_flags & O_NONBLOCK) { /* * GPFS can return ETIMEDOUT for pread on * nonblocking file descriptors when files * migrated to tape need to be recalled. I * could imagine this happens elsewhere * too. With blocking file descriptors this * does not happen. */ ret = set_blocking(fsp->fh->fd, true); if (ret == -1) { status = map_nt_error_from_unix(errno); DBG_WARNING("Could not set fd to blocking: " "%s\n", strerror(errno)); fd_close(fsp); return status; } } where fsp->fh->fd is a fake gluster-created fd. Yes, ultimately we need to add SMB_VFS_FCNTL() for operation on fd's returned from the VFS. This isn't really a problem with the original patch, it's a more generic issue with how we're handling fd's returned from the VFS and making sure we only vector through the VFS for this. A test infrastructure for gluster/ceph that is integrated with Samba would help find these things before they go it.
In the meantime, vfs_gluster can fix this by creating a local fd using pipe(), and then returning one end of this from: vfs_gluster_open(), instead of 13371337. Does this also cause a problem for ceph ? We might want to fix this in master by adding SMB_VFS_FCNTL(), and leave the local kernel oplock bug alone in 4.11 and below until we get the fixed VFS in 4.12.
(In reply to Jeremy Allison from comment #9) I will try suggested changes and confirm whether it can fix the issue for now.
Created attachment 15436 [details] vfs_glusterfs temp fix for master Ok. After offline discussions I have come up with the attached patch which fixes the issue. patch is also available at https://lists.samba.org/archive/samba-technical/2019-August/134128.html
(In reply to Anoop C S from comment #11) Hi Anoop - that patch looks good to me (once you remove the //FIXME comment :-). Can you get Guenther to +1 it and we'll push to master. Can you log a separate bug for this (and make it depend on this one) so we can track the backports separately. Thanks, Jeremy.
(In reply to Jeremy Allison from comment #12) Wait.. I am confused on our way forward w.r.t this bug. Based on your comment #9 it was my impression that we implement SMB_VFS_FCNTL in future and fix it with 4.12 leaving current stable branches with original bug. Following that vfs_glusterfs fix was prepared out of your suggestion as a temporary code change in just master. What did I get wrong here?
I think 4.12 was forked from master and contains the SMB_VFS_FCNTL() call, so we don't need it for 4.12. For 4.11 and below you can add the "dummy-fd" fix. Does that answer the question Anoop ? Jeremy.
(In reply to Jeremy Allison from comment #14) We don't need fake-fd fix in v4.11 and below because the original fixes for this bug are only present in v4.12 and master. I could see the backport patches for v4.11, v4.10 and v4.9 as attachments and reviewed but not pulled in yet. I will attach fake-fd fix backports in bug #14241 as soon as original fixes for this bug gets into v4.11 and v4.10.
OK, that works. Thanks for your help !
@Volker, Given that v4.10 is discontinued and v4.11 is in "security fixes only" mode, shall we close this bug as RESOLVED as actual vfs_glusterfs fix has been merged and present from v4.12.
Closing as we have required patches in current stable release versions(v4.12 and above).