Hey Dwight, On Fri, Mar 7, 2014 at 6:48 PM, Dwight Engen <dwight.en...@oracle.com> wrote: > On Fri, 7 Mar 2014 16:12:23 -0600 > Serge Hallyn <serge.hal...@ubuntu.com> wrote: > >> Quoting Dwight Engen (dwight.en...@oracle.com): >> > Opening a debug log for every thread at every iteration of >> > test-concurrent causes it to quickly run out of fd's because this >> > fd is leaked. Fix this by adding a new api: lxc_log_close(). >> > >> > As Caglar noted, the log handling is in general a bit "interesting" >> > because a logfile can be opened through the per-container api >> > c->set_config_item("lxc.logfile") but lxc_log_fd is now per-thread >> > data. It just so happens in test-concurrent that there is a 1:1 >> > mapping of threads to logfiles. >> >> Will at_exit work at thread exit? > > I don't think it does. I thought a little about trying to do the close > automatically internal to lxc on the last ref in lxc_container_put() > but that doesn't really make sense. I guess that's what seems weird to > me, that opening the log is via a per-container api, but logging is > really per-process (or per-thread with Caglar's recent change).
Just wondering, why do you think closing it automatically is not a good idea? >> > Split out getting debug logs from quiet since I think they are >> > useful separately. If debug is specified, get a log of any mode, >> > not just during start. >> > >> > Signed-off-by: Dwight Engen <dwight.en...@oracle.com> >> >> Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com> >> >> > --- >> > src/lxc/log.c | 10 ++++++++++ >> > src/lxc/lxccontainer.h | 5 +++++ >> > src/tests/concurrent.c | 19 ++++++++++++++----- >> > 3 files changed, 29 insertions(+), 5 deletions(-) >> > >> > diff --git a/src/lxc/log.c b/src/lxc/log.c >> > index a9f98a4..d5b862e 100644 >> > --- a/src/lxc/log.c >> > +++ b/src/lxc/log.c >> > @@ -368,6 +368,16 @@ extern int lxc_log_init(const char *name, >> > const char *file, return ret; >> > } >> > >> > +extern void lxc_log_close(void) >> > +{ >> > + if (lxc_log_fd == -1) >> > + return; >> > + close(lxc_log_fd); >> > + lxc_log_fd = -1; >> > + free(log_fname); >> > + log_fname = NULL; >> > +} >> > + >> > /* >> > * This is called when we read a lxc.loglevel entry in a lxc.conf >> > file. This >> > * happens after processing command line arguments, which override >> > the .conf diff --git a/src/lxc/lxccontainer.h >> > b/src/lxc/lxccontainer.h index b9873eb..ba15ab7 100644 >> > --- a/src/lxc/lxccontainer.h >> > +++ b/src/lxc/lxccontainer.h >> > @@ -865,6 +865,11 @@ int list_active_containers(const char >> > *lxcpath, char ***names, struct lxc_contai */ >> > int list_all_containers(const char *lxcpath, char ***names, struct >> > lxc_container ***cret); >> > +/*! >> > + * \brief Close log file. >> > + */ >> > +void lxc_log_close(void); >> > + >> > #ifdef __cplusplus >> > } >> > #endif >> > diff --git a/src/tests/concurrent.c b/src/tests/concurrent.c >> > index c4d2ad5..acabbed 100644 >> > --- a/src/tests/concurrent.c >> > +++ b/src/tests/concurrent.c >> > @@ -29,6 +29,7 @@ >> > >> > static int nthreads = 5; >> > static int iterations = 1; >> > +static int debug = 0; >> > static int quiet = 0; >> > static int delay = 0; >> > static const char *template = "busybox"; >> > @@ -40,6 +41,7 @@ static const struct option options[] = { >> > { "delay", required_argument, NULL, 'd' }, >> > { "modes", required_argument, NULL, 'm' }, >> > { "quiet", no_argument, NULL, 'q' }, >> > + { "debug", no_argument, NULL, 'D' }, >> > { "help", no_argument, NULL, '?' }, >> > { 0, 0, 0, 0 }, >> > }; >> > @@ -54,6 +56,7 @@ static void usage(void) { >> > " -d, --delay=N Delay in seconds between >> > start and stop\n" " -m, --modes=<mode,mode,...> Modes to run >> > (create, start, stop, destroy)\n" " -q, --quiet >> > Don't produce any output\n" >> > + " -D, --debug Create a debug log\n" >> > " -?, --help Give this help list\n" >> > "\n" >> > "Mandatory or optional arguments to long options are also >> > mandatory or optional\n" @@ -81,6 +84,11 @@ static void >> > do_function(void *arguments) return; >> > } >> > >> > + if (debug) { >> > + c->set_config_item(c, "lxc.loglevel", "DEBUG"); >> > + c->set_config_item(c, "lxc.logfile", name); >> > + } >> > + >> > if (strcmp(args->mode, "create") == 0) { >> > if (!c->is_defined(c)) { >> > if (!c->create(c, template, NULL, NULL, 1, NULL)) { >> > @@ -91,10 +99,6 @@ static void do_function(void *arguments) >> > } else if(strcmp(args->mode, "start") == 0) { >> > if (c->is_defined(c) && !c->is_running(c)) { >> > c->want_daemonize(c, true); >> > - if (!quiet) { >> > - c->set_config_item(c, "lxc.loglevel", "DEBUG"); >> > - c->set_config_item(c, "lxc.logfile", name); >> > - } >> > if (!c->start(c, false, NULL)) { >> > fprintf(stderr, "Starting the container (%s) >> > failed...\n", name); goto out; >> > @@ -127,6 +131,8 @@ static void do_function(void *arguments) >> > args->return_code = 0; >> > out: >> > lxc_container_put(c); >> > + if (debug) >> > + lxc_log_close(); >> > } >> > >> > static void *concurrent(void *arguments) >> > @@ -148,7 +154,7 @@ int main(int argc, char *argv[]) { >> > >> > pthread_attr_init(&attr); >> > >> > - while ((opt = getopt_long(argc, argv, "j:i:t:d:m:q", options, >> > NULL)) != -1) { >> > + while ((opt = getopt_long(argc, argv, "j:i:t:d:m:qD", options, >> > NULL)) != -1) { switch(opt) { >> > case 'j': >> > nthreads = atoi(optarg); >> > @@ -165,6 +171,9 @@ int main(int argc, char *argv[]) { >> > case 'q': >> > quiet = 1; >> > break; >> > + case 'D': >> > + debug = 1; >> > + break; >> > case 'm': { >> > char *mode_tok, *tok, *saveptr = NULL; >> > >> > -- >> > 1.8.5.3 >> > >> > > -- S.Çağlar Onur <cag...@10ur.org> _______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel