a3470f
From d2d4750a70b30174a52f63a39b174f48ce0879e9 Mon Sep 17 00:00:00 2001
a3470f
From: Ravishankar N <ravishankar@redhat.com>
a3470f
Date: Mon, 11 Dec 2017 16:14:03 +0530
a3470f
Subject: [PATCH 107/128] feature/bitrot: remove internal xattrs from lookup
a3470f
 cbk
a3470f
a3470f
Backport of https://review.gluster.org/#/c/19021/
a3470f
Problem:
a3470f
afr requests all xattrs in lookup via the list-xattr key. If bitrot is
a3470f
enabled and later disabled, or if the bitrot xattrs were present due to
a3470f
an older version of bitrot which used to create the xattrs without
a3470f
enabling the feature, the xattrs (trusted.bit-rot.version in particular)
a3470f
was not getting filtered and ended up reaching the client stack. AFR, on
a3470f
noticing different values of the xattr across bricks of the replica,
a3470f
started triggering spurious metadata heals.
a3470f
a3470f
Fix:
a3470f
Filter all internal xattrs in bitrot xlator before unwinding lookup,
a3470f
(f)getxattr.
a3470f
a3470f
Thanks to Kotresh for the help in RCA'ing.
a3470f
a3470f
Change-Id: I5bc70e4b901359c3daefc67b8e4fa6ddb47f046c
a3470f
BUG: 1519740
a3470f
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
a3470f
Reviewed-on: https://code.engineering.redhat.com/gerrit/126154
a3470f
Tested-by: RHGS Build Bot <nigelb@redhat.com>
a3470f
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
a3470f
---
a3470f
 xlators/features/bit-rot/src/stub/bit-rot-stub.c | 23 ++++++++++++++++-------
a3470f
 xlators/features/bit-rot/src/stub/bit-rot-stub.h |  5 +++++
a3470f
 2 files changed, 21 insertions(+), 7 deletions(-)
a3470f
a3470f
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.c b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
a3470f
index fb187a3..4be7caa 100644
a3470f
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.c
a3470f
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.c
a3470f
@@ -1585,6 +1585,7 @@ br_stub_getxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
a3470f
         int32_t              ret          = 0;
a3470f
         size_t               totallen     = 0;
a3470f
         size_t               signaturelen = 0;
a3470f
+        br_stub_private_t   *priv         = NULL;
a3470f
         br_version_t        *obuf         = NULL;
a3470f
         br_signature_t      *sbuf         = NULL;
a3470f
         br_isignature_out_t *sign         = NULL;
a3470f
@@ -1592,9 +1593,15 @@ br_stub_getxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
a3470f
         br_stub_local_t     *local        = NULL;
a3470f
         inode_t             *inode        = NULL;
a3470f
         gf_boolean_t         bad_object   = _gf_false;
a3470f
+        gf_boolean_t         ver_enabled  = _gf_false;
a3470f
+
a3470f
+        BR_STUB_VER_ENABLED_IN_CALLPATH(frame, ver_enabled);
a3470f
+        priv = this->private;
a3470f
 
a3470f
         if (op_ret < 0)
a3470f
                 goto unwind;
a3470f
+        BR_STUB_VER_COND_GOTO (priv, (!ver_enabled), delkeys);
a3470f
+
a3470f
         if (cookie != (void *) BR_STUB_REQUEST_COOKIE)
a3470f
                 goto unwind;
a3470f
 
a3470f
@@ -1740,8 +1747,7 @@ br_stub_getxattr (call_frame_t *frame, xlator_t *this,
a3470f
                 goto unwind;
a3470f
 
a3470f
         priv = this->private;
a3470f
-        if (!priv->do_versioning)
a3470f
-                goto wind;
a3470f
+        BR_STUB_VER_NOT_ACTIVE_THEN_GOTO (frame, priv, wind);
a3470f
 
a3470f
         /**
a3470f
          * If xattr is node-uuid and the inode is marked bad, return EIO.
a3470f
@@ -1762,6 +1768,7 @@ br_stub_getxattr (call_frame_t *frame, xlator_t *this,
a3470f
                           strlen (GLUSTERFS_GET_BR_STUB_INIT_TIME)) == 0)
a3470f
             && ((gf_uuid_compare (loc->gfid, rootgfid) == 0)
a3470f
                 || (gf_uuid_compare (loc->inode->gfid, rootgfid) == 0))) {
a3470f
+                BR_STUB_RESET_LOCAL_NULL (frame);
a3470f
                 br_stub_send_stub_init_time (frame, this);
a3470f
                 return 0;
a3470f
         }
a3470f
@@ -1792,6 +1799,7 @@ br_stub_getxattr (call_frame_t *frame, xlator_t *this,
a3470f
                        FIRST_CHILD (this)->fops->getxattr, loc, name, xdata);
a3470f
         return 0;
a3470f
 unwind:
a3470f
+        BR_STUB_RESET_LOCAL_NULL (frame);
a3470f
         STACK_UNWIND_STRICT (getxattr, frame, op_ret, op_errno, NULL, NULL);
a3470f
                 return 0;
a3470f
 }
a3470f
@@ -1809,6 +1817,7 @@ br_stub_fgetxattr (call_frame_t *frame, xlator_t *this,
a3470f
         br_stub_private_t  *priv = NULL;
a3470f
 
a3470f
         rootgfid[15] = 1;
a3470f
+        priv = this->private;
a3470f
 
a3470f
         if (!name) {
a3470f
                 cbk = br_stub_listxattr_cbk;
a3470f
@@ -1818,9 +1827,7 @@ br_stub_fgetxattr (call_frame_t *frame, xlator_t *this,
a3470f
         if (br_stub_is_internal_xattr (name))
a3470f
                 goto unwind;
a3470f
 
a3470f
-        priv = this->private;
a3470f
-        if (!priv->do_versioning)
a3470f
-                goto wind;
a3470f
+        BR_STUB_VER_NOT_ACTIVE_THEN_GOTO (frame, priv, wind);
a3470f
 
a3470f
         /**
a3470f
          * If xattr is node-uuid and the inode is marked bad, return EIO.
a3470f
@@ -1840,6 +1847,7 @@ br_stub_fgetxattr (call_frame_t *frame, xlator_t *this,
a3470f
             && (strncmp (name, GLUSTERFS_GET_BR_STUB_INIT_TIME,
a3470f
                          strlen (GLUSTERFS_GET_BR_STUB_INIT_TIME)) == 0)
a3470f
             && (gf_uuid_compare (fd->inode->gfid, rootgfid) == 0)) {
a3470f
+                BR_STUB_RESET_LOCAL_NULL (frame);
a3470f
                 br_stub_send_stub_init_time (frame, this);
a3470f
                 return 0;
a3470f
         }
a3470f
@@ -1870,6 +1878,7 @@ br_stub_fgetxattr (call_frame_t *frame, xlator_t *this,
a3470f
                        FIRST_CHILD (this)->fops->fgetxattr, fd, name, xdata);
a3470f
         return 0;
a3470f
 unwind:
a3470f
+        BR_STUB_RESET_LOCAL_NULL (frame);
a3470f
         STACK_UNWIND_STRICT (fgetxattr, frame, op_ret, op_errno, NULL, NULL);
a3470f
         return 0;
a3470f
 }
a3470f
@@ -2867,13 +2876,14 @@ br_stub_lookup_cbk (call_frame_t *frame, void *cookie,
a3470f
 
a3470f
         BR_STUB_VER_ENABLED_IN_CALLPATH(frame, ver_enabled);
a3470f
         priv = this->private;
a3470f
-        BR_STUB_VER_COND_GOTO (priv, (!ver_enabled), unwind);
a3470f
 
a3470f
         if (op_ret < 0) {
a3470f
                 (void) br_stub_handle_lookup_error (this, inode, op_errno);
a3470f
                 goto unwind;
a3470f
         }
a3470f
 
a3470f
+        BR_STUB_VER_COND_GOTO (priv, (!ver_enabled), delkey);
a3470f
+
a3470f
         if (!IA_ISREG (stbuf->ia_type))
a3470f
                 goto unwind;
a3470f
 
a3470f
@@ -2892,7 +2902,6 @@ br_stub_lookup_cbk (call_frame_t *frame, void *cookie,
a3470f
                         op_errno = EIO;
a3470f
                         goto unwind;
a3470f
                 }
a3470f
-
a3470f
                 goto delkey;
a3470f
         }
a3470f
 
a3470f
diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub.h b/xlators/features/bit-rot/src/stub/bit-rot-stub.h
a3470f
index 433fa68..8f1b185 100644
a3470f
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub.h
a3470f
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub.h
a3470f
@@ -54,6 +54,11 @@
a3470f
                         frame->local = NULL;                \
a3470f
         } while (0)
a3470f
 
a3470f
+#define BR_STUB_RESET_LOCAL_NULL(frame) do {           \
a3470f
+                if (frame->local == (void *)0x1)      \
a3470f
+                        frame->local = NULL;           \
a3470f
+        } while (0)
a3470f
+
a3470f
 typedef int (br_stub_version_cbk) (call_frame_t *, void *,
a3470f
                                    xlator_t *, int32_t, int32_t, dict_t *);
a3470f
 
a3470f
-- 
a3470f
1.8.3.1
a3470f