diff options
author | Niels de Vos <ndevos@redhat.com> | 2017-02-27 18:45:16 -0800 |
---|---|---|
committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2017-04-26 01:18:56 +0000 |
commit | 1538c98f5e33e0794830d5153f17a96ff28c9914 (patch) | |
tree | e72011f97e32840903e6d88ea8b3025a375870d2 | |
parent | 0451909e0533d357a45dd427226028e095240dac (diff) |
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>
-rw-r--r-- | libglusterfs/src/graph.c | 79 | ||||
-rw-r--r-- | tests/basic/gfapi/Makefile.am | 2 | ||||
-rw-r--r-- | tests/basic/gfapi/gfapi-load-volfile.c | 65 | ||||
-rw-r--r-- | tests/basic/gfapi/gfapi-load-volfile.t | 28 | ||||
-rw-r--r-- | tests/basic/gfapi/protocol-client.vol.in | 14 |
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 + |