[Patch 2/3] chunkd: Implement flexible CLD paths

[Patch 2/3] chunkd: Implement flexible CLD paths

[Pete Zaitcev]

The current code encoded policy with the <Group> clause, which may
be undesirable. The group layout with 1 directory depth was largely
an artefact of CLD API being a pain to use. This patch removes
the rigid group structure and allows for any valid CLD paths.

Signed-off-by: Pete Zaitcev <zaitcev@redhat.com>

---
 doc/setup.txt        |   63 +++++++++++++-------
 server/chunkd.h      |    4 -
 server/cldu.c        |  124 +++++++++++++++++++----------------------
 server/config.c      |   40 ++++++++-----
 server/server.c      |    2 
 test/server-test.cfg |    2 
 6 files changed, 131 insertions(+), 104 deletions(-)

diff --git a/doc/setup.txt b/doc/setup.txt
index 5ef7225..b586909 100644
--- a/doc/setup.txt
+++ b/doc/setup.txt
@@ -14,11 +14,8 @@ _cld._udp.phx2.ex.com has SRV record 10 50 8081 maika.phx2.ex.com.
    Also, make sure that your hostname has a domain. We don't want to search
    for CLD in the DNS root, do we?
 
-*) create a volume (directory that stores data):
-
-	mkdir -p /disk1/chunkd
-
-*) create XML-like configuration file (filled in, in the following steps)
+*) create XML-like configuration file (filled in, in the following steps).
+   Notice that clause names are case-sensitive, unlike XML.
 
 *) choose TCP listen port for server. For an unknown reason, this clause
    has no default, so you need to spell it out:
@@ -29,24 +26,36 @@ _cld._udp.phx2.ex.com has SRV record 10 50 8081 maika.phx2.ex.com.
 
    For clouds and their many cheap nodes with one Ethernet it usually is
    not a great idea to specify interfaces, since they often use IPv6 or
-   acquire IP addresses from DHCP. So, just specify a port.
+   acquire IP addresses from DHCP. So, just specify a port and the Chunk
+   will listen on a wildcard socket.
+
+   However, just in case anyone needs it, the following syntax works as
+   expected to limit where Chunk listens:
+
+	<Listen>
+		<Node>192.168.128.1</Node>
+		<Port>8082</Port>
+	</Listen>
 
-   An option exists to let the server to listen on a random port with
+   An option also exists to let the server to listen on a random port with
    the "auto" keyword:
 
 	<Listen>auto</Listen>
 
    This works with CLD-aware clients that find the connection information
-   from records like /chunk-picbak/*. It's not commonly used in real clouds
+   from Chunk's record in CLD. It's not commonly used in real clouds
    but may be beneficial in chroots and other odd environments.
 
 *) choose pathname (dir + filename) where daemon should store its
    PID file. Default is /var/run/chunkd.pid, but it limits you to
-   one Chunk daemon per node, which is usual in clouds.
+   one Chunk daemon per node, since each Chunk instance wants a separate
+   PID file.
 
 	<PID>/home/developer/run/chunkd.pid</PID>
 
-*) create the volume entry (same as in mkdir above):
+*) choose a pathname and create the directory where Chunk will keep
+   all of its data. For example, run "mkdir -p /disk1/chunkd" and add
+   an configuration entry like this:
 
 	<Path>/disk1/chunkd</Path>
 
@@ -57,20 +66,8 @@ _cld._udp.phx2.ex.com has SRV record 10 50 8081 maika.phx2.ex.com.
 		<Cert>/etc/pki/cert-public-key.pem</Cert>
 	</SSL>
 
-*) configure the group. The group is used by upper level applications to
-   find chunkservers that are assigned to them. So, typically, a group
-   is named after the application.
-
-        <Group>ultracart2</Group>
-
-   It's best to use only ASCII letters, numbers, dash ('-'), underscore ('_'),
-   and period ('.') in group names.
-
-   Keep in mind that if you do not assign a group, the chunkserver joins
-   the default group called "default".
-
 *) For a typical Chunk configuration (not running it standalone),
-   configure Node ID (NID). The NID follows the data, so
+   configure Node ID (NID). It's an integer. The NID follows the data, so
    the best practice for NIDs is to have site-wide node bring-up scripts
    create a unique identifier from a per-group counter, instead of using
    the MAC or IP address. One common trick is to use the creation time,
@@ -83,6 +80,26 @@ _cld._udp.phx2.ex.com has SRV record 10 50 8081 maika.phx2.ex.com.
 
 	<NID>1247713739</NID>
 
+*) configure a path in CLD where the configuration record is kept
+   (unless you want to use a legacy "group"). The path is like an absolute
+   filename, from the root of local CLD cell. A typical policy would be
+   to use the same directory name for all related Chunk instances and
+   name it according to their function, as in the following example:
+
+	<InfoPath>/chunk/ultracart2/chunk-5-12</InfoPath>
+
+   For legacy applications that expect a "group", the following pattern
+   should be followed:
+
+	<InfoPath>/chunk-GROUP/NID</InfoPath>
+
+   For example, for the default group (that is called "default"):
+
+	<InfoPath>/chunk-default/1247713739</InfoPath>
+
+   It's best to use only ASCII letters, numbers, dash ('-'), underscore ('_'),
+   and period ('.') in names of CLD path components.
+
 *) configure the location information. The Chunk works fine without, so
    it's only used by clients that want to spread redundant data in a
    certain pattern. For example, tabled avoids putting all replicas of
diff --git a/server/chunkd.h b/server/chunkd.h
index dd86cee..c6c2a27 100644
--- a/server/chunkd.h
+++ b/server/chunkd.h
@@ -220,7 +220,7 @@ struct server {
 
 	char			*ourhost;
 	char			*vol_path;
-	char			*group;
+	char			*info_path;
 	uint32_t		nid;
 	struct geo		loc;
 
@@ -291,7 +291,7 @@ extern void cli_in_end(struct client *cli);
 /* cldu.c */
 extern void cld_init(void);
 extern void cldu_add_host(const char *host, unsigned int port);
-extern int cld_begin(const char *thishost, const char *thisgroup, uint32_t nid,
+extern int cld_begin(const char *thishost, uint32_t nid, char *infopath,
 		     struct geo *locp, void (*cb)(enum st_cld));
 extern void cld_end(void);
 
diff --git a/server/cldu.c b/server/cldu.c
index 4409fb4..51d01a3 100644
--- a/server/cldu.c
+++ b/server/cldu.c
@@ -48,9 +48,8 @@ struct cld_session {
 
 	struct timer timer;
 
-	char *cfname;		/* /chunk-GROUP directory */
-	char *ffname;		/* /chunk-GROUP/NID */
-	struct ncld_fh *ffh;	/* /chunk-GROUP/NID, keep open for lock */
+	char *ffname;
+	struct ncld_fh *ffh;	/* keep open for lock */
 	uint32_t nid;
 	const char *ourhost;	/* N.B. points to some global data. */
 	struct geo *ploc;	/* N.B. points to some global data. */
@@ -60,9 +59,6 @@ struct cld_session {
 
 static int cldu_set_cldc(struct cld_session *cs, int newactive);
 
-#define SVC "chunk"
-static char svc[] = SVC;
-
 struct hail_log cldu_hail_log = {
 	.func		= applog,
 };
@@ -89,44 +85,14 @@ static int cldu_nextactive(struct cld_session *cs)
 	return cs->actx;
 }
 
-static int cldu_setgroup(struct cld_session *sp, const char *thisgroup,
-			 uint32_t thisnid, const char *thishost,
-			 struct geo *loc)
+static void cldu_saveargs(struct cld_session *sp, char *infopath,
+			  uint32_t thisnid, const char *thishost,
+			  struct geo *loc)
 {
-	size_t cnlen;
-	size_t mlen;
-	char nbuf[11];	/* 32 bits in decimal and nul */
-	char *mem;
-
-	if (thisgroup == NULL) {
-		thisgroup = "default";
-	}
-
-	snprintf(nbuf, 11, "%u", thisnid);
-
-	cnlen = strlen(thisgroup);
-
-	mlen = sizeof("/" SVC "-")-1;
-	mlen += cnlen;
-	mlen++;	// '\0'
-	mem = malloc(mlen);
-	sprintf(mem, "/%s-%s", svc, thisgroup);
-	sp->cfname = mem;
-
-	mlen = sizeof("/" SVC "-")-1;
-	mlen += cnlen;
-	mlen++;	// '/'
-	mlen += strlen(nbuf);
-	mlen++;	// '\0'
-	mem = malloc(mlen);
-	sprintf(mem, "/%s-%s/%s", svc, thisgroup, nbuf);
-	sp->ffname = mem;
-
+	sp->ffname = infopath;
 	sp->nid = thisnid;
 	sp->ourhost = thishost;
 	sp->ploc = loc;
-
-	return 0;
 }
 
 static void cldu_timer_event(struct timer *timer)
@@ -182,6 +148,54 @@ static void cldu_sess_event(void *priv, uint32_t what)
 }
 
 /*
+ * Create the directories: mkdir -p $(dirname $path).
+ */
+static int cldu_make_path(struct ncld_sess *nsess, const char *path)
+{
+	const char *compdir;	/* the current component directory */
+	const char *compend;	/* the component's end (position of slash) */
+	char *dir;
+	unsigned int mode;
+	struct ncld_fh *dh;
+	int error;
+
+	/* Our configurator has this check too, but let's be safe. */
+	if (path[0] != '/')
+		return -1;
+	compdir = path + sizeof('/');
+
+	for (;;) {
+		if (!compdir[0]) {
+			applog(LOG_ERR, "CLD path (%s) is invalid", path);
+			return -1;	/* Path ends with slash - error */
+		}
+		compend = strchr(compdir, '/');
+		if (!compend)		/* It's a file, all done */
+			return 0;
+
+		dir = strndup(path, compend - path);	/* always absolute */
+		if (!dir) {
+			applog(LOG_ERR, "No core (%d)", compend - path);
+			return -1;
+		}
+
+		mode = COM_READ | COM_WRITE | COM_CREATE | COM_DIRECTORY,
+		dh = ncld_open(nsess, dir, mode, &error, 0, NULL, NULL);
+		if (!dh) {
+			applog(LOG_ERR, "CLD open(%s) failed: %d", dir, error);
+			free(dir);
+			return -1;
+		}
+
+		free(dir);
+		ncld_close(dh);
+
+		compdir = compend + 1;
+	}
+	return 0;
+}
+
+/*
  * Create the file with our parameters in memory, return as ret.
  */
 static int cldu_make_ffile(char **ret, struct cld_session *cs)
@@ -282,7 +296,6 @@ error:
 static int cldu_set_cldc(struct cld_session *cs, int newactive)
 {
 	struct cldc_host *hp;
-	struct ncld_fh *cfh;	/* /chunk-GROUP directory fh */
 	struct timespec tm;
 	char *buf = NULL; /* stupid gcc 4.4.1 throws a warning */
 	int len;
@@ -324,22 +337,11 @@ static int cldu_set_cldc(struct cld_session *cs, int newactive)
 	/*
 	 * First, make sure the base directory exists.
 	 */
-	cfh = ncld_open(cs->nsess, cs->cfname,
-			COM_READ | COM_WRITE | COM_CREATE | COM_DIRECTORY,
-			&error, 0 /* CE_MASTER_FAILOVER | CE_SESS_FAILED */,
-			NULL, NULL);
-	if (!cfh) {
-		applog(LOG_ERR, "CLD open(%s) failed: %d", cs->cfname, error);
-		goto err_copen;
-	}
-
-	/*
-	 * We don't use directory handle to open files in it, so close it.
-	 */
-	ncld_close(cfh);
+	if (cldu_make_path(cs->nsess, cs->ffname))
+		goto err_path;
 
 	/*
-	 * Then, create the membership file for us.
+	 * Path done, create the membership file for us, keep it open.
 	 *
 	 * It is a bit racy to create a file like this, applications can see
 	 * an empty file, or a file with stale contents. But what to do?
@@ -404,7 +406,7 @@ err_buf:
 err_lock:
 	ncld_close(cs->ffh);	/* session-close closes these, maybe drop */
 err_fopen:
-err_copen:
+err_path:
 	ncld_sess_close(cs->nsess);
 	cs->nsess = NULL;
 err_nsess:
@@ -431,7 +433,7 @@ void cld_init()
  * by reference, so their lifetime must exceed the lifetime of the session
  * (the time between cld_begin and cld_end).
  */
-int cld_begin(const char *thishost, const char *thisgroup, uint32_t nid,
+int cld_begin(const char *thishost, uint32_t nid, char *infopath,
 	      struct geo *locp, void (*cb)(enum st_cld))
 {
 	static struct cld_session *cs = &ses;
@@ -449,10 +451,7 @@ int cld_begin(const char *thishost, const char *thisgroup, uint32_t nid,
 
 	timer_init(&cs->timer, "chunkd_cldu_timer", cldu_timer_event, cs);
 
-	if (cldu_setgroup(cs, thisgroup, nid, thishost, locp)) {
-		/* Already logged error */
-		goto err_group;
-	}
+	cldu_saveargs(cs, infopath, nid, thishost, locp);
 
 	if (!cs->forced_hosts) {
 		GList *tmp, *host_list = NULL;
@@ -498,7 +497,6 @@ int cld_begin(const char *thishost, const char *thisgroup, uint32_t nid,
 
 err_net:
 err_addr:
-err_group:
 	return -1;
 }
 
@@ -548,8 +546,6 @@ void cld_end(void)
 		}
 	}
 
-	free(cs->cfname);
-	cs->cfname = NULL;
 	free(cs->ffname);
 	cs->ffname = NULL;
 }
diff --git a/server/config.c b/server/config.c
index 0422239..6c714c1 100644
--- a/server/config.c
+++ b/server/config.c
@@ -30,10 +30,9 @@
 #include <ctype.h>
 #include <errno.h>
 #include <syslog.h>
-#include <stdio.h>
 #include <openssl/hmac.h>
 #include <glib.h>
-#include <cldc.h>
+#include <cld_common.h>
 #include "chunkd.h"
 
 struct config_context {
@@ -56,18 +55,17 @@ struct config_context {
 	struct listen_cfg tmp_listen;
 };
 
-static bool is_good_group_name(const char *s)
+static bool is_good_info_name(const char *s)
 {
 	char c;
 	int n;
 
 	n = 0;
 	while ((c = *s++) != 0) {
-		if (n >= 64)
+		if (n >= CLD_INODE_NAME_MAX)
 			return false;
-		/* whatever we allow in the future, we must filter '/' */
 		if (!(isalpha(c) || isdigit(c) ||
-		    c == '-' || c == '_' || c == '.'))
+		    c == '-' || c == '_' || c == '.' || c == '/'))
 			return false;
 		n++;
 	}
@@ -398,12 +396,6 @@ static void cfg_elm_end (GMarkupParseContext *context,
 		}
 	}
 
-	else if (!strcmp(element_name, "Group") && cc->text) {
-		free(chunkd_srv.group);
-		chunkd_srv.group = cc->text;
-		cc->text = NULL;
-	}
-
 	else if (!strcmp(element_name, "NID") && cc->text) {
 		n = strtol(cc->text, NULL, 10);
 		if (n <= 0 || n >= LONG_MAX) {
@@ -424,6 +416,16 @@ static void cfg_elm_end (GMarkupParseContext *context,
 		cc->text = NULL;
 	}
 
+	else if (!strcmp(element_name, "InfoPath")) {
+		if (!cc->text) {
+			applog(LOG_WARNING, "InfoPath element empty");
+			return;
+		}
+		free(chunkd_srv.info_path);
+		chunkd_srv.info_path = cc->text;
+		cc->text = NULL;
+	}
+
 	else if (!strcmp(element_name, "Geo") && cc->text) {
 		cfg_elm_end_geo(cc);
 		cc->in_geo = false;
@@ -515,8 +517,18 @@ void read_config(void)
 		}
 	}
 
-	if (chunkd_srv.group && !is_good_group_name(chunkd_srv.group)) {
-		applog(LOG_ERR, "Group name '%s' is invalid", chunkd_srv.group);
+	if (!chunkd_srv.info_path) {
+		applog(LOG_ERR, "error: no InfoPath defined in cfg file");
+		exit(1);
+	}
+	if (chunkd_srv.info_path[0] != '/') {
+		/* Special-case because most likely a script error or such. */
+		applog(LOG_ERR, "error: InfoPath is not absolute");
+		exit(1);
+	}
+	if (!is_good_info_name(chunkd_srv.info_path)) {
+		applog(LOG_ERR, "error: InfoPath '%s' is invalid",
+		       chunkd_srv.info_path);
 		exit(1);
 	}
 
diff --git a/server/server.c b/server/server.c
index b2a30bc..69f3973 100644
--- a/server/server.c
+++ b/server/server.c
@@ -1826,7 +1826,7 @@ int main (int argc, char *argv[])
 			goto err_out_listen;
 	}
 
-	if (cld_begin(chunkd_srv.ourhost, chunkd_srv.group, chunkd_srv.nid,
+	if (cld_begin(chunkd_srv.ourhost, chunkd_srv.nid, chunkd_srv.info_path,
 		      &chunkd_srv.loc, NULL)) {
 		rc = 1;
 		goto err_out_cld;
diff --git a/test/server-test.cfg b/test/server-test.cfg
index 29b69e2..0f9d896 100644
--- a/test/server-test.cfg
+++ b/test/server-test.cfg
@@ -21,6 +21,8 @@
 	<Host>localhost</Host>
 </CLD>
 
+<InfoPath>/chunkd-test/1</InfoPath>
+
 <Check>
 	<User>testuser</User>
 	<User>testuser2</User>