The Samba-Bugzilla – Attachment 10151 Details for
Bug 10741
VFS gpfs offline bit is flapping
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
complete patchset for 4.1
bug-10741.v4-1-test.patchset (text/plain), 15.74 KB, created by
Michael Adam
on 2014-07-24 13:00:39 UTC
(
hide
)
Description:
complete patchset for 4.1
Filename:
MIME Type:
Creator:
Michael Adam
Created:
2014-07-24 13:00:39 UTC
Size:
15.74 KB
patch
obsolete
>From ee97bc696ffd331d8c4af35201724d9dfc8c4313 Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Thu, 3 Jul 2014 10:07:37 +0200 >Subject: [PATCH 1/6] s3:vfs:gpfs: store the winAttrs in the struct_ex when we > got them in vfs_gpfs_fstat() > >This may (e.g.) have lead to some occurrences of flapping offline bits. > >Signed-off-by: Michael Adam <obnox@samba.org> >Reviewed-by: Christof Schmitt <cs@samba.org> >(cherry picked from commit 573ca6ef6b8376800d8fc988d67909e103b74656) > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=10741 >--- > source3/modules/vfs_gpfs.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c >index 4a53bf8..ca8f80d 100644 >--- a/source3/modules/vfs_gpfs.c >+++ b/source3/modules/vfs_gpfs.c >@@ -1476,6 +1476,7 @@ static int vfs_gpfs_fstat(struct vfs_handle_struct *handle, > sbuf->st_ex_calculated_birthtime = false; > sbuf->st_ex_btime.tv_sec = attrs.creationTime.tv_sec; > sbuf->st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec; >+ sbuf->vfs_private = attrs.winAttrs; > } > return 0; > } >-- >1.9.1 > > >From bb379eb29a217b78e85d104f5d4b324c4a2058dd Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Thu, 3 Jul 2014 10:10:11 +0200 >Subject: [PATCH 2/6] s3:vfs:gpfs: fix flapping offline: always get winAttrs > from gpfs for is_offline > >There is a problem of flapping offline due to uninitialized >stat buffers. Due to a optimization in vfswrap_readdir which >directly calling fastatat (i.e. not through vfs), marking the >stat buffer valid, there is nothing this module can do about >it and hence can not currently not rely on the vaildity of >the stat buffer. > >By always calling out to GPFS even when the stat buffer is >flagged valid, we can always return correct offline information, >thereby sacrificing the readdir optimization. > >Pair-Programmed-With: Volker Lendecke <vl@samba.org> > >Signed-off-by: Michael Adam <obnox@samba.org> >Reviewed-by: Christof Schmitt <cs@samba.org> >(cherry picked from commit 31e67507144aae8d5a8ec49587ac89d2d94636f0) > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=10741 >--- > source3/modules/vfs_gpfs.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > >diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c >index ca8f80d..614c56d 100644 >--- a/source3/modules/vfs_gpfs.c >+++ b/source3/modules/vfs_gpfs.c >@@ -1652,9 +1652,7 @@ static bool vfs_gpfs_is_offline(struct vfs_handle_struct *handle, > return -1; > } > >- if (VALID_STAT(*sbuf)) { >- attrs.winAttrs = sbuf->vfs_private; >- } else { >+ { > int ret; > ret = get_gpfs_winattrs(path, &attrs); > >-- >1.9.1 > > >From 7198af762bb9992c7ada1b3570f6c783215bd98f Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Thu, 17 Jul 2014 17:22:09 +0200 >Subject: [PATCH 3/6] s3:vfs:gpfs: Remove all reading uses of > stat_ex.vfs_private from vfs_gfs. > >This was used as a cache for offline-info in the stat buffer. >But as the implementation of gpfs_is_offline() showed, this cache >does not always carry valid information when the stat itself is valid >(since at least one call goes to fstatat() directly, circumventing > the vfs). > >So the correct thing is to always call SMB_VFS_IS_OFFLINE() >when checking whether a file is offline. For the pread and pwrite >calls, we need to call IS_OFFLINE before the actual read >and check afterwards if the file was offline before (as a basis >whether to send notifications). > >Signed-off-by: Michael Adam <obnox@samba.org> >Reviewed-by: Christof Schmitt <cs@samba.org> >(cherry picked from commit 16a040f8ef7f2f594505ef07e6f9b77df8f1d725) > >Conflicts: > source3/modules/vfs_gpfs.c > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=10741 >--- > source3/modules/vfs_gpfs.c | 41 ++++++++++++++++++++--------------------- > 1 file changed, 20 insertions(+), 21 deletions(-) > >diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c >index 614c56d..a1612e2 100644 >--- a/source3/modules/vfs_gpfs.c >+++ b/source3/modules/vfs_gpfs.c >@@ -1681,7 +1681,8 @@ static ssize_t vfs_gpfs_sendfile(vfs_handle_struct *handle, int tofd, > files_struct *fsp, const DATA_BLOB *hdr, > off_t offset, size_t n) > { >- if ((fsp->fsp_name->st.vfs_private & GPFS_WINATTR_OFFLINE) != 0) { >+ if (SMB_VFS_IS_OFFLINE(handle->conn, fsp->fsp_name, &fsp->fsp_name->st)) >+ { > errno = ENOSYS; > return -1; > } >@@ -1952,14 +1953,14 @@ static ssize_t vfs_gpfs_pread(vfs_handle_struct *handle, files_struct *fsp, > void *data, size_t n, off_t offset) > { > ssize_t ret; >+ bool was_offline; > >- ret = SMB_VFS_NEXT_PREAD(handle, fsp, data, n, offset); >+ was_offline = SMB_VFS_IS_OFFLINE(handle->conn, fsp->fsp_name, >+ &fsp->fsp_name->st); > >- DEBUG(10, ("vfs_private = %x\n", >- (unsigned int)fsp->fsp_name->st.vfs_private)); >+ ret = SMB_VFS_NEXT_PREAD(handle, fsp, data, n, offset); > >- if ((ret != -1) && >- ((fsp->fsp_name->st.vfs_private & GPFS_WINATTR_OFFLINE) != 0)) { >+ if ((ret != -1) && was_offline) { > fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > notify_fname(handle->conn, NOTIFY_ACTION_MODIFIED, > FILE_NOTIFY_CHANGE_ATTRIBUTES, >@@ -1973,6 +1974,7 @@ struct vfs_gpfs_pread_state { > struct files_struct *fsp; > ssize_t ret; > int err; >+ bool was_offline; > }; > > static void vfs_gpfs_pread_done(struct tevent_req *subreq); >@@ -1991,6 +1993,8 @@ static struct tevent_req *vfs_gpfs_pread_send(struct vfs_handle_struct *handle, > if (req == NULL) { > return NULL; > } >+ state->was_offline = SMB_VFS_IS_OFFLINE(handle->conn, fsp->fsp_name, >+ &fsp->fsp_name->st); > state->fsp = fsp; > subreq = SMB_VFS_NEXT_PREAD_SEND(state, ev, handle, fsp, data, > n, offset); >@@ -2024,11 +2028,7 @@ static ssize_t vfs_gpfs_pread_recv(struct tevent_req *req, int *err) > } > *err = state->err; > >- DEBUG(10, ("vfs_private = %x\n", >- (unsigned int)fsp->fsp_name->st.vfs_private)); >- >- if ((state->ret != -1) && >- ((fsp->fsp_name->st.vfs_private & GPFS_WINATTR_OFFLINE) != 0)) { >+ if ((state->ret != -1) && state->was_offline) { > fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > DEBUG(10, ("sending notify\n")); > notify_fname(fsp->conn, NOTIFY_ACTION_MODIFIED, >@@ -2043,14 +2043,14 @@ static ssize_t vfs_gpfs_pwrite(vfs_handle_struct *handle, files_struct *fsp, > const void *data, size_t n, off_t offset) > { > ssize_t ret; >+ bool was_offline; > >- ret = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset); >+ was_offline = SMB_VFS_IS_OFFLINE(handle->conn, fsp->fsp_name, >+ &fsp->fsp_name->st); > >- DEBUG(10, ("vfs_private = %x\n", >- (unsigned int)fsp->fsp_name->st.vfs_private)); >+ ret = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset); > >- if ((ret != -1) && >- ((fsp->fsp_name->st.vfs_private & GPFS_WINATTR_OFFLINE) != 0)) { >+ if ((ret != -1) && was_offline) { > fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > notify_fname(handle->conn, NOTIFY_ACTION_MODIFIED, > FILE_NOTIFY_CHANGE_ATTRIBUTES, >@@ -2064,6 +2064,7 @@ struct vfs_gpfs_pwrite_state { > struct files_struct *fsp; > ssize_t ret; > int err; >+ bool was_offline; > }; > > static void vfs_gpfs_pwrite_done(struct tevent_req *subreq); >@@ -2083,6 +2084,8 @@ static struct tevent_req *vfs_gpfs_pwrite_send( > if (req == NULL) { > return NULL; > } >+ state->was_offline = SMB_VFS_IS_OFFLINE(handle->conn, fsp->fsp_name, >+ &fsp->fsp_name->st); > state->fsp = fsp; > subreq = SMB_VFS_NEXT_PWRITE_SEND(state, ev, handle, fsp, data, > n, offset); >@@ -2116,11 +2119,7 @@ static ssize_t vfs_gpfs_pwrite_recv(struct tevent_req *req, int *err) > } > *err = state->err; > >- DEBUG(10, ("vfs_private = %x\n", >- (unsigned int)fsp->fsp_name->st.vfs_private)); >- >- if ((state->ret != -1) && >- ((fsp->fsp_name->st.vfs_private & GPFS_WINATTR_OFFLINE) != 0)) { >+ if ((state->ret != -1) && state->was_offline) { > fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > DEBUG(10, ("sending notify\n")); > notify_fname(fsp->conn, NOTIFY_ACTION_MODIFIED, >-- >1.9.1 > > >From b684d9cbe7762049fc4d71557df0b2d76cf85f39 Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Thu, 17 Jul 2014 23:35:58 +0200 >Subject: [PATCH 4/6] s3:vfs:gpfs: remove all writing uses of > stat_ex.vfs_private from vfs_gpfs. > >Now that the vfs_private cache is never read in vfs_gpfs, there is >no need any more to write it. > >With this change, vfs_gpfs does not use vfs_private any more. > >Signed-off-by: Michael Adam <obnox@samba.org> >Reviewed-by: Christof Schmitt <cs@samba.org> >(cherry picked from commit d87d13f4c2a77c03bbffcd0fe4fc9464d9024cae) > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=10741 >--- > source3/modules/vfs_gpfs.c | 7 ------- > 1 file changed, 7 deletions(-) > >diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c >index a1612e2..193cfad 100644 >--- a/source3/modules/vfs_gpfs.c >+++ b/source3/modules/vfs_gpfs.c >@@ -1444,7 +1444,6 @@ static int vfs_gpfs_stat(struct vfs_handle_struct *handle, > smb_fname->st.st_ex_calculated_birthtime = false; > smb_fname->st.st_ex_btime.tv_sec = attrs.creationTime.tv_sec; > smb_fname->st.st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec; >- smb_fname->st.vfs_private = attrs.winAttrs; > } > return 0; > } >@@ -1476,7 +1475,6 @@ static int vfs_gpfs_fstat(struct vfs_handle_struct *handle, > sbuf->st_ex_calculated_birthtime = false; > sbuf->st_ex_btime.tv_sec = attrs.creationTime.tv_sec; > sbuf->st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec; >- sbuf->vfs_private = attrs.winAttrs; > } > return 0; > } >@@ -1513,7 +1511,6 @@ static int vfs_gpfs_lstat(struct vfs_handle_struct *handle, > smb_fname->st.st_ex_calculated_birthtime = false; > smb_fname->st.st_ex_btime.tv_sec = attrs.creationTime.tv_sec; > smb_fname->st.st_ex_btime.tv_nsec = attrs.creationTime.tv_nsec; >- smb_fname->st.vfs_private = attrs.winAttrs; > } > return 0; > } >@@ -1961,7 +1958,6 @@ static ssize_t vfs_gpfs_pread(vfs_handle_struct *handle, files_struct *fsp, > ret = SMB_VFS_NEXT_PREAD(handle, fsp, data, n, offset); > > if ((ret != -1) && was_offline) { >- fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > notify_fname(handle->conn, NOTIFY_ACTION_MODIFIED, > FILE_NOTIFY_CHANGE_ATTRIBUTES, > fsp->fsp_name->base_name); >@@ -2029,7 +2025,6 @@ static ssize_t vfs_gpfs_pread_recv(struct tevent_req *req, int *err) > *err = state->err; > > if ((state->ret != -1) && state->was_offline) { >- fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > DEBUG(10, ("sending notify\n")); > notify_fname(fsp->conn, NOTIFY_ACTION_MODIFIED, > FILE_NOTIFY_CHANGE_ATTRIBUTES, >@@ -2051,7 +2046,6 @@ static ssize_t vfs_gpfs_pwrite(vfs_handle_struct *handle, files_struct *fsp, > ret = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset); > > if ((ret != -1) && was_offline) { >- fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > notify_fname(handle->conn, NOTIFY_ACTION_MODIFIED, > FILE_NOTIFY_CHANGE_ATTRIBUTES, > fsp->fsp_name->base_name); >@@ -2120,7 +2114,6 @@ static ssize_t vfs_gpfs_pwrite_recv(struct tevent_req *req, int *err) > *err = state->err; > > if ((state->ret != -1) && state->was_offline) { >- fsp->fsp_name->st.vfs_private &= ~GPFS_WINATTR_OFFLINE; > DEBUG(10, ("sending notify\n")); > notify_fname(fsp->conn, NOTIFY_ACTION_MODIFIED, > FILE_NOTIFY_CHANGE_ATTRIBUTES, >-- >1.9.1 > > >From d6c95abf58434f5610d1b65f8ba9a7aecbebfcb2 Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Thu, 17 Jul 2014 17:06:32 +0200 >Subject: [PATCH 5/6] s3:vfs:gpfs: remove a block and reduce indentation in > gpfs_is_offline() > >Signed-off-by: Michael Adam <obnox@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >Reviewed-by: Christof Schmitt <cs@samba.org> >(cherry picked from commit eb0577dca04a2fde4691094a006954d417d1cf22) > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=10741 >--- > source3/modules/vfs_gpfs.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > >diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c >index 193cfad..f9eb7e8 100644 >--- a/source3/modules/vfs_gpfs.c >+++ b/source3/modules/vfs_gpfs.c >@@ -1634,6 +1634,7 @@ static bool vfs_gpfs_is_offline(struct vfs_handle_struct *handle, > char *path = NULL; > NTSTATUS status; > struct gpfs_config_data *config; >+ int ret; > > SMB_VFS_HANDLE_GET_DATA(handle, config, > struct gpfs_config_data, >@@ -1649,15 +1650,12 @@ static bool vfs_gpfs_is_offline(struct vfs_handle_struct *handle, > return -1; > } > >- { >- int ret; >- ret = get_gpfs_winattrs(path, &attrs); >- >- if (ret == -1) { >- TALLOC_FREE(path); >- return false; >- } >+ ret = get_gpfs_winattrs(path, &attrs); >+ if (ret == -1) { >+ TALLOC_FREE(path); >+ return false; > } >+ > if ((attrs.winAttrs & GPFS_WINATTR_OFFLINE) != 0) { > DEBUG(10, ("%s is offline\n", path)); > TALLOC_FREE(path); >-- >1.9.1 > > >From c6b966c21ca9b90b98f801f9895e37cdc91fdb55 Mon Sep 17 00:00:00 2001 >From: Michael Adam <obnox@samba.org> >Date: Thu, 17 Jul 2014 17:27:17 +0200 >Subject: [PATCH 6/6] s3: remove stat_ex.vfs_private completely > >It is not used any more. > >Signed-off-by: Michael Adam <obnox@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >Reviewed-by: Christof Schmitt <cs@samba.org> > >Autobuild-User(master): Michael Adam <obnox@samba.org> >Autobuild-Date(master): Thu Jul 24 14:23:11 CEST 2014 on sn-devel-104 > >(cherry picked from commit cd95937369b1729e2417d78f3c903bce5d32da93) > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=10741 >--- > source3/include/includes.h | 8 -------- > source3/librpc/idl/open_files.idl | 1 - > source3/smbd/durable.c | 14 -------------- > 3 files changed, 23 deletions(-) > >diff --git a/source3/include/includes.h b/source3/include/includes.h >index 1b22a57..e8434f2 100644 >--- a/source3/include/includes.h >+++ b/source3/include/includes.h >@@ -322,14 +322,6 @@ struct stat_ex { > > uint32_t st_ex_flags; > uint32_t st_ex_mask; >- >- /* >- * Add space for VFS internal extensions. The initial user of this >- * would be the onefs modules, passing the snapid from the stat calls >- * to the file_id_create call. Maybe we'll have to expand this later, >- * but the core of Samba should never look at this field. >- */ >- uint64_t vfs_private; > }; > > typedef struct stat_ex SMB_STRUCT_STAT; >diff --git a/source3/librpc/idl/open_files.idl b/source3/librpc/idl/open_files.idl >index 686bc02..42ecb02 100644 >--- a/source3/librpc/idl/open_files.idl >+++ b/source3/librpc/idl/open_files.idl >@@ -77,7 +77,6 @@ interface open_files > hyper st_ex_blocks; > uint32 st_ex_flags; > uint32 st_ex_mask; >- hyper vfs_private; > } vfs_default_durable_stat; > > typedef [public] struct { >diff --git a/source3/smbd/durable.c b/source3/smbd/durable.c >index 9b05d48..c3d0a6f 100644 >--- a/source3/smbd/durable.c >+++ b/source3/smbd/durable.c >@@ -121,7 +121,6 @@ NTSTATUS vfs_default_durable_cookie(struct files_struct *fsp, > cookie.stat_info.st_ex_blocks = fsp->fsp_name->st.st_ex_blocks; > cookie.stat_info.st_ex_flags = fsp->fsp_name->st.st_ex_flags; > cookie.stat_info.st_ex_mask = fsp->fsp_name->st.st_ex_mask; >- cookie.stat_info.vfs_private = fsp->fsp_name->st.vfs_private; > > ndr_err = ndr_push_struct_blob(cookie_blob, mem_ctx, &cookie, > (ndr_push_flags_fn_t)ndr_push_vfs_default_durable_cookie); >@@ -275,7 +274,6 @@ NTSTATUS vfs_default_durable_disconnect(struct files_struct *fsp, > cookie.stat_info.st_ex_blocks = fsp->fsp_name->st.st_ex_blocks; > cookie.stat_info.st_ex_flags = fsp->fsp_name->st.st_ex_flags; > cookie.stat_info.st_ex_mask = fsp->fsp_name->st.st_ex_mask; >- cookie.stat_info.vfs_private = fsp->fsp_name->st.vfs_private; > > ndr_err = ndr_push_struct_blob(&new_cookie_blob, mem_ctx, &cookie, > (ndr_push_flags_fn_t)ndr_push_vfs_default_durable_cookie); >@@ -536,18 +534,6 @@ static bool vfs_default_durable_reconnect_check_stat( > return false; > } > >- if (cookie_st->vfs_private != fsp_st->vfs_private) { >- DEBUG(1, ("vfs_default_durable_reconnect (%s): " >- "stat_ex.%s differs: " >- "cookie:%llu != stat:%llu, " >- "denying durable reconnect\n", >- name, >- "vfs_private", >- (unsigned long long)cookie_st->vfs_private, >- (unsigned long long)fsp_st->vfs_private)); >- return false; >- } >- > return true; > } > >-- >1.9.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=10151">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
cs
:
review+
obnox
:
review+
Actions:
View
Attachments on
bug 10741
: 10151