From ee97bc696ffd331d8c4af35201724d9dfc8c4313 Mon Sep 17 00:00:00 2001 From: Michael Adam 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 Reviewed-by: Christof Schmitt (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 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 Signed-off-by: Michael Adam Reviewed-by: Christof Schmitt (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 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 Reviewed-by: Christof Schmitt (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 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 Reviewed-by: Christof Schmitt (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 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 Reviewed-by: Volker Lendecke Reviewed-by: Christof Schmitt (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 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 Reviewed-by: Volker Lendecke Reviewed-by: Christof Schmitt Autobuild-User(master): Michael Adam 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