[Devel] [RFC rh7 v4] ve/tty: vt -- Implement per VE support for console and terminals
Vladimir Davydov
vdavydov at parallels.com
Fri Aug 21 07:15:52 PDT 2015
Hi,
On Thu, Aug 20, 2015 at 04:20:11PM +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
>
> v4:
> - Use lockdep_assert_held in vtty @map operations to make sure
> we're under @tty_mutex
> - vtty_install now requests for port memory earlier for vtty_install_peer
> simplification
> - Drop tty_vhangup call from vtty_close, as been found it doesn't
> bring any benefit
> - Drop TTY_BUFFER_PAGE and fix typo in vtty_write_room
> - Rework tty counting to be the same as in pcs6: drivers
> became TTY_DRIVER_TYPE_PTY and @count adjusted accordingly
>
> 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>
> ---
>
> Guys, take a look please. Hopefully I addressed all the commeents,
> but review is still would be highly appreciated. The main change
> is that I swicthed to pcs6 tty coutning scheme.
>
> drivers/tty/pty.c | 609 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 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, 641 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,619 @@ 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. Also there is a per-VE spinlock to order
> + * close() operation.
I don't see any spinlocks in your code.
> + *
> + * 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. Because the @master may be hanging around
> + * unconnected during the whole lifetime of a pair, we're
> + * assiging an extra refernce on it so kernel won't complain
> + * that link has nonzero @count but no real file descriptor
> + * assigned.
> + */
> +
> +#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);
> +
> +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 idx %2d cnt %2d fl 0x%-8lx\n", \
> + (__tty), (__tty)->index, (__tty)->count, (__tty)->flags)
> +#define vtty_printk_pair(__tty) \
> + vtty_printk("tty %p idx %2d cnt %2d fl 0x%-8lx " \
> + "link %p idx %2d cnt %2d fl 0x%-8lx\n", \
> + (__tty), (__tty)->index, (__tty)->count, (__tty)->flags, \
> + (__tty)->link, (__tty)->link ? (__tty)->link->index : -1, \
> + (__tty)->link ? (__tty)->link->count : -1, \
> + (__tty)->link ? (__tty)->link->flags : -1ul)
tty_io.c uses pr_debug. Why should we be different?
> +#else
> +#define vtty_printk(fmt, ...)
> +#define vtty_printk_one(__tty)
> +#define vtty_printk_pair(__tty)
> +#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)
> +{
> + lockdep_assert_held(&tty_mutex);
> + return idr_find(&vtty_idr, veid);
> +}
> +
> +static void vtty_map_set(vtty_map_t *map, struct tty_struct *tty, int index)
> +{
> + lockdep_assert_held(&tty_mutex);
> + 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)
> +{
> + lockdep_assert_held(&tty_mutex);
> + 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)
> +{
> + lockdep_assert_held(&tty_mutex);
> + 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);
> +
> + lockdep_assert_held(&tty_mutex);
> + 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)
> +{
> + lockdep_assert_held(&tty_mutex);
> + current_veid = veid;
> +}
> +
> +static void vtty_drop_context(void)
> +{
> + lockdep_assert_held(&tty_mutex);
> + current_veid = VTTY_USE_EXEC_VEID;
> +}
> +
> +static envid_t vtty_get_context(void)
> +{
> + BUILD_BUG_ON(sizeof(current_veid) < sizeof(envid_t));
> + lockdep_assert_held(&tty_mutex);
> +
> + if (likely(current_veid == VTTY_USE_EXEC_VEID))
Nit: likely is redundant here, it's not a hot path.
BTW, since vttys should never be used by ve0 we could use 0 instead of
VTTY_USE_EXEC_VEID. Then the body of this function would look like this:
return current_veid ?: get_exec_env()->veid;
(I like ?: with the first choice omitted :-)
> + 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;
> +
> + /*
> + * XXX: Strictly speaking there should not
> + * 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.
> + */
I wonder if we need to handle this open-master-from-ct case now. This
won't work if there's no slave, because there is no working install
method for master, so we will have to write new code anyway if we need
it. May be, better just zap it for now, and only introduce hunks like
this when they are really necessary?
> + tty = map->vttys[idx];
> + if (tty) {
> + if (vtty_is_exiting(tty) || vtty_is_exiting(tty->link))
1. There is absolutely no point in checking this for 'tty', because
tty_reopen, which is normally called right after ->lookup, already
takes care of it.
2. What's the point in checking TTY_HUPPING and TTY_LDISC_CHANGING for
'tty->link'? We are opening 'tty' here.
> + tty = ERR_PTR(-ENXIO);
Just curious, why ENXIO? Why not EIO?
Anyway, returning an error here means tty_open failure. Now suppose
somebody attempts to open slave vtty while the pair is being destroyed.
A failure would be unexpected then. AFAIU we should return NULL here for
tty_open to create a new pair.
> + 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->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,
> + struct tty_port *port, int index)
> +{
> + struct tty_struct *tty;
> +
> + tty = alloc_tty_struct();
> + if (!tty)
> + return ERR_PTR(-ENOMEM);
> + initialize_tty_struct(tty, driver, index);
> +
> + tty->port = port;
> + 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_port *peer_port;
> + struct tty_struct *peer;
> + vtty_map_t *map;
> + int ret;
> +
> + 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);
> + peer_port = kzalloc(sizeof(*peer_port), GFP_KERNEL);
> + if (!tty->port || !peer_port) {
> + ret = -ENOMEM;
> + goto err_free;
> + }
> +
> + /*
> + * The peer will have no real @fd assigned yet,
> + * so add an extra @fd sign over so kernel won't
> + * treat it as closing until a real file appear
> + * on the peer. Sill this spare peer may be in this
> + * state during the whole pair lifetime (imagine
> + * noone ever opened it and @tty get closed).
> + */
> + peer = vtty_install_peer(map, vttym_driver, peer_port, tty->index);
> + if (IS_ERR(peer)) {
> + vtty_map_del(map, tty, tty->index);
Do we need it here?
> + ret = PTR_ERR(peer);
> + goto err_free;
> + }
> +
> + vtty_standard_install(map, vttys_driver, tty);
> + vtty_map_set(map, tty, tty->index);
> +
> + tty->count++;
> + tty->count++; /* master hold slave reference */
> + peer->count++; /* slave hold master reference */
> + set_bit(TTY_EXTRA_REF, &peer->flags);
> +
> + tty->link = peer;
> + peer->link = tty;
> +
> + vtty_printk_pair(tty);
> + return 0;
> +
> +err_free:
> + kfree(tty->port);
> + kfree(peer_port);
> + return ret;
> +}
> +
> +static int vtty_open(struct tty_struct *tty, struct file *filp)
> +{
> + if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
> + return -EIO;
What's the point in this check? You only set TTY_OTHER_CLOSED when both
ends are closed and hence the pair is dead.
> +
> + clear_bit(TTY_IO_ERROR, &tty->flags);
Ditto?
> + 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;
> + vtty_map_t *map = tty->driver_data;
map is unused in this function.
> +
> + /*
> + * Some other file is still using us.
> + */
> + if (tty->count > 2)
> + return;
> +
> + if (test_bit(TTY_IO_ERROR, &tty->flags))
> + return;
Why?
> +
> + if (tty->driver == vttys_driver) {
> + if (peer->count == 1) {
> + clear_bit(TTY_EXTRA_REF, &peer->flags);
> + peer->count--;
Here you read and modify peer->count. Why is it safe? You hold neither
peer->legacy_mutex nor tty_mutex here.
Another question regarding synchronization is what prevents the
following race, which results in leaking a vtty pair?
CPU0 CPU1
---- ----
opened slave opened slave
[ master->count = 1, slave->count = 3 ]
closing slave closing slave
tty_release
->close (vtty_close)
tty->count > 2 => return
tty_release
->close (vtty_close)
tty->count > 2 => return
lock tty_mutex
tty->count--;
unlock tty_mutex
lock tty_mutex
tty->count--;
unlock tty_mutex
[ master->count = 1, slave->count = 1 ]
If this race can happen, is it relevant to pty too?
> + tty->count--;
> + }
> + } else {
> + if (peer->count == 1) {
> + clear_bit(TTY_EXTRA_REF, &tty->flags);
> + tty->count--;
> + } else
> + peer->count++;
It is not obvious why we need to increment peer->count here. Please add
a comment.
> + }
> +
> + if (tty->count == 1) {
> + set_bit(TTY_IO_ERROR, &tty->flags);
> + set_bit(TTY_OTHER_CLOSED, &peer->flags);
> + }
> +
> + tty->packet = 0;
You don't implement TIOCPKT ioctl so tty->packet is always 0 and this is
redundant.
> + wake_up_interruptible(&tty->read_wait);
> + wake_up_interruptible(&tty->write_wait);
> +
> + peer->packet = 0;
Ditto.
> + wake_up_interruptible(&peer->read_wait);
> + wake_up_interruptible(&peer->write_wait);
> +
> + 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)
> +{
> + /*
> + * 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)
> + tty_perform_flush(peer, TCIFLUSH);
> + }
> + }
> +
> + return count;
> +}
> +
> +static int vtty_write_room(struct tty_struct *tty)
> +{
> + struct tty_struct *peer = tty->link;
> +
> + if (tty->stopped)
> + return 0;
> +
> + if (peer->count < 2)
> + return 2048;
> +
> + 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;
> + int i;
> +
> + mutex_lock(&tty_mutex);
> + map = vtty_map_lookup(ve->veid);
> + if (map) {
> + 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;
> + }
> + vtty_map_free(map);
> + }
> + mutex_unlock(&tty_mutex);
> +}
> +
> +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_DYNAMIC_DEV | \
> + TTY_DRIVER_INSTALLED | \
> + 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_PTY;
> + vttym_driver->subtype = PTY_TYPE_MASTER;
> + 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_PTY;
> + vttys_driver->subtype = PTY_TYPE_SLAVE;
> + 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 holding
> + * 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) {
> + tty = tty_init_dev(vttys_driver, idx);
> + if (IS_ERR(tty))
> + goto err_install;
> + tty->count--;
> + tty_unlock(tty);
> + tty = tty->link;
> + }
> +
> + vtty_drop_context();
> +
> + WARN_ON(!test_bit(TTY_LDISC, &tty->flags));
> +
> + tty_add_file(tty, file);
> + tty->count++;
> + 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)) {
This is the only place where vtty_match() is used. Wouldn't it be better
to fold it in vtty_driver()? I mean
struct tty_driver *vtty_driver(dev_t dev, int *index)
{
if (MAJOR(device) == TTY_MAJOR &&
vtty_match_index(MINOR(device))) {
*index = MINOR(device);
return vttys_driver;
}
return NULL;
}
> + 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; }
If !CONFIG_TTY, these functions will never be used so remove the stubs
please.
Thanks,
Vladimir
> +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