[lxc-devel] [PATCH] separate console device from console log
Serge Hallyn
serge.hallyn at canonical.com
Wed Jan 2 22:26:33 UTC 2013
Quoting Stéphane Graber (stgraber at ubuntu.com):
> On 12/27/2012 05:01 PM, Dwight Engen wrote:
> > lxc-start -c makes the named file/device the container's console, but using
> > this with a regular file in order to get a log of the console output does
> > not work very well if you also want to login on the console. This change
> > implements an additional option (-L) to simply log the console's output to
> > a file.
> >
> > Both options can be used separately or together. For example to get a usable
> > console and log: lxc-start -n name -c /dev/tty8 -L console.log
> >
> > The console state is cleaned up more when lxc_delete_console is called, and
> > some of the clean up paths in lxc_create_console were fixed.
> >
> > The lxc_priv and lxc_unpriv macros were modified to make use of gcc's local
> > label feature so they can be expanded more than once in the same function.
> >
> > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
Thanks, looks good, and agreed this is useful.
>
> I think I'd like Serge to take a quick look at that one as I have very
> limited knowledge of what's going on with the console handling.
> I didn't spot any specific problem though.
>
> Just one comment below related to the manpage change.
>
> > ---
> > doc/lxc-start.sgml.in | 28 ++++++++++------
> > src/lxc/arguments.h | 1 +
> > src/lxc/caps.h | 12 ++++---
> > src/lxc/conf.c | 2 ++
> > src/lxc/conf.h | 2 ++
> > src/lxc/console.c | 58 +++++++++++++++++++++++++++-----
> > src/lxc/lxc_start.c | 91 ++++++++++++++++++++++++++++++---------------------
> > 7 files changed, 134 insertions(+), 60 deletions(-)
> >
> > diff --git a/doc/lxc-start.sgml.in b/doc/lxc-start.sgml.in
> > index 5c98a25..e4036f4 100644
> > --- a/doc/lxc-start.sgml.in
> > +++ b/doc/lxc-start.sgml.in
> > @@ -51,7 +51,8 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> > <command>lxc-start</command>
> > <arg choice="req">-n <replaceable>name</replaceable></arg>
> > <arg choice="opt">-f <replaceable>config_file</replaceable></arg>
> > - <arg choice="opt">-c <replaceable>console_file</replaceable></arg>
> > + <arg choice="opt">-c <replaceable>console_device</replaceable></arg>
> > + <arg choice="opt">-L <replaceable>console_logfile</replaceable></arg>
> > <arg choice="opt">-d</arg>
> > <arg choice="opt">-p <replaceable>pid_file</replaceable></arg>
> > <arg choice="opt">-s KEY=VAL</arg>
> > @@ -76,11 +77,6 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> > defined, the default isolation is used.
> > </para>
> > <para>
> > - The orphan process group
> > - and daemon are not supported by this command, use
> > - the <command>lxc-execute</command> command instead.
> > - </para>
> > - <para>
>
> Why was that dropped? I don't see any mention in the commit message
> about it. Is that no longer relevant?
>
> > If no command is specified, <command>lxc-start</command> will
> > use the default
> > <command>"/sbin/init"</command> command to run a system
> > @@ -139,13 +135,25 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> > <varlistentry>
> > <term>
> > <option>-c,
> > - --console <replaceable>console_file</replaceable></option>
> > + --console <replaceable>console_device</replaceable></option>
> > + </term>
> > + <listitem>
> > + <para>
> > + Specify a device to use for the container's console, for example
> > + /dev/tty8. If this option is not specified the current terminal
> > + will be used unless <option>-d</option> is specified.
> > + </para>
> > + </listitem>
> > + </varlistentry>
> > +
> > + <varlistentry>
> > + <term>
> > + <option>-L,
> > + --console-log <replaceable>console_logfile</replaceable></option>
> > </term>
> > <listitem>
> > <para>
> > - Specify a file to output the container console. If the
> > - option is not specified the output will go the terminal
> > - except if the <option>-d</option> is specified.
> > + Specify a file to log the container's console output to.
> > </para>
> > </listitem>
> > </varlistentry>
> > diff --git a/src/lxc/arguments.h b/src/lxc/arguments.h
> > index 188c460..5f2ecba 100644
> > --- a/src/lxc/arguments.h
> > +++ b/src/lxc/arguments.h
> > @@ -45,6 +45,7 @@ struct lxc_arguments {
> > int daemonize;
> > const char *rcfile;
> > const char *console;
> > + const char *console_log;
> > const char *pidfile;
> >
> > /* for lxc-checkpoint/restart */
> > diff --git a/src/lxc/caps.h b/src/lxc/caps.h
> > index 0cf8460..88cf09e 100644
> > --- a/src/lxc/caps.h
> > +++ b/src/lxc/caps.h
> > @@ -33,27 +33,29 @@ extern int lxc_caps_last_cap(void);
> >
> > #define lxc_priv(__lxc_function) \
> > ({ \
> > + __label__ out; \
> > int __ret, __ret2, __errno = 0; \
> > __ret = lxc_caps_up(); \
> > if (__ret) \
> > - goto __out; \
> > + goto out; \
> > __ret = __lxc_function; \
> > if (__ret) \
> > __errno = errno; \
> > __ret2 = lxc_caps_down(); \
> > - __out: __ret ? errno = __errno,__ret : __ret2; \
> > + out: __ret ? errno = __errno,__ret : __ret2; \
> > })
> >
> > -#define lxc_unpriv(__lxc_function) \
> > +#define lxc_unpriv(__lxc_function) \
> > ({ \
> > + __label__ out; \
> > int __ret, __ret2, __errno = 0; \
> > __ret = lxc_caps_down(); \
> > if (__ret) \
> > - goto __out; \
> > + goto out; \
> > __ret = __lxc_function; \
> > if (__ret) \
> > __errno = errno; \
> > __ret2 = lxc_caps_up(); \
> > - __out: __ret ? errno = __errno,__ret : __ret2; \
> > + out: __ret ? errno = __errno,__ret : __ret2; \
> > })
> > #endif
> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > index c82e759..4f041dc 100644
> > --- a/src/lxc/conf.c
> > +++ b/src/lxc/conf.c
> > @@ -2005,6 +2005,8 @@ struct lxc_conf *lxc_conf_init(void)
> > memset(new, 0, sizeof(*new));
> >
> > new->personality = -1;
> > + new->console.log_path = NULL;
> > + new->console.log_fd = -1;
> > new->console.path = NULL;
> > new->console.peer = -1;
> > new->console.master = -1;
> > diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> > index d496916..b576893 100644
> > --- a/src/lxc/conf.h
> > +++ b/src/lxc/conf.h
> > @@ -175,6 +175,8 @@ struct lxc_console {
> > int master;
> > int peer;
> > char *path;
> > + char *log_path;
> > + int log_fd;
> > char name[MAXPATHLEN];
> > struct termios *tios;
> > };
> > diff --git a/src/lxc/console.c b/src/lxc/console.c
> > index 73bec78..cb756f0 100644
> > --- a/src/lxc/console.c
> > +++ b/src/lxc/console.c
> > @@ -195,11 +195,21 @@ int lxc_create_console(struct lxc_conf *conf)
> > goto err;
> > }
> >
> > + if (console->log_path) {
> > + fd = lxc_unpriv(open(console->log_path, O_CLOEXEC | O_RDWR | O_CREAT | O_APPEND, 0600));
> > + if (fd < 0) {
> > + SYSERROR("failed to open '%s'", console->log_path);
> > + goto err;
> > + }
> > + DEBUG("using '%s' as console log", console->log_path);
> > + console->log_fd = fd;
> > + }
> > +
> > fd = lxc_unpriv(open(console->path, O_CLOEXEC | O_RDWR | O_CREAT |
> > O_APPEND, 0600));
> > if (fd < 0) {
> > SYSERROR("failed to open '%s'", console->path);
> > - goto err;
> > + goto err_close_console_log;
> > }
> >
> > DEBUG("using '%s' as console", console->path);
> > @@ -212,7 +222,7 @@ int lxc_create_console(struct lxc_conf *conf)
> > console->tios = malloc(sizeof(tios));
> > if (!console->tios) {
> > SYSERROR("failed to allocate memory");
> > - goto err;
> > + goto err_close_console;
> > }
> >
> > /* Get termios */
> > @@ -241,26 +251,54 @@ int lxc_create_console(struct lxc_conf *conf)
> >
> > err_free:
> > free(console->tios);
> > +
> > +err_close_console:
> > + close(console->peer);
> > + console->peer = -1;
> > +
> > +err_close_console_log:
> > + if (console->log_fd >= 0) {
> > + close(console->log_fd);
> > + console->log_fd = -1;
> > + }
> > +
> > err:
> > close(console->master);
> > + console->master = -1;
> > +
> > close(console->slave);
> > + console->slave = -1;
> > return -1;
> > }
> >
> > -void lxc_delete_console(const struct lxc_console *console)
> > +void lxc_delete_console(struct lxc_console *console)
> > {
> > if (console->tios &&
> > tcsetattr(console->peer, TCSAFLUSH, console->tios))
> > WARN("failed to set old terminal settings");
> > + free(console->tios);
> > + console->tios = NULL;
> > +
> > + close(console->peer);
> > + console->peer = -1;
> > +
> > + if (console->log_fd >= 0) {
> > + close(console->log_fd);
> > + console->log_fd = -1;
> > + }
> > +
> > close(console->master);
> > + console->master = -1;
> > +
> > close(console->slave);
> > + console->slave = -1;
> > }
> >
> > static int console_handler(int fd, void *data, struct lxc_epoll_descr *descr)
> > {
> > struct lxc_console *console = (struct lxc_console *)data;
> > char buf[1024];
> > - int r;
> > + int r,w;
> >
> > r = read(fd, buf, sizeof(buf));
> > if (r < 0) {
> > @@ -280,10 +318,14 @@ static int console_handler(int fd, void *data, struct lxc_epoll_descr *descr)
> > return 0;
> >
> > if (console->peer == fd)
> > - r = write(console->master, buf, r);
> > - else
> > - r = write(console->peer, buf, r);
> > -
> > + w = write(console->master, buf, r);
> > + else {
> > + w = write(console->peer, buf, r);
> > + if (console->log_fd > 0)
> > + w = write(console->log_fd, buf, r);
> > + }
> > + if (w != r)
> > + WARN("console short write");
> > return 0;
> > }
> >
> > diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
> > index fb756dd..184fb04 100644
> > --- a/src/lxc/lxc_start.c
> > +++ b/src/lxc/lxc_start.c
> > @@ -54,10 +54,46 @@ lxc_log_define(lxc_start_ui, lxc_start);
> >
> > static struct lxc_list defines;
> >
> > +static int ensure_path(char **confpath, const char *path)
> > +{
> > + int err = -1, fd;
> > + char *fullpath = NULL;
> > +
> > + if (path) {
> > + if (access(path, W_OK)) {
> > + fd = creat(path, 0600);
> > + if (fd < 0) {
> > + SYSERROR("failed to create '%s'", path);
> > + goto err;
> > + }
> > + close(fd);
> > + }
> > +
> > + fullpath = realpath(path, NULL);
> > + if (!fullpath) {
> > + SYSERROR("failed to get the real path of '%s'", path);
> > + goto err;
> > + }
> > +
> > + *confpath = strdup(fullpath);
> > + if (!*confpath) {
> > + ERROR("failed to dup string '%s'", fullpath);
> > + goto err;
> > + }
> > + }
> > + err = 0;
> > +
> > +err:
> > + if (fullpath)
> > + free(fullpath);
> > + return err;
> > +}
> > +
> > static int my_parser(struct lxc_arguments* args, int c, char* arg)
> > {
> > switch (c) {
> > case 'c': args->console = arg; break;
> > + case 'L': args->console_log = arg; break;
> > case 'd': args->daemonize = 1; args->close_all_fds = 1; break;
> > case 'f': args->rcfile = arg; break;
> > case 'C': args->close_all_fds = 1; break;
> > @@ -72,6 +108,7 @@ static const struct option my_longopts[] = {
> > {"rcfile", required_argument, 0, 'f'},
> > {"define", required_argument, 0, 's'},
> > {"console", required_argument, 0, 'c'},
> > + {"console-log", required_argument, 0, 'L'},
> > {"close-all-fds", no_argument, 0, 'C'},
> > {"pidfile", required_argument, 0, 'p'},
> > LXC_COMMON_OPTIONS
> > @@ -85,15 +122,16 @@ static struct lxc_arguments my_args = {
> > lxc-start start COMMAND in specified container NAME\n\
> > \n\
> > Options :\n\
> > - -n, --name=NAME NAME for name of the container\n\
> > - -d, --daemon daemonize the container\n\
> > - -p, --pidfile=FILE Create a file with the process id\n\
> > - -f, --rcfile=FILE Load configuration file FILE\n\
> > - -c, --console=FILE Set the file output for the container console\n\
> > - -C, --close-all-fds If any fds are inherited, close them\n\
> > - If not specified, exit with failure instead\n\
> > - Note: --daemon implies --close-all-fds\n\
> > - -s, --define KEY=VAL Assign VAL to configuration variable KEY\n",
> > + -n, --name=NAME NAME for name of the container\n\
> > + -d, --daemon daemonize the container\n\
> > + -p, --pidfile=FILE Create a file with the process id\n\
> > + -f, --rcfile=FILE Load configuration file FILE\n\
> > + -c, --console=FILE Use specified FILE for the container console\n\
> > + -L, --console-log=FILE Log container console output to FILE\n\
> > + -C, --close-all-fds If any fds are inherited, close them\n\
> > + If not specified, exit with failure instead\n\
> > + Note: --daemon implies --close-all-fds\n\
> > + -s, --define KEY=VAL Assign VAL to configuration variable KEY\n",
> > .options = my_longopts,
> > .parser = my_parser,
> > .checker = NULL,
> > @@ -177,35 +215,14 @@ int main(int argc, char *argv[])
> > return err;
> > }
> >
> > - if (my_args.console) {
> > -
> > - char *console, fd;
> > -
> > - if (access(my_args.console, W_OK)) {
> > -
> > - fd = creat(my_args.console, 0600);
> > - if (fd < 0) {
> > - SYSERROR("failed to touch file '%s'",
> > - my_args.console);
> > - return err;
> > - }
> > - close(fd);
> > - }
> > -
> > - console = realpath(my_args.console, NULL);
> > - if (!console) {
> > - SYSERROR("failed to get the real path of '%s'",
> > - my_args.console);
> > - return err;
> > - }
> > -
> > - conf->console.path = strdup(console);
> > - if (!conf->console.path) {
> > - ERROR("failed to dup string '%s'", console);
> > - return err;
> > - }
> > + if (ensure_path(&conf->console.path, my_args.console) < 0) {
> > + ERROR("failed to ensure console path '%s'", my_args.console);
> > + return err;
> > + }
> >
> > - free(console);
> > + if (ensure_path(&conf->console.log_path, my_args.console_log) < 0) {
> > + ERROR("failed to ensure console log '%s'", my_args.console_log);
> > + return err;
> > }
> >
> > if (my_args.pidfile != NULL) {
> >
>
>
> --
> Stéphane Graber
> Ubuntu developer
> http://www.ubuntu.com
>
More information about the lxc-devel
mailing list