Quoting Dwight Engen (dwight.en...@oracle.com):
> Make lxc_cmd_console() return the fd from the socket connection to the
> caller. This fd keeps the tty slot allocated until the caller closes
> it. Returning the fd allows for a long lived process to close the fd
> and reuse consoles.
>
> Add API function for console allocation.
>
> Create test program for console API.
>
> Signed-off-by: Dwight Engen <dwight.en...@oracle.com>
It looks good and tests fine, so overall
Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>
However, do you think it would be better to call this function
lxcapi_console_getfd(), and have lxcapi_console() be a higher
level function which actually runs the lxc_mainloop() the
way lxc_console() does, either binding to the caller's fds
0,1,2, or to 3 passed-in fds?
> ---
> .gitignore | 6 +-
> src/lxc/commands.c | 44 +++++++-----
> src/lxc/commands.h | 2 +-
> src/lxc/lxc_console.c | 9 ++-
> src/lxc/lxccontainer.c | 11 +++
> src/lxc/lxccontainer.h | 15 +++++
> src/tests/Makefile.am | 6 +-
> src/tests/console.c | 177
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 8 files changed, 245 insertions(+), 25 deletions(-)
> create mode 100644 src/tests/console.c
>
> diff --git a/.gitignore b/.gitignore
> index f9d17cb..335ae08 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -29,6 +29,7 @@ templates/lxc-alpine
> templates/lxc-altlinux
> templates/lxc-archlinux
> templates/lxc-busybox
> +templates/lxc-cirros
> templates/lxc-debian
> templates/lxc-fedora
> templates/lxc-opensuse
> @@ -71,17 +72,20 @@ src/lxc/legacy/lxc-ls
> src/python-lxc/build/
> src/python-lxc/lxc/__pycache__/
>
> +src/tests/lxc-test-cgpath
> +src/tests/lxc-test-clonetest
> +src/tests/lxc-test-console
> src/tests/lxc-test-containertests
> src/tests/lxc-test-createtest
> src/tests/lxc-test-destroytest
> src/tests/lxc-test-get_item
> src/tests/lxc-test-getkeys
> src/tests/lxc-test-locktests
> +src/tests/lxc-test-lxcpath
> src/tests/lxc-test-saveconfig
> src/tests/lxc-test-shutdowntest
> src/tests/lxc-test-startone
>
> -
> config/compile
> config/config.guess
> config/config.sub
> diff --git a/src/lxc/commands.c b/src/lxc/commands.c
> index 169914e..b4afc07 100644
> --- a/src/lxc/commands.c
> +++ b/src/lxc/commands.c
> @@ -115,7 +115,8 @@ static const char *lxc_cmd_str(lxc_cmd_t cmd)
> * stored directly in data and datalen will be 0.
> *
> * As a special case, the response for LXC_CMD_CONSOLE is created
> - * here as it contains an fd passed through the unix socket.
> + * here as it contains an fd for the master pty passed through the
> + * unix socket.
> */
> static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd)
> {
> @@ -132,13 +133,19 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr
> *cmd)
> if (cmd->req.cmd == LXC_CMD_CONSOLE) {
> struct lxc_cmd_console_rsp_data *rspdata;
>
> + /* recv() returns 0 bytes when a tty cannot be allocated,
> + * rsp->ret is < 0 when the peer permission check failed
> + */
> + if (ret == 0 || rsp->ret < 0)
> + return 0;
> +
> rspdata = malloc(sizeof(*rspdata));
> if (!rspdata) {
> ERROR("command %s couldn't allocate response buffer",
> lxc_cmd_str(cmd->req.cmd));
> return -1;
> }
> - rspdata->fd = rspfd;
> + rspdata->masterfd = rspfd;
> rspdata->ttynum = PTR_TO_INT(rsp->data);
> rsp->data = rspdata;
> }
> @@ -214,8 +221,9 @@ static int lxc_cmd_rsp_send(int fd, struct lxc_cmd_rsp
> *rsp)
> * the fd cannot be closed because it is used as a placeholder to indicate
> * that a particular tty slot is in use. The fd is also used as a signal to
> * the container that when the caller dies or closes the fd, the container
> - * will notice the fd in its mainloop select and then free the slot with
> - * lxc_cmd_fd_cleanup().
> + * will notice the fd on its side of the socket in its mainloop select and
> + * then free the slot with lxc_cmd_fd_cleanup(). The socket fd will be
> + * returned in the cmd response structure.
> */
> static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped,
> const char *lxcpath)
> @@ -262,8 +270,10 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr
> *cmd, int *stopped,
>
> ret = lxc_cmd_rsp_recv(sock, cmd);
> out:
> - if (!stay_connected || ret < 0)
> + if (!stay_connected || ret <= 0)
> close(sock);
> + if (stay_connected && ret > 0)
> + cmd->rsp.ret = sock;
>
> return ret;
> }
> @@ -544,7 +554,7 @@ static int lxc_cmd_stop_callback(int fd, struct
> lxc_cmd_req *req,
> * @fd : out: file descriptor for master side of pty
> * @lxcpath : the lxcpath in which the container is running
> *
> - * Returns 0 on success, < 0 on failure
> + * Returns fd holding tty allocated on success, < 0 on failure
> */
> int lxc_cmd_console(const char *name, int *ttynum, int *fd, const char
> *lxcpath)
> {
> @@ -557,31 +567,29 @@ int lxc_cmd_console(const char *name, int *ttynum, int
> *fd, const char *lxcpath)
> ret = lxc_cmd(name, &cmd, &stopped, lxcpath);
> if (ret < 0)
> return ret;
> - if (ret == 0) {
> - ERROR("console %d invalid or all consoles busy", *ttynum);
> +
> + if (cmd.rsp.ret < 0) {
> + ERROR("console access denied: %s", strerror(-cmd.rsp.ret));
> ret = -1;
> goto out;
> }
>
> - ret = -1;
> - #if 1 /* FIXME: how can this happen? */
> - if (cmd.rsp.ret) {
> - ERROR("console access denied: %s",
> - strerror(-cmd.rsp.ret));
> + if (ret == 0) {
> + ERROR("console %d invalid,busy or all consoles busy", *ttynum);
> + ret = -1;
> goto out;
> }
> - #endif
>
> rspdata = cmd.rsp.data;
> - if (rspdata->fd < 0) {
> + if (rspdata->masterfd < 0) {
> ERROR("unable to allocate fd for tty %d", rspdata->ttynum);
> goto out;
> }
>
> - ret = 0;
> - *fd = rspdata->fd;
> + ret = cmd.rsp.ret; /* sock fd */
> + *fd = rspdata->masterfd;
> *ttynum = rspdata->ttynum;
> - INFO("tty %d allocated", rspdata->ttynum);
> + INFO("tty %d allocated fd %d sock %d", rspdata->ttynum, *fd, ret);
> out:
> free(cmd.rsp.data);
> return ret;
> diff --git a/src/lxc/commands.h b/src/lxc/commands.h
> index 08bde9c..c3738cd 100644
> --- a/src/lxc/commands.h
> +++ b/src/lxc/commands.h
> @@ -61,7 +61,7 @@ struct lxc_cmd_rr {
> };
>
> struct lxc_cmd_console_rsp_data {
> - int fd;
> + int masterfd;
> int ttynum;
> };
>
> diff --git a/src/lxc/lxc_console.c b/src/lxc/lxc_console.c
> index 820794a..efc8919 100644
> --- a/src/lxc/lxc_console.c
> +++ b/src/lxc/lxc_console.c
> @@ -184,7 +184,7 @@ static int master_handler(int fd, void *data, struct
> lxc_epoll_descr *descr)
>
> int main(int argc, char *argv[])
> {
> - int err, std_in = 1;
> + int err, ttyfd, std_in = 1;
> struct lxc_epoll_descr descr;
> struct termios newtios, oldtios;
>
> @@ -203,9 +203,11 @@ int main(int argc, char *argv[])
> return -1;
> }
>
> - err = lxc_cmd_console(my_args.name, &my_args.ttynum, &master,
> my_args.lxcpath[0]);
> - if (err)
> + ttyfd = lxc_cmd_console(my_args.name, &my_args.ttynum, &master,
> my_args.lxcpath[0]);
> + if (ttyfd < 0) {
> + err = ttyfd;
> goto out;
> + }
>
> fprintf(stderr, "\n\
> Connected to tty %1$d\n\
> @@ -249,6 +251,7 @@ Type <Ctrl+%2$c q> to exit the console, \
> goto out_mainloop_open;
> }
>
> + close(ttyfd);
> err = 0;
>
> out_mainloop_open:
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 2ea9556..af60f0b 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -350,6 +350,16 @@ static bool lxcapi_unfreeze(struct lxc_container *c)
> return true;
> }
>
> +static int lxcapi_console(struct lxc_container *c, int *ttynum, int
> *masterfd)
> +{
> + int ttyfd;
> + if (!c)
> + return -1;
> +
> + ttyfd = lxc_cmd_console(c->name, ttynum, masterfd, c->config_path);
> + return ttyfd;
> +}
> +
> static pid_t lxcapi_init_pid(struct lxc_container *c)
> {
> if (!c)
> @@ -1979,6 +1989,7 @@ struct lxc_container *lxc_container_new(const char
> *name, const char *configpath
> c->is_running = lxcapi_is_running;
> c->freeze = lxcapi_freeze;
> c->unfreeze = lxcapi_unfreeze;
> + c->console = lxcapi_console;
> c->init_pid = lxcapi_init_pid;
> c->load_config = lxcapi_load_config;
> c->want_daemonize = lxcapi_want_daemonize;
> diff --git a/src/lxc/lxccontainer.h b/src/lxc/lxccontainer.h
> index bd3d22a..5078f03 100644
> --- a/src/lxc/lxccontainer.h
> +++ b/src/lxc/lxccontainer.h
> @@ -115,6 +115,21 @@ struct lxc_container {
> const char *lxcpath, int flags, const char *bdevtype,
> const char *bdevdata, unsigned long newsize, char **hookargs);
>
> + /* lxcapi_console: allocate a console tty from container @c
> + *
> + * @c : the running container
> + * @ttynum : in : tty number to attempt to allocate or -1 to
> + * allocate the first available tty
> + * out: the tty number that was allocated
> + * @masterfd : out: fd refering to the master side of pty
> + *
> + * Returns "ttyfd" on success, -1 on failure. The returned "ttyfd" is
> + * used to keep the tty allocated. The caller should close "ttyfd" to
> + * indicate that it is done with the allocated console so that it can
> + * be allocated by another caller.
> + */
> + int (*console)(struct lxc_container *c, int *ttynum, int *masterfd);
> +
> #if 0
> bool (*commit_cgroups)(struct lxc_container *c);
> bool (*reread_cgroups)(struct lxc_container *c);
> diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
> index c0ce648..ba24d2c 100644
> --- a/src/tests/Makefile.am
> +++ b/src/tests/Makefile.am
> @@ -14,6 +14,7 @@ lxc_test_getkeys_SOURCES = getkeys.c
> lxc_test_lxcpath_SOURCES = lxcpath.c
> lxc_test_cgpath_SOURCES = cgpath.c
> lxc_test_clonetest_SOURCES = clonetest.c
> +lxc_test_console_SOURCES = console.c
>
> AM_CFLAGS=-I$(top_srcdir)/src \
> -DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \
> @@ -24,7 +25,7 @@ AM_CFLAGS=-I$(top_srcdir)/src \
> bin_PROGRAMS = lxc-test-containertests lxc-test-locktests lxc-test-startone \
> lxc-test-destroytest lxc-test-saveconfig lxc-test-createtest \
> lxc-test-shutdowntest lxc-test-get_item lxc-test-getkeys
> lxc-test-lxcpath \
> - lxc-test-cgpath lxc-test-clonetest
> + lxc-test-cgpath lxc-test-clonetest lxc-test-console
>
> endif
>
> @@ -40,4 +41,5 @@ EXTRA_DIST = \
> saveconfig.c \
> shutdowntest.c \
> clonetest.c \
> - startone.c
> + startone.c \
> + console.c
> diff --git a/src/tests/console.c b/src/tests/console.c
> new file mode 100644
> index 0000000..a3e7bca
> --- /dev/null
> +++ b/src/tests/console.c
> @@ -0,0 +1,177 @@
> +/* liblxcapi
> + *
> + * Copyright © 2013 Oracle.
> + *
> + * Authors:
> + * Dwight Engen <dwight.en...@oracle.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include "../lxc/lxccontainer.h"
> +
> +#include <errno.h>
> +#include <unistd.h>
> +
> +#define TTYCNT 4
> +#define TTYCNT_STR "4"
> +#define TSTNAME "lxcconsoletest"
> +#define MAXCONSOLES 512
> +
> +#define TSTERR(fmt, ...) do { \
> + fprintf(stderr, "%s:%d " fmt "\n", __FILE__, __LINE__, ##__VA_ARGS__); \
> +} while (0)
> +
> +static void test_console_close_all(int ttyfd[MAXCONSOLES],
> + int masterfd[MAXCONSOLES])
> +{
> + int i;
> +
> + for (i = 0; i < MAXCONSOLES; i++) {
> + if (masterfd[i] != -1) {
> + close(masterfd[i]);
> + masterfd[i] = -1;
> + }
> + if (ttyfd[i] != -1) {
> + close(ttyfd[i]);
> + ttyfd[i] = -1;
> + }
> + }
> +}
> +
> +static int test_console_running_container(struct lxc_container *c)
> +{
> + int nrconsoles, i, ret = -1;
> + int ttynum [MAXCONSOLES];
> + int ttyfd [MAXCONSOLES];
> + int masterfd[MAXCONSOLES];
> +
> + for (i = 0; i < MAXCONSOLES; i++)
> + ttynum[i] = ttyfd[i] = masterfd[i] = -1;
> +
> + ttynum[0] = 1;
> + ret = c->console(c, &ttynum[0], &masterfd[0]);
> + if (ret < 0) {
> + TSTERR("console allocate failed");
> + goto err1;
> + }
> + ttyfd[0] = ret;
> + if (ttynum[0] != 1) {
> + TSTERR("console allocate got bad ttynum %d", ttynum[0]);
> + goto err2;
> + }
> +
> + /* attempt to alloc same ttynum */
> + ret = c->console(c, &ttynum[0], &masterfd[1]);
> + if (ret != -1) {
> + TSTERR("console allocate should fail for allocated ttynum %d",
> ttynum[0]);
> + goto err2;
> + }
> + close(masterfd[0]); masterfd[0] = -1;
> + close(ttyfd[0]); ttyfd[0] = -1;
> +
> + /* ensure we can allocate all consoles, we do this a few times to
> + * show that the closes are freeing up the allocated slots
> + */
> + for (i = 0; i < 10; i++) {
> + for (nrconsoles = 0; nrconsoles < MAXCONSOLES; nrconsoles++) {
> + ret = c->console(c, &ttynum[nrconsoles],
> &masterfd[nrconsoles]);
> + if (ret < 0)
> + break;
> + ttyfd[nrconsoles] = ret;
> + }
> + if (nrconsoles != TTYCNT) {
> + TSTERR("didn't allocate all consoles %d != %d",
> nrconsoles, TTYCNT);
> + goto err2;
> + }
> + test_console_close_all(ttyfd, masterfd);
> + }
> + ret = 0;
> +
> +err2:
> + test_console_close_all(ttyfd, masterfd);
> +err1:
> + return ret;
> +}
> +
> +/* test_container: test console function
> + *
> + * @lxcpath : the lxcpath in which to create the container
> + * @group : name of the container group or NULL for default "lxc"
> + * @name : name of the container
> + * @template : template to use when creating the container
> + */
> +static int test_console(const char *lxcpath,
> + const char *group, const char *name,
> + const char *template)
> +{
> + int ret;
> + struct lxc_container *c = NULL;
> +
> + if (lxcpath) {
> + ret = mkdir(lxcpath, 0755);
> + if (ret < 0 && errno != EEXIST) {
> + TSTERR("failed to mkdir %s %s", lxcpath,
> strerror(errno));
> + goto out1;
> + }
> + }
> + ret = -1;
> +
> + if ((c = lxc_container_new(name, lxcpath)) == NULL) {
> + TSTERR("instantiating container %s", name);
> + goto out1;
> + }
> + if (c->is_defined(c)) {
> + c->stop(c);
> + c->destroy(c);
> + c = lxc_container_new(name, lxcpath);
> + }
> + if (!c->createl(c, template, NULL, NULL, NULL)) {
> + TSTERR("creating container %s", name);
> + goto out2;
> + }
> + c->load_config(c, NULL);
> + c->set_config_item(c, "lxc.tty", TTYCNT_STR);
> + c->save_config(c, NULL);
> + c->want_daemonize(c);
> + if (!c->startl(c, 0, NULL)) {
> + TSTERR("starting container %s", name);
> + goto out3;
> + }
> +
> + ret = test_console_running_container(c);
> +
> + c->stop(c);
> +out3:
> + c->destroy(c);
> +out2:
> + lxc_container_put(c);
> +out1:
> + return ret;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + int ret;
> + ret = test_console(NULL, NULL, TSTNAME, "busybox");
> + if (ret < 0)
> + goto err1;
> +
> + ret = test_console("/var/lib/lxctest2", NULL, TSTNAME, "busybox");
> + if (ret < 0)
> + goto err1;
> + printf("All tests passed\n");
> +err1:
> + return ret;
> +}
> --
> 1.8.1.4
>
>
> ------------------------------------------------------------------------------
> Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
> Get 100% visibility into your production application - at no cost.
> Code-level diagnostics for performance bottlenecks with <2% overhead
> Download for free and get started troubleshooting in minutes.
> http://p.sf.net/sfu/appdyn_d2d_ap1
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel
------------------------------------------------------------------------------
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with <2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel
