afr: Ignore gluster internal (virtual) xattrs in metadata heal check

Problem:
In arbiter configuration, posix-xlator in the arbiter brick always sets
the GF_CONTENT_KEY in the response dict with a value 0. If the file size on
the data bricks is more than quick-read's max-file-size (64kb default),
those bricks don't set the key. Because of this difference in the no. of dict
elements, afr triggers metadata heal in lookup code path, in turn
leading to extra lookups+inodelks.

Fix:
Changed afr dict comparison logic to ignore all virtual xattrs and the
on-disk ones that we should not be healing.

Also removed is_virtual_xattr() function. The original callers to this
function (upcall) don't seem to need it anymore.


> Reviewed-on: http://review.gluster.org/15548
> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

(cherry picked from commit 5afc6aba906a21aee19c2f1baaa7d9eb762ae0ac)

Change-Id: I05730bdd39d8fb0b9a49a5fc9c0bb01f0d3bb308
BUG: 1379528
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Reviewed-on: http://review.gluster.org/15577
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

4 files changed, 18 insertions, 50 deletions

diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 351eb4daaec..14a344958fd 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -4535,37 +4535,20 @@ gf_zero_fill_stat (struct iatt *buf)
         buf->ia_ctime = 0;
 }
 
-/* This function checks if the input key is a virtual or on disk xattr.
- * Its not simple to identify a virtual xattr as there is no common
- * format used to identify virtual xattrs. One way is to list each of the
- * xattrs in different categories and compare the input against every xattr
- * in the list, this is very inefficient as there are 100s of xattrs.
- *
- * Currently the only consumer of this function is upcall and md-cache,
- * hence tailoring this function to their needs; in their case its allowed
- * to consider disk xattrs as virtual xattrs, but not vice versa, i.e.
- * virtual xattrs should not be considered as on disk xattr. Hence, being
- * conservative, we consider anything that starts from user.*, security.*,
- * system.* as on disk xattrs. trusted.* and glusterfs.* cannot be considered
- * as virtual xattrs as there are some on disk xattrs which start from
- * glusterfs.* and trusted.*
- *
- * Hence, this function could return an on disk xattr as virtual xattr but
- * never a virtual xattr as on disk xattr.
- */
 gf_boolean_t
-is_virtual_xattr (const char *k)
+gf_is_valid_xattr_namespace (char *key)
 {
-        gf_boolean_t ret = _gf_true;
+        static char *xattr_namespaces[] = {"trusted.", "security.", "system.",
+                                           "user.", NULL };
+        int i = 0;
 
-        if ((strncmp (k, "user.", strlen ("user.")) == 0) ||
-            (strncmp (k, "security.", strlen ("security.")) == 0) ||
-            (strncmp (k, "system.", strlen ("system.")) == 0) ||
-            (GF_POSIX_ACL_REQUEST (k))) {
-                ret = _gf_false;
+        for (i = 0; xattr_namespaces[i]; i++) {
+                if (strncmp (key, xattr_namespaces[i],
+                             strlen (xattr_namespaces[i])) == 0)
+                        return _gf_true;
         }
 
-        return ret;
+        return _gf_false;
 }
 
 ino_t
diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h
index 286b70ea708..72613acba3b 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -853,7 +853,7 @@ void
 gf_zero_fill_stat (struct iatt *buf);
 
 gf_boolean_t
-is_virtual_xattr (const char *k);
+gf_is_valid_xattr_namespace (char *k);
 
 const char *
 gf_inode_type_to_str (ia_type_t type);
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index 8c59da7ecca..a92be5fe85c 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -1816,10 +1816,6 @@ afr_frame_return (call_frame_t *frame)
 }
 
 static char *afr_ignore_xattrs[] = {
-        GLUSTERFS_OPEN_FD_COUNT,
-        GLUSTERFS_PARENT_ENTRYLK,
-        GLUSTERFS_ENTRYLK_COUNT,
-        GLUSTERFS_INODELK_COUNT,
         GF_SELINUX_XATTR_KEY,
         QUOTA_SIZE_KEY,
         NULL
@@ -1840,8 +1836,13 @@ afr_is_xattr_ignorable (char *key)
 }
 
 static gf_boolean_t
-afr_xattr_match (dict_t *this, char *key1, data_t *value1, void *data)
+afr_xattr_match_needed (dict_t *this, char *key1, data_t *value1, void *data)
 {
+        /* Ignore all non-disk (i.e. virtual) xattrs right away. */
+        if (!gf_is_valid_xattr_namespace (key1))
+                return _gf_false;
+
+        /* Ignore on-disk xattrs that AFR doesn't need to heal. */
         if (!afr_is_xattr_ignorable (key1))
                 return _gf_true;
 
@@ -1851,7 +1852,7 @@ afr_xattr_match (dict_t *this, char *key1, data_t *value1, void *data)
 gf_boolean_t
 afr_xattrs_are_equal (dict_t *dict1, dict_t *dict2)
 {
-        return are_dicts_equal (dict1, dict2, afr_xattr_match, NULL);
+        return are_dicts_equal (dict1, dict2, afr_xattr_match_needed, NULL);
 }
 
 static int
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index 6162f2649e1..becbd94c226 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -117,22 +117,6 @@ posix_xattr_ignorable (char *key)
         return _is_in_array (posix_ignore_xattrs, key);
 }
 
-static gf_boolean_t
-posix_is_valid_namespace (char *key)
-{
-        static char *xattr_namespaces[] = {"trusted.", "security.", "system.",
-                                           "user.", NULL };
-        int i = 0;
-
-        for (i = 0; xattr_namespaces[i]; i++) {
-                if (strncmp (key, xattr_namespaces[i],
-                             strlen (xattr_namespaces[i])) == 0)
-                        return _gf_true;
-        }
-
-        return _gf_false;
-}
-
 static int
 _posix_xattr_get_set_from_backend (posix_xattr_filler_t *filler, char *key)
 {
@@ -142,7 +126,7 @@ _posix_xattr_get_set_from_backend (posix_xattr_filler_t *filler, char *key)
         char     val_buf[256] = {0};
         gf_boolean_t have_val   = _gf_false;
 
-        if (!posix_is_valid_namespace (key)) {
+        if (!gf_is_valid_xattr_namespace (key)) {
                 ret = -1;
                 goto out;
         }