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.en...@oracle.com>
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
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Master Java SE, Java EE, Eclipse, Spring, Hibernate, JavaScript, jQuery and much more. Keep your Java skills current with LearnJavaNow - 200+ hours of step-by-step video tutorials by Java experts. SALE $49.99 this month only -- learn more at: http://p.sf.net/sfu/learnmore_122612
_______________________________________________ Lxc-devel mailing list Lxc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/lxc-devel
