[Devel] [RFC rh7 v3] ve/tty: vt -- Implement per VE support for console and terminals

Vladimir Davydov vdavydov at parallels.com
Mon Aug 17 09:33:36 PDT 2015


On Fri, Aug 07, 2015 at 01:04:49PM +0300, Cyrill Gorcunov wrote:
> Previously in commit 8674c044330fad1458bd59b02f9037fb97e8b7af stubs for
> virtual terminals have been added, they support writes from kernel side
> which simply drops into the void.
> 
> In the patch the code has been moved from kernel/ve/console.c
> to drivers/tty/pty.c to reuse a couple of pty helpers.
> 
> Now we support up to MAX_NR_VTTY_CONSOLES virtual consoles inside container.
> For /dev/console we reserve the first virtual terminal.
> 
> Some details on the driver itself:
> 
>  - The drivers carries per-VE tty instances in @vtty_idr map, once
>    VE tries to open a terminal we allocate tty map internally and
>    keep it intact until VE destructed, this allow us to not bind
>    into device namespaces (ie not rely on tty_class);
> 
>  - Unlike buffered IO to unix98 driver once internal port buffer
>    get full we don't block write operations if there is no reader
>    assigned yet but zap them. This is done intentionally to behave
>    closely to native consoles;
> 
>  - The kernel choose which VE request terminal using get_exec_env
>    helper, but for opening master peer from the nodes ve0 it uses
>    vtty_set_context/vtty_get_context/vtty_drop_context to notify
>    tty layer which @vtty_idr to use instead of get_exec_env.
> 
> https://jira.sw.ru/browse/PSBM-34533
> https://jira.sw.ru/browse/PSBM-34532
> https://jira.sw.ru/browse/PSBM-34107
> https://jira.sw.ru/browse/PSBM-32686
> https://jira.sw.ru/browse/PSBM-32685
> 
> v2:
>  - Rename terminals from vtz to vtty
>  - Merge code into /drivers/tty/pty.c to reuse some of
>    pty functionality
>  - Get rid of two array of indices, use one for master
>    peers and fetch slaves via @link
>  - Drop TTY_VT_OPEN and wait() on it
>  - Add vtty_open_slave helper
> 
> v3:
>  - Reverse the scheme, the peers opened from inside of
>    container are the slave peers as it were in pcs6
>  - Add vtty_set_context/vtty_drop_context/vtty_get_context
>    to open needed tty from ve0 context
>  - In vtty_open_master reuse existing vtty_lookup, vtty_open
>    helpers
>  - In ve_vtty_fini zap active tty tracking, such ttys are
>    sitting here because the node has been opening the console
>    and didn't release file descriptors yet with tty associated.
>    The kernel will clean them up once they are closed but the
>    tacking map pointer should be zapped to escape nil dereference
> 
> FIXME: Once this is applied need to drop kernel/ve/console.c
> from the source tree, dropping it immediately ruines my queue
> series, because there are other patches not yet merged but
> changing kernel/ve/console.c code.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>
> CC: Vladimir Davydov <vdavydov at virtuozzo.com>
> CC: Konstantin Khorenko <khorenko at virtuozzo.com>
> CC: Pavel Emelyanov <xemul at virtuozzo.com>
> CC: Andrey Vagin <avagin at virtuozzo.com>
> CC: Igor Sukhih <igor at parallels.com>
> ---
> 
> Vladimir, take a look please, once time permit. The problems
> I met which forced me to rewrite the code being far from pcs6
> variant:
>  - no owner_ve in tty_struct
>  - termios no longer dynamically allocated
>  - tty-write now takes dif arguments type
>  - can't use alloc_tty_driver anymore, since cdevs are dynamic
>  - tty-port no loner static
> 
> Hope I addressed all your concerns but still might be missing
> somehting since it was really huge thread.
> 
>  drivers/tty/pty.c    |  580 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/tty/tty_io.c |   29 --
>  include/linux/tty.h  |    3 
>  include/linux/ve.h   |   28 +-
>  kernel/ve/Makefile   |    2 
>  kernel/ve/vecalls.c  |    3 
>  6 files changed, 612 insertions(+), 33 deletions(-)
> 
> Index: linux-pcs7.git/drivers/tty/pty.c
> ===================================================================
> --- linux-pcs7.git.orig/drivers/tty/pty.c
> +++ linux-pcs7.git/drivers/tty/pty.c
> @@ -820,10 +820,590 @@ static void __init unix98_pty_init(void)
>  static inline void unix98_pty_init(void) { }
>  #endif
>  
> +#if defined(CONFIG_VE) && defined(CONFIG_UNIX98_PTYS)
> +
> +/*
> + * VTTY architecture overview.
> + *
> + * With VTTY we make /dev/console and /dev/tty[X] virtualized
> + * per container (note the real names may vary because the
> + * kernel itself uses major:minor numbers to distinguish
> + * devices and doesn't care how they are named inside /dev.
> + * /dev/console stands for TTYAUX_MAJOR:1 while /dev/tty[X]
> + * stands for TTY_MAJOR:[0:12]. That said from inside of
> + * VTTY /dev/console is the same as /dev/tty0.
> + *
> + * For every container here is a tty map represented by
> + * vtty_map_t. It carries @veid of VE and associated slave
> + * tty peers.
> + *
> + * map
> + *  veid -> CTID
> + *    vttys -> [ 0 ]
> + *               `- @slave -> link -> @master
> + *             [ 1 ]
> + *               `- @slave -> link -> @master
> + *
> + * When container is opening tty by self (init process
> + * or whatever) we create @slave peer and a linked master,
> + * and carry them inside map until the kernel explicitly
> + * close them.
> + *
> + * There is one exception though: connecting into container's
> + * console from the node. For such case we lookup for existing
> + * slave first and if it doesn't exist we allocate a new pair.
> + * The main problem is peer closing here: the container may
> + * be stopped while node still carries ttys. So we zap such
> + * pairs from tracking in fini() routine leaving the kernel
> + */
> +
> +#include <linux/ve.h>
> +#include <linux/file.h>
> +#include <linux/anon_inodes.h>
> +
> +static struct tty_driver *vttym_driver;
> +static struct tty_driver *vttys_driver;
> +static DEFINE_IDR(vtty_idr);

Nit: It would be nice to have a comment explaining synchronization rules
here. If vtty_idr is protected by a mutex (tty_mutex?), I would also
recommend inserting lockdep_assert_held to all the helpers for
accessing/modifying vtty_idr below.

> +
> +static struct file_operations vtty_fops;
> +
> +#undef VTTY_DEBUG
> +#ifdef VTTY_DEBUG
> +#define vtty_printk(fmt, ...)								\
> +	printk("vtty: %20s: %4d: " fmt,							\
> +	       __func__, __LINE__, ##__VA_ARGS__)
> +#define vtty_printk_one(__tty)								\
> +	vtty_printk("tty %p count %4d flags 0x%-8lx\n",					\
> +		    (__tty), (__tty)->count, (__tty)->flags)
> +#define vtty_printk_pair(__tty)								\
> +	vtty_printk("tty %p count %4d flags 0x%-8lx link %p count %4d flags 0x%-8lx\n",	\
> +		    (__tty), (__tty)->count, (__tty)->flags,				\
> +		    (__tty)->link, (__tty)->link ? (__tty)->link->count : -1,		\
> +		    (__tty)->link ? (__tty)->link->flags : -1ul)
> +#else
> +#define vtty_printk(fmt, ...)
> +#define vtty_printk_one(__tty)
> +#define vtty_printk_pair(__tty)

Nit:

if (A)
	vtt_printk(...);

won't compile if !VTTY_DEBUG. Use do { } while (0) stubs to overcome
this.

> +#endif
> +
> +#define vtty_match_index(idx)		((idx) >= 0 && (idx) < MAX_NR_VTTY_CONSOLES)
> +
> +typedef struct {
> +	envid_t			veid;
> +	struct tty_struct	*vttys[MAX_NR_VTTY_CONSOLES];
> +} vtty_map_t;
> +
> +static vtty_map_t *vtty_map_lookup(envid_t veid)
> +{
> +	return idr_find(&vtty_idr, veid);
> +}
> +
> +static void vtty_map_set(vtty_map_t *map, struct tty_struct *tty, int index)
> +{
> +	vtty_printk("map %p id %d tty %p index %d\n", map, map->veid, tty, index);
> +	map->vttys[index] = tty;
> +}
> +
> +static void vtty_map_del(vtty_map_t *map, struct tty_struct *tty, int index)
> +{
> +	vtty_printk("map %p id %d tty %p index %d\n", map, map ? map->veid : -1, tty, index);
> +	if (map)
> +		map->vttys[index] = NULL;
> +}
> +
> +static void vtty_map_free(vtty_map_t *map)
> +{
> +	vtty_printk("map %p id %d\n", map, map->veid);
> +	idr_remove(&vtty_idr, map->veid);
> +	kfree(map);
> +}
> +
> +static vtty_map_t *vtty_map_alloc(envid_t veid)
> +{
> +	vtty_map_t *map = kzalloc(sizeof(*map), GFP_KERNEL);
> +
> +	if (map) {
> +		map->veid = veid;
> +		veid = idr_alloc(&vtty_idr, map, veid, veid + 1, GFP_KERNEL);
> +		if (veid < 0) {
> +			kfree(map);
> +			return ERR_PTR(veid);
> +		}
> +	} else
> +		map = ERR_PTR(-ENOMEM);
> +
> +	vtty_printk("map %p id %d\n", map, veid);
> +	return map;
> +}
> +
> +static inline int vtty_is_exiting(struct tty_struct *tty)
> +{
> +	return test_bit(TTY_CLOSING, &tty->flags)		||
> +		test_bit(TTY_HUPPING, &tty->flags)		||
> +		test_bit(TTY_LDISC_CHANGING, &tty->flags);
> +}
> +
> +#define VTTY_USE_EXEC_VEID	(ULONG_MAX)
> +static unsigned long current_veid = VTTY_USE_EXEC_VEID;
> +
> +static void vtty_set_context(envid_t veid)
> +{
> +	WARN_ON_ONCE(!mutex_is_locked(&tty_mutex));

Nit: locked_assert_held

> +	current_veid = veid;
> +}
> +
> +static void vtty_drop_context(void)
> +{
> +	WARN_ON_ONCE(!mutex_is_locked(&tty_mutex));

ditto

> +	current_veid = VTTY_USE_EXEC_VEID;
> +}
> +
> +static envid_t vtty_get_context(void)
> +{
> +	BUILD_BUG_ON(sizeof(current_veid) < sizeof(envid_t));
> +	WARN_ON_ONCE(!mutex_is_locked(&tty_mutex));

ditto

> +
> +	if (likely(current_veid == VTTY_USE_EXEC_VEID))
> +		return get_exec_env()->veid;
> +
> +	return current_veid;
> +}
> +
> +static struct tty_struct *vtty_lookup(struct tty_driver *driver,
> +				      struct inode *inode, int idx)
> +{
> +	vtty_map_t *map = vtty_map_lookup(vtty_get_context());
> +	struct tty_struct *tty;
> +
> +	WARN_ON_ONCE((driver != vttym_driver) &&
> +		     (driver != vttys_driver));
> +
> +	if (!vtty_match_index(idx))
> +		return ERR_PTR(-EIO);
> +
> +	vtty_printk("driver %s index %d\n", driver->driver_name, idx);
> +	/*
> +	 * Nothing ever been opened yet, allocate a new
> +	 * tty map together with both peers from the scratch
> +	 * in install procedure.
> +	 */
> +	if (!map)
> +		return NULL;
> +
> +	/*
> +	 * FIXME: Strictly speaking there should not

Nit: s/FIXME/XXX :-)

> +	 * be requests for master peers from
> +	 * inside of container (ie lookup for
> +	 * vttym_driver). Even vtty_open_master
> +	 * may simply fetch tty->link by self
> +	 * after lookup for slave returned valid
> +	 * tty pointer. But I leave it this way
> +	 * simply because not sure yet how to
> +	 * c/r containers with live connection
> +	 * from nodes and this provides an easier
> +	 * way for testing.
> +	 */
> +	tty = map->vttys[idx];
> +	if (tty) {
> +		if (vtty_is_exiting(tty) ||
> +		    vtty_is_exiting(tty->link))
> +			tty = ERR_PTR(-ENXIO);
> +		else {
> +			if (driver == vttym_driver)
> +				tty = tty->link;
> +			WARN_ON(!tty);
> +		}
> +	}
> +	vtty_printk("tty %p count %4d\n",
> +		    tty, IS_ERR_OR_NULL(tty) ? -1 : tty->count);
> +	return tty;
> +}
> +
> +static void vtty_standard_install(vtty_map_t *map, struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	WARN_ON(tty_init_termios(tty));
> +
> +	tty_driver_kref_get(driver);
> +	tty->count++;
> +	tty->driver_data = map;
> +
> +	tty_port_init(tty->port);
> +	tty->port->itty = tty;
> +}
> +
> +static struct tty_struct *vtty_install_peer(vtty_map_t *map, struct tty_driver *driver, int index)
> +{
> +	struct tty_struct *tty;
> +
> +	tty = alloc_tty_struct();
> +	if (!tty)
> +		return ERR_PTR(-ENOMEM);
> +	initialize_tty_struct(tty, driver, index);
> +
> +	tty->port = kzalloc(sizeof(*tty->port), GFP_KERNEL);
> +	if (!tty->port) {
> +		deinitialize_tty_struct(tty);

Nit: Move port allocation before initialize_tty_struct and you won't
have to call deinitialize_tty_struct on error path.

> +		free_tty_struct(tty);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +	vtty_standard_install(map, driver, tty);
> +	return tty;
> +}
> +
> +static int vtty_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> +	envid_t veid = vtty_get_context();
> +	struct tty_struct *peer;
> +	vtty_map_t *map;
> +
> +	WARN_ON_ONCE(driver != vttys_driver);
> +
> +	map = vtty_map_lookup(veid);
> +	if (!map) {
> +		map = vtty_map_alloc(veid);
> +		if (IS_ERR(map))
> +			return PTR_ERR(map);
> +	}
> +
> +	tty->port = kzalloc(sizeof(*tty->port), GFP_KERNEL);
> +	if (!tty->port)
> +		return -ENOMEM;
> +
> +	peer = vtty_install_peer(map, vttym_driver, tty->index);
> +	if (IS_ERR(peer)) {
> +		vtty_map_del(map, tty, tty->index);
> +		kfree(tty->port);
> +		return PTR_ERR(peer);
> +	}
> +	set_bit(TTY_EXTRA_REF, &peer->flags);
> +
> +	vtty_standard_install(map, vttys_driver, tty);
> +	vtty_map_set(map, tty, tty->index);
> +
> +	tty->link = peer;
> +	peer->link = tty;
> +

tty->count++;

?

> +	vtty_printk_pair(tty);
> +	return 0;
> +}
> +
> +static int vtty_open(struct tty_struct *tty, struct file *filp)
> +{
> +	if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
> +		return -EIO;
> +
> +	clear_bit(TTY_IO_ERROR, &tty->flags);
> +	clear_bit(TTY_OTHER_CLOSED, &tty->link->flags);
> +	set_bit(TTY_THROTTLED, &tty->flags);
> +
> +	vtty_printk_pair(tty);
> +	return 0;
> +}
> +
> +static void vtty_close(struct tty_struct *tty, struct file *filp)
> +{
> +	struct tty_struct *peer = tty->link;
> +
> +	if (tty->count > 1)
> +		return;
> +
> +	if (test_bit(TTY_IO_ERROR, &tty->flags))
> +		return;
> 
> +	wake_up_interruptible(&tty->read_wait);
> +	wake_up_interruptible(&tty->write_wait);
> +
> +	wake_up_interruptible(&peer->read_wait);
> +	wake_up_interruptible(&peer->write_wait);
> +
> +	set_bit(TTY_IO_ERROR, &tty->flags);
> +	set_bit(TTY_OTHER_CLOSED, &peer->flags);
> +
> +	if (test_bit(TTY_EXTRA_REF, &peer->flags)) {
> +		clear_bit(TTY_EXTRA_REF, &peer->flags);
> +		peer->count--;
> +
> +		tty_unlock(tty);
> +		tty_vhangup(peer);
> +		tty_lock(tty);

Why do you think we need to do this?

> +	}
> +
> +	vtty_printk_pair(tty);
> +}
> +
> +static void vtty_shutdown(struct tty_struct *tty)
> +{
> +	vtty_printk_pair(tty);
> +	vtty_map_del(tty->driver_data, tty, tty->index);
> +}
> +
> +static void vtty_cleanup(struct tty_struct *tty)

I'd use pty_cleanup instead.

> +{
> +	/*
> +	 * Make sure line discipline is off already.
> +	 * It is just for debug reasons.
> +	 */
> +	WARN_ON_ONCE(!test_bit(TTY_LDISC_HALTED, &tty->flags));
> +	WARN_ON_ONCE(test_bit(TTY_LDISC, &tty->flags));
> +
> +	tty_port_put(tty->port);
> +	vtty_printk_one(tty);
> +}
> +
> +static int vtty_write(struct tty_struct *tty, const unsigned char *buf, int count)
> +{
> +	struct tty_struct *peer = tty->link;
> +
> +	if (tty->stopped)
> +		return 0;
> +
> +	if (count > 0) {
> +		count = tty_insert_flip_string(peer->port, buf, count);
> +		if (count) {
> +			tty_flip_buffer_push(peer->port);
> +			tty_wakeup(tty);
> +		} else {
> +			/*
> +			 * Flush the slave reader if noone
> +			 * is actually hooked on. Otherwise
> +			 * wait until reader fetch all data.
> +			 */
> +			if (peer->count < 2)

I.e. we always perform flush on master to slave writes here, right? If
so, this is incorrect. Missed a slave reference in tty_install?

> +				tty_perform_flush(peer, TCIFLUSH);
> +		}
> +	}
> +
> +	return count;
> +}
> +
> +static int vtty_write_room(struct tty_struct *tty)
> +{
> +	struct tty_struct *peer = tty->link;
> +
> +	if (peer->stopped)

tty->stopped

> +		return 0;
> +
> +	if (peer->count < 2)
> +		return TTY_BUFFER_PAGE;

TTY_BUFFER_PAGE looks confusing here, because our write_room method has
nothing to do with it. Please use a numeric constant (e.g. 8192).

> +
> +	return pty_space(peer);
> +}
> +
> +static const struct tty_operations vtty_ops = {
> +	.lookup		= vtty_lookup,
> +	.install	= vtty_install,
> +	.open		= vtty_open,
> +	.close		= vtty_close,
> +	.shutdown	= vtty_shutdown,
> +	.cleanup	= vtty_cleanup,
> +	.write		= vtty_write,
> +	.write_room	= vtty_write_room,
> +	.set_termios	= pty_set_termios,
> +	.unthrottle	= pty_unthrottle,
> +	.remove		= pty_unix98_remove,
> +};
> +
> +struct tty_driver *vtty_console_driver(int *index)
> +{
> +	*index = 0;
> +	return vttys_driver;
> +}
> +
> +int vtty_match(dev_t device)
> +{
> +	return MAJOR(device) == TTY_MAJOR && MINOR(device) < MAX_NR_VTTY_CONSOLES;
> +}
> +
> +struct tty_driver *vtty_driver(dev_t dev, int *index)
> +{
> +	BUG_ON(MINOR(dev) >= MAX_NR_VTTY_CONSOLES);
> +
> +	*index = MINOR(dev);
> +	return vttys_driver;
> +}
> +
> +static void ve_vtty_fini(void *data)
> +{
> +	struct ve_struct *ve = data;
> +	vtty_map_t *map = vtty_map_lookup(ve->veid);
> +	int i;
> +
> +	mutex_lock(&tty_mutex);
> +	for (i = 0; i < MAX_NR_VTTY_CONSOLES; i++) {
> +		struct tty_struct *tty = map->vttys[i];
> +		if (!tty)
> +			continue;
> +		vtty_printk("active tty at %d\n", i);
> +		vtty_printk_pair(tty);
> +		tty->driver_data = tty->link->driver_data = NULL;
> +	}
> +	mutex_unlock(&tty_mutex);
> +	vtty_map_free(map);

What protects vtty_idr here?

> +}
> +
> +static struct ve_hook vtty_hook = {
> +	.fini           = ve_vtty_fini,
> +	.priority       = HOOK_PRIO_DEFAULT,
> +	.owner          = THIS_MODULE,
> +};
> +
> +static int __init vtty_init(void)
> +{
> +#define VTTY_DRIVER_ALLOC_FLAGS			\
> +	(TTY_DRIVER_REAL_RAW		|	\
> +	 TTY_DRIVER_RESET_TERMIOS	|	\
> +	 TTY_DRIVER_DEVPTS_MEM)
> +
> +	vttym_driver = tty_alloc_driver(MAX_NR_VTTY_CONSOLES, VTTY_DRIVER_ALLOC_FLAGS);
> +	if (IS_ERR(vttym_driver))
> +		panic(pr_fmt("Can't allocate master vtty driver\n"));
> +
> +	vttys_driver = tty_alloc_driver(MAX_NR_VTTY_CONSOLES, VTTY_DRIVER_ALLOC_FLAGS);
> +	if (IS_ERR(vttys_driver))
> +		panic(pr_fmt("Can't allocate slave vtty driver\n"));
> +
> +	vttym_driver->driver_name		= "vtty_master";
> +	vttym_driver->name			= "vttym";
> +	vttym_driver->name_base			= 0;
> +	vttym_driver->major			= 0;
> +	vttym_driver->minor_start		= 0;
> +	vttym_driver->type			= TTY_DRIVER_TYPE_CONSOLE;
> +	vttym_driver->init_termios		= tty_std_termios;
> +	vttym_driver->init_termios.c_iflag	= 0;
> +	vttym_driver->init_termios.c_oflag	= 0;
> +
> +	/* 38400 boud rate, 8 bit char size, enable receiver */
> +	vttym_driver->init_termios.c_cflag	= B38400 | CS8 | CREAD;
> +	vttym_driver->init_termios.c_lflag	= 0;
> +	vttym_driver->init_termios.c_ispeed	= 38400;
> +	vttym_driver->init_termios.c_ospeed	= 38400;
> +	tty_set_operations(vttym_driver, &vtty_ops);
> +
> +	vttys_driver->driver_name		= "vtty_slave";
> +	vttys_driver->name			= "vttys";
> +	vttys_driver->name_base			= 0;
> +	vttys_driver->major			= 0;
> +	vttys_driver->minor_start		= 0;
> +	vttys_driver->type			= TTY_DRIVER_TYPE_CONSOLE;
> +	vttys_driver->init_termios		= tty_std_termios;
> +	vttys_driver->init_termios.c_iflag	= 0;
> +	vttys_driver->init_termios.c_oflag	= 0;
> +	vttys_driver->init_termios.c_cflag	= B38400 | CS8 | CREAD;
> +	vttys_driver->init_termios.c_lflag	= 0;
> +	vttys_driver->init_termios.c_ispeed	= 38400;
> +	vttys_driver->init_termios.c_ospeed	= 38400;
> +	tty_set_operations(vttys_driver, &vtty_ops);
> +
> +	if (tty_register_driver(vttym_driver))
> +		panic(pr_fmt("Can't register master vtty driver\n"));
> +
> +	if (tty_register_driver(vttys_driver))
> +		panic(pr_fmt("Can't register slave vtty driver\n"));
> +
> +	ve_hook_register(VE_SS_CHAIN, &vtty_hook);
> +	tty_default_fops(&vtty_fops);
> +	return 0;
> +}
> +
> +int vtty_open_master(envid_t veid, int idx)
> +{
> +	struct tty_struct *tty;
> +	struct file *file;
> +	char devname[64];
> +	int fd, ret;
> +
> +	if (!vtty_match_index(idx))
> +		return -EIO;
> +
> +	vtty_printk("veid %d index %d\n", (int)veid, idx);
> +
> +	fd = get_unused_fd_flags(0);
> +	if (fd < 0)
> +		return fd;
> +
> +	snprintf(devname, sizeof(devname), "v%utty%d", veid, idx);
> +	file = anon_inode_getfile(devname, &vtty_fops, NULL, O_RDWR);
> +	if (IS_ERR(file)) {
> +		ret = PTR_ERR(file);
> +		goto err_put_unused_fd;
> +	}
> +	nonseekable_open(NULL, file);
> +
> +	ret = tty_alloc_file(file);
> +	if (ret)
> +		goto err_fput;
> +
> +	/*
> +	 * Opening comes from ve0 context so
> +	 * setup VE's context until master fetched.
> +	 * This is done under @tty_mutex so noone
> +	 * else would access it while we're hilding
> +	 * the lock.
> +	 */
> +	mutex_lock(&tty_mutex);
> +	vtty_set_context(veid);
> +
> +	tty = vtty_lookup(vttym_driver, NULL, idx);
> +	if (IS_ERR(tty)) {
> +		ret = -ENODEV;
> +		goto err_install;
> +	} else if (!tty) {
> +		/*
> +		 * Allocate a new tty pair, the slave
> +		 * won't be having @count here ecause
> +		 * it's not opened by anyone yet.
> +		 */

Hey, wait! It's opened by us, isn't it? Where is the reference?

> +		tty = tty_init_dev(vttys_driver, idx);
> +		if (IS_ERR(tty))
> +			goto err_install;
> +		tty->count--;
> +		tty_unlock(tty);
> +		tty = tty->link;
> +	}
> +
> +	/*
> +	 * Make sure the slave peer won't disappear
> +	 * while we're keeping the tty opened.
> +	 */
> +	set_bit(TTY_EXTRA_REF, &tty->link->flags);
> +	tty->link->count++;
> +
> +	vtty_drop_context();
> +
> +	WARN_ON(!test_bit(TTY_LDISC, &tty->flags));
> +
> +	clear_bit(TTY_EXTRA_REF, &tty->flags);

This juggling with TTY_EXTRA_REF looks suspicious. AFAIU we only need
TTY_EXTRA_REF set for master, so that it won't pass away on close unless
its slave is dead too, and master must always pin its slave, no?

> +	tty_add_file(tty, file);
> +	fd_install(fd, file);
> +	WARN_ON(vtty_open(tty, file));
> +
> +	mutex_unlock(&tty_mutex);
> +	ret = fd;
> +
> +	vtty_printk_pair(tty);
> +out:
> +	return ret;
> +
> +err_install:
> +	vtty_drop_context();
> +	mutex_unlock(&tty_mutex);
> +	tty_free_file(file);
> +err_fput:
> +	file->f_op = NULL;
> +	fput(file);
> +err_put_unused_fd:
> +	put_unused_fd(fd);
> +	goto out;
> +}
> +#else
> +static void vtty_init(void) { };
> +#endif /* CONFIG_VE && CONFIG_UNIX98_PTYS */
> +
>  static int __init pty_init(void)
>  {
>  	legacy_pty_init();
>  	unix98_pty_init();
> +	vtty_init();
>  	return 0;
>  }
>  module_init(pty_init);
> Index: linux-pcs7.git/drivers/tty/tty_io.c
> ===================================================================
> --- linux-pcs7.git.orig/drivers/tty/tty_io.c
> +++ linux-pcs7.git/drivers/tty/tty_io.c
> @@ -306,6 +306,10 @@ static int check_tty_count(struct tty_st
>  	    tty->driver->subtype == PTY_TYPE_SLAVE &&
>  	    tty->link && tty->link->count)
>  		count++;
> +#ifdef CONFIG_VE
> +	if (test_bit(TTY_EXTRA_REF, &tty->flags))
> +		count++;
> +#endif
>  	if (tty->count != count) {
>  		printk(KERN_WARNING "Warning: dev (%s) tty->count(%d) "
>  				    "!= #fd's(%d) in %s\n",
> @@ -1931,14 +1935,15 @@ static struct tty_driver *tty_lookup_dri
>  	struct tty_driver *driver;
>  
>  #ifdef CONFIG_VE
> -	extern struct tty_driver *vz_vt_device(struct ve_struct *ve, dev_t dev, int *index);
> -	if (!ve_is_super(get_exec_env()) &&
> -	    (MAJOR(device) == TTY_MAJOR && MINOR(device) < VZ_VT_MAX_DEVS)) {
> -		driver = tty_driver_kref_get(vz_vt_device(get_exec_env(), device, index));
> +	struct ve_struct *ve = get_exec_env();
> +
> +	if (!ve_is_super(ve) && vtty_match(device)) {
> +		driver = tty_driver_kref_get(vtty_driver(device, index));
>  		*noctty = 1;
>  		return driver;
>  	}
>  #endif
> +
>  	switch (device) {
>  #ifdef CONFIG_VT
>  	case MKDEV(TTY_MAJOR, 0): {
> @@ -1952,10 +1957,8 @@ static struct tty_driver *tty_lookup_dri
>  	case MKDEV(TTYAUX_MAJOR, 1): {
>  		struct tty_driver *console_driver = console_device(index);
>  #ifdef CONFIG_VE
> -		if (!ve_is_super(get_exec_env())) {
> -			extern struct tty_driver *vz_console_device(int *index);
> -			console_driver = vz_console_device(index);
> -		}
> +		if (!ve_is_super(ve))
> +			console_driver = vtty_console_driver(index);
>  #endif
>  		if (console_driver) {
>  			driver = tty_driver_kref_get(console_driver);
> @@ -3628,9 +3631,6 @@ static DEVICE_ATTR(active, S_IRUGO, show
>  
>  #ifdef CONFIG_VE
>  
> -extern int vz_con_ve_init(struct ve_struct *ve);
> -extern void vz_con_ve_fini(struct ve_struct *ve);
> -
>  void console_sysfs_notify(void)
>  {
>  	struct ve_struct *ve = get_exec_env();
> @@ -3647,7 +3647,6 @@ void ve_tty_console_fini(struct ve_struc
>  	device_remove_file(consdev, &dev_attr_active);
>  	device_destroy_namespace(tty_class, MKDEV(TTYAUX_MAJOR, 1), ve);
>  	device_destroy_namespace(tty_class, MKDEV(TTYAUX_MAJOR, 0), ve);
> -	vz_con_ve_fini(ve);
>  }
>  
>  int ve_tty_console_init(struct ve_struct *ve)
> @@ -3669,15 +3668,9 @@ int ve_tty_console_init(struct ve_struct
>  	if (err)
>  		goto err_consfile;
>  
> -	err = vz_con_ve_init(ve);
> -	if (err)
> -		goto err_vzcon;
> -
>  	ve->consdev = dev;
>  	return 0;
>  
> -err_vzcon:
> -	device_remove_file(dev, &dev_attr_active);
>  err_consfile:
>  	device_destroy_namespace(tty_class, MKDEV(TTYAUX_MAJOR, 1), ve);
>  err_consdev:
> Index: linux-pcs7.git/include/linux/tty.h
> ===================================================================
> --- linux-pcs7.git.orig/include/linux/tty.h
> +++ linux-pcs7.git/include/linux/tty.h
> @@ -322,6 +322,9 @@ struct tty_file_private {
>  #define TTY_HUPPING 		21	/* ->hangup() in progress */
>  #define TTY_LDISC_HALTED	22	/* Line discipline is halted */
>  #define TTY_CHARGED		23	/* Charged as ub resource */
> +#ifdef CONFIG_VE
> +#define TTY_EXTRA_REF		24	/* Carries extra reference */
> +#endif
>  
>  #define TTY_WRITE_FLUSH(tty) tty_write_flush((tty))
>  
> Index: linux-pcs7.git/include/linux/ve.h
> ===================================================================
> --- linux-pcs7.git.orig/include/linux/ve.h
> +++ linux-pcs7.git/include/linux/ve.h
> @@ -66,13 +66,6 @@ struct ve_struct {
>  	struct binfmt_misc	*binfmt_misc;
>  #endif
>  
> -#define VZ_VT_MAX_DEVS		12
> -	struct tty_driver	*vz_vt_driver;
> -	struct tty_struct	*vz_tty_vt[VZ_VT_MAX_DEVS];
> -
> -	struct tty_struct	*vz_tty_conm;
> -	struct tty_struct	*vz_tty_cons;
> -
>  #ifdef CONFIG_TTY
>  	struct device		*consdev;
>  #endif
> @@ -209,17 +202,24 @@ extern unsigned long long ve_relative_cl
>  extern void monotonic_abs_to_ve(clockid_t which_clock, struct timespec *tp);
>  extern void monotonic_ve_to_abs(clockid_t which_clock, struct timespec *tp);
>  
> -#ifdef CONFIG_VTTYS
> -extern int vtty_open_master(int veid, int idx);
> -extern struct tty_driver *vtty_driver;
> -#else
> -static inline int vtty_open_master(int veid, int idx) { return -ENODEV; }
> -#endif
> -
>  void ve_stop_ns(struct pid_namespace *ns);
>  void ve_exit_ns(struct pid_namespace *ns);
>  int ve_start_container(struct ve_struct *ve);
>  
> +#define MAX_NR_VTTY_CONSOLES		(12)
> +
> +#ifdef CONFIG_TTY
> +extern int vtty_match(dev_t device);
> +extern struct tty_driver *vtty_driver(dev_t dev, int *index);
> +extern struct tty_driver *vtty_console_driver(int *index);
> +extern int vtty_open_master(envid_t veid, int idx);
> +#else /* CONFIG_TTY */
> +static inline int vtty_match(dev_t device) { return 0; }
> +static inline struct tty_driver *vtty_driver(dev_t dev, int *index) { return NULL; }
> +static inline struct tty_driver *vtty_console_driver(int *index) { return NULL; }
> +static inline int vtty_open_master(envid_t veid, int idx) { return -ENODEV; }
> +#endif /* CONFIG_TTY */
> +
>  #else	/* CONFIG_VE */
>  
>  #define ve_uevent_seqnum uevent_seqnum
> Index: linux-pcs7.git/kernel/ve/Makefile
> ===================================================================
> --- linux-pcs7.git.orig/kernel/ve/Makefile
> +++ linux-pcs7.git/kernel/ve/Makefile
> @@ -4,7 +4,7 @@
>  # Copyright (c) 2000-2015 Parallels IP Holdings GmbH
>  #
>  
> -obj-$(CONFIG_VE) = ve.o veowner.o hooks.o vzstat_core.o ve-kobject.o console.o
> +obj-$(CONFIG_VE) = ve.o veowner.o hooks.o vzstat_core.o ve-kobject.o
>  obj-$(CONFIG_VZ_WDOG) += vzwdog.o
>  obj-$(CONFIG_VE_CALLS) += vzmon.o
>  
> Index: linux-pcs7.git/kernel/ve/vecalls.c
> ===================================================================
> --- linux-pcs7.git.orig/kernel/ve/vecalls.c
> +++ linux-pcs7.git/kernel/ve/vecalls.c
> @@ -998,6 +998,9 @@ static int ve_configure(envid_t veid, un
>  	case VE_CONFIGURE_OS_RELEASE:
>  		err = init_ve_osrelease(ve, data);
>  		break;
> +	case VE_CONFIGURE_OPEN_TTY:
> +		err = vtty_open_master(ve->veid, val);
> +		break;
>  	}
>  
>  	put_ve(ve);
> 



More information about the Devel mailing list