rpc: switch to MT-safe block RPC routines

blockResponse *
block_delete_1(blockDelete *argp, CLIENT *clnt)
{
        static blockResponse clnt_res; <<<<<<-------- Same memory is used by everyone

        memset((char *)&clnt_res, 0, sizeof(clnt_res)); <<<<<---- Here memset is happening
        if (clnt_call (clnt, BLOCK_DELETE,
                (xdrproc_t) xdr_blockDelete, (caddr_t) argp,
                (xdrproc_t) xdr_blockResponse, (caddr_t) &clnt_res,
                TIMEOUT) != RPC_SUCCESS) {
                return (NULL);
        }
        return (&clnt_res); <<<<<---- ptr to this memory is returned.
}

So while Thread-1 is returned "return (&clnt_res);" another thread could be
doing "memset((char *)&clnt_res, 0, sizeof(clnt_res));"

This seem to be a day-1 gluster-blockd bug from the looks of it.

Change-Id: I3fc76d7814c4fe5b286577586ec44d752dcc73f0
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>

4 files changed, 149 insertions, 42 deletions

diff --git a/cli/gluster-block.c b/cli/gluster-block.c
index 85b0e80..aca7de5 100644
--- a/cli/gluster-block.c
+++ b/cli/gluster-block.c
@@ -59,7 +59,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
   blockInfoCli *info_obj;
   blockListCli *list_obj;
   blockModifyCli *modify_obj;
-  blockResponse *reply = NULL;
+  blockResponse reply = {0,};
   char          errMsg[2048] = {0};
 
 
@@ -103,8 +103,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
   switch(opt) {
   case CREATE_CLI:
     create_obj = cobj;
-    reply = block_create_cli_1(create_obj, clnt);
-    if (!reply) {
+    if (block_create_cli_1(create_obj, &reply, clnt) != RPC_SUCCESS) {
       LOG("cli", GB_LOG_ERROR,
           "%sblock %s create on volume %s with hosts %s failed\n",
           clnt_sperror(clnt, "block_create_cli_1"), create_obj->block_name,
@@ -114,8 +113,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
     break;
   case DELETE_CLI:
     delete_obj = cobj;
-    reply = block_delete_cli_1(delete_obj, clnt);
-    if (!reply) {
+    if (block_delete_cli_1(delete_obj, &reply, clnt) != RPC_SUCCESS) {
       LOG("cli", GB_LOG_ERROR, "%sblock %s delete on volume %s failed",
           clnt_sperror(clnt, "block_delete_cli_1"),
           delete_obj->block_name, delete_obj->volume);
@@ -124,8 +122,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
     break;
   case INFO_CLI:
     info_obj = cobj;
-    reply = block_info_cli_1(info_obj, clnt);
-    if (!reply) {
+    if (block_info_cli_1(info_obj, &reply, clnt) != RPC_SUCCESS) {
       LOG("cli", GB_LOG_ERROR, "%sblock %s info on volume %s failed",
           clnt_sperror(clnt, "block_info_cli_1"),
           info_obj->block_name, info_obj->volume);
@@ -134,8 +131,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
     break;
   case LIST_CLI:
     list_obj = cobj;
-    reply = block_list_cli_1(list_obj, clnt);
-    if (!reply) {
+    if (block_list_cli_1(list_obj, &reply, clnt) != RPC_SUCCESS) {
       LOG("cli", GB_LOG_ERROR, "%sblock list on volume %s failed",
           clnt_sperror(clnt, "block_list_cli_1"), list_obj->volume);
       goto out;
@@ -143,8 +139,7 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
     break;
   case MODIFY_CLI:
     modify_obj = cobj;
-    reply = block_modify_cli_1(modify_obj, clnt);
-    if (!reply) {
+    if (block_modify_cli_1(modify_obj, &reply, clnt) != RPC_SUCCESS) {
       LOG("cli", GB_LOG_ERROR, "%sblock modify on volume %s failed",
           clnt_sperror(clnt, "block_modify_cli_1"), modify_obj->volume);
       goto out;
@@ -153,9 +148,9 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
   }
 
  out:
-  if (reply) {
-    ret = reply->exit;
-    MSG("%s", reply->out);
+  if (reply.out) {
+    ret = reply.exit;
+    MSG("%s", reply.out);
   } else if (errMsg[0]) {
     LOG("cli", GB_LOG_ERROR, "%s", errMsg);
     MSG("%s\n", errMsg);
@@ -165,8 +160,9 @@ glusterBlockCliRPC_1(void *cobj, clioperations opt)
     ret = -1;
   }
 
-  if (clnt && reply) {
-    if (!clnt_freeres(clnt, (xdrproc_t)xdr_blockResponse, (char *)reply)) {
+  if (clnt) {
+    if (reply.out && !clnt_freeres(clnt, (xdrproc_t)xdr_blockResponse,
+                                   (char *)&reply)) {
       LOG("cli", GB_LOG_ERROR, "%s",
           clnt_sperror(clnt, "clnt_freeres failed"));
     }
diff --git a/rpc/block_svc_routines.c b/rpc/block_svc_routines.c
index d47a841..51b56ec 100644
--- a/rpc/block_svc_routines.c
+++ b/rpc/block_svc_routines.c
@@ -363,7 +363,7 @@ glusterBlockCallRPC_1(char *host, void *cobj,
   int ret = -1;
   int sockfd;
   int errsv = 0;
-  blockResponse *reply =  NULL;
+  blockResponse reply = {0,};
   struct sockaddr_in *sain = NULL;
 
 
@@ -386,8 +386,7 @@ glusterBlockCallRPC_1(char *host, void *cobj,
     strcpy(((blockCreate *)cobj)->ipaddr, host);
     *rpc_sent = TRUE;
 
-    reply = block_create_1((blockCreate *)cobj, clnt);
-    if (!reply) {
+    if (block_create_1((blockCreate *)cobj, &reply, clnt) != RPC_SUCCESS) {
       LOG("mgmt", GB_LOG_ERROR, "%son host %s",
           clnt_sperror(clnt, "block remote create failed"), host);
       goto out;
@@ -395,8 +394,7 @@ glusterBlockCallRPC_1(char *host, void *cobj,
     break;
   case DELETE_SRV:
     *rpc_sent = TRUE;
-    reply = block_delete_1((blockDelete *)cobj, clnt);
-    if (!reply) {
+    if (block_delete_1((blockDelete *)cobj, &reply, clnt) != RPC_SUCCESS) {
       LOG("mgmt", GB_LOG_ERROR, "%son host %s",
           clnt_sperror(clnt, "block remote delete failed"), host);
       goto out;
@@ -404,8 +402,7 @@ glusterBlockCallRPC_1(char *host, void *cobj,
     break;
   case MODIFY_SRV:
     *rpc_sent = TRUE;
-    reply = block_modify_1((blockModify *)cobj, clnt);
-    if (!reply) {
+    if (block_modify_1((blockModify *)cobj, &reply, clnt) != RPC_SUCCESS) {
       LOG("mgmt", GB_LOG_ERROR, "%son host %s",
           clnt_sperror(clnt, "block remote modify failed"), host);
       goto out;
@@ -417,16 +414,15 @@ glusterBlockCallRPC_1(char *host, void *cobj,
       goto out;
   }
 
-  if (reply) {
-    if (GB_STRDUP(*out, reply->out) < 0) {
-      goto out;
-    }
-    ret = reply->exit;
+  if (GB_STRDUP(*out, reply.out) < 0) {
+    goto out;
   }
+  ret = reply.exit;
 
  out:
-  if (clnt && reply) {
-    if (!clnt_freeres(clnt, (xdrproc_t)xdr_blockResponse, (char *)reply)) {
+  if (clnt) {
+    if (reply.out && !clnt_freeres(clnt, (xdrproc_t)xdr_blockResponse,
+                                   (char *)&reply)) {
       LOG("mgmt", GB_LOG_ERROR, "%s",
           clnt_sperror(clnt, "clnt_freeres failed"));
 
@@ -1467,7 +1463,7 @@ blockModifyCliFormatResponse (blockModifyCli *blk, struct blockModify *mobj,
 }
 
 blockResponse *
-block_modify_cli_1_svc(blockModifyCli *blk, struct svc_req *rqstp)
+block_modify_cli_1_svc_st(blockModifyCli *blk, struct svc_req *rqstp)
 {
   int ret = -1;
   static blockModify mobj = {0};
@@ -1781,7 +1777,7 @@ blockCreateCliFormatResponse(struct glfs *glfs, blockCreateCli *blk,
 }
 
 blockResponse *
-block_create_cli_1_svc(blockCreateCli *blk, struct svc_req *rqstp)
+block_create_cli_1_svc_st(blockCreateCli *blk, struct svc_req *rqstp)
 {
   int errCode = -1;
   uuid_t uuid;
@@ -2048,7 +2044,7 @@ out:
 
 
 blockResponse *
-block_create_1_svc(blockCreate *blk, struct svc_req *rqstp)
+block_create_1_svc_st(blockCreate *blk, struct svc_req *rqstp)
 {
   char *tmp = NULL;
   char *backstore = NULL;
@@ -2276,7 +2272,7 @@ blockDeleteCliFormatResponse(blockDeleteCli *blk, int errCode, char *errMsg,
 }
 
 blockResponse *
-block_delete_cli_1_svc(blockDeleteCli *blk, struct svc_req *rqstp)
+block_delete_cli_1_svc_st(blockDeleteCli *blk, struct svc_req *rqstp)
 {
   blockRemoteDeleteResp *savereply = NULL;
   MetaInfo *info = NULL;
@@ -2377,7 +2373,7 @@ block_delete_cli_1_svc(blockDeleteCli *blk, struct svc_req *rqstp)
 
 
 blockResponse *
-block_delete_1_svc(blockDelete *blk, struct svc_req *rqstp)
+block_delete_1_svc_st(blockDelete *blk, struct svc_req *rqstp)
 {
   int ret;
   char *iqn = NULL;
@@ -2445,7 +2441,7 @@ block_delete_1_svc(blockDelete *blk, struct svc_req *rqstp)
 
 
 blockResponse *
-block_modify_1_svc(blockModify *blk, struct svc_req *rqstp)
+block_modify_1_svc_st(blockModify *blk, struct svc_req *rqstp)
 {
   int ret;
   char *authattr = NULL;
@@ -2570,7 +2566,7 @@ block_modify_1_svc(blockModify *blk, struct svc_req *rqstp)
 
 
 blockResponse *
-block_list_cli_1_svc(blockListCli *blk, struct svc_req *rqstp)
+block_list_cli_1_svc_st(blockListCli *blk, struct svc_req *rqstp)
 {
   blockResponse *reply;
   struct glfs *glfs;
@@ -2815,7 +2811,7 @@ blockInfoCliFormatResponse(blockInfoCli *blk, int errCode,
 }
 
 blockResponse *
-block_info_cli_1_svc(blockInfoCli *blk, struct svc_req *rqstp)
+block_info_cli_1_svc_st(blockInfoCli *blk, struct svc_req *rqstp)
 {
   blockResponse *reply;
   struct glfs *glfs;
@@ -2884,3 +2880,106 @@ block_info_cli_1_svc(blockInfoCli *blk, struct svc_req *rqstp)
 
   return reply;
 }
+
+
+bool_t
+block_create_1_svc(blockCreate *blk, blockResponse *reply, struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(create, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+bool_t
+block_delete_1_svc(blockDelete *blk, blockResponse *reply, struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(delete, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+bool_t
+block_modify_1_svc(blockModify *blk, blockResponse *reply, struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(modify, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+bool_t
+block_create_cli_1_svc(blockCreateCli *blk, blockResponse *reply,
+                       struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(create_cli, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+bool_t
+block_modify_cli_1_svc(blockModifyCli *blk, blockResponse *reply,
+                       struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(modify_cli, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+bool_t
+block_list_cli_1_svc(blockListCli *blk, blockResponse *reply,
+                     struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(list_cli, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+bool_t
+block_info_cli_1_svc(blockInfoCli *blk, blockResponse *reply,
+                     struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(info_cli, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+bool_t
+block_delete_cli_1_svc(blockDeleteCli *blk, blockResponse *reply,
+                       struct svc_req *rqstp)
+{
+  int ret;
+
+  GB_RPC_CALL(delete_cli, blk, reply, rqstp, ret);
+  return ret;
+}
+
+
+int
+gluster_block_1_freeresult (SVCXPRT *transp, xdrproc_t xdr_result, caddr_t result)
+{
+  xdr_free (xdr_result, result);
+
+  return 1;
+}
+
+
+int
+gluster_block_cli_1_freeresult (SVCXPRT *transp, xdrproc_t xdr_result, caddr_t result)
+{
+  xdr_free (xdr_result, result);
+
+  return 1;
+}
diff --git a/rpc/rpcl/Makefile.am b/rpc/rpcl/Makefile.am
index 38b1dbf..08a40ea 100644
--- a/rpc/rpcl/Makefile.am
+++ b/rpc/rpcl/Makefile.am
@@ -10,17 +10,17 @@ DISTCLEANFILES = Makefile.in $(BUILT_SOURCES)
 CLEANFILES = *~ $(BUILT_SOURCES)
 
 block.h: block.x
-	rpcgen -h -o $(top_builddir)/rpc/rpcl/$@ $^
+	rpcgen -hM -o $(top_builddir)/rpc/rpcl/$@ $^
 
 block_xdr.c: block.x
-	rpcgen -c -o $(top_builddir)/rpc/rpcl/$@ $^
+	rpcgen -cM -o $(top_builddir)/rpc/rpcl/$@ $^
 
 block_clnt.c: block.x
-	rpcgen -l -o $(top_builddir)/rpc/rpcl/$@ $^
+	rpcgen -lM -o $(top_builddir)/rpc/rpcl/$@ $^
 	$(SED) -i 's|TIMEOUT = { 25, 0 }|TIMEOUT = { 300, 0 }|' $(top_builddir)/rpc/rpcl/$@
 
 block_svc.c: block.x
-	rpcgen -m -o $(top_builddir)/rpc/rpcl/$@ $^
+	rpcgen -mM -o $(top_builddir)/rpc/rpcl/$@ $^
 
 dist-hook:
 	find $(distdir) -type f \
diff --git a/utils/utils.h b/utils/utils.h
index d3b1027..ce1ef52 100644
--- a/utils/utils.h
+++ b/utils/utils.h
@@ -278,6 +278,18 @@ extern struct gbConf gbConf;
            GB_FREE(tmp);                                               \
          } while (0)
 
+# define GB_RPC_CALL(op, blk, reply, rqstp, ret)                    \
+        do {                                                        \
+          blockResponse *resp = block_##op##_1_svc_st(blk, rqstp);  \
+          if (resp) {                                               \
+            memcpy(reply, resp, sizeof(*reply));                    \
+            GB_FREE(resp);                                          \
+            ret = true;                                             \
+          } else {                                                  \
+            ret = false;                                            \
+          }                                                         \
+        } while (0)
+
 
 # define  CALLOC(x)                                                  \
             calloc(1, x)