Attachment 18414 Details for Bug 15686 – patch from master for v4-20-test (part 2)

[patch] patch from master for v4-20-test (part 2)

tmp.patch (text/plain), 15.28 KB, created by Anoop C S on 2024-08-20 13:32:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Anoop C S

Created: 2024-08-20 13:32:57 UTC

Size: 15.28 KB

patch

obsolete

>From 4ccada4ab754fbf2e2beb7adf24fdf4ab3f25952 Mon Sep 17 00:00:00 2001
>From: Shachar Sharon <ssharon@redhat.com>
>Date: Thu, 23 May 2024 17:15:40 +0300
>Subject: [PATCH 1/3] vfs_ceph: replace WRAP_RETURN macro with convenience
> helpers
>
>The WRAP_RETURN is a non-hygienic macro, and as such has the potential
>of creating bogus code (e.g. 'return WRAP_RETURN(ret);' which existed
>in the code in the past but did not yield any compiler warning). Prefer
>simple convenience helper functions instead, which are also type safe.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15686
>
>Signed-off-by: Shachar Sharon <ssharon@redhat.com>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Guenther Deschner <gd@samba.org>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>(cherry picked from commit 691a397b2707f2924e3f6910c9c574e01d811a97)
>---
> source3/modules/vfs_ceph.c | 109 ++++++++++++++++++++-----------------
> 1 file changed, 58 insertions(+), 51 deletions(-)
>
>diff --git a/source3/modules/vfs_ceph.c b/source3/modules/vfs_ceph.c
>index c9ee5414f03..a180e96cea5 100644
>--- a/source3/modules/vfs_ceph.c
>+++ b/source3/modules/vfs_ceph.c
>@@ -54,16 +54,30 @@
> #define llu(_var) ((long long unsigned)_var)
> 
> /*
>- * Note, libcephfs's return code model is to return -errno! So we have to
>- * convert to what Samba expects, which is to set errno to -return and return -1
>+ * Note, libcephfs's return code model is to return -errno. Thus we have to
>+ * convert to what Samba expects: set errno to non-negative value and return -1.
>+ *
>+ * Using convenience helper functions to avoid non-hygienic macro.
>  */
>-#define WRAP_RETURN(_res) \
>-	errno = 0; \
>-	if (_res < 0) { \
>-		errno = -_res; \
>-		return -1; \
>-	} \
>-	return _res \
>+static inline int status_code(int ret)
>+{
>+	if (ret < 0) {
>+		errno = -ret;
>+		return -1;
>+	}
>+	errno = 0;
>+	return ret;
>+}
>+
>+static inline ssize_t lstatus_code(intmax_t ret)
>+{
>+	if (ret < 0) {
>+		errno = -((int)ret);
>+		return -1;
>+	}
>+	errno = 0;
>+	return (ssize_t)ret;
>+}
> 
> /*
>  * Track unique connections, as virtual mounts, to cephfs file systems.
>@@ -344,7 +358,7 @@ static uint64_t cephwrap_disk_free(struct vfs_handle_struct *handle,
> 		return *dfree;
> 	} else {
> 		DBG_DEBUG("[CEPH] ceph_statfs returned %d\n", ret);
>-		WRAP_RETURN(ret);
>+		return status_code(ret);
> 	}
> }
> 
>@@ -401,7 +415,7 @@ static int cephwrap_statvfs(struct vfs_handle_struct *handle,
> 
> 	ret = ceph_statfs(handle->data, smb_fname->base_name, &statvfs_buf);
> 	if (ret < 0) {
>-		WRAP_RETURN(ret);
>+		return status_code(ret);
> 	}
> 
> 	statbuf->OptimalTransferSize = statvfs_buf.f_frsize;
>@@ -493,7 +507,7 @@ static int cephwrap_mkdirat(struct vfs_handle_struct *handle,
> 
> 	DBG_DEBUG("[CEPH] mkdirat(...) = %d\n", result);
> 
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #else
> 	struct smb_filename *full_fname = NULL;
> 
>@@ -511,7 +525,7 @@ static int cephwrap_mkdirat(struct vfs_handle_struct *handle,
> 
> 	TALLOC_FREE(full_fname);
> 
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #endif
> }
> 
>@@ -522,7 +536,7 @@ static int cephwrap_closedir(struct vfs_handle_struct *handle, DIR *dirp)
> 	DBG_DEBUG("[CEPH] closedir(%p, %p)\n", handle, dirp);
> 	result = ceph_closedir(handle->data, (struct ceph_dir_result *) dirp);
> 	DBG_DEBUG("[CEPH] closedir(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> /* File operations */
>@@ -604,7 +618,7 @@ out:
> 	TALLOC_FREE(name);
> 	fsp->fsp_flags.have_proc_fds = false;
> 	DBG_DEBUG("[CEPH] open(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static int cephwrap_close(struct vfs_handle_struct *handle, files_struct *fsp)
>@@ -614,8 +628,7 @@ static int cephwrap_close(struct vfs_handle_struct *handle, files_struct *fsp)
> 	DBG_DEBUG("[CEPH] close(%p, %p)\n", handle, fsp);
> 	result = ceph_close(handle->data, fsp_get_pathref_fd(fsp));
> 	DBG_DEBUG("[CEPH] close(...) = %d\n", result);
>-
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static ssize_t cephwrap_pread(struct vfs_handle_struct *handle, files_struct *fsp, void *data,
>@@ -627,7 +640,7 @@ static ssize_t cephwrap_pread(struct vfs_handle_struct *handle, files_struct *fs
> 
> 	result = ceph_read(handle->data, fsp_get_io_fd(fsp), data, n, offset);
> 	DBG_DEBUG("[CEPH] pread(...) = %llu\n", llu(result));
>-	WRAP_RETURN(result);
>+	return lstatus_code(result);
> }
> 
> struct cephwrap_pread_state {
>@@ -690,7 +703,7 @@ static ssize_t cephwrap_pwrite(struct vfs_handle_struct *handle, files_struct *f
> 	DBG_DEBUG("[CEPH] pwrite(%p, %p, %p, %llu, %llu)\n", handle, fsp, data, llu(n), llu(offset));
> 	result = ceph_write(handle->data, fsp_get_io_fd(fsp), data, n, offset);
> 	DBG_DEBUG("[CEPH] pwrite(...) = %llu\n", llu(result));
>-	WRAP_RETURN(result);
>+	return lstatus_code(result);
> }
> 
> struct cephwrap_pwrite_state {
>@@ -751,7 +764,7 @@ static off_t cephwrap_lseek(struct vfs_handle_struct *handle, files_struct *fsp,
> 
> 	DBG_DEBUG("[CEPH] cephwrap_lseek\n");
> 	result = ceph_lseek(handle->data, fsp_get_io_fd(fsp), offset, whence);
>-	WRAP_RETURN(result);
>+	return lstatus_code(result);
> }
> 
> static ssize_t cephwrap_sendfile(struct vfs_handle_struct *handle, int tofd, files_struct *fromfsp, const DATA_BLOB *hdr,
>@@ -818,7 +831,7 @@ static int cephwrap_renameat(struct vfs_handle_struct *handle,
> 	TALLOC_FREE(full_fname_src);
> 	TALLOC_FREE(full_fname_dst);
> 
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> /*
>@@ -924,7 +937,7 @@ static int cephwrap_stat(struct vfs_handle_struct *handle,
> 				SAMBA_STATX_ATTR_MASK, 0);
> 	DBG_DEBUG("[CEPH] statx(...) = %d\n", result);
> 	if (result < 0) {
>-		WRAP_RETURN(result);
>+		return status_code(result);
> 	}
> 
> 	init_stat_ex_from_ceph_statx(&smb_fname->st, &stx);
>@@ -943,7 +956,7 @@ static int cephwrap_fstat(struct vfs_handle_struct *handle, files_struct *fsp, S
> 				SAMBA_STATX_ATTR_MASK, 0);
> 	DBG_DEBUG("[CEPH] fstat(...) = %d\n", result);
> 	if (result < 0) {
>-		WRAP_RETURN(result);
>+		return status_code(result);
> 	}
> 
> 	init_stat_ex_from_ceph_statx(sbuf, &stx);
>@@ -987,7 +1000,7 @@ static int cephwrap_fstatat(struct vfs_handle_struct *handle,
> 
> 	DBG_DEBUG("[CEPH] fstatat(...) = %d\n", result);
> 	if (result < 0) {
>-		WRAP_RETURN(result);
>+		return status_code(result);
> 	}
> 
> 	init_stat_ex_from_ceph_statx(sbuf, &stx);
>@@ -1013,7 +1026,7 @@ static int cephwrap_lstat(struct vfs_handle_struct *handle,
> 				SAMBA_STATX_ATTR_MASK, AT_SYMLINK_NOFOLLOW);
> 	DBG_DEBUG("[CEPH] lstat(...) = %d\n", result);
> 	if (result < 0) {
>-		WRAP_RETURN(result);
>+		return status_code(result);
> 	}
> 
> 	init_stat_ex_from_ceph_statx(&smb_fname->st, &stx);
>@@ -1095,7 +1108,7 @@ static int cephwrap_unlinkat(struct vfs_handle_struct *handle,
> 			       smb_fname->base_name,
> 			       flags);
> 	DBG_DEBUG("[CEPH] unlinkat(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #else
> 	struct smb_filename *full_fname = NULL;
> 
>@@ -1122,7 +1135,7 @@ static int cephwrap_unlinkat(struct vfs_handle_struct *handle,
> 	}
> 	TALLOC_FREE(full_fname);
> 	DBG_DEBUG("[CEPH] unlink(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #endif
> }
> 
>@@ -1145,7 +1158,7 @@ static int cephwrap_fchmod(struct vfs_handle_struct *handle, files_struct *fsp,
> 				    mode);
> 	}
> 	DBG_DEBUG("[CEPH] fchmod(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static int cephwrap_fchown(struct vfs_handle_struct *handle, files_struct *fsp, uid_t uid, gid_t gid)
>@@ -1172,7 +1185,7 @@ static int cephwrap_fchown(struct vfs_handle_struct *handle, files_struct *fsp,
> 	}
> 
> 	DBG_DEBUG("[CEPH] fchown(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static int cephwrap_lchown(struct vfs_handle_struct *handle,
>@@ -1184,7 +1197,7 @@ static int cephwrap_lchown(struct vfs_handle_struct *handle,
> 	DBG_DEBUG("[CEPH] lchown(%p, %s, %d, %d)\n", handle, smb_fname->base_name, uid, gid);
> 	result = ceph_lchown(handle->data, smb_fname->base_name, uid, gid);
> 	DBG_DEBUG("[CEPH] lchown(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static int cephwrap_chdir(struct vfs_handle_struct *handle,
>@@ -1194,7 +1207,7 @@ static int cephwrap_chdir(struct vfs_handle_struct *handle,
> 	DBG_DEBUG("[CEPH] chdir(%p, %s)\n", handle, smb_fname->base_name);
> 	result = ceph_chdir(handle->data, smb_fname->base_name);
> 	DBG_DEBUG("[CEPH] chdir(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static struct smb_filename *cephwrap_getwd(struct vfs_handle_struct *handle,
>@@ -1234,13 +1247,13 @@ static int strict_allocate_ftruncate(struct vfs_handle_struct *handle, files_str
> 	/* Shrink - just ftruncate. */
> 	if (pst->st_ex_size > len) {
> 		result = ceph_ftruncate(handle->data, fsp_get_io_fd(fsp), len);
>-		WRAP_RETURN(result);
>+		return status_code(result);
> 	}
> 
> 	space_to_write = len - pst->st_ex_size;
> 	result = ceph_fallocate(handle->data, fsp_get_io_fd(fsp), 0, pst->st_ex_size,
> 				space_to_write);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static int cephwrap_ftruncate(struct vfs_handle_struct *handle, files_struct *fsp, off_t len)
>@@ -1254,7 +1267,7 @@ static int cephwrap_ftruncate(struct vfs_handle_struct *handle, files_struct *fs
> 	}
> 
> 	result = ceph_ftruncate(handle->data, fsp_get_io_fd(fsp), len);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static int cephwrap_fallocate(struct vfs_handle_struct *handle,
>@@ -1270,7 +1283,7 @@ static int cephwrap_fallocate(struct vfs_handle_struct *handle,
> 	/* unsupported mode flags are rejected by libcephfs */
> 	result = ceph_fallocate(handle->data, fsp_get_io_fd(fsp), mode, offset, len);
> 	DBG_DEBUG("[CEPH] fallocate(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static bool cephwrap_lock(struct vfs_handle_struct *handle, files_struct *fsp, int op, off_t offset, off_t count, int type)
>@@ -1362,7 +1375,7 @@ static int cephwrap_symlinkat(struct vfs_handle_struct *handle,
> 				dirfd,
> 				new_smb_fname->base_name);
> 	DBG_DEBUG("[CEPH] symlinkat(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #else
> 	struct smb_filename *full_fname = NULL;
> 
>@@ -1382,7 +1395,7 @@ static int cephwrap_symlinkat(struct vfs_handle_struct *handle,
> 			full_fname->base_name);
> 	TALLOC_FREE(full_fname);
> 	DBG_DEBUG("[CEPH] symlink(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #endif
> }
> 
>@@ -1410,7 +1423,7 @@ static int cephwrap_readlinkat(struct vfs_handle_struct *handle,
> 				 bufsiz);
> 
> 	DBG_DEBUG("[CEPH] readlinkat(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #else
> 	struct smb_filename *full_fname = NULL;
> 
>@@ -1427,7 +1440,7 @@ static int cephwrap_readlinkat(struct vfs_handle_struct *handle,
> 	result = ceph_readlink(handle->data, full_fname->base_name, buf, bufsiz);
> 	TALLOC_FREE(full_fname);
> 	DBG_DEBUG("[CEPH] readlink(...) = %d\n", result);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> #endif
> }
> 
>@@ -1466,7 +1479,7 @@ static int cephwrap_linkat(struct vfs_handle_struct *handle,
> 	DBG_DEBUG("[CEPH] link(...) = %d\n", result);
> 	TALLOC_FREE(full_fname_old);
> 	TALLOC_FREE(full_fname_new);
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> static int cephwrap_mknodat(struct vfs_handle_struct *handle,
>@@ -1491,7 +1504,7 @@ static int cephwrap_mknodat(struct vfs_handle_struct *handle,
> 
> 	TALLOC_FREE(full_fname);
> 
>-	WRAP_RETURN(result);
>+	return status_code(result);
> }
> 
> /*
>@@ -1600,10 +1613,7 @@ static ssize_t cephwrap_fgetxattr(struct vfs_handle_struct *handle,
> 				    size);
> 	}
> 	DBG_DEBUG("[CEPH] fgetxattr(...) = %d\n", ret);
>-	if (ret < 0) {
>-		WRAP_RETURN(ret);
>-	}
>-	return (ssize_t)ret;
>+	return lstatus_code(ret);
> }
> 
> static ssize_t cephwrap_flistxattr(struct vfs_handle_struct *handle, struct files_struct *fsp, char *list, size_t size)
>@@ -1629,10 +1639,7 @@ static ssize_t cephwrap_flistxattr(struct vfs_handle_struct *handle, struct file
> 					size);
> 	}
> 	DBG_DEBUG("[CEPH] flistxattr(...) = %d\n", ret);
>-	if (ret < 0) {
>-		WRAP_RETURN(ret);
>-	}
>-	return (ssize_t)ret;
>+	return lstatus_code(ret);
> }
> 
> static int cephwrap_fremovexattr(struct vfs_handle_struct *handle, struct files_struct *fsp, const char *name)
>@@ -1653,7 +1660,7 @@ static int cephwrap_fremovexattr(struct vfs_handle_struct *handle, struct files_
> 					name);
> 	}
> 	DBG_DEBUG("[CEPH] fremovexattr(...) = %d\n", ret);
>-	WRAP_RETURN(ret);
>+	return status_code(ret);
> }
> 
> static int cephwrap_fsetxattr(struct vfs_handle_struct *handle, struct files_struct *fsp, const char *name, const void *value, size_t size, int flags)
>@@ -1682,7 +1689,7 @@ static int cephwrap_fsetxattr(struct vfs_handle_struct *handle, struct files_str
> 				flags);
> 	}
> 	DBG_DEBUG("[CEPH] fsetxattr(...) = %d\n", ret);
>-	WRAP_RETURN(ret);
>+	return status_code(ret);
> }
> 
> static bool cephwrap_aio_force(struct vfs_handle_struct *handle, struct files_struct *fsp)
>-- 
>2.46.0
>
>
>From 5ae4a6efa047f6d2db81774ffed6aa0ba786084c Mon Sep 17 00:00:00 2001
>From: Shachar Sharon <ssharon@redhat.com>
>Date: Thu, 30 May 2024 11:02:37 +0300
>Subject: [PATCH 2/3] vfs_ceph: explicit cast to uint64_t upon failure of
> ceph_statfs
>
>When a call to 'ceph_statfs' from with 'cephwrap_disk_free' returns
>non-zero status do an explicit cast to uint64_t for the negative (-1)
>value returned by 'status_code'.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15686
>
>Signed-off-by: Shachar Sharon <ssharon@redhat.com>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>Reviewed-by: David Disseldorp <ddiss@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Guenther Deschner <gd@samba.org>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>(cherry picked from commit a7d34ec597fe810090d28bfda636b7450ecb06e5)
>---
> source3/modules/vfs_ceph.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/modules/vfs_ceph.c b/source3/modules/vfs_ceph.c
>index a180e96cea5..cbe98a24c85 100644
>--- a/source3/modules/vfs_ceph.c
>+++ b/source3/modules/vfs_ceph.c
>@@ -358,7 +358,7 @@ static uint64_t cephwrap_disk_free(struct vfs_handle_struct *handle,
> 		return *dfree;
> 	} else {
> 		DBG_DEBUG("[CEPH] ceph_statfs returned %d\n", ret);
>-		return status_code(ret);
>+		return (uint64_t)status_code(ret);
> 	}
> }
> 
>-- 
>2.46.0
>
>
>From a725325b479fd85061032ff8b04506b873518045 Mon Sep 17 00:00:00 2001
>From: Shachar Sharon <ssharon@redhat.com>
>Date: Tue, 30 Jul 2024 09:55:44 +0300
>Subject: [PATCH 3/3] vfs_ceph{_new}: do not set errno upon successful call to
> libcephfs
>
>There is code in Samba that expects errno from a previous system call
>to be preserved through a subsequent system call. Thus, avoid setting
>"errno = 0" in status_code() and lstatus_code() upon successful return
>from libcephfs API call.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15686
>
>Signed-off-by: Shachar Sharon <ssharon@redhat.com>
>Reviewed-by: Guenther Deschner <gd@samba.org>
>Reviewed-by: Anoop C S <anoopcs@samba.org>
>(cherry picked from commit a7f4e2bd47c7f4728f3ac8d90af693156a69c557)
>---
> source3/modules/vfs_ceph.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/source3/modules/vfs_ceph.c b/source3/modules/vfs_ceph.c
>index cbe98a24c85..c36a095d47e 100644
>--- a/source3/modules/vfs_ceph.c
>+++ b/source3/modules/vfs_ceph.c
>@@ -65,7 +65,6 @@ static inline int status_code(int ret)
> 		errno = -ret;
> 		return -1;
> 	}
>-	errno = 0;
> 	return ret;
> }
> 
>@@ -75,7 +74,6 @@ static inline ssize_t lstatus_code(intmax_t ret)
> 		errno = -((int)ret);
> 		return -1;
> 	}
>-	errno = 0;
> 	return (ssize_t)ret;
> }
> 
>-- 
>2.46.0
>

Flags:

anoopcs: review+

Actions: View

Attachments on bug 15686: 18398 | 18399 | 18400 | 18412 | 18414 | 18417 | 18428 | 18429