Attachment 17560 Details for Bug 15198 – patch for v4-17

[patch] patch for v4-17

tmp.patch (text/plain), 7.69 KB, created by Anoop C S on 2022-10-13 06:18:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Anoop C S

Created: 2022-10-13 06:18:26 UTC

Size: 7.69 KB

patch

obsolete

>From 3211a899fff11934bc2f2383e42c10a8d93fb791 Mon Sep 17 00:00:00 2001
>From: Anoop C S <anoopcs@samba.org>
>Date: Tue, 11 Oct 2022 23:25:46 +0530
>Subject: [PATCH 1/4] vfs_glusterfs: Simplify SMB_VFS_GET_REAL_FILENAME_AT
> implementation
>
>It was unnecessary to construct full directory path as "dir/." which is
>same as "dir". We could just directly use dirfsp->fsp_name->base_name
>for glfs_getxattr() and return the result.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15198
>
>Signed-off-by: Anoop C S <anoopcs@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 8cbd9e63724d80c06565d0c90bd107166dfd9bbe)
>---
> source3/modules/vfs_glusterfs.c | 31 +++++--------------------------
> 1 file changed, 5 insertions(+), 26 deletions(-)
>
>diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
>index 8fef8c8bf9c..5d43ca6261c 100644
>--- a/source3/modules/vfs_glusterfs.c
>+++ b/source3/modules/vfs_glusterfs.c
>@@ -2268,9 +2268,6 @@ static NTSTATUS vfs_gluster_get_real_filename_at(
> 	char val_buf[GLUSTER_NAME_MAX + 1];
> #ifdef HAVE_GFAPI_VER_7_11
> 	glfs_fd_t *pglfd = NULL;
>-#else
>-	struct smb_filename *smb_fname_dot = NULL;
>-	struct smb_filename *full_fname = NULL;
> #endif
> 
> 	if (strlen(name) >= GLUSTER_NAME_MAX) {
>@@ -2289,29 +2286,11 @@ static NTSTATUS vfs_gluster_get_real_filename_at(
> 
> 	ret = glfs_fgetxattr(pglfd, key_buf, val_buf, GLUSTER_NAME_MAX + 1);
> #else
>-	smb_fname_dot = synthetic_smb_fname(mem_ctx,
>-					    ".",
>-					    NULL,
>-					    NULL,
>-					    0,
>-					    0);
>-	if (smb_fname_dot == NULL) {
>-		return NT_STATUS_NO_MEMORY;
>-	}
>-
>-	full_fname = full_path_from_dirfsp_atname(talloc_tos(),
>-						  dirfsp,
>-						  smb_fname_dot);
>-	if (full_fname == NULL) {
>-		TALLOC_FREE(smb_fname_dot);
>-		return NT_STATUS_NO_MEMORY;
>-	}
>-
>-	ret = glfs_getxattr(handle->data, full_fname->base_name,
>-			    key_buf, val_buf, GLUSTER_NAME_MAX + 1);
>-
>-	TALLOC_FREE(smb_fname_dot);
>-	TALLOC_FREE(full_fname);
>+	ret = glfs_getxattr(handle->data,
>+			    dirfsp->fsp_name->base_name,
>+			    key_buf,
>+			    val_buf,
>+			    GLUSTER_NAME_MAX + 1);
> #endif
> 
> 	if (ret == -1) {
>-- 
>2.37.3
>
>
>From 8f18dcbed83d75e4bf26161c18203b6661ff7663 Mon Sep 17 00:00:00 2001
>From: Anoop C S <anoopcs@samba.org>
>Date: Tue, 11 Oct 2022 23:27:37 +0530
>Subject: [PATCH 2/4] vfs_glusterfs: Do not use glfs_fgetxattr() for
> SMB_VFS_GET_REAL_FILENAME_AT
>
>glfs_fgetxattr() or generally fgetxattr() will return EBADF as dirfsp
>here is a pathref fsp. GlusterFS client log had following entries
>indicating the error:
>
>W [MSGID: 114031] [client-rpc-fops_v2.c:993:client4_0_fgetxattr_cbk] \
>  0-vol-client-0: remote operation failed. [{errno=9}, {error=Bad file descriptor}]
>
>Therefore use glfs_getxattr() only for implementing get_real_filename_at
>logic.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15198
>
>Signed-off-by: Anoop C S <anoopcs@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 6a6bd1a0530424def64d2d462b54e4c1f4f9bebb)
>---
> source3/modules/vfs_glusterfs.c | 14 --------------
> 1 file changed, 14 deletions(-)
>
>diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
>index 5d43ca6261c..dd7fecda4fc 100644
>--- a/source3/modules/vfs_glusterfs.c
>+++ b/source3/modules/vfs_glusterfs.c
>@@ -2266,9 +2266,6 @@ static NTSTATUS vfs_gluster_get_real_filename_at(
> 	int ret;
> 	char key_buf[GLUSTER_NAME_MAX + 64];
> 	char val_buf[GLUSTER_NAME_MAX + 1];
>-#ifdef HAVE_GFAPI_VER_7_11
>-	glfs_fd_t *pglfd = NULL;
>-#endif
> 
> 	if (strlen(name) >= GLUSTER_NAME_MAX) {
> 		return NT_STATUS_OBJECT_NAME_INVALID;
>@@ -2277,22 +2274,11 @@ static NTSTATUS vfs_gluster_get_real_filename_at(
> 	snprintf(key_buf, GLUSTER_NAME_MAX + 64,
> 		 "glusterfs.get_real_filename:%s", name);
> 
>-#ifdef HAVE_GFAPI_VER_7_11
>-	pglfd = vfs_gluster_fetch_glfd(handle, dirfsp);
>-	if (pglfd == NULL) {
>-		DBG_ERR("Failed to fetch gluster fd\n");
>-		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
>-	}
>-
>-	ret = glfs_fgetxattr(pglfd, key_buf, val_buf, GLUSTER_NAME_MAX + 1);
>-#else
> 	ret = glfs_getxattr(handle->data,
> 			    dirfsp->fsp_name->base_name,
> 			    key_buf,
> 			    val_buf,
> 			    GLUSTER_NAME_MAX + 1);
>-#endif
>-
> 	if (ret == -1) {
> 		if (errno == ENOATTR) {
> 			errno = ENOENT;
>-- 
>2.37.3
>
>
>From c382b89580b970bd4e5f854a600661b7a20c4e32 Mon Sep 17 00:00:00 2001
>From: Anoop C S <anoopcs@samba.org>
>Date: Mon, 10 Oct 2022 20:29:13 +0530
>Subject: [PATCH 3/4] vfs_glusterfs: Add path based fallback mechanism for
> SMB_VFS_FGETXATTR
>
>Fallback mechanism was missing in vfs_gluster_fgetxattr() for path based
>call. Therefore adding a similar mechanism as seen with other calls like
>vfs_gluster_fsetxattr, vfs_gluster_flistxattr etc.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15198
>
>Signed-off-by: Anoop C S <anoopcs@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 7af4bfe8285714c137b6347b17305c9cd0702bdd)
>---
> source3/modules/vfs_glusterfs.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
>index dd7fecda4fc..bccb072d358 100644
>--- a/source3/modules/vfs_glusterfs.c
>+++ b/source3/modules/vfs_glusterfs.c
>@@ -2312,7 +2312,21 @@ static ssize_t vfs_gluster_fgetxattr(struct vfs_handle_struct *handle,
> 		return -1;
> 	}
> 
>-	return glfs_fgetxattr(glfd, name, value, size);
>+	if (!fsp->fsp_flags.is_pathref) {
>+		/*
>+		 * We can use an io_fd to retrieve xattr value.
>+		 */
>+		return glfs_fgetxattr(glfd, name, value, size);
>+	}
>+
>+	/*
>+	 * This is no longer a handle based call.
>+	 */
>+	return glfs_getxattr(handle->data,
>+			     fsp->fsp_name->base_name,
>+			     name,
>+			     value,
>+			     size);
> }
> 
> static ssize_t vfs_gluster_flistxattr(struct vfs_handle_struct *handle,
>-- 
>2.37.3
>
>
>From 153194b3782d44bae3c2d6692c67edab9030bd81 Mon Sep 17 00:00:00 2001
>From: Anoop C S <anoopcs@samba.org>
>Date: Tue, 11 Oct 2022 23:02:48 +0530
>Subject: [PATCH 4/4] vfs_glusterfs: Simplify SMB_VFS_FDOPENDIR implementation
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>It was unnecessary to construct full directory path as "dir/." which is
>same as "dir". We could just directly use fsp->fsp_name->base_name and
>return directory stream obtained from glfs_opendir().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15198
>
>Signed-off-by: Anoop C S <anoopcs@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>
>Autobuild-User(master): Ralph BÃ¶hme <slow@samba.org>
>Autobuild-Date(master): Wed Oct 12 12:48:50 UTC 2022 on sn-devel-184
>
>(cherry picked from commit cc397175cb9a1b06f268ecf6b3d62f621947cbba)
>---
> source3/modules/vfs_glusterfs.c | 28 +---------------------------
> 1 file changed, 1 insertion(+), 27 deletions(-)
>
>diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c
>index bccb072d358..6e0f87a15bf 100644
>--- a/source3/modules/vfs_glusterfs.c
>+++ b/source3/modules/vfs_glusterfs.c
>@@ -626,38 +626,12 @@ static DIR *vfs_gluster_fdopendir(struct vfs_handle_struct *handle,
> 				  uint32_t attributes)
> {
> 	glfs_fd_t *glfd = NULL;
>-	struct smb_filename *full_fname = NULL;
>-	struct smb_filename *smb_fname_dot = NULL;
>-
>-	smb_fname_dot = synthetic_smb_fname(fsp->fsp_name,
>-					    ".",
>-					    NULL,
>-					    NULL,
>-					    0,
>-					    0);
> 
>-	if (smb_fname_dot == NULL) {
>-		return NULL;
>-	}
>-
>-	full_fname = full_path_from_dirfsp_atname(talloc_tos(),
>-						  fsp,
>-						  smb_fname_dot);
>-	if (full_fname == NULL) {
>-		TALLOC_FREE(smb_fname_dot);
>-		return NULL;
>-	}
>-
>-	glfd = glfs_opendir(handle->data, full_fname->base_name);
>+	glfd = glfs_opendir(handle->data, fsp->fsp_name->base_name);
> 	if (glfd == NULL) {
>-		TALLOC_FREE(full_fname);
>-		TALLOC_FREE(smb_fname_dot);
> 		return NULL;
> 	}
> 
>-	TALLOC_FREE(full_fname);
>-	TALLOC_FREE(smb_fname_dot);
>-
> 	return (DIR *)glfd;
> }
> 
>-- 
>2.37.3
>

Flags:

anoopcs: review-

Actions: View

Attachments on bug 15198: 17560 | 17662