The Samba-Bugzilla – Attachment 18641 Details for
Bug 15819
vfs_ceph_snapshots fails to list snapshots for entries at any level beyond share root
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch from master for v4-21-test
v4.21.patch (text/plain), 5.08 KB, created by
Anoop C S
on 2025-04-30 14:36:55 UTC
(
hide
)
Description:
patch from master for v4-21-test
Filename:
MIME Type:
Creator:
Anoop C S
Created:
2025-04-30 14:36:55 UTC
Size:
5.08 KB
patch
obsolete
>From aacc5f02fbb7bce2913800e874992dd883a92236 Mon Sep 17 00:00:00 2001 >From: Anoop C S <anoopcs@samba.org> >Date: Tue, 4 Mar 2025 15:09:33 +0530 >Subject: [PATCH 1/2] vfs_ceph_snapshots: Use full path from dirfsp at > smb_fname > >In ceph_snap_gmt_openat() we hand in the incoming smb_fname as it is >to ceph_snap_gmt_strip_snapshot() which is then passed on to derive >the actual snapshot path using ceph_snap_gmt_convert(). But this can >go wrong in ceph_snap_gmt_convert_dir() while opening the snapdir. >Unless we constitute the full path from dirfsp at the first place we >always end up opening the snapdir from the parent directory with >OpenDir(). > >For example with dirfsp("foobar") and smb_fname("shift.txt"), we open >snapdir from share root because parent is calculated as empty string >via ceph_snap_get_parent_path(). Instead we could construct the full >path from dirfsp using full_path_from_dirfsp_atname() to ensure we >don't open the wrong snapdir. > >Since we have access to the twrp token at VFS layer it doesn't make >much sense to make use of ceph_snap_gmt_strip_snapshot() in openat. >We could instead directly act based on already available twrp token >avoiding an extra copy of incoming smb_filename. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15819 > >Signed-off-by: Anoop C S <anoopcs@samba.org> >Reviewed-by: David Disseldorp ddiss@samba.org >(cherry picked from commit ad8b2dbb67d87db22f2fa2df814dd91cbe071e60) >--- > source3/modules/vfs_ceph_snapshots.c | 29 +++++++++++++--------------- > 1 file changed, 13 insertions(+), 16 deletions(-) > >diff --git a/source3/modules/vfs_ceph_snapshots.c b/source3/modules/vfs_ceph_snapshots.c >index 98b8f5f6b5d..7400bef008e 100644 >--- a/source3/modules/vfs_ceph_snapshots.c >+++ b/source3/modules/vfs_ceph_snapshots.c >@@ -938,21 +938,11 @@ static int ceph_snap_gmt_openat(vfs_handle_struct *handle, > { > time_t timestamp = 0; > struct smb_filename *smb_fname = NULL; >- char stripped[PATH_MAX + 1]; > char conv[PATH_MAX + 1]; > int ret; > int saved_errno = 0; > >- ret = ceph_snap_gmt_strip_snapshot(handle, >- smb_fname_in, >- ×tamp, >- stripped, >- sizeof(stripped)); >- if (ret < 0) { >- errno = -ret; >- return -1; >- } >- if (timestamp == 0) { >+ if (smb_fname_in->twrp == 0) { > return SMB_VFS_NEXT_OPENAT(handle, > dirfsp, > smb_fname_in, >@@ -960,8 +950,18 @@ static int ceph_snap_gmt_openat(vfs_handle_struct *handle, > how); > } > >+ timestamp = nt_time_to_unix(smb_fname_in->twrp); >+ >+ smb_fname = full_path_from_dirfsp_atname(talloc_tos(), >+ dirfsp, >+ smb_fname_in); >+ if (smb_fname == NULL) { >+ errno = ENOMEM; >+ return -1; >+ } >+ > ret = ceph_snap_gmt_convert(handle, >- stripped, >+ smb_fname->base_name, > timestamp, > conv, > sizeof(conv)); >@@ -969,10 +969,7 @@ static int ceph_snap_gmt_openat(vfs_handle_struct *handle, > errno = -ret; > return -1; > } >- smb_fname = cp_smb_filename(talloc_tos(), smb_fname_in); >- if (smb_fname == NULL) { >- return -1; >- } >+ > smb_fname->base_name = conv; > > ret = SMB_VFS_NEXT_OPENAT(handle, >-- >2.49.0 > > >From 8954b417d29fec8b186b6b367ec5f79233e76b69 Mon Sep 17 00:00:00 2001 >From: Anoop C S <anoopcs@samba.org> >Date: Tue, 4 Mar 2025 16:15:05 +0530 >Subject: [PATCH 2/2] vfs_ceph_snapshots: Always calculate absolute snapshot > path > >Use the same logic from shadow_copy2 module to always prepend the >connectpath to the relative snapshot path so as to return converted >path corresponding to the file's share root. > >Please note that with the current working directory staying at the >connectpath level we are safe to prefix it to the smb_filename. In >other words it seems we never get past the connectpath internally >during normal file system operations via chdir(). Since all relative >paths are now based on dirfsp we could constitute absolute path by >prepending the connectpath to full_path_from_dirfsp_atname() output >ignoring the current working directory. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15819 > >Signed-off-by: Anoop C S <anoopcs@samba.org> >Reviewed-by: David Disseldorp <ddiss@samba.org> > >Autobuild-User(master): Anoop C S <anoopcs@samba.org> >Autobuild-Date(master): Wed Apr 30 11:32:59 UTC 2025 on atb-devel-224 > >(cherry picked from commit 95a2b50b1983a6ba810a96f50b27db7c992c02c0) >--- > source3/modules/vfs_ceph_snapshots.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > >diff --git a/source3/modules/vfs_ceph_snapshots.c b/source3/modules/vfs_ceph_snapshots.c >index 7400bef008e..f5ba0752614 100644 >--- a/source3/modules/vfs_ceph_snapshots.c >+++ b/source3/modules/vfs_ceph_snapshots.c >@@ -531,10 +531,13 @@ static int ceph_snap_gmt_convert_dir(struct vfs_handle_struct *handle, > * Temporally use the caller's return buffer for this. > */ > if (strlen(name) == 0) { >- ret = strlcpy(_converted_buf, snapdir, buflen); >+ ret = snprintf(_converted_buf, buflen, "%s/%s", >+ handle->conn->connectpath, snapdir); > } else { >- ret = snprintf(_converted_buf, buflen, "%s/%s", name, snapdir); >+ ret = snprintf(_converted_buf, buflen, "%s/%s/%s", >+ handle->conn->connectpath, name, snapdir); > } >+ > if (ret >= buflen) { > ret = -EINVAL; > goto err_out; >-- >2.49.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
ddiss
:
review+
Actions:
View
Attachments on
bug 15819
:
18640
| 18641