[lxc-devel] [PATCH v2] console API improvements

Serge Hallyn serge.hallyn at ubuntu.com
Thu Jun 13 16:39:44 UTC 2013


Quoting Dwight Engen (dwight.engen at oracle.com):
> On Wed, 12 Jun 2013 11:15:10 -0500
> Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
> 
> > Quoting Dwight Engen (dwight.engen at oracle.com):
> > > Add a higher level console API that opens a tty/console and runs the
> > > mainloop as well. Rename existing API to console_getfd(). Use these
> > > in the python binding.
> > > 
> > > Allow attaching a console peer after container bootup, including if
> > > the container was launched with -d. This is made possible by
> > > allocation of a "proxy" pty as the peer when the console is
> > > attached to.
> > > 
> > > Improve handling of SIGWINCH, the pty size will be correctly set at
> > > the beginning of a session and future changes when using the
> > > lxc_console() API will be propagated to it as well.
> > > 
> > > Refactor some common code between lxc_console.c and console.c. The
> > > variable wait4q (renamed to saw_escape) was static, making the
> > > mainloop callback not safe across threads. This wasn't a problem
> > > when the callback was in the non-threaded lxc-console, but now that
> > > it is internal to console.c, we have to take care of it. This is
> > > now contained in a per-tty state structure.
> > > 
> > > Don't attempt to open /dev/null as the console peer since /dev/null
> > > cannot be added to the mainloop (epoll_ctl() fails with EPERM).
> > > This isn't needed to get the console setup (and the log to work)
> > > since the case of not having a peer at console init time has to be
> > > handled to allow for attaching to it later.
> > > 
> > > Move signalfd libc wrapper/replacement to utils.h.
> > > 
> > > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> > > ---
> > >  doc/lxc-console.sgml.in                  |  11 +-
> > >  src/lxc/commands.c                       |  64 +--
> > >  src/lxc/commands.h                       |   2 +
> > >  src/lxc/conf.c                           |  11 +-
> > >  src/lxc/conf.h                           |   5 +
> > >  src/lxc/console.c                        | 742
> > > +++++++++++++++++++++++++------
> > > src/lxc/console.h                        |  18 +-
> > > src/lxc/lxc.h                            |   9 -
> > > src/lxc/lxc_console.c                    | 189 +-------
> > > src/lxc/lxccontainer.c                   |  12 +-
> > > src/lxc/lxccontainer.h                   |  21 +-
> > > src/lxc/start.c                          |  83 +---
> > > src/lxc/utils.h                          |  65 +++
> > > src/python-lxc/examples/pyconsole-vte.py |  58 +++
> > > src/python-lxc/examples/pyconsole.py     |  33 ++
> > > src/python-lxc/lxc.c                     |  46 ++
> > > src/python-lxc/lxc/__init__.py           |  18 +-
> > > src/tests/console.c                      |   6 +- 18 files changed,
> > > 962 insertions(+), 431 deletions(-) create mode 100755
> > > src/python-lxc/examples/pyconsole-vte.py create mode 100755
> > > src/python-lxc/examples/pyconsole.py
> > > 
> > > diff --git a/doc/lxc-console.sgml.in b/doc/lxc-console.sgml.in
> > > index 9299778..f4737d1 100644
> > > --- a/doc/lxc-console.sgml.in
> > > +++ b/doc/lxc-console.sgml.in
> > > @@ -78,6 +78,12 @@ Foundation, Inc., 59 Temple Place, Suite 330,
> > > Boston, MA 02111-1307 USA </para>
> > >  
> > >      <para>
> > > +      A <replaceable>ttynum</replaceable> of 0 may be given to
> > > attach
> > > +      to the container's /dev/console instead of its
> > > +      dev/tty<<replaceable>ttynum</replaceable>>.
> > > +    </para>
> > > +
> > > +    <para>
> > >        A keyboard escape sequence may be used to disconnect from
> > > the tty and quit lxc-console. The default escape sequence is
> > > <Ctrl+a q>. </para>
> > > @@ -107,8 +113,9 @@ Foundation, Inc., 59 Temple Place, Suite 330,
> > > Boston, MA 02111-1307 USA </term>
> > >  	<listitem>
> > >  	  <para>
> > > -	    Specify the tty number to connect, if not specified a
> > > tty
> > > -	    number will be automatically choosen by the container.
> > > +	    Specify the tty number to connect to or 0 for the
> > > console. If not
> > > +	    specified the next available tty number will be
> > > automatically
> > > +	    choosen by the container.
> > >  	  </para>
> > >  	</listitem>
> > >        </varlistentry>
> > > diff --git a/src/lxc/commands.c b/src/lxc/commands.c
> > > index b4afc07..b23eb98 100644
> > > --- a/src/lxc/commands.c
> > > +++ b/src/lxc/commands.c
> > > @@ -40,6 +40,7 @@
> > >  #include <lxc/utils.h>
> > >  
> > >  #include "commands.h"
> > > +#include "console.h"
> > >  #include "confile.h"
> > >  #include "mainloop.h"
> > >  #include "af_unix.h"
> > > @@ -546,6 +547,37 @@ static int lxc_cmd_stop_callback(int fd,
> > > struct lxc_cmd_req *req, }
> > >  
> > >  /*
> > > + * lxc_cmd_console_winch: To process as if a SIGWINCH were received
> > > + *
> > > + * @name      : name of container to connect to
> > > + * @lxcpath   : the lxcpath in which the container is running
> > > + *
> > > + * Returns 0 on success, < 0 on failure
> > > + */
> > > +int lxc_cmd_console_winch(const char *name, const char *lxcpath)
> > > +{
> > > +	int ret, stopped = 0;
> > > +	struct lxc_cmd_rr cmd = {
> > > +		.req = { .cmd = LXC_CMD_CONSOLE_WINCH },
> > > +	};
> > > +
> > > +	ret = lxc_cmd(name, &cmd, &stopped, lxcpath);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int lxc_cmd_console_winch_callback(int fd, struct
> > > lxc_cmd_req *req,
> > > +					  struct lxc_handler
> > > *handler) +{
> > > +	struct lxc_cmd_rsp rsp = { .data = 0 };
> > > +
> > > +	lxc_console_sigwinch(SIGWINCH);
> > > +	return lxc_cmd_rsp_send(fd, &rsp);
> > > +}
> > > +
> > > +/*
> > >   * lxc_cmd_console: Open an fd to a tty in the container
> > >   *
> > >   * @name           : name of container to connect to
> > > @@ -599,39 +631,21 @@ static int lxc_cmd_console_callback(int fd,
> > > struct lxc_cmd_req *req, struct lxc_handler *handler)
> > >  {
> > >  	int ttynum = PTR_TO_INT(req->data);
> > > -	struct lxc_tty_info *tty_info = &handler->conf->tty_info;
> > > +	int masterfd;
> > >  	struct lxc_cmd_rsp rsp;
> > >  
> > > -	if (ttynum > 0) {
> > > -		if (ttynum > tty_info->nbtty)
> > > -			goto out_close;
> > > -
> > > -		if (tty_info->pty_info[ttynum - 1].busy)
> > > -			goto out_close;
> > > -
> > > -		/* the requested tty is available */
> > > -		goto out_send;
> > > -	}
> > > -
> > > -	/* search for next available tty, fixup index tty1 => [0]
> > > */
> > > -	for (ttynum = 1;
> > > -	     ttynum <= tty_info->nbtty &&
> > > tty_info->pty_info[ttynum - 1].busy;
> > > -	     ttynum++);
> > > -
> > > -	/* we didn't find any available slot for tty */
> > > -	if (ttynum > tty_info->nbtty)
> > > +	masterfd = lxc_console_allocate(handler->conf, fd,
> > > &ttynum);
> > > +	if (masterfd < 0)
> > >  		goto out_close;
> > >  
> > > -out_send:
> > >  	memset(&rsp, 0, sizeof(rsp));
> > >  	rsp.data = INT_TO_PTR(ttynum);
> > > -	if (lxc_af_unix_send_fd(fd, tty_info->pty_info[ttynum -
> > > 1].master,
> > > -				&rsp, sizeof(rsp)) < 0) {
> > > +	if (lxc_af_unix_send_fd(fd, masterfd, &rsp, sizeof(rsp)) <
> > > 0) { ERROR("failed to send tty to client");
> > > +		lxc_console_free(handler->conf, fd);
> > >  		goto out_close;
> > >  	}
> > >  
> > > -	tty_info->pty_info[ttynum - 1].busy = fd;
> > >  	return 0;
> > >  
> > >  out_close:
> > > @@ -650,6 +664,7 @@ static int lxc_cmd_process(int fd, struct
> > > lxc_cmd_req *req, 
> > >  	callback cb[LXC_CMD_MAX] = {
> > >  		[LXC_CMD_CONSOLE]         =
> > > lxc_cmd_console_callback,
> > > +		[LXC_CMD_CONSOLE_WINCH]   =
> > > lxc_cmd_console_winch_callback, [LXC_CMD_STOP]            =
> > > lxc_cmd_stop_callback, [LXC_CMD_GET_STATE]       =
> > > lxc_cmd_get_state_callback, [LXC_CMD_GET_INIT_PID]    =
> > > lxc_cmd_get_init_pid_callback, @@ -668,8 +683,7 @@ static int
> > > lxc_cmd_process(int fd, struct lxc_cmd_req *req, static void
> > > lxc_cmd_fd_cleanup(int fd, struct lxc_handler *handler, struct
> > > lxc_epoll_descr *descr) {
> > > -	extern void lxc_console_remove_fd(int, struct lxc_tty_info
> > > *);
> > > -	lxc_console_remove_fd(fd, &handler->conf->tty_info);
> > > +	lxc_console_free(handler->conf, fd);
> > >  	lxc_mainloop_del_handler(descr, fd);
> > >  	close(fd);
> > >  }
> > > diff --git a/src/lxc/commands.h b/src/lxc/commands.h
> > > index c3738cd..46806cb 100644
> > > --- a/src/lxc/commands.h
> > > +++ b/src/lxc/commands.h
> > > @@ -34,6 +34,7 @@
> > >  
> > >  typedef enum {
> > >  	LXC_CMD_CONSOLE,
> > > +	LXC_CMD_CONSOLE_WINCH,
> > >  	LXC_CMD_STOP,
> > >  	LXC_CMD_GET_STATE,
> > >  	LXC_CMD_GET_INIT_PID,
> > > @@ -65,6 +66,7 @@ struct lxc_cmd_console_rsp_data {
> > >  	int ttynum;
> > >  };
> > >  
> > > +extern int lxc_cmd_console_winch(const char *name, const char
> > > *lxcpath); extern int lxc_cmd_console(const char *name, int
> > > *ttynum, int *fd, const char *lxcpath);
> > >  extern char *lxc_cmd_get_cgroup_path(const char *name, const char
> > > *lxcpath); diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > index 5700eff..90aea60 100644
> > > --- a/src/lxc/conf.c
> > > +++ b/src/lxc/conf.c
> > > @@ -1293,8 +1293,8 @@ static int setup_dev_console(const struct
> > > lxc_rootfs *rootfs, return 0;
> > >  	}
> > >  
> > > -	if (console->peer == -1) {
> > > -		INFO("no console output required");
> > > +	if (console->master < 0) {
> > > +		INFO("no console");
> > >  		return 0;
> > >  	}
> > >  
> > > @@ -1359,8 +1359,8 @@ static int setup_ttydir_console(const struct
> > > lxc_rootfs *rootfs, if (ret >= 0)
> > >  		close(ret);
> > >  
> > > -	if (console->peer == -1) {
> > > -		INFO("no console output required");
> > > +	if (console->master < 0) {
> > > +		INFO("no console");
> > >  		return 0;
> > >  	}
> > >  
> > > @@ -2127,6 +2127,9 @@ struct lxc_conf *lxc_conf_init(void)
> > >  	new->console.log_fd = -1;
> > >  	new->console.path = NULL;
> > >  	new->console.peer = -1;
> > > +	new->console.peerpty.busy = -1;
> > > +	new->console.peerpty.master = -1;
> > > +	new->console.peerpty.slave = -1;
> > >  	new->console.master = -1;
> > >  	new->console.slave = -1;
> > >  	new->console.name[0] = '\0';
> > > diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> > > index a2efa7c..2fd3ab1 100644
> > > --- a/src/lxc/conf.h
> > > +++ b/src/lxc/conf.h
> > > @@ -188,6 +188,8 @@ struct lxc_tty_info {
> > >  	struct lxc_pty_info *pty_info;
> > >  };
> > >  
> > > +struct lxc_tty_state;
> > > +
> > >  /*
> > >   * Defines the structure to store the console information
> > >   * @peer   : the file descriptor put/get console traffic
> > > @@ -197,11 +199,14 @@ struct lxc_console {
> > >  	int slave;
> > >  	int master;
> > >  	int peer;
> > > +	struct lxc_pty_info peerpty;
> > > +	struct lxc_epoll_descr *descr;
> > >  	char *path;
> > >  	char *log_path;
> > >  	int log_fd;
> > >  	char name[MAXPATHLEN];
> > >  	struct termios *tios;
> > > +	struct lxc_tty_state *tty_state;
> > >  };
> > >  
> > >  /*
> > > diff --git a/src/lxc/console.c b/src/lxc/console.c
> > > index 93c16b5..3720c5b 100644
> > > --- a/src/lxc/console.c
> > > +++ b/src/lxc/console.c
> > > @@ -21,6 +21,8 @@
> > >   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
> > > 02111-1307 USA */
> > >  
> > > +#include <assert.h>
> > > +#include <signal.h>
> > >  #include <stdio.h>
> > >  #include <stdlib.h>
> > >  #include <unistd.h>
> > > @@ -29,6 +31,7 @@
> > >  #include <sys/types.h>
> > >  #include <termios.h>
> > >  
> > > +#include "lxccontainer.h"
> > >  #include "log.h"
> > >  #include "conf.h"
> > >  #include "config.h"
> > > @@ -37,6 +40,8 @@
> > >  #include "commands.h"
> > >  #include "mainloop.h"
> > >  #include "af_unix.h"
> > > +#include "lxclock.h"
> > > +#include "utils.h"
> > >  
> > >  #if HAVE_PTY_H
> > >  #include <pty.h>
> > > @@ -46,156 +51,490 @@
> > >  
> > >  lxc_log_define(lxc_console, lxc);
> > >  
> > > -extern void lxc_console_remove_fd(int fd, struct lxc_tty_info
> > > *tty_info) -{
> > > -	int i;
> > > +static struct lxc_list lxc_ttys;
> > >  
> > > -	for (i = 0; i < tty_info->nbtty; i++) {
> > > -
> > > -		if (tty_info->pty_info[i].busy != fd)
> > > -			continue;
> > > +typedef void (*sighandler_t)(int);
> > > +struct lxc_tty_state
> > > +{
> > > +	struct lxc_list node;
> > > +	int stdinfd;
> > > +	int stdoutfd;
> > > +	int masterfd;
> > > +	int escape;
> > > +	int saw_escape;
> > > +	const char *winch_proxy;
> > > +	const char *winch_proxy_lxcpath;
> > > +	int sigfd;
> > > +	sigset_t oldmask;
> > > +};
> > > +
> > > +__attribute__((constructor))
> > > +void lxc_console_init(void)
> > > +{
> > > +	lxc_list_init(&lxc_ttys);
> > > +}
> > >  
> > > -		tty_info->pty_info[i].busy = 0;
> > > +/* lxc_console_winsz: propagte winsz from one terminal to another
> > > + *
> > > + * @srcfd : terminal to get size from (typically a slave pty)
> > > + * @dstfd : terminal to set size on (typically a master pty)
> > > + */
> > > +static void lxc_console_winsz(int srcfd, int dstfd)
> > > +{
> > > +	struct winsize wsz;
> > > +	if (isatty(srcfd) && ioctl(srcfd, TIOCGWINSZ, &wsz) == 0) {
> > > +		DEBUG("set winsz dstfd:%d cols:%d rows:%d", dstfd,
> > > +		      wsz.ws_col, wsz.ws_row);
> > > +		ioctl(dstfd, TIOCSWINSZ, &wsz);
> > >  	}
> > > +}
> > >  
> > > -	return;
> > > +static void lxc_console_winch(struct lxc_tty_state *ts)
> > > +{
> > > +	lxc_console_winsz(ts->stdinfd, ts->masterfd);
> > > +	if (ts->winch_proxy) {
> > > +		lxc_cmd_console_winch(ts->winch_proxy,
> > > +				      ts->winch_proxy_lxcpath);
> > > +	}
> > >  }
> > >  
> > > -static int get_default_console(char **console)
> > > +void lxc_console_sigwinch(int sig)
> > >  {
> > > -	int fd;
> > > +	if (process_lock() == 0) {
> > > +		struct lxc_list *it;
> > > +		struct lxc_tty_state *ts;
> > >  
> > > -	if (!access("/dev/tty", F_OK)) {
> > > -		fd = open("/dev/tty", O_RDWR);
> > > -		if (fd >= 0) {
> > > -			close(fd);
> > > -			*console = strdup("/dev/tty");
> > > -			goto out;
> > > +		lxc_list_for_each(it, &lxc_ttys) {
> > > +			ts = it->elem;
> > > +			lxc_console_winch(ts);
> > >  		}
> > > +		process_unlock();
> > >  	}
> > > +}
> > >  
> > > -	if (!access("/dev/null", F_OK)) {
> > > -		*console = strdup("/dev/null");
> > > -		goto out;
> > > +static int lxc_console_cb_sigwinch_fd(int fd, void *cbdata,
> > > +				      struct lxc_epoll_descr
> > > *descr) +{
> > > +	struct signalfd_siginfo siginfo;
> > > +	struct lxc_tty_state *ts = cbdata;
> > > +
> > > +	if (read(fd, &siginfo, sizeof(siginfo)) < 0) {
> > > +		ERROR("failed to read signal info");
> > > +		return -1;
> > >  	}
> > >  
> > > -	ERROR("No suitable default console");
> > > +	lxc_console_winch(ts);
> > > +	return 0;
> > > +}
> > > +
> > > +/*
> > > + * lxc_console_sigwinch_init: install SIGWINCH handler
> > > + *
> > > + * @srcfd  : src for winsz in SIGWINCH handler
> > > + * @dstfd  : dst for winsz in SIGWINCH handler
> > > + *
> > > + * Returns lxc_tty_state structure on success or NULL on failure.
> > > The sigfd
> > > + * member of the returned lxc_tty_state can be
> > > select()/poll()ed/epoll()ed
> > > + * on (ie added to a mainloop) for SIGWINCH.
> > > + *
> > > + * Must be called with process_lock held to protect the lxc_ttys
> > > list, or
> > > + * from a non-threaded context.
> > 
> > Would it be worth putting lxc_ttys list in lxc_container struct,
> > passing in the lxc_container and doing a mem_lock?
> 
> I'm not sure that makes sense, the lxc_ttys list is per process. It
> exists so that when the process is the recipient of an
> LXC_CMD_CONSOLE_WINCH it can iterate this list and update the size of
> any terminals it has open. I don't think there is any existing list of
> all containers the process may have open, but I guess we could do that
> instead and then the lxc_ttys list could be hung off the lxc_container
> struct, but I'm not sure that is what you were suggesting :)

Nope.

> To clarify, the LXC_CMD_CONSOLE_WINCH mechanism is only used in the
> case of /dev/console peer proxy pty, ie. when you start out without
> specifying a console (lxc-start -d) and then attach to the console
> afterwards (lxc-console -t 0). Its purpose is to forward the winch from
> the peer proxy pty to the real (console) pty.

All right, sounds like it was just a bad idea then.  I should have
looked more closely at the lxc_ttys.  (It was a gut reaction to the
rather long process_lock() time, which I wasn't 100% comfortable
with.)

thanks,
-serge




More information about the lxc-devel mailing list