From e472b92b2306c37350ca66678d2b642ff510d8b4 Mon Sep 17 00:00:00 2001

From: Krishnan Parthasarathi <kparthas@redhat.com>

Date: Fri, 14 Aug 2015 16:01:05 +0530

Subject: [PATCH 270/279] rpc: add owner xlator argument to rpc_clnt_new

        Backport of http://review.gluster.com/11908

The @owner argument tells RPC layer the xlator that owns the connection

and to which xlator THIS needs be set during network notifications like

CONNECT and DISCONNECT.

Code paths that originate from the head of a (volume) graph and use

STACK_WIND ensure that the RPC local endpoint has the right xlator saved

in the frame of the call (callback pair). This guarantees that the

callback is executed in the right xlator context.

The client handshake process which includes fetching of brick ports from

glusterd, setting lk-version on the brick for the session, don't have

the correct xlator set in their frames. The problem lies with RPC

notifications. It doesn't have the provision to set THIS with the xlator

that is registered with the corresponding RPC programs. e.g,

RPC_CLNT_CONNECT event received by protocol/client doesn't have THIS set

to its xlator. This implies, call(-callbacks) originating from this

thread don't have the right xlator set too.

The fix would be to save the xlator registered with the RPC connection

during rpc_clnt_new. e.g, protocol/client's xlator would be saved with

the RPC connection that it 'owns'. RPC notifications such as CONNECT,

DISCONNECT, etc inherit THIS from the RPC connection's xlator.

BUG: 1235571

Change-Id: I6138c7f2ffa9a99a15eec020e1f829400b186ce4

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

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

Reviewed-by: Raghavendra Gowdappa <rgowdapp@redhat.com>

Tested-by: Raghavendra Gowdappa <rgowdapp@redhat.com>

---

 api/src/glfs-internal.h                            |    1 -

 api/src/glfs-mgmt.c                                |    2 +-

 cli/src/cli-quotad-client.c                        |    2 +-

 cli/src/cli.c                                      |    2 +-

 glusterfsd/src/glusterfsd-mgmt.c                   |    2 +-

 libglusterfs/src/globals.h                         |    1 +

 rpc/rpc-lib/src/rpc-clnt.c                         |   19 ++++++++++++++++++-

 rpc/rpc-lib/src/rpc-clnt.h                         |    4 +++-

 .../features/changelog/src/changelog-rpc-common.c  |    2 +-

 xlators/features/quota/src/quota-enforcer-client.c |    2 +-

 .../snapview-server/src/snapview-server-mgmt.c     |    2 +-

 xlators/mgmt/glusterd/src/glusterd-conn-mgmt.c     |    2 +-

 xlators/mgmt/glusterd/src/glusterd-handler.c       |    2 +-

 xlators/nfs/server/src/nlm4.c                      |    2 +-

 xlators/protocol/client/src/client.c               |    2 +-

 15 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/api/src/glfs-internal.h b/api/src/glfs-internal.h

index 83b4ade..8ff78de 100644

--- a/api/src/glfs-internal.h

+++ b/api/src/glfs-internal.h

@@ -199,7 +199,6 @@ int glfs_first_lookup (xlator_t *subvol);

 void glfs_process_upcall_event (struct glfs *fs, void *data);

         GFAPI_PRIVATE(glfs_process_upcall_event, 3.7.0);

-#define DECLARE_OLD_THIS xlator_t *old_THIS = NULL

 #define __GLFS_ENTRY_VALIDATE_FS(fs, label)                         \

 do {                                                                \

diff --git a/api/src/glfs-mgmt.c b/api/src/glfs-mgmt.c

index 2159d67..903b102 100644

--- a/api/src/glfs-mgmt.c

+++ b/api/src/glfs-mgmt.c

@@ -868,7 +868,7 @@ glfs_mgmt_init (struct glfs *fs)

 	if (ret)

 		goto out;

-	rpc = rpc_clnt_new (options, ctx, THIS->name, 8);

+	rpc = rpc_clnt_new (options, THIS, THIS->name, 8);

 	if (!rpc) {

 		ret = -1;

 		gf_msg (THIS->name, GF_LOG_WARNING, 0,

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

index 7c16519..d3a2312 100644

--- a/cli/src/cli-quotad-client.c

+++ b/cli/src/cli-quotad-client.c

@@ -131,7 +131,7 @@ cli_quotad_clnt_init (xlator_t *this, dict_t *options)

         if (ret)

                 goto out;

-        rpc = rpc_clnt_new (options, this->ctx, this->name, 16);

+        rpc = rpc_clnt_new (options, this, this->name, 16);

         if (!rpc)

                 goto out;

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

index 525ec4b..851178b 100644

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

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

@@ -627,7 +627,7 @@ cli_rpc_init (struct cli_state *state)

                         goto out;

         }

-        rpc = rpc_clnt_new (options, this->ctx, this->name, 16);

+        rpc = rpc_clnt_new (options, this, this->name, 16);

         if (!rpc)

                 goto out;

diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c

index 1cff44b..aa7ee4f 100644

--- a/glusterfsd/src/glusterfsd-mgmt.c

+++ b/glusterfsd/src/glusterfsd-mgmt.c

@@ -2038,7 +2038,7 @@ glusterfs_mgmt_init (glusterfs_ctx_t *ctx)

         if (ret)

                 goto out;

-        rpc = rpc_clnt_new (options, THIS->ctx, THIS->name, 8);

+        rpc = rpc_clnt_new (options, THIS, THIS->name, 8);

         if (!rpc) {

                 ret = -1;

                 gf_log (THIS->name, GF_LOG_WARNING, "failed to create rpc clnt");

diff --git a/libglusterfs/src/globals.h b/libglusterfs/src/globals.h

index 9aca3c3..9579c45 100644

--- a/libglusterfs/src/globals.h

+++ b/libglusterfs/src/globals.h

@@ -68,6 +68,7 @@

 /* THIS */

 #define THIS (*__glusterfs_this_location())

+#define DECLARE_OLD_THIS        xlator_t *old_THIS = THIS

 xlator_t **__glusterfs_this_location ();

 xlator_t *glusterfs_this_get ();

diff --git a/rpc/rpc-lib/src/rpc-clnt.c b/rpc/rpc-lib/src/rpc-clnt.c

index 5fbe072..1d87866 100644

--- a/rpc/rpc-lib/src/rpc-clnt.c

+++ b/rpc/rpc-lib/src/rpc-clnt.c

@@ -817,6 +817,16 @@ out:

 static void

 rpc_clnt_destroy (struct rpc_clnt *rpc);

+#define RPC_THIS_SAVE(xl) do {                                  \

+        old_THIS = THIS ;                                       \

+        if (!old_THIS)                                          \

+                gf_log_callingfn ("rpc", GF_LOG_CRITICAL,       \

+                                  "THIS is not initialised.");  \

+        THIS = xl;                                              \

+} while (0)

+

+#define RPC_THIS_RESTORE        (THIS = old_THIS)

+

 int

 rpc_clnt_notify (rpc_transport_t *trans, void *mydata,

                  rpc_transport_event_t event, void *data, ...)

@@ -828,6 +838,7 @@ rpc_clnt_notify (rpc_transport_t *trans, void *mydata,

         rpc_transport_pollin_t *pollin      = NULL;

         struct timespec         ts          = {0, };

         void                   *clnt_mydata = NULL;

+        DECLARE_OLD_THIS;

         conn = mydata;

         if (conn == NULL) {

@@ -837,6 +848,8 @@ rpc_clnt_notify (rpc_transport_t *trans, void *mydata,

         if (!clnt)

                 goto out;

+        RPC_THIS_SAVE (clnt->owner);

+

         switch (event) {

         case RPC_TRANSPORT_DISCONNECT:

         {

@@ -937,6 +950,7 @@ rpc_clnt_notify (rpc_transport_t *trans, void *mydata,

         }

 out:

+        RPC_THIS_RESTORE;

         return ret;

 }

@@ -1041,11 +1055,13 @@ out:

 }

 struct rpc_clnt *

-rpc_clnt_new (dict_t *options, glusterfs_ctx_t *ctx, char *name,

+rpc_clnt_new (dict_t *options, xlator_t *owner, char *name,

               uint32_t reqpool_size)

 {

         int                    ret  = -1;

         struct rpc_clnt       *rpc  = NULL;

+        glusterfs_ctx_t       *ctx  = owner->ctx;

+

         rpc = GF_CALLOC (1, sizeof (*rpc), gf_common_mt_rpcclnt_t);

         if (!rpc) {

@@ -1054,6 +1070,7 @@ rpc_clnt_new (dict_t *options, glusterfs_ctx_t *ctx, char *name,

         pthread_mutex_init (&rpc->lock, NULL);

         rpc->ctx = ctx;

+        rpc->owner = owner;

         if (!reqpool_size)

                 reqpool_size = RPC_CLNT_DEFAULT_REQUEST_COUNT;

diff --git a/rpc/rpc-lib/src/rpc-clnt.h b/rpc/rpc-lib/src/rpc-clnt.h

index ee46a9a..6a4bb40 100644

--- a/rpc/rpc-lib/src/rpc-clnt.h

+++ b/rpc/rpc-lib/src/rpc-clnt.h

@@ -188,9 +188,11 @@ typedef struct rpc_clnt {

         int                   refcount;

         int                   auth_null;

         char                  disabled;

+        xlator_t             *owner;

 } rpc_clnt_t;

-struct rpc_clnt *rpc_clnt_new (dict_t *options, glusterfs_ctx_t *ctx,

+

+struct rpc_clnt *rpc_clnt_new (dict_t *options, xlator_t *owner,

                                char *name, uint32_t reqpool_size);

 int rpc_clnt_start (struct rpc_clnt *rpc);

diff --git a/xlators/features/changelog/src/changelog-rpc-common.c b/xlators/features/changelog/src/changelog-rpc-common.c

index de3a730..b3b14fa 100644

--- a/xlators/features/changelog/src/changelog-rpc-common.c

+++ b/xlators/features/changelog/src/changelog-rpc-common.c

@@ -52,7 +52,7 @@ changelog_rpc_client_init (xlator_t *this, void *cbkdata,

                 goto dealloc_dict;

         }

-        rpc = rpc_clnt_new (options, this->ctx, this->name, 16);

+        rpc = rpc_clnt_new (options, this, this->name, 16);

         if (!rpc)

                 goto dealloc_dict;

diff --git a/xlators/features/quota/src/quota-enforcer-client.c b/xlators/features/quota/src/quota-enforcer-client.c

index 067db6d..10bd24f 100644

--- a/xlators/features/quota/src/quota-enforcer-client.c

+++ b/xlators/features/quota/src/quota-enforcer-client.c

@@ -453,7 +453,7 @@ quota_enforcer_init (xlator_t *this, dict_t *options)

         if (ret)

                 goto out;

-        rpc = rpc_clnt_new (options, this->ctx, this->name, 16);

+        rpc = rpc_clnt_new (options, this, this->name, 16);

         if (!rpc) {

                 ret = -1;

                 goto out;

diff --git a/xlators/features/snapview-server/src/snapview-server-mgmt.c b/xlators/features/snapview-server/src/snapview-server-mgmt.c

index 0fe3687..4b61326 100644

--- a/xlators/features/snapview-server/src/snapview-server-mgmt.c

+++ b/xlators/features/snapview-server/src/snapview-server-mgmt.c

@@ -85,7 +85,7 @@ svs_mgmt_init (xlator_t *this)

                 goto out;

         }

-        priv->rpc = rpc_clnt_new (options, this->ctx, this->name, 8);

+        priv->rpc = rpc_clnt_new (options, this, this->name, 8);

         if (!priv->rpc) {

                 gf_log (this->name, GF_LOG_ERROR, "failed to initialize RPC");

                 goto out;

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

index fca9323..607a065 100644

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

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

@@ -46,7 +46,7 @@ glusterd_conn_init (glusterd_conn_t *conn, char *sockpath,

                 goto out;

         /* @options is free'd by rpc_transport when destroyed */

-        rpc = rpc_clnt_new (options, this->ctx, (char *)svc->name, 16);

+        rpc = rpc_clnt_new (options, this, (char *)svc->name, 16);

         if (!rpc) {

                 ret = -1;

                 goto out;

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

index 82bd7b1..ed5d6ff 100644

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

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

@@ -3295,7 +3295,7 @@ glusterd_rpc_create (struct rpc_clnt **rpc,

         GF_ASSERT (options);

         /* TODO: is 32 enough? or more ? */

-        new_rpc = rpc_clnt_new (options, this->ctx, this->name, 16);

+        new_rpc = rpc_clnt_new (options, this, this->name, 16);

         if (!new_rpc)

                 goto out;

diff --git a/xlators/nfs/server/src/nlm4.c b/xlators/nfs/server/src/nlm4.c

index a1127a3..ac45b4f 100644

--- a/xlators/nfs/server/src/nlm4.c

+++ b/xlators/nfs/server/src/nlm4.c

@@ -1055,7 +1055,7 @@ nlm4_establish_callback (void *csarg)

         }

         /* TODO: is 32 frames in transit enough ? */

-        rpc_clnt = rpc_clnt_new (options, cs->nfsx->ctx, "NLM-client", 32);

+        rpc_clnt = rpc_clnt_new (options, cs->nfsx, "NLM-client", 32);

         if (rpc_clnt == NULL) {

                 gf_msg (GF_NLM, GF_LOG_ERROR, EINVAL, NFS_MSG_INVALID_ENTRY,

                         "rpc_clnt NULL");

diff --git a/xlators/protocol/client/src/client.c b/xlators/protocol/client/src/client.c

index db7a07a..ccb26dd 100644

--- a/xlators/protocol/client/src/client.c

+++ b/xlators/protocol/client/src/client.c

@@ -2286,7 +2286,7 @@ client_init_rpc (xlator_t *this)

                 goto out;

         }

-        conf->rpc = rpc_clnt_new (this->options, this->ctx, this->name, 0);

+        conf->rpc = rpc_clnt_new (this->options, this, this->name, 0);

         if (!conf->rpc) {

                 gf_msg (this->name, GF_LOG_ERROR, 0, PC_MSG_RPC_INIT_FAILED,

                         "failed to initialize RPC");

-- 

1.7.1