protocol/server: don't assume there would be a volfile id

Earlier glusterfs never had an assumption someone would start it with
right arguments, and brick processes would be spawned by a management
layer. It just assume the role based on the volfile. Other than
volfile, no other arguments should be technically mandatory for
working of glusterfs. With this patch, that assumption holds true.

Updates: github issue # 352

A note on why this particular issue for this basic sanity?

As per the design of thin-arbiter/tie-breaker, it can be started
independently on any machine, without need of glusterd. So, similar
to 'glusterd', we should be able to spawn a process with any translator
without options/volume id etc.

fixes: bz#1569399
Change-Id: I5c0650fe0bfde35ad94ccba60e63f6cdcd1ae5ff
Signed-off-by: Amar Tumballi <amarts@redhat.com>

3 files changed, 47 insertions, 2 deletions

diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c
index e30d9287575..5666a532696 100644
--- a/glusterfsd/src/glusterfsd-mgmt.c
+++ b/glusterfsd/src/glusterfsd-mgmt.c
@@ -861,6 +861,13 @@ glusterfs_handle_attach (rpcsvc_request_t *req)
         this = THIS;
         GF_ASSERT (this);
 
+        ctx = this->ctx;
+        if (!ctx->cmd_args.volfile_id) {
+                gf_log (THIS->name, GF_LOG_ERROR,
+                        "No volfile-id provided, erroring out");
+                return -1;
+        }
+
         ret = xdr_to_generic (req->msg[0], &xlator_req,
                              (xdrproc_t)xdr_gd1_mgmt_brick_op_req);
 
@@ -870,7 +877,6 @@ glusterfs_handle_attach (rpcsvc_request_t *req)
                 return -1;
         }
         ret = 0;
-        ctx = this->ctx;
 
         LOCK (&ctx->volfile_lock);
         {
@@ -2089,6 +2095,11 @@ glusterfs_volfile_fetch_one (glusterfs_ctx_t *ctx, char *volfile_id)
         cmd_args = &ctx->cmd_args;
         if (!volfile_id) {
                 volfile_id = ctx->cmd_args.volfile_id;
+                if (!volfile_id) {
+                        gf_log (THIS->name, GF_LOG_ERROR,
+                                "No volfile-id provided, erroring out");
+                        return -1;
+                }
         }
 
         frame = create_frame (THIS, ctx->pool);
diff --git a/tests/basic/volfile-sanity.t b/tests/basic/volfile-sanity.t
new file mode 100644
index 00000000000..04d1d2869c2
--- /dev/null
+++ b/tests/basic/volfile-sanity.t
@@ -0,0 +1,26 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+
+cleanup;
+
+## Start and create a volume
+TEST glusterd
+TEST pidof glusterd
+
+TEST $CLI volume create $V0  $H0:$B0/${V0}1
+
+killall glusterd
+
+## Mount FUSE with caching disabled (read-write)
+TEST $GFS -f /var/lib/glusterd/vols/${V0}/${V0}.${H0}.*.vol
+TEST $GFS -f /var/lib/glusterd/vols/${V0}/${V0}.tcp-fuse.vol $M0
+
+TEST $(df -h $M0 | grep -q ${V0})
+TEST $(cat /proc/mounts | grep -q $M0)
+
+TEST ! stat $M0/newfile;
+TEST touch $M0/newfile;
+TEST rm $M0/newfile;
+
+cleanup;
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index 254a5ca6c62..eef956cc21a 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -1210,7 +1210,15 @@ server_init (xlator_t *this)
                 }
         }
 #endif
-
+        if (!this->ctx->cmd_args.volfile_id) {
+                /* In some use cases this is a valid case, but
+                   document this to be annoying log in that case */
+                gf_msg (this->name, GF_LOG_WARNING, EINVAL,
+                        PS_MSG_VOL_FILE_OPEN_FAILED,
+                        "volfile-id argument not given. "
+                        "This is mandatory argument, defaulting to 'gluster'");
+                this->ctx->cmd_args.volfile_id = gf_strdup ("gluster");
+        }
         FIRST_CHILD(this)->volfile_id
                 = gf_strdup (this->ctx->cmd_args.volfile_id);