capabilities: read capabilities is one-time job

I do not see any value in reading the capabilities every time a command
kicks in, in fact this is a major bug.

Say we are running a daemon of version X, in case if the gluster-block package
is upgraded to version Y, then X capabilities file will be overridden by
Y version capability file, even before restarting the daemon the gluster-blockd
process of version X can return wrong capabilities of version Y.

Also, we are adding delay in the gluster-blockd overall response by
unnecessarily reading the capabilities on every command from the file.

Tested-by: Xiubo Li <[email protected]>
Reviewed-by: Xiubo Li <[email protected]>
Reviewed-by: Pranith Kumar K <[email protected]>
Signed-off-by: Prasanna Kumar Kalever <[email protected]>
Signed-off-by: Xiubo Li <[email protected]>

daemon/gluster-blockd.c

@@ -21,6 +21,7 @@
 # include  "lru.h"
 # include  "block.h"
 # include  "block_svc.h"
+# include  "capabilities.h"
 
 # define   GB_TGCLI_GLOBALS     "targetcli set "                               \
                                 "global auto_add_default_portal=false "        \
@@ -303,6 +304,21 @@ blockNodeSanityCheck(void)
   return 0;
 }
 
+
+static int
+initDaemonCapabilities(void)
+{
+
+  gbSetCapabilties();
+  if (!globalCapabilities) {
+    LOG("mgmt", GB_LOG_ERROR, "%s", "capabilities fetching failed");
+    return -1;
+  }
+
+  return 0;
+}
+
+
 int
 main (int argc, char **argv)
 {
@@ -344,6 +360,11 @@ main (int argc, char **argv)
     exit(errnosv);
   }
 
+  if (initDaemonCapabilities()) {
+    exit(EXIT_FAILURE);
+  }
+  LOG("mgmt", GB_LOG_INFO, "%s", "capabilities fetched successfully");
+
   initCache();
 
   /* set signal */

rpc/block_svc_routines.c

@@ -4793,10 +4793,13 @@ block_delete_1_svc_st(blockDelete *blk, struct svc_req *rqstp)
   return reply;
 }
 
+
 blockResponse *
 block_version_1_svc_st(void *data, struct svc_req *rqstp)
 {
   blockResponse *reply = NULL;
+  gbCapObj *caps;
+  int i;
 
 
   LOG("mgmt", GB_LOG_DEBUG, "%s", "version check request");
@@ -4805,15 +4808,26 @@ block_version_1_svc_st(void *data, struct svc_req *rqstp)
     return NULL;
   }
 
-  reply->exit = gbSetCapabilties(&reply);
+  if (GB_ALLOC_N(caps, GB_CAP_MAX) < 0) {
+    GB_FREE(reply);
+    return NULL;
+  }
+
+  for (i = 0; i < GB_CAP_MAX; i++) {
+    GB_STRCPYSTATIC(caps[i].cap, globalCapabilities[i].cap);
+    caps[i].status = globalCapabilities[i].status;
+  }
 
-  GB_ASPRINTF(&reply->out, "remote version check returns %d", reply->exit);
+  reply->xdata.xdata_len = GB_CAP_MAX * sizeof(gbCapObj);
+  reply->xdata.xdata_val = (char *) caps;
+  reply->exit = 0;
 
-  LOG("mgmt", GB_LOG_DEBUG, "command exit code, %d", reply->exit);
+  GB_ASPRINTF(&reply->out, "version check routine");
 
   return reply;
 }
 
+
 blockResponse *
 block_modify_1_svc_st(blockModify *blk, struct svc_req *rqstp)
 {

utils/capabilities.c

@@ -12,6 +12,9 @@
 # include "capabilities.h"
 
 
+gbCapObj *globalCapabilities;
+
+
 int
 gbCapabilitiesEnumParse(const char *cap)
 {
@@ -32,27 +35,29 @@ gbCapabilitiesEnumParse(const char *cap)
 }
 
 
-int
-gbSetCapabilties(blockResponse **c)
+void
+gbSetCapabilties(void)
 {
-  FILE * fp;
+  FILE *fp = NULL;
   char *line = NULL;
   size_t len = 0;
   int count = 0;
-  int ret = 0;
-  blockResponse *reply = *c;
+  int ret;
   gbCapObj *caps = NULL;
   char *p, *sep;
 
 
   fp = fopen(GB_CAPS_FILE, "r");
   if (fp == NULL) {
-    return -1;
+    LOG("mgmt", GB_LOG_ERROR,
+        "gbSetCapabilties: fopen() failed (%s)", strerror(errno));
+    goto out;
   }
 
   if (GB_ALLOC_N(caps, GB_CAP_MAX) < 0) {
-    fclose(fp);
-    return -1;
+    LOG("mgmt", GB_LOG_ERROR,
+        "gbSetCapabilties: calloc() failed (%s)", strerror(errno));
+    goto out;
   }
 
   while ((getline(&line, &len, fp)) != -1) {
@@ -83,7 +88,9 @@ gbSetCapabilties(blockResponse **c)
       if (ret >= 0) {
         caps[count].status = ret;
       } else {
-        ret = -1;
+        LOG("mgmt", GB_LOG_ERROR,
+            "gbSetCapabilties: convertStringToTrillianParse(%s) failed (%s)", p,
+            strerror(errno));
         goto out;
       }
       count++;
@@ -93,17 +100,20 @@ gbSetCapabilties(blockResponse **c)
   }
 
   if (GB_CAP_MAX != count) {
-    ret = -1;
+    LOG("mgmt", GB_LOG_ERROR, "gbSetCapabilties: GB_CAP_MAX != %d", count);
     goto out;
   }
 
-  reply->xdata.xdata_len = GB_CAP_MAX * sizeof(gbCapObj);
-  reply->xdata.xdata_val = (char *) caps;
+  globalCapabilities = caps;
 
-  ret = 0;
  out:
+  if (!globalCapabilities) {
+    GB_FREE(caps);
+  }
   GB_FREE(line);
-  fclose(fp);
+  if (fp) {
+    fclose(fp);
+  }
 
-  return ret;
+  return;
 }

utils/capabilities.h

@@ -73,5 +73,8 @@ static const char *const gbCapabilitiesLookup[] = {
 };
 
 
+extern gbCapObj *globalCapabilities;
+
+
 int gbCapabilitiesEnumParse(const char *cap);
-int gbSetCapabilties (blockResponse **c);
+void gbSetCapabilties(void);