From 2c6c4ad77ba5511a62846af932840deb5bc389ae Mon Sep 17 00:00:00 2001

From: Tamar Shacked <tshacked@redhat.com>

Date: Mon, 7 Jun 2021 12:25:57 +0300

Subject: [PATCH 584/584] dht - fixing xattr inconsistency

The scenario of setting an xattr to a dir, killing one of the bricks,

removing the xattr, bringing back the brick results in xattr

inconsistency - The downed brick will still have the xattr, but the rest

won't.

This patch add a mechanism that will remove the extra xattrs during

lookup.

Backport of:

> Upstream-patch-link: https://review.gluster.org/#/c/glusterfs/+/24687/

> fixes: #1324

> Change-Id: Ifec0b7aea6cd40daa8b0319b881191cf83e031d1

> Signed-off-by: Barak Sason Rofman <bsasonro@redhat.com>

BUG: 1600379

Change-Id: I588f69b283e5354cd362d74486d6ec6d226ecc96

Signed-off-by: Tamar Shacked <tshacked@redhat.com>

Signed-off-by: srijan-sivakumar <ssivakum@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/c/rhs-glusterfs/+/245560

Tested-by: RHGS Build Bot <nigelb@redhat.com>

Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>

---

 libglusterfs/src/common-utils.c                | 20 +++++++-

 libglusterfs/src/glusterfs/common-utils.h      |  6 +++

 tests/bugs/distribute/bug-1600379.t            | 54 ++++++++++++++++++++

 xlators/cluster/dht/src/dht-common.c           | 14 ++----

 xlators/cluster/dht/src/dht-common.h           |  4 --

 xlators/cluster/dht/src/dht-helper.c           |  4 ++

 xlators/cluster/dht/src/dht-selfheal.c         | 11 ++++

 xlators/storage/posix/src/posix-helpers.c      | 19 +++++++

 xlators/storage/posix/src/posix-inode-fd-ops.c | 69 ++++++++++++++++++++++++++

 xlators/storage/posix/src/posix.h              |  3 ++

 10 files changed, 189 insertions(+), 15 deletions(-)

 create mode 100644 tests/bugs/distribute/bug-1600379.t

diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c

index c2dfe28..d8b7c6e 100644

--- a/libglusterfs/src/common-utils.c

+++ b/libglusterfs/src/common-utils.c

@@ -54,6 +54,7 @@

 #include "xxhash.h"

 #include <ifaddrs.h>

 #include "glusterfs/libglusterfs-messages.h"

+#include "glusterfs/glusterfs-acl.h"

 #include "protocol-common.h"

 #ifdef __FreeBSD__

 #include <pthread_np.h>

@@ -82,12 +83,21 @@ gf_boolean_t gf_signal_on_assert = false;

 typedef int32_t (*rw_op_t)(int32_t fd, char *buf, int32_t size);

 typedef int32_t (*rwv_op_t)(int32_t fd, const struct iovec *buf, int32_t size);

-void gf_assert(void)

+char *xattrs_to_heal[] = {"user.",

+                          POSIX_ACL_ACCESS_XATTR,

+                          POSIX_ACL_DEFAULT_XATTR,

+                          QUOTA_LIMIT_KEY,

+                          QUOTA_LIMIT_OBJECTS_KEY,

+                          GF_SELINUX_XATTR_KEY,

+                          GF_XATTR_MDATA_KEY,

+                          NULL};

+

+void

+gf_assert(void)

 {

     if (gf_signal_on_assert) {

         raise(SIGCONT);

     }

-

 }

 void

@@ -5430,3 +5440,9 @@ gf_d_type_from_ia_type(ia_type_t type)

             return DT_UNKNOWN;

     }

 }

+

+char **

+get_xattrs_to_heal()

+{

+    return xattrs_to_heal;

+}

diff --git a/libglusterfs/src/glusterfs/common-utils.h b/libglusterfs/src/glusterfs/common-utils.h

index bd48b6f..8439bb6 100644

--- a/libglusterfs/src/glusterfs/common-utils.h

+++ b/libglusterfs/src/glusterfs/common-utils.h

@@ -183,6 +183,12 @@ enum _gf_xlator_ipc_targets {

 typedef enum _gf_special_pid gf_special_pid_t;

 typedef enum _gf_xlator_ipc_targets _gf_xlator_ipc_targets_t;

+/* Array to hold custom xattr keys */

+extern char *xattrs_to_heal[];

+

+char **

+get_xattrs_to_heal();

+

 /* The DHT file rename operation is not a straightforward rename.

  * It involves creating linkto and linkfiles, and can unlink or rename the

  * source file depending on the hashed and cached subvols for the source

diff --git a/tests/bugs/distribute/bug-1600379.t b/tests/bugs/distribute/bug-1600379.t

new file mode 100644

index 0000000..8d2f615

--- /dev/null

+++ b/tests/bugs/distribute/bug-1600379.t

@@ -0,0 +1,54 @@

+#!/bin/bash

+

+. $(dirname $0)/../../include.rc

+. $(dirname $0)/../../volume.rc

+

+# Initialize

+#------------------------------------------------------------

+cleanup;

+

+# Start glusterd

+TEST glusterd;

+TEST pidof glusterd;

+TEST $CLI volume info;

+

+# Create a volume

+TEST $CLI volume create $V0 $H0:$B0/${V0}{1,2}

+

+# Verify volume creation

+EXPECT "$V0" volinfo_field $V0 'Volume Name';

+EXPECT 'Created' volinfo_field $V0 'Status';

+

+# Start volume and verify successful start

+TEST $CLI volume start $V0;

+EXPECT 'Started' volinfo_field $V0 'Status';

+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 --entry-timeout=0 $M0;

+#------------------------------------------------------------

+

+# Test case - Remove xattr from killed brick on lookup

+#------------------------------------------------------------

+# Create a dir and set custom xattr

+TEST mkdir $M0/testdir

+TEST setfattr -n user.attr -v val $M0/testdir

+xattr_val=`getfattr -d $B0/${V0}2/testdir | awk '{print $1}'`;

+TEST ${xattr_val}='user.attr="val"';

+

+# Kill 2nd brick process

+TEST kill_brick $V0 $H0 $B0/${V0}2

+EXPECT_WITHIN ${PROCESS_UP_TIMEOUT} "1" online_brick_count

+

+# Remove custom xattr

+TEST setfattr -x user.attr $M0/testdir

+

+# Bring up the killed brick process

+TEST $CLI volume start $V0 force

+

+# Perform lookup

+sleep 5

+TEST ls $M0/testdir

+

+# Check brick xattrs

+xattr_val_2=`getfattr -d $B0/${V0}2/testdir`;

+TEST [ ${xattr_val_2} = ''] ;

+

+cleanup;

diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c

index ce0fbbf..edfc6e7 100644

--- a/xlators/cluster/dht/src/dht-common.c

+++ b/xlators/cluster/dht/src/dht-common.c

@@ -19,6 +19,7 @@

 #include <glusterfs/byte-order.h>

 #include <glusterfs/quota-common-utils.h>

 #include <glusterfs/upcall-utils.h>

+#include <glusterfs/common-utils.h>

 #include <sys/time.h>

 #include <libgen.h>

@@ -127,15 +128,6 @@ dht_read_iatt_from_xdata(xlator_t *this, dict_t *xdata, struct iatt *stbuf)

 int

 dht_rmdir_unlock(call_frame_t *frame, xlator_t *this);

-char *xattrs_to_heal[] = {"user.",

-                          POSIX_ACL_ACCESS_XATTR,

-                          POSIX_ACL_DEFAULT_XATTR,

-                          QUOTA_LIMIT_KEY,

-                          QUOTA_LIMIT_OBJECTS_KEY,

-                          GF_SELINUX_XATTR_KEY,

-                          GF_XATTR_MDATA_KEY,

-                          NULL};

-

 char *dht_dbg_vxattrs[] = {DHT_DBG_HASHED_SUBVOL_PATTERN, NULL};

 /* Return true if key exists in array

@@ -143,6 +135,8 @@ char *dht_dbg_vxattrs[] = {DHT_DBG_HASHED_SUBVOL_PATTERN, NULL};

 static gf_boolean_t

 dht_match_xattr(const char *key)

 {

+    char **xattrs_to_heal = get_xattrs_to_heal();

+

     return gf_get_index_by_elem(xattrs_to_heal, (char *)key) >= 0;

 }

@@ -5399,11 +5393,13 @@ dht_dir_common_set_remove_xattr(call_frame_t *frame, xlator_t *this, loc_t *loc,

     int call_cnt = 0;

     dht_local_t *local = NULL;

     char gfid_local[GF_UUID_BUF_SIZE] = {0};

+    char **xattrs_to_heal;

     conf = this->private;

     local = frame->local;

     call_cnt = conf->subvolume_cnt;

     local->flags = flags;

+    xattrs_to_heal = get_xattrs_to_heal();

     if (!gf_uuid_is_null(local->gfid)) {

         gf_uuid_unparse(local->gfid, gfid_local);

diff --git a/xlators/cluster/dht/src/dht-common.h b/xlators/cluster/dht/src/dht-common.h

index 132b3b3..b856c68 100644

--- a/xlators/cluster/dht/src/dht-common.h

+++ b/xlators/cluster/dht/src/dht-common.h

@@ -54,10 +54,6 @@

 #define DHT_DBG_HASHED_SUBVOL_PATTERN "dht.file.hashed-subvol.*"

 #define DHT_DBG_HASHED_SUBVOL_KEY "dht.file.hashed-subvol."

-/* Array to hold custom xattr keys

- */

-extern char *xattrs_to_heal[];

-

 /* Rebalance nodeuuid flags */

 #define REBAL_NODEUUID_MINE 0x01

diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c

index 4f7370d..4c3940a 100644

--- a/xlators/cluster/dht/src/dht-helper.c

+++ b/xlators/cluster/dht/src/dht-helper.c

@@ -2289,6 +2289,7 @@ dht_dir_set_heal_xattr(xlator_t *this, dht_local_t *local, dict_t *dst,

     int luret = -1;

     int luflag = -1;

     int i = 0;

+    char **xattrs_to_heal;

     if (!src || !dst) {

         gf_msg(this->name, GF_LOG_WARNING, EINVAL, DHT_MSG_DICT_SET_FAILED,

@@ -2305,6 +2306,9 @@ dht_dir_set_heal_xattr(xlator_t *this, dht_local_t *local, dict_t *dst,

        and set it to dst dict, here index start from 1 because

        user xattr already checked in previous statement

     */

+

+    xattrs_to_heal = get_xattrs_to_heal();

+

     for (i = 1; xattrs_to_heal[i]; i++) {

         keyval = dict_get(src, xattrs_to_heal[i]);

         if (keyval) {

diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c

index f4e17d1..8af7301 100644

--- a/xlators/cluster/dht/src/dht-selfheal.c

+++ b/xlators/cluster/dht/src/dht-selfheal.c

@@ -2315,6 +2315,15 @@ dht_dir_heal_xattrs(void *data)

         if (subvol == mds_subvol)

             continue;

         if (uret || uflag) {

+            /* Custom xattr heal is required - let posix handle it */

+            ret = dict_set_int8(xdata, "sync_backend_xattrs", _gf_true);

+            if (ret) {

+                gf_smsg(this->name, GF_LOG_WARNING, 0, DHT_MSG_DICT_SET_FAILED,

+                        "path=%s", local->loc.path, "key=%s",

+                        "sync_backend_xattrs", NULL);

+                goto out;

+            }

+

             ret = syncop_setxattr(subvol, &local->loc, user_xattr, 0, xdata,

                                   NULL);

             if (ret) {

@@ -2325,6 +2334,8 @@ dht_dir_heal_xattrs(void *data)

                        "user xattr on path %s on "

                        "subvol %s, gfid = %s ",

                        local->loc.path, subvol->name, gfid);

+            } else {

+                dict_del(xdata, "sync_backend_xattrs");

             }

         }

     }

diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c

index 16351d8..40a9ee4 100644

--- a/xlators/storage/posix/src/posix-helpers.c

+++ b/xlators/storage/posix/src/posix-helpers.c

@@ -3656,3 +3656,22 @@ out:

     return is_stale;

 }

+

+/* Delete user xattr from the file at the file-path specified by data and from

+ * dict */

+int

+posix_delete_user_xattr(dict_t *dict, char *k, data_t *v, void *data)

+{

+    int ret;

+    char *real_path = data;

+

+    ret = sys_lremovexattr(real_path, k);

+    if (ret) {

+        gf_msg("posix-helpers", GF_LOG_ERROR, P_MSG_XATTR_NOT_REMOVED, errno,

+               "removexattr failed. key %s path %s", k, real_path);

+    }

+

+    dict_del(dict, k);

+

+    return ret;

+}

diff --git a/xlators/storage/posix/src/posix-inode-fd-ops.c b/xlators/storage/posix/src/posix-inode-fd-ops.c

index 4c2983a..be22c5e 100644

--- a/xlators/storage/posix/src/posix-inode-fd-ops.c

+++ b/xlators/storage/posix/src/posix-inode-fd-ops.c

@@ -62,6 +62,7 @@

 #include <glusterfs/events.h>

 #include "posix-gfid-path.h"

 #include <glusterfs/compat-uuid.h>

+#include <glusterfs/common-utils.h>

 extern char *marker_xattrs[];

 #define ALIGN_SIZE 4096

@@ -2733,6 +2734,7 @@ posix_setxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,

     int32_t ret = 0;

     ssize_t acl_size = 0;

     dict_t *xattr = NULL;

+    dict_t *subvol_xattrs = NULL;

     posix_xattr_filler_t filler = {

         0,

     };

@@ -2748,6 +2750,10 @@ posix_setxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,

     struct mdata_iatt mdata_iatt = {

         0,

     };

+    int8_t sync_backend_xattrs = _gf_false;

+    data_pair_t *custom_xattrs;

+    data_t *keyval = NULL;

+    char **xattrs_to_heal = get_xattrs_to_heal();

     DECLARE_OLD_FS_ID_VAR;

     SET_FS_ID(frame->root->uid, frame->root->gid);

@@ -2930,6 +2936,66 @@ posix_setxattr(call_frame_t *frame, xlator_t *this, loc_t *loc, dict_t *dict,

         goto out;

     }

+    ret = dict_get_int8(xdata, "sync_backend_xattrs", &sync_backend_xattrs);

+    if (ret) {

+        gf_msg_debug(this->name, -ret, "Unable to get sync_backend_xattrs");

+    }

+

+    if (sync_backend_xattrs) {

+        /* List all custom xattrs */

+        subvol_xattrs = dict_new();

+        if (!subvol_xattrs)

+            goto out;

+

+        ret = dict_set_int32_sizen(xdata, "list-xattr", 1);

+        if (ret) {

+            gf_msg(this->name, GF_LOG_ERROR, 0, ENOMEM,

+                   "Unable to set list-xattr in dict ");

+            goto out;

+        }

+

+        subvol_xattrs = posix_xattr_fill(this, real_path, loc, NULL, -1, xdata,

+                                         NULL);

+

+        /* Remove all user xattrs from the file */

+        dict_foreach_fnmatch(subvol_xattrs, "user.*", posix_delete_user_xattr,

+                             real_path);

+

+        /* Remove all custom xattrs from the file */

+        for (i = 1; xattrs_to_heal[i]; i++) {

+            keyval = dict_get(subvol_xattrs, xattrs_to_heal[i]);

+            if (keyval) {

+                ret = sys_lremovexattr(real_path, xattrs_to_heal[i]);

+                if (ret) {

+                    gf_msg(this->name, GF_LOG_ERROR, P_MSG_XATTR_NOT_REMOVED,

+                           errno, "removexattr failed. key %s path %s",

+                           xattrs_to_heal[i], loc->path);

+                    goto out;

+                }

+

+                dict_del(subvol_xattrs, xattrs_to_heal[i]);

+                keyval = NULL;

+            }

+        }

+

+        /* Set custom xattrs based on info provided by DHT */

+        custom_xattrs = dict->members_list;

+

+        while (custom_xattrs != NULL) {

+            ret = sys_lsetxattr(real_path, custom_xattrs->key,

+                                custom_xattrs->value->data,

+                                custom_xattrs->value->len, flags);

+            if (ret) {

+                op_errno = errno;

+                gf_log(this->name, GF_LOG_ERROR, "setxattr failed - %s %d",

+                       custom_xattrs->key, ret);

+                goto out;

+            }

+

+            custom_xattrs = custom_xattrs->next;

+        }

+    }

+

     xattr = dict_new();

     if (!xattr)

         goto out;

@@ -3037,6 +3103,9 @@ out:

     if (xattr)

         dict_unref(xattr);

+    if (subvol_xattrs)

+        dict_unref(subvol_xattrs);

+

     return 0;

 }

diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h

index 4be979c..b357d34 100644

--- a/xlators/storage/posix/src/posix.h

+++ b/xlators/storage/posix/src/posix.h

@@ -686,4 +686,7 @@ posix_update_iatt_buf(struct iatt *buf, int fd, char *loc, dict_t *xdata);

 gf_boolean_t

 posix_is_layout_stale(dict_t *xdata, char *par_path, xlator_t *this);

+int

+posix_delete_user_xattr(dict_t *dict, char *k, data_t *v, void *data);

+

 #endif /* _POSIX_H */

-- 

1.8.3.1