From: phk Date: Mon, 25 Jun 2007 21:12:17 +0000 (+0000) Subject: Redo the -n argument code, it had too many problems: X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e52707a8f6fe022285cf130b293703463646bb68;p=varnish Redo the -n argument code, it had too many problems: We need to process -P and -f arguments before we change directory. (ticket 120) (XXX: what about storage and hash arguments ??) The daemon(3) call should not change our directory subsequently. (ticket 121) There is no need to enforce a hostname style format on the argument, a directory nam makes much more sense, since that is what we need. Defaulting to /tmp instead of our hostname makes more sense (ticket 119). This also allows the admin to use a different directory if /tmp is mounted noexec (ticket 111) Put the directoryname used in the proctitle (via heritage) XXX: for docs: vcl.load CLI commands will work relative to the -n directory. git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@1572 d4fa192b-c00b-0410-8231-f00ffab90ce4 --- diff --git a/varnish-cache/bin/varnishd/heritage.h b/varnish-cache/bin/varnishd/heritage.h index fd4e04b5..42c5c7e6 100644 --- a/varnish-cache/bin/varnishd/heritage.h +++ b/varnish-cache/bin/varnishd/heritage.h @@ -62,6 +62,8 @@ struct heritage { /* Hash method */ struct hash_slinger *hash; + + const char *n_arg; }; struct params { diff --git a/varnish-cache/bin/varnishd/mgt.h b/varnish-cache/bin/varnishd/mgt.h index 8bf530bf..20c820b4 100644 --- a/varnish-cache/bin/varnishd/mgt.h +++ b/varnish-cache/bin/varnishd/mgt.h @@ -58,7 +58,7 @@ void MCF_ParamSet(struct cli *, const char *param, const char *val); /* mgt_vcc.c */ void mgt_vcc_init(void); -int mgt_vcc_default(const char *bflag, const char *fflag, int Cflag); +int mgt_vcc_default(const char *bflag, const char *fflag, int f_fd, int Cflag); int mgt_push_vcls_and_start(unsigned *status, char **p); #include "stevedore.h" diff --git a/varnish-cache/bin/varnishd/mgt_child.c b/varnish-cache/bin/varnishd/mgt_child.c index 70a34aef..e45d16bf 100644 --- a/varnish-cache/bin/varnishd/mgt_child.c +++ b/varnish-cache/bin/varnishd/mgt_child.c @@ -204,7 +204,7 @@ start_child(void) AZ(close(heritage.fds[0])); AZ(close(heritage.fds[3])); - setproctitle("Varnish-Chld"); + setproctitle("Varnish-Chld %s", heritage.n_arg); signal(SIGINT, SIG_DFL); signal(SIGTERM, SIG_DFL); @@ -408,7 +408,7 @@ mgt_run(int dflag, const char *T_arg) e->name = "mgt_sigchild"; AZ(ev_add(mgt_evb, e)); - setproctitle("Varnish-Mgr"); + setproctitle("Varnish-Mgr %s", heritage.n_arg); sac.sa_handler = SIG_IGN; sac.sa_flags = SA_RESTART; diff --git a/varnish-cache/bin/varnishd/mgt_param.c b/varnish-cache/bin/varnishd/mgt_param.c index ad7cb5b8..289fd021 100644 --- a/varnish-cache/bin/varnishd/mgt_param.c +++ b/varnish-cache/bin/varnishd/mgt_param.c @@ -499,72 +499,6 @@ tweak_ping_interval(struct cli *cli, struct parspec *par, const char *arg) /*--------------------------------------------------------------------*/ -#define NAME_RE "^([0-9A-Za-z-]+)(\\.[0-9A-Za-z-]+)*$" -static regex_t *name_re; - -static void -tweak_name(struct cli *cli, struct parspec *par, const char *arg) -{ - char hostname[1024], path[1024]; - int error; - - (void)par; - - if (arg == NULL) { - cli_out(cli, "\"%s\"", master.name); - return; - } - - /* empty string -> hostname */ - if (*arg == '\0') { - if (gethostname(hostname, sizeof hostname) == 0) - arg = hostname; - else - arg = "localhost"; - } - - /* check that the new name follows hostname convention */ - if (name_re == NULL) { - name_re = malloc(sizeof *name_re); - AN(name_re); - AZ(regcomp(name_re, NAME_RE, REG_EXTENDED|REG_NOSUB)); - } - if (regexec(name_re, arg, 0, NULL, 0) != 0) { - cli_out(cli, "Invalid instance name"); - cli_result(cli, CLIS_PARAM); - return; - } - - error = 0; - snprintf(path, sizeof path, "/tmp/%s", arg); /* XXX overflow */ - if (master.name && *master.name) { - struct stat old_st; - char old_path[1024]; - - /* rename old directory */ - snprintf(old_path, sizeof old_path, "/tmp/%s", master.name); /* XXX overflow */ - if (stat(old_path, &old_st) == 0 && S_ISDIR(old_st.st_mode)) { - error = rename(old_path, path); - } else { - error = (mkdir(path, 0755) != 0 && errno != EEXIST); - } - } else { - error = (mkdir(path, 0755) != 0 && errno != EEXIST); - } - - if (error || chdir(path) != 0) { - cli_out(cli, "could not create %s: %s", path, strerror(errno)); - cli_result(cli, CLIS_CANT); - return; - } - - /* Everything is fine, store the (new) name */ - master.name = strdup(arg); - XXXAN(master.name); -} - -/*--------------------------------------------------------------------*/ - /* * Make sure to end all lines with either a space or newline of the * formatting will go haywire. @@ -738,12 +672,6 @@ static struct parspec parspec[] = { "it possible to attach a debugger to the child.\n" MUST_RESTART, "3", "seconds" }, - { "name", tweak_name, - "Name of varnishd instance. Must follow hostname " - "naming conventions. Makes it possible to run " - "multiple varnishd instances on one server.\n" - EXPERIMENTAL, - "" }, { NULL, NULL, NULL } }; diff --git a/varnish-cache/bin/varnishd/mgt_vcc.c b/varnish-cache/bin/varnishd/mgt_vcc.c index ad10083a..15d12059 100644 --- a/varnish-cache/bin/varnishd/mgt_vcc.c +++ b/varnish-cache/bin/varnishd/mgt_vcc.c @@ -254,11 +254,11 @@ mgt_VccCompile(struct vsb *sb, const char *b, const char *e, int C_flag) } static char * -mgt_VccCompileFile(struct vsb *sb, const char *fn, int C_flag) +mgt_VccCompileFile(struct vsb *sb, const char *fn, int C_flag, int fd) { char *csrc, *vf = NULL; - csrc = VCC_CompileFile(sb, fn, -1); + csrc = VCC_CompileFile(sb, fn, fd); if (csrc != NULL) { if (C_flag) fputs(csrc, stdout); @@ -314,7 +314,7 @@ mgt_vcc_delbyname(const char *name) /*--------------------------------------------------------------------*/ int -mgt_vcc_default(const char *b_arg, const char *f_arg, int C_flag) +mgt_vcc_default(const char *b_arg, const char *f_arg, int f_fd, int C_flag) { char *addr, *port; char *buf, *vf; @@ -349,7 +349,7 @@ mgt_vcc_default(const char *b_arg, const char *f_arg, int C_flag) vf = mgt_VccCompile(sb, buf, NULL, C_flag); free(buf); } else { - vf = mgt_VccCompileFile(sb, f_arg, C_flag); + vf = mgt_VccCompileFile(sb, f_arg, C_flag, f_fd); } vsb_finish(sb); if (vsb_len(sb) > 0) { @@ -461,7 +461,7 @@ mcf_config_load(struct cli *cli, char **av, void *priv) sb = vsb_new(NULL, NULL, 0, VSB_AUTOEXTEND); XXXAN(sb); - vf = mgt_VccCompileFile(sb, av[3], 0); + vf = mgt_VccCompileFile(sb, av[3], 0, -1); vsb_finish(sb); if (vsb_len(sb) > 0) { cli_out(cli, "%s", vsb_data(sb)); diff --git a/varnish-cache/bin/varnishd/varnishd.c b/varnish-cache/bin/varnishd/varnishd.c index e7e17470..cc4e1b2d 100644 --- a/varnish-cache/bin/varnishd/varnishd.c +++ b/varnish-cache/bin/varnishd/varnishd.c @@ -44,6 +44,7 @@ #include #include #include +#include #ifndef HAVE_DAEMON #include "compat/daemon.h" @@ -178,7 +179,8 @@ usage(void) " -h classic [default]"); fprintf(stderr, " %-28s # %s\n", "", " -h classic,"); - fprintf(stderr, " %-28s # %s\n", "-n name", "varnishd instance name"); + fprintf(stderr, " %-28s # %s\n", "-n dir", + "varnishd working directory"); fprintf(stderr, " %-28s # %s\n", "-P file", "PID file"); fprintf(stderr, " %-28s # %s\n", "-p param=value", "set parameter"); @@ -405,7 +407,9 @@ main(int argc, char *argv[]) unsigned F_flag = 0; const char *b_arg = NULL; const char *f_arg = NULL; + int f_fd = -1; const char *h_arg = "classic"; + const char *n_arg = "/tmp"; const char *P_arg = NULL; const char *s_arg = "file"; const char *T_arg = NULL; @@ -455,7 +459,7 @@ main(int argc, char *argv[]) h_arg = optarg; break; case 'n': - MCF_ParamSet(cli, "name", optarg); + n_arg = optarg; break; case 'P': P_arg = optarg; @@ -499,6 +503,7 @@ main(int argc, char *argv[]) usage(); } + /* XXX: we can have multiple CLI actions above, is this enough ? */ if (cli[0].result != CLIS_OK) { fprintf(stderr, "Parameter errors:\n"); vsb_finish(cli[0].sb); @@ -519,14 +524,39 @@ main(int argc, char *argv[]) fprintf(stderr, "One of -b or -f must be specified\n"); usage(); } + + if (f_arg != NULL) { + f_fd = open(f_arg, O_RDONLY); + if (f_fd < 0) { + fprintf(stderr, "Cannot open '%s': %s\n", + f_arg, strerror(errno)); + exit(1); + } + } + + if (mkdir(n_arg, 0755) < 0 && errno != EEXIST) { + fprintf(stderr, "Cannot create working directory '%s': %s\n", + n_arg, strerror(errno)); + exit(1); + } + if (chdir(n_arg) < 0) { + fprintf(stderr, "Cannot change to working directory '%s': %s\n", + n_arg, strerror(errno)); + exit(1); + } + + heritage.n_arg = n_arg; + + /* XXX: should this be relative to the -n arg ? */ if (P_arg && (pfh = vpf_open(P_arg, 0600, NULL)) == NULL) { perror(P_arg); exit(1); } - if (mgt_vcc_default(b_arg, f_arg, C_flag)) + if (mgt_vcc_default(b_arg, f_arg, f_fd, C_flag)) exit (2); + if (C_flag) exit (0); @@ -538,7 +568,7 @@ main(int argc, char *argv[]) if (d_flag == 1) DebugStunt(); if (d_flag < 2 && !F_flag) - daemon(d_flag, d_flag); + daemon(1, d_flag); if (d_flag == 1) printf("%d\n", getpid());