glusterfs icon indicating copy to clipboard operation
glusterfs copied to clipboard

Allow opening snapshot directory(entrypoint) via `glfs_open()`/`glfs_h_open()`

Open anoopcs9 opened this issue 2 years ago • 3 comments

With #3650, we now have the support for opening directories using glfs_h_open() in addition to glfs_open(). Therefore it makes sense to allow open request for snapshot directory coming via open() and not just opendir().

fixes: #3666

anoopcs9 avatar Jul 25 '22 09:07 anoopcs9

CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format

index a4b41b91b..d8502bfe9 100644
--- a/xlators/features/snapview-server/src/snapview-server.c
+++ b/xlators/features/snapview-server/src/snapview-server.c
@@ -2218,8 +2218,8 @@ svs_open(call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t flags,
         op_errno = 0;
         goto out;
     } else {
-        SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret, op_errno,
-                               out);
+        SVS_GET_INODE_CTX_INFO(inode_ctx, fs, object, this, loc, op_ret,
+                               op_errno, out);
 
         op_ret = gf_setcredentials(&root->uid, &root->gid, root->ngrps,
                                    root->groups);

gluster-ant avatar Jul 25 '22 10:07 gluster-ant

/run regression

anoopcs9 avatar Jul 29 '22 09:07 anoopcs9

Adding some context for the proposed change:

When GlusterFS snapshots are configured to be accessible from Windows clients via Samba directory opens, especially .snaps directory, happen via glfs_open() these days instead of glfs_opendir() which ends up in svs_open() and gets terminated with SIGABRT. To make snapshots consumable via Samba, opening snap directory in svs_open() should go through the same route as svs_opendir() without any purposeful abort. As a side-effect various snapshot directories under .snaps are also opened via svs_open() using glfs_h_open() which brings in the additional change to use glfs_close() instead of glfs_closedir() to generalize the closure of either file or directory.

anoopcs9 avatar Aug 05 '22 13:08 anoopcs9

@rafikc30 @mohit84 @xhernandez Is this good to go?

anoopcs9 avatar Aug 16 '22 07:08 anoopcs9

I think we have enough approvals to get this merged.

anoopcs9 avatar Aug 22 '22 06:08 anoopcs9