From 854ab79dbef449c39adf66e3faebb4681359fce4 Mon Sep 17 00:00:00 2001

From: mohit84 <moagrawa@redhat.com>

Date: Thu, 18 Feb 2021 09:40:44 +0530

Subject: [PATCH 533/538] glusterd: Rebalance cli is not showing correct status

 after reboot (#2172)

Rebalance cli is not showing correct status after reboot.

The CLI is not correct status because defrag object is not

valid at the time of creating a rpc connection to show the status.

The defrag object is not valid because at the time of start a glusterd

glusterd_restart_rebalance can be call almost at the same time by two

different synctask and glusterd got a disconnect on rpc object and it

cleanup the defrag object.

Solution: To avoid the defrag object populate a reference count before

          create a defrag rpc object.

>Fixes: #1339

>Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>

>Change-Id: Ia284015d79beaa3d703ebabb92f26870a5aaafba

Upstream Patch : https://github.com/gluster/glusterfs/pull/2172

BUG: 1832306

Change-Id: Ia284015d79beaa3d703ebabb92f26870a5aaafba

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

Reviewed-on: https://code.engineering.redhat.com/gerrit/228249

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

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

---

 xlators/mgmt/glusterd/src/glusterd-rebalance.c | 35 ++++++++++-----

 xlators/mgmt/glusterd/src/glusterd-syncop.c    |  1 +

 xlators/mgmt/glusterd/src/glusterd-utils.c     | 59 +++++++++++++++++++++++++-

 xlators/mgmt/glusterd/src/glusterd-utils.h     |  5 +++

 xlators/mgmt/glusterd/src/glusterd.h           |  1 +

 5 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c

index b419a89..fcd5318 100644

--- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c

+++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c

@@ -86,6 +86,7 @@ __glusterd_defrag_notify(struct rpc_clnt *rpc, void *mydata,

     glusterd_conf_t *priv = NULL;

     xlator_t *this = NULL;

     int pid = -1;

+    int refcnt = 0;

     this = THIS;

     if (!this)

@@ -125,11 +126,12 @@ __glusterd_defrag_notify(struct rpc_clnt *rpc, void *mydata,

         }

         case RPC_CLNT_DISCONNECT: {

-            if (!defrag->connected)

-                return 0;

-

             LOCK(&defrag->lock);

             {

+                if (!defrag->connected) {

+                    UNLOCK(&defrag->lock);

+                    return 0;

+                }

                 defrag->connected = 0;

             }

             UNLOCK(&defrag->lock);

@@ -146,11 +148,11 @@ __glusterd_defrag_notify(struct rpc_clnt *rpc, void *mydata,

             glusterd_defrag_rpc_put(defrag);

             if (defrag->cbk_fn)

                 defrag->cbk_fn(volinfo, volinfo->rebal.defrag_status);

-

-            GF_FREE(defrag);

+            refcnt = glusterd_defrag_unref(defrag);

             gf_msg(this->name, GF_LOG_INFO, 0, GD_MSG_REBALANCE_DISCONNECTED,

-                   "Rebalance process for volume %s has disconnected.",

-                   volinfo->volname);

+                   "Rebalance process for volume %s has disconnected"

+                   " and defrag refcnt is %d.",

+                   volinfo->volname, refcnt);

             break;

         }

         case RPC_CLNT_DESTROY:

@@ -309,7 +311,11 @@ glusterd_handle_defrag_start(glusterd_volinfo_t *volinfo, char *op_errstr,

         gf_msg_debug("glusterd", 0, "rebalance command failed");

         goto out;

     }

-

+    /* Take reference before sleep to save defrag object cleanup while

+       glusterd_restart_rebalance call for other bricks by syncktask

+       at the time of restart a glusterd.

+    */

+    glusterd_defrag_ref(defrag);

     sleep(5);

     ret = glusterd_rebalance_rpc_create(volinfo);

@@ -372,6 +378,7 @@ glusterd_rebalance_rpc_create(glusterd_volinfo_t *volinfo)

     GF_ASSERT(this);

     priv = this->private;

     GF_ASSERT(priv);

+    struct rpc_clnt *rpc = NULL;

     // rebalance process is not started

     if (!defrag)

@@ -396,13 +403,21 @@ glusterd_rebalance_rpc_create(glusterd_volinfo_t *volinfo)

     }

     glusterd_volinfo_ref(volinfo);

-    ret = glusterd_rpc_create(&defrag->rpc, options, glusterd_defrag_notify,

-                              volinfo, _gf_true);

+    ret = glusterd_rpc_create(&rpc, options, glusterd_defrag_notify, volinfo,

+                              _gf_false);

     if (ret) {

         gf_msg(THIS->name, GF_LOG_ERROR, 0, GD_MSG_RPC_CREATE_FAIL,

                "Glusterd RPC creation failed");

         goto out;

     }

+    LOCK(&defrag->lock);

+    {

+        if (!defrag->rpc)

+            defrag->rpc = rpc;

+        else

+            rpc_clnt_unref(rpc);

+    }

+    UNLOCK(&defrag->lock);

     ret = 0;

 out:

     if (options)

diff --git a/xlators/mgmt/glusterd/src/glusterd-syncop.c b/xlators/mgmt/glusterd/src/glusterd-syncop.c

index df78fef..05c9e11 100644

--- a/xlators/mgmt/glusterd/src/glusterd-syncop.c

+++ b/xlators/mgmt/glusterd/src/glusterd-syncop.c

@@ -1732,6 +1732,7 @@ gd_brick_op_phase(glusterd_op_t op, dict_t *op_ctx, dict_t *req_dict,

         if (!rpc) {

             if (pending_node->type == GD_NODE_REBALANCE && pending_node->node) {

                 volinfo = pending_node->node;

+                glusterd_defrag_ref(volinfo->rebal.defrag);

                 ret = glusterd_rebalance_rpc_create(volinfo);

                 if (ret) {

                     ret = 0;

diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c

index bc188a2..9fb8eab 100644

--- a/xlators/mgmt/glusterd/src/glusterd-utils.c

+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c

@@ -93,6 +93,44 @@

 #define NLMV4_VERSION 4

 #define NLMV1_VERSION 1

+int

+glusterd_defrag_ref(glusterd_defrag_info_t *defrag)

+{

+    int refcnt = 0;

+

+    if (!defrag)

+        goto out;

+

+    LOCK(&defrag->lock);

+    {

+        refcnt = ++defrag->refcnt;

+    }

+    UNLOCK(&defrag->lock);

+

+out:

+    return refcnt;

+}

+

+int

+glusterd_defrag_unref(glusterd_defrag_info_t *defrag)

+{

+    int refcnt = -1;

+

+    if (!defrag)

+        goto out;

+

+    LOCK(&defrag->lock);

+    {

+        refcnt = --defrag->refcnt;

+        if (refcnt <= 0)

+            GF_FREE(defrag);

+    }

+    UNLOCK(&defrag->lock);

+

+out:

+    return refcnt;

+}

+

 gf_boolean_t

 is_brick_mx_enabled(void)

 {

@@ -9370,6 +9408,7 @@ glusterd_volume_defrag_restart(glusterd_volinfo_t *volinfo, char *op_errstr,

     char pidfile[PATH_MAX] = "";

     int ret = -1;

     pid_t pid = 0;

+    int refcnt = 0;

     this = THIS;

     GF_ASSERT(this);

@@ -9410,7 +9449,25 @@ glusterd_volume_defrag_restart(glusterd_volinfo_t *volinfo, char *op_errstr,

                              volinfo->volname);

                     goto out;

                 }

-                ret = glusterd_rebalance_rpc_create(volinfo);

+                refcnt = glusterd_defrag_ref(volinfo->rebal.defrag);

+                /* If refcnt value is 1 it means either defrag object is

+                   poulated by glusterd_rebalance_defrag_init or previous

+                   rpc creation was failed.If it is not 1 it means it(defrag)

+                   was populated at the time of start a rebalance daemon.

+                   We need to create a rpc object only while a previous

+                   rpc connection was not established successfully at the

+                   time of restart a rebalance daemon by

+                   glusterd_handle_defrag_start otherwise rebalance cli

+                   does not show correct status after just reboot a node and try

+                   to print the rebalance status because defrag object has been

+                   destroyed during handling of rpc disconnect.

+                */

+                if (refcnt == 1) {

+                    ret = glusterd_rebalance_rpc_create(volinfo);

+                } else {

+                    ret = 0;

+                    glusterd_defrag_unref(volinfo->rebal.defrag);

+                }

                 break;

             }

         case GF_DEFRAG_STATUS_NOT_STARTED:

diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h

index 02d85d2..4541471 100644

--- a/xlators/mgmt/glusterd/src/glusterd-utils.h

+++ b/xlators/mgmt/glusterd/src/glusterd-utils.h

@@ -886,4 +886,9 @@ int32_t

 glusterd_check_brick_order(dict_t *dict, char *err_str, int32_t type,

                            int32_t sub_count);

+int

+glusterd_defrag_ref(glusterd_defrag_info_t *defrag);

+

+int

+glusterd_defrag_unref(glusterd_defrag_info_t *defrag);

 #endif

diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h

index efe4d0e..9de3f28 100644

--- a/xlators/mgmt/glusterd/src/glusterd.h

+++ b/xlators/mgmt/glusterd/src/glusterd.h

@@ -321,6 +321,7 @@ struct glusterd_defrag_info_ {

     uint64_t total_data;

     uint64_t num_files_lookedup;

     uint64_t total_failures;

+    int refcnt;

     gf_lock_t lock;

     int cmd;

     pthread_t th;

-- 

1.8.3.1