From d2e47ddb888d84d1dcdd7baca6d47063d7656df4 Mon Sep 17 00:00:00 2001 From: phk Date: Sat, 31 May 2008 09:34:28 +0000 Subject: [PATCH] Go over the Telnet/CLI code in the manager process. Implement "quit" (ticket 125) and drop the planned aliases. Add CLIS_CLOSE return status, to communicate that close is happening. Fix logical bug in fd-table allocation bug in event-engine. Plug memory leak in CLI session destruction. Increase listen depth of telnet socket to 10. Ignore trailing whitespace and empty CLI input lines. Log CLI sessions and commands into syslog. git-svn-id: svn+ssh://projects.linpro.no/svn/varnish/trunk@2644 d4fa192b-c00b-0410-8231-f00ffab90ce4 --- varnish-cache/bin/varnishd/mgt_cli.c | 117 +++++++++++++++++-------- varnish-cache/bin/varnishd/mgt_event.c | 8 +- varnish-cache/include/cli.h | 15 +--- 3 files changed, 87 insertions(+), 53 deletions(-) diff --git a/varnish-cache/bin/varnishd/mgt_cli.c b/varnish-cache/bin/varnishd/mgt_cli.c index eed2b430..456659f9 100644 --- a/varnish-cache/bin/varnishd/mgt_cli.c +++ b/varnish-cache/bin/varnishd/mgt_cli.c @@ -37,8 +37,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -78,6 +80,8 @@ mcf_stats(struct cli *cli, const char * const *av, void *priv) #undef MAC_STAT } +/*--------------------------------------------------------------------*/ + static void mcf_help(struct cli *cli, const char * const *av, void *priv) { @@ -98,6 +102,17 @@ mcf_help(struct cli *cli, const char * const *av, void *priv) /*--------------------------------------------------------------------*/ +static void +mcf_close(struct cli *cli, const char *const *av, void *priv) +{ + + (void)av; + (void)priv; + cli_out(cli, "Closing CLI connection"); + cli_result(cli, CLIS_CLOSE); +} + +/*--------------------------------------------------------------------*/ /* XXX: what order should this list be in ? */ static struct cli_proto cli_proto[] = { @@ -115,13 +130,12 @@ static struct cli_proto cli_proto[] = { { CLI_VCL_SHOW, mcf_config_show, NULL }, { CLI_PARAM_SHOW, mcf_param_show, NULL }, { CLI_PARAM_SET, mcf_param_set, NULL }, + + { CLI_QUIT, mcf_close, NULL}, #if 0 { CLI_SERVER_RESTART }, { CLI_ZERO }, { CLI_VERBOSE, m_cli_func_verbose, NULL }, - { CLI_EXIT, m_cli_func_exit, NULL}, - { CLI_QUIT }, - { CLI_BYE }, #endif { NULL } }; @@ -204,27 +218,6 @@ struct cli_port { char *name; }; -static int -mgt_cli_close(const struct cli_port *cp) -{ - - CHECK_OBJ_NOTNULL(cp, CLI_PORT_MAGIC); - fprintf(stderr, "CLI closing: %s\n", cp->name); - vsb_delete(cp->cli->sb); - VLU_Destroy(cp->vlu); - free(cp->name); - (void)close(cp->fdi); - if (cp->fdi == 0) - assert(open("/dev/null", O_RDONLY) == 0); - (void)close(cp->fdo); - if (cp->fdo == 1) { - assert(open("/dev/null", O_WRONLY) == 1); - (void)close(2); - assert(open("/dev/null", O_WRONLY) == 2); - } - return (1); -} - static int mgt_cli_vlu(void *priv, const char *p) { @@ -235,6 +228,17 @@ mgt_cli_vlu(void *priv, const char *p) CAST_OBJ_NOTNULL(cp, priv, CLI_PORT_MAGIC); vsb_clear(cp->cli->sb); + + /* Remove trailing whitespace */ + q = strchr(p, '\0'); + while (q > p && isspace(q[-1])) + q--; + *q = '\0'; + + /* Ignore empty lines */ + if (*p == 0) + return (0); + cli_dispatch(cp->cli, cli_proto, p); vsb_finish(cp->cli->sb); AZ(vsb_overflowed(cp->cli->sb)); @@ -266,30 +270,71 @@ mgt_cli_vlu(void *priv, const char *p) } /* send the result back */ - if (cli_writeres(cp->fdo, cp->cli)) - return (mgt_cli_close(cp)); + syslog(LOG_INFO, "CLI %d result %d \"%s\"", + cp->fdi, cp->cli->result, p); + if (cli_writeres(cp->fdo, cp->cli) || cp->cli->result == CLIS_CLOSE) + return (1); return (0); } +/*-------------------------------------------------------------------- + * Get rid of a CLI session. + * + * Always and only called from mgt_cli_callback(). + * + * We must get rid of everything but the event, which gets GC'ed by + * ev_schdule_one() when mgt_cli_callback, through our return value + * returns non-zero. + */ + +static int +mgt_cli_close(struct cli_port *cp) +{ + + CHECK_OBJ_NOTNULL(cp, CLI_PORT_MAGIC); + syslog(LOG_NOTICE, "CLI %d closed", cp->fdi); + free(cp->name); + vsb_delete(cp->cli->sb); + VLU_Destroy(cp->vlu); + + (void)close(cp->fdi); + if (cp->fdo != cp->fdi) + (void)close(cp->fdo); + + /* Special case for stdin/out/err */ + if (cp->fdi == 0) + assert(open("/dev/null", O_RDONLY) == 0); + if (cp->fdo == 1) { + assert(open("/dev/null", O_WRONLY) == 1); + (void)close(2); + assert(open("/dev/null", O_WRONLY) == 2); + } + + free(cp); + return (1); +} + +/*-------------------------------------------------------------------- + * Callback whenever something happens to the input fd of the session. + */ + static int mgt_cli_callback(const struct ev *e, int what) { struct cli_port *cp; - int i; CAST_OBJ_NOTNULL(cp, e->priv, CLI_PORT_MAGIC); if (what & (EV_ERR | EV_HUP | EV_GONE)) return (mgt_cli_close(cp)); - i = VLU_Fd(cp->fdi, cp->vlu); - if (i != 0) { - mgt_cli_close(cp); - return (1); - } + if (VLU_Fd(cp->fdi, cp->vlu)) + return (mgt_cli_close(cp)); return (0); } +/*--------------------------------------------------------------------*/ + void mgt_cli_setup(int fdi, int fdo, int verbose, const char *ident) { @@ -299,9 +344,9 @@ mgt_cli_setup(int fdi, int fdo, int verbose, const char *ident) XXXAN(cp); cp->vlu = VLU_New(cp, mgt_cli_vlu, params->cli_buffer); - asprintf(&cp->name, "cli %s fds{%d,%d}", ident, fdi, fdo); + cp->name = strdup(ident); XXXAN(cp->name); - fprintf(stderr, "CLI opened: %s\n", cp->name); + syslog(LOG_NOTICE, "CLI %d open from %s", fdi, cp->name); cp->magic = CLI_PORT_MAGIC; cp->fdi = fdi; @@ -339,7 +384,7 @@ telnet_accept(const struct ev *ev, int what) TCP_myname(ev->fd, abuf1, sizeof abuf1, pbuf1, sizeof pbuf1); TCP_name((void*)&addr, addrlen, abuf2, sizeof abuf2, pbuf2, sizeof pbuf2); - asprintf(&p, "telnet{%s:%s,%s:%s}", abuf2, pbuf2, abuf1, pbuf1); + asprintf(&p, "telnet %s:%s %s:%s", abuf2, pbuf2, abuf1, pbuf1); XXXAN(p); mgt_cli_setup(i, i, 0, p); @@ -363,7 +408,7 @@ mgt_cli_telnet(const char *T_arg) exit(2); } for (i = 0; i < n; ++i) { - int sock = VSS_listen(ta[i], 1); + int sock = VSS_listen(ta[i], 10); struct ev *ev = ev_new(); XXXAN(ev); ev->fd = sock; diff --git a/varnish-cache/bin/varnishd/mgt_event.c b/varnish-cache/bin/varnishd/mgt_event.c index f1264fa8..4c538165 100644 --- a/varnish-cache/bin/varnishd/mgt_event.c +++ b/varnish-cache/bin/varnishd/mgt_event.c @@ -107,13 +107,13 @@ ev_get_pfd(struct evbase *evb) unsigned u; void *p; - if (evb->lpfd < evb->npfd) + if (evb->lpfd + 1 < evb->npfd) return (0); - if (evb->npfd > 256) - u = evb->npfd + 256; - else if (evb->npfd < 8) + if (evb->npfd < 8) u = 8; + else if (evb->npfd > 256) + u = evb->npfd + 256; else u = evb->npfd * 2; p = realloc(evb->pfd, sizeof *evb->pfd * u); diff --git a/varnish-cache/include/cli.h b/varnish-cache/include/cli.h index 44b6124e..7b13918f 100644 --- a/varnish-cache/include/cli.h +++ b/varnish-cache/include/cli.h @@ -200,24 +200,12 @@ "\tEnable/Disable verbosity", \ 0, 0 -#define CLI_EXIT \ - "exit", \ - "exit", \ - "\tClose connection", \ - 0, 0 - #define CLI_QUIT \ "quit", \ "quit", \ "\tClose connection", \ 0, 0 -#define CLI_BYE \ - "bye", \ - "bye", \ - "\tClose connection", \ - 0, 0 - # define CLI_SERVER_STATUS \ "status", \ "status", \ @@ -237,7 +225,8 @@ enum cli_status_e { CLIS_PARAM = 106, CLIS_OK = 200, CLIS_CANT = 300, - CLIS_COMMS = 400 + CLIS_COMMS = 400, + CLIS_CLOSE = 500 }; /* Length of first line of response */ -- 2.39.5