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