[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