From bc759c626e16af3be07f82bb8203a3c455973757 Mon Sep 17 00:00:00 2001

From: vmallika <vmallika@redhat.com>

Date: Mon, 24 Aug 2015 08:07:14 +0530

Subject: [PATCH 310/320] cli: on error invoke cli_cmd_broadcast_response function in separate thread

This is a backport of http://review.gluster.org/#/c/11990/

There is a problem in current CLI framework

CLI holds the lock when processing command.

When processing quota list command, below sequence of steps executed in the

same thread and causing deadlock

1) CLI holds the lock

2) Send rpc_clnt_submit request to quotad for quota usage

3) If quotad is down, rpc_clnt_submit invokes cbk function with error

4) cbk function cli_quotad_getlimit_cbk tries to hold lock to broadcast

   the results and hangs, because same thread has already holding the lock

This patch fixes the problem by creating seperate thread for

broadcasting the result

> Change-Id: I53be006eadf6aaf348083d9168535530d70a8ab3

> BUG: 1242819

> Signed-off-by: vmallika <vmallika@redhat.com>

Change-Id: Id010b95be689a947dd7ef75500d6c00d0f78cd3c

BUG: 1242803

Signed-off-by: vmallika <vmallika@redhat.com>

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

Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com>

Tested-by: Krishnan Parthasarathi <kparthas@redhat.com>

---

 cli/src/cli-cmd-volume.c |   12 ------

 cli/src/cli-rpc-ops.c    |   94 ++++++++++++++++++++++++++++++++++++++++++---

 cli/src/cli.c            |    1 +

 cli/src/cli.h            |    1 +

 4 files changed, 89 insertions(+), 19 deletions(-)

diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c

index 6dd3058..ca18108 100644

--- a/cli/src/cli-cmd-volume.c

+++ b/cli/src/cli-cmd-volume.c

@@ -1381,18 +1381,6 @@ cli_cmd_quota_handle_list_all (const char **words, dict_t *options)

         CLI_LOCAL_INIT (local, words, frame, xdata);

         proc = &cli_quotad_clnt.proctable[GF_AGGREGATOR_GETLIMIT];

-        if (!(global_state->mode & GLUSTER_MODE_XML)) {

-                print_quota_list_header (type);

-        } else {

-                ret = cli_xml_output_vol_quota_limit_list_begin

-                        (local, 0, 0, NULL);

-                if (ret) {

-                        gf_log ("cli", GF_LOG_ERROR, "Error in printing "

-                                "xml output");

-                        goto out;

-                }

-        }

-

         gfid_str = GF_CALLOC (1, gf_common_mt_char, 64);

         if (!gfid_str) {

                 ret = -1;

diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c

index dfc5c6b..fd76df7 100644

--- a/cli/src/cli-rpc-ops.c

+++ b/cli/src/cli-rpc-ops.c

@@ -3232,7 +3232,8 @@ out:

 }

 int

-print_quota_list_from_quotad (call_frame_t *frame, dict_t *rsp_dict)

+print_quota_list_from_quotad (call_frame_t *frame, dict_t *rsp_dict,

+                              int32_t list_count)

 {

         char             *path          = NULL;

         char             *default_sl    = NULL;

@@ -3244,11 +3245,11 @@ print_quota_list_from_quotad (call_frame_t *frame, dict_t *rsp_dict)

         quota_limits_t  *size_limits    = NULL;

         int32_t          type           = 0;

+        GF_ASSERT (frame);

+

         local = frame->local;

         gd_rsp_dict = local->dict;

-        GF_ASSERT (frame);

-

         ret = dict_get_int32 (rsp_dict, "type", &type);

         if (ret) {

                 gf_log ("cli", GF_LOG_ERROR, "Failed to get type");

@@ -3306,12 +3307,37 @@ print_quota_list_from_quotad (call_frame_t *frame, dict_t *rsp_dict)

                 goto out;

         }

+        if (list_count == 0) {

+                if (!(global_state->mode & GLUSTER_MODE_XML)) {

+                        print_quota_list_header (type);

+                } else {

+                        ret = cli_xml_output_vol_quota_limit_list_begin

+                                (local, 0, 0, NULL);

+                        if (ret) {

+                                gf_log ("cli", GF_LOG_ERROR, "Error in "

+                                        "printing xml output");

+                                goto out;

+                        }

+                }

+        }

+

         ret = print_quota_list_output (local, path, default_sl, &limits,

                                        &used_space, type);

 out:

         return ret;

 }

+void*

+cli_cmd_broadcast_response_detached (void *opaque)

+{

+        int32_t ret = 0;

+

+        ret = (intptr_t) opaque;

+        cli_cmd_broadcast_response (ret);

+

+        return NULL;

+}

+

 int

 cli_quotad_getlimit_cbk (struct rpc_req *req, struct iovec *iov,

                           int count, void *myframe)

@@ -3321,12 +3347,41 @@ cli_quotad_getlimit_cbk (struct rpc_req *req, struct iovec *iov,

         int                ret         = -1;

         dict_t            *dict        = NULL;

         call_frame_t      *frame       = NULL;

+        cli_local_t       *local       = NULL;

+        dict_t            *gd_rsp_dict = NULL;

+        int32_t            list_count  = 0;

+        pthread_t          th_id       = {0, };

-        if (-1 == req->rpc_status) {

+        frame = myframe;

+        GF_ASSERT (frame);

+

+        local = frame->local;

+        gd_rsp_dict = local->dict;

+

+        LOCK (&local->lock);

+        {

+                ret = dict_get_int32 (gd_rsp_dict, "quota-list-count",

+                                      &list_count);

+                if (ret)

+                        list_count = 0;

+                ret = dict_set_int32 (gd_rsp_dict, "quota-list-count",

+                                      list_count + 1);

+        }

+        UNLOCK (&local->lock);

+

+        if (ret) {

+                gf_log ("cli", GF_LOG_ERROR, "Failed to set "

+                        "quota-list-count in dict");

                 goto out;

         }

-        frame = myframe;

+        if (-1 == req->rpc_status) {

+                if (list_count == 0)

+                        cli_err ("Connection failed. Please check if quota "

+                                 "daemon is operational.");

+                ret = -1;

+                goto out;

+        }

         ret = xdr_to_generic (*iov, &rsp, (xdrproc_t)xdr_gf_cli_rsp);

         if (ret < 0) {

@@ -3357,11 +3412,36 @@ cli_quotad_getlimit_cbk (struct rpc_req *req, struct iovec *iov,

                                 "unserialize req-buffer to dictionary");

                         goto out;

                 }

-                print_quota_list_from_quotad (frame, dict);

+                print_quota_list_from_quotad (frame, dict, list_count);

         }

 out:

-        cli_cmd_broadcast_response (ret);

+        /* Bad Fix: CLI holds the lock to process a command.

+         * When processing quota list command, below sequence of steps executed

+         * in the same thread and causing deadlock

+         *

+         * 1) CLI holds the lock

+         * 2) Send rpc_clnt_submit request to quotad for quota usage

+         * 3) If quotad is down, rpc_clnt_submit invokes cbk function with error

+         * 4) cbk function cli_quotad_getlimit_cbk invokes

+         *    cli_cmd_broadcast_response which tries to hold lock to broadcast

+         *    the results and hangs, because same thread has already holding

+         *    the lock

+         *

+         * Broadcasting response in a seperate thread which is not a

+         * good fix. This needs to be re-visted with better solution

+         */

+        if (ret == -1) {

+                ret = pthread_create (&th_id, NULL,

+                                cli_cmd_broadcast_response_detached,

+                                (void *)-1);

+                if (ret)

+                        gf_log ("cli", GF_LOG_ERROR, "pthread_create failed: "

+                                "%s", strerror (errno));

+        } else {

+                cli_cmd_broadcast_response (ret);

+        }

+

         if (dict)

                 dict_unref (dict);

diff --git a/cli/src/cli.c b/cli/src/cli.c

index 851178b..fa5b682 100644

--- a/cli/src/cli.c

+++ b/cli/src/cli.c

@@ -653,6 +653,7 @@ cli_local_get ()

         cli_local_t     *local = NULL;

         local = GF_CALLOC (1, sizeof (*local), cli_mt_cli_local_t);

+        LOCK_INIT (&local->lock);

         return local;

 }

diff --git a/cli/src/cli.h b/cli/src/cli.h

index 6c0fbce..d831af0 100644

--- a/cli/src/cli.h

+++ b/cli/src/cli.h

@@ -151,6 +151,7 @@ struct cli_local {

         xmlDocPtr               doc;

         int                     vol_count;

 #endif

+        gf_lock_t               lock;

 };

 struct cli_volume_status {

-- 

1.7.1