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 <gorcu...@virtuozzo.com>
> CC: Vladimir Davydov <vdavy...@virtuozzo.com>
> CC: Konstantin Khorenko <khore...@virtuozzo.com>
> CC: Pavel Emelyanov <xe...@virtuozzo.com>
> CC: Andrey Vagin <ava...@virtuozzo.com>
> CC: Igor Sukhih <i...@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);
>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
