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 <[email protected]> --- 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> -- To unsubscribe from this list: send the line "unsubscribe hail-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
