libglusterfs: accept random volname in glusterfs_graph_prepare()

When the call to glfs_new("volname") passes a name for the volume and it
does not match the name of the subvolume in the graph, glfs_init() will
fail. This is easily reproducible by a gfapi program that loads the
volume from a .vol file, and not from a GlusterD server.

Change-Id: I33e77fbee7d12eaefe7c384fad6aecfa3582ea5a
BUG: 1425623
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: https://review.gluster.org/16796
Smoke: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
Reviewed-by: Prashanth Pai <ppai@redhat.com>
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>

5 files changed, 160 insertions, 28 deletions

diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c
index 916942c3a2d..1edcf20eda6 100644
--- a/libglusterfs/src/graph.c
+++ b/libglusterfs/src/graph.c
@@ -407,25 +407,27 @@ fill_uuid (char *uuid, int size)
 }
 
 
-int
-glusterfs_graph_settop (glusterfs_graph_t *graph, glusterfs_ctx_t *ctx,
-                        char *volume_name)
+static int
+glusterfs_graph_settop (glusterfs_graph_t *graph, char *volume_name,
+                        gf_boolean_t exact_match)
 {
+        int         ret  = -1;
         xlator_t   *trav = NULL;
 
-        if (!volume_name) {
+        if (!volume_name || !exact_match) {
                 graph->top = graph->first;
-                return 0;
-        }
-
-        for (trav = graph->first; trav; trav = trav->next) {
-                if (strcmp (trav->name, volume_name) == 0) {
-                        graph->top = trav;
-                        return 0;
+                ret = 0;
+        } else {
+                for (trav = graph->first; trav; trav = trav->next) {
+                        if (strcmp (trav->name, volume_name) == 0) {
+                                graph->top = trav;
+                                ret = 0;
+                                break;
+                        }
                 }
         }
 
-        return -1;
+        return ret;
 }
 
 
@@ -462,19 +464,32 @@ glusterfs_graph_prepare (glusterfs_graph_t *graph, glusterfs_ctx_t *ctx,
         /* XXX: CHECKSUM */
 
         /* XXX: attach to -n volname */
-        ret = glusterfs_graph_settop (graph, ctx, volume_name);
-        if (ret) {
-                char *slash = rindex (volume_name, '/');
-                if (slash) {
-                        ret = glusterfs_graph_settop (graph, ctx, slash + 1);
-                        if (!ret) {
-                                goto ok;
-                        }
-                }
-                gf_msg ("graph", GF_LOG_ERROR, 0, LG_MSG_GRAPH_ERROR,
-                        "glusterfs graph settop failed");
-                return -1;
+        /* A '/' in the volume name suggests brick multiplexing is used, find
+         * the top of the (sub)graph. The volname MUST match the subvol in this
+         * case. In other cases (like for gfapi) the default top for the
+         * (sub)graph is ok. */
+        if (!volume_name) {
+                /* GlusterD does not pass a volume_name */
+                ret = glusterfs_graph_settop (graph, volume_name, _gf_false);
+        } else if (strncmp (volume_name, "/snaps/", 7) == 0) {
+                /* snap shots have their top xlator named like "/snaps/..."  */
+                ret = glusterfs_graph_settop (graph, volume_name,
+                                              _gf_false);
+        } else if (volume_name[0] == '/') {
+                /* brick multiplexing passes the brick path */
+                ret = glusterfs_graph_settop (graph, volume_name,
+                                              _gf_true);
+        } else {
+                ret = glusterfs_graph_settop (graph, volume_name,
+                                              _gf_false);
         }
+        if (!ret) {
+                goto ok;
+        }
+
+        gf_msg ("graph", GF_LOG_ERROR, 0, LG_MSG_GRAPH_ERROR,
+                "glusterfs graph settop failed");
+        return -1;
 ok:
 
         /* XXX: WORM VOLUME */
@@ -1085,9 +1100,19 @@ glusterfs_graph_attach (glusterfs_graph_t *orig_graph, char *path)
         }
 
         /* TBD: memory leaks everywhere */
-        glusterfs_graph_prepare (graph, this->ctx, xl->name);
-        glusterfs_graph_init (graph);
-        glusterfs_xlator_link (orig_graph->top, graph->top);
+        if (glusterfs_graph_prepare (graph, this->ctx, xl->name)) {
+                gf_log (this->name, GF_LOG_WARNING,
+                        "failed to prepare graph for xlator %s", xl->name);
+                return -EIO;
+        } else if (glusterfs_graph_init (graph)) {
+                gf_log (this->name, GF_LOG_WARNING,
+                        "failed to initialize graph for xlator %s", xl->name);
+                return -EIO;
+        } else if (glusterfs_xlator_link (orig_graph->top, graph->top)) {
+                gf_log (this->name, GF_LOG_WARNING,
+                        "failed to link the graphs for xlator %s ", xl->name);
+                return -EIO;
+        }
 
         return 0;
 }
diff --git a/tests/basic/gfapi/Makefile.am b/tests/basic/gfapi/Makefile.am
index 3cad969672e..e30fefea5b9 100644
--- a/tests/basic/gfapi/Makefile.am
+++ b/tests/basic/gfapi/Makefile.am
@@ -5,7 +5,7 @@ CFLAGS   = -Wall -g $(shell pkg-config --cflags glusterfs-api)
 LDFLAGS  = $(shell pkg-config --libs glusterfs-api)
 
 BINARIES = upcall-cache-invalidate libgfapi-fini-hang anonymous_fd seek \
-	bug1283983 bug1291259 gfapi-ssl-test
+	bug1283983 bug1291259 gfapi-ssl-test gfapi-load-volfile
 
 %: %.c
 	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^
diff --git a/tests/basic/gfapi/gfapi-load-volfile.c b/tests/basic/gfapi/gfapi-load-volfile.c
new file mode 100644
index 00000000000..91d5677bd44
--- /dev/null
+++ b/tests/basic/gfapi/gfapi-load-volfile.c
@@ -0,0 +1,65 @@
+/*
+ * Create a glfs instance based on a .vol file
+ *
+ * This is used to measure memory leaks by initializing a graph through a .vol
+ * file and destroying it again.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <api/glfs.h>
+
+#define PROGNAME "gfapi-load-volfile"
+
+void
+usage(FILE *output)
+{
+	fprintf(output, "Usage: " PROGNAME " <volfile>\n");
+}
+
+void
+main(int argc, char **argv)
+{
+	int ret = 0;
+	glfs_t *fs = NULL;
+
+	if (argc != 2) {
+		usage(stderr);
+		exit(EXIT_FAILURE);
+	}
+
+	if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "-h")) {
+		usage(stdout);
+		exit(EXIT_SUCCESS);
+	}
+
+	fs = glfs_new(PROGNAME);
+	if (!fs) {
+		perror("glfs_new failed");
+		exit(EXIT_FAILURE);
+	}
+
+	glfs_set_logging(fs, PROGNAME ".log", 9);
+
+	ret = glfs_set_volfile(fs, argv[1]);
+	if (ret) {
+		perror("glfs_set_volfile failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+
+	ret = glfs_init(fs);
+	if (ret) {
+		perror("glfs_init failed");
+		ret = EXIT_FAILURE;
+		goto out;
+	}
+
+	ret = EXIT_SUCCESS;
+out:
+	glfs_fini(fs);
+
+	exit(ret);
+}
diff --git a/tests/basic/gfapi/gfapi-load-volfile.t b/tests/basic/gfapi/gfapi-load-volfile.t
new file mode 100644
index 00000000000..d914cacd819
--- /dev/null
+++ b/tests/basic/gfapi/gfapi-load-volfile.t
@@ -0,0 +1,28 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup
+
+TEST glusterd
+
+TEST $CLI volume create ${V0} ${H0}:${B0}/brick0
+EXPECT 'Created' volinfo_field ${V0} 'Status'
+
+TEST $CLI volume start ${V0}
+EXPECT 'Started' volinfo_field ${V0} 'Status'
+
+TEST build_tester $(dirname ${0})/gfapi-load-volfile.c -lgfapi
+
+sed -e "s,@@HOSTNAME@@,${H0},g" -e "s,@@BRICKPATH@@,${B0}/brick0,g" \
+            $(dirname ${0})/protocol-client.vol.in \
+            > $(dirname ${0})/protocol-client.vol
+
+TEST ./$(dirname ${0})/gfapi-load-volfile \
+             $(dirname ${0})/protocol-client.vol
+
+cleanup_tester $(dirname ${0})/gfapi-load-volfile
+cleanup_tester $(dirname ${0})/protocol-client.vol
+
+cleanup
diff --git a/tests/basic/gfapi/protocol-client.vol.in b/tests/basic/gfapi/protocol-client.vol.in
new file mode 100644
index 00000000000..ef35001e29f
--- /dev/null
+++ b/tests/basic/gfapi/protocol-client.vol.in
@@ -0,0 +1,14 @@
+#
+# This .vol file expects that there is
+#
+#  1. GlusterD listening on @@HOSTNAME@@
+#  2. a volume that provides a brick on @@BRICKPATH@@
+#  3. the volume with the brick has been started
+#
+volume test
+    type protocol/client
+    option remote-host @@HOSTNAME@@
+    option remote-subvolume @@BRICKPATH@@
+    option transport-type socket
+end-volume
+