Helper patches for PTY namespaces

PATCH In review [email protected] 15 participants 34 messages Apr 12, 2008 – Aug 3, 2008 9 messages not in the archive Char and misc drivers

lore

Share & tools

lore

[email protected]

author

2008-04-12 17:29 UTC #0 lore raw

cover Helper patches for PTY namespaces

Some simple helper patches to enable implementation of multiple PTY (or device) namespaces.

[PATCH 1/4]: Propagate error code from devpts_pty_new [PATCH 2/4]: Factor out PTY index allocation [PATCH 3/4]: Move devpts globals into init_pts_ns [PATCH 3/4]: Enable multiple mounts of /dev/pts

This patchset is based on earlier versions developed by Serge Hallyn and Matt Helsley.

[email protected]

author

2008-04-12 17:32 UTC #1 lore raw

PATCH 1/4 : Propagate error code from devpts_pty_new

From: Sukadev Bhattiprolu <[email protected]>
Subject: [PATCH 1/4]: Propagate error code from devpts_pty_new

Have ptmx_open() propagate any error code returned by devpts_pty_new() (which returns either 0 or -ENOMEM anyway).

Signed-off-by Sukadev Bhattiprolu <[email protected]>

---
 drivers/char/tty_io.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/char/tty_io.c +2 -2

Index: 2.6.25-rc8-mm1/drivers/char/tty_io.c
===================================================================
@@ -2835,8 +2835,8 @@ static int ptmx_open(struct inode *inode
 	filp->private_data = tty;
 	file_move(filp, &tty->tty_files);
 
-	retval = -ENOMEM;
-	if (devpts_pty_new(tty->link))
+	retval = devpts_pty_new(tty->link);
+	if (retval)
 		goto out1;
 
 	check_tty_count(tty, "tty_open");

[email protected]

author

2008-04-12 17:32 UTC #2 lore raw

PATCH 2/4 : Factor out PTY index allocation

In reply to [email protected]

From: Sukadev Bhattiprolu <[email protected]>
Subject: [PATCH 2/4]: Factor out PTY index allocation

Factor out the code used to allocate/free a pts index into new interfaces, devpts_new_index() and devpts_kill_index(). This localizes the external data structures used in managing the pts indices.

Signed-off-by Sukadev Bhattiprolu <[email protected]>

Signed-off-by Serge Hallyn<[email protected]>

Signed-off-by Matt Helsley<[email protected]>

---
 drivers/char/tty_io.c     |   40 ++++++----------------------------------
 fs/devpts/inode.c         |   42 +++++++++++++++++++++++++++++++++++++++++-
 include/linux/devpts_fs.h |    4 ++++
 3 files changed, 51 insertions(+), 35 deletions(-)

include/linux/devpts_fs.h +4 -0

Index: 2.6.25-rc5-mm1/include/linux/devpts_fs.h
===================================================================
@@ -17,6 +17,8 @@
 
 #ifdef CONFIG_UNIX98_PTYS
 
+int devpts_new_index(void);
+void devpts_kill_index(int idx);
 int devpts_pty_new(struct tty_struct *tty);      /* mknod in devpts */
 struct tty_struct *devpts_get_tty(int number);	 /* get tty structure */
 void devpts_pty_kill(int number);		 /* unlink */

@@ -24,6 +26,8 @@ void devpts_pty_kill(int number);		 /* u
 #else
 
 /* Dummy stubs in the no-pty case */
+static inline int devpts_new_index(void) { return -EINVAL; }
+static inline void devpts_kill_index(int idx) { }
 static inline int devpts_pty_new(struct tty_struct *tty) { return -EINVAL; }
 static inline struct tty_struct *devpts_get_tty(int number) { return NULL; }
 static inline void devpts_pty_kill(int number) { }

drivers/char/tty_io.c +6 ~~-34~~

Index: 2.6.25-rc5-mm1/drivers/char/tty_io.c
===================================================================
@@ -91,7 +91,6 @@
 #include <linux/module.h>
 #include <linux/smp_lock.h>
 #include <linux/device.h>
-#include <linux/idr.h>
 #include <linux/wait.h>
 #include <linux/bitops.h>
 #include <linux/delay.h>

@@ -137,9 +136,6 @@ EXPORT_SYMBOL(tty_mutex);
 
 #ifdef CONFIG_UNIX98_PTYS
 extern struct tty_driver *ptm_driver;	/* Unix98 pty masters; for /dev/ptmx */
-extern int pty_limit;			/* Config limit on Unix98 ptys */
-static DEFINE_IDR(allocated_ptys);
-static DEFINE_MUTEX(allocated_ptys_lock);
 static int ptmx_open(struct inode *, struct file *);
 #endif

@@ -2636,15 +2632,9 @@ static void release_dev(struct file *fil
 	 */
 	release_tty(tty, idx);
 
-#ifdef CONFIG_UNIX98_PTYS
 	/* Make this pty number available for reallocation */
-	if (devpts) {
-		mutex_lock(&allocated_ptys_lock);
-		idr_remove(&allocated_ptys, idx);
-		mutex_unlock(&allocated_ptys_lock);
-	}
-#endif
-
+	if (devpts)
+		devpts_kill_index(idx);
 }
 
 /**

@@ -2800,29 +2790,13 @@ static int ptmx_open(struct inode *inode
 	struct tty_struct *tty;
 	int retval;
 	int index;
-	int idr_ret;
 
 	nonseekable_open(inode, filp);
 
 	/* find a device that is not in use. */
-	mutex_lock(&allocated_ptys_lock);
-	if (!idr_pre_get(&allocated_ptys, GFP_KERNEL)) {
-		mutex_unlock(&allocated_ptys_lock);
-		return -ENOMEM;
-	}
-	idr_ret = idr_get_new(&allocated_ptys, NULL, &index);
-	if (idr_ret < 0) {
-		mutex_unlock(&allocated_ptys_lock);
-		if (idr_ret == -EAGAIN)
-			return -ENOMEM;
-		return -EIO;
-	}
-	if (index >= pty_limit) {
-		idr_remove(&allocated_ptys, index);
-		mutex_unlock(&allocated_ptys_lock);
-		return -EIO;
-	}
-	mutex_unlock(&allocated_ptys_lock);
+	index = devpts_new_index();
+	if (index < 0)
+		return index;
 
 	mutex_lock(&tty_mutex);
 	retval = init_dev(ptm_driver, index, &tty);

@@ -2847,9 +2821,7 @@ out1:
 	release_dev(filp);
 	return retval;
 out:
-	mutex_lock(&allocated_ptys_lock);
-	idr_remove(&allocated_ptys, index);
-	mutex_unlock(&allocated_ptys_lock);
+	devpts_kill_index(index);
 	return retval;
 }
 #endif

fs/devpts/inode.c +41 -1

62 lines not on this page · raw message

[email protected]

author

2008-04-12 17:33 UTC #3 lore raw

PATCH 3/4 : Move devpts globals into init_pts_ns

In reply to [email protected]

Matt, Serge, please sign-off on this version.
---
From: Sukadev Bhattiprolu <[email protected]>
Subject: [PATCH 3/4]: Move devpts globals into init_pts_ns

Move devpts global variables 'allocated_ptys' and 'devpts_mnt' into a new 'pts_namespace' and remove the 'devpts_root'.

Changelog:
- Split these relatively simpler changes off from the patch that supports remounting devpts.

Signed-off-by Sukadev Bhattiprolu <[email protected]>

---
 fs/devpts/inode.c         |   84 ++++++++++++++++++++++++++++++++--------------
 include/linux/devpts_fs.h |   10 +++++
 2 files changed, 70 insertions(+), 24 deletions(-)

fs/devpts/inode.c +60 ~~-24~~

Index: 2.6.25-rc8-mm1/fs/devpts/inode.c
===================================================================
@@ -28,12 +28,8 @@
 #define DEVPTS_DEFAULT_MODE 0600
 
 extern int pty_limit;			/* Config limit on Unix98 ptys */
-static DEFINE_IDR(allocated_ptys);
 static DECLARE_MUTEX(allocated_ptys_lock);
 
-static struct vfsmount *devpts_mnt;
-static struct dentry *devpts_root;
-
 static struct {
 	int setuid;
 	int setgid;

@@ -54,6 +50,14 @@ static match_table_t tokens = {
 	{Opt_err, NULL}
 };
 
+struct pts_namespace init_pts_ns = {
+	.kref = {
+		.refcount = ATOMIC_INIT(2),
+	},
+	.allocated_ptys = IDR_INIT(init_pts_ns.allocated_ptys),
+	.mnt = NULL,
+};
+
 static int devpts_remount(struct super_block *sb, int *flags, char *data)
 {
 	char *p;

@@ -140,7 +144,7 @@ devpts_fill_super(struct super_block *s,
 	inode->i_fop = &simple_dir_operations;
 	inode->i_nlink = 2;
 
-	devpts_root = s->s_root = d_alloc_root(inode);
+	s->s_root = d_alloc_root(inode);
 	if (s->s_root)
 		return 0;

@@ -168,10 +172,9 @@ static struct file_system_type devpts_fs
  * to the System V naming convention
  */
 
-static struct dentry *get_node(int num)
+static struct dentry *get_node(struct dentry *root, int num)
 {
 	char s[12];
-	struct dentry *root = devpts_root;
 	mutex_lock(&root->d_inode->i_mutex);
 	return lookup_one_len(s, root, sprintf(s, "%d", num));
 }

@@ -180,14 +183,17 @@ int devpts_new_index(void)
 {
 	int index;
 	int idr_ret;
+	struct pts_namespace *pts_ns;
+
+	pts_ns = &init_pts_ns;
 
 retry:
-	if (!idr_pre_get(&allocated_ptys, GFP_KERNEL)) {
+	if (!idr_pre_get(&pts_ns->allocated_ptys, GFP_KERNEL)) {
 		return -ENOMEM;
 	}
 
 	down(&allocated_ptys_lock);
-	idr_ret = idr_get_new(&allocated_ptys, NULL, &index);
+	idr_ret = idr_get_new(&pts_ns->allocated_ptys, NULL, &index);
 	if (idr_ret < 0) {
 		up(&allocated_ptys_lock);
 		if (idr_ret == -EAGAIN)

@@ -196,7 +202,7 @@ retry:
 	}
 
 	if (index >= pty_limit) {
-		idr_remove(&allocated_ptys, index);
+		idr_remove(&pts_ns->allocated_ptys, index);
 		up(&allocated_ptys_lock);
 		return -EIO;
 	}

@@ -206,8 +212,10 @@ retry:
 
 void devpts_kill_index(int idx)
 {
+	struct pts_namespace *pts_ns = &init_pts_ns;
+
 	down(&allocated_ptys_lock);
-	idr_remove(&allocated_ptys, idx);
+	idr_remove(&pts_ns->allocated_ptys, idx);
 	up(&allocated_ptys_lock);
 }

@@ -217,12 +225,26 @@ int devpts_pty_new(struct tty_struct *tt
 	struct tty_driver *driver = tty->driver;
 	dev_t device = MKDEV(driver->major, driver->minor_start+number);
 	struct dentry *dentry;
-	struct inode *inode = new_inode(devpts_mnt->mnt_sb);
+	struct dentry *root;
+	struct inode *inode;
+	struct pts_namespace *pts_ns;
 
 	/* We're supposed to be given the slave end of a pty */
 	BUG_ON(driver->type != TTY_DRIVER_TYPE_PTY);
 	BUG_ON(driver->subtype != PTY_TYPE_SLAVE);
 
+	pts_ns = &init_pts_ns;
+	root = pts_ns->mnt->mnt_root;
+
+	mutex_lock(&root->d_inode->i_mutex);
+	inode = idr_find(&pts_ns->allocated_ptys, number);
+	mutex_unlock(&root->d_inode->i_mutex);
+
+	if (inode && !IS_ERR(inode))
+		return -EEXIST;
+
+	inode = new_inode(pts_ns->mnt->mnt_sb);
+
 	if (!inode)
 		return -ENOMEM;

@@ -232,23 +254,28 @@ int devpts_pty_new(struct tty_struct *tt
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
 	init_special_inode(inode, S_IFCHR|config.mode, device);
 	inode->i_private = tty;
+	idr_replace(&pts_ns->allocated_ptys, inode, number);
 
-	dentry = get_node(number);
+	dentry = get_node(root, number);
 	if (!IS_ERR(dentry) && !dentry->d_inode) {
 		d_instantiate(dentry, inode);
-		fsnotify_create(devpts_root->d_inode, dentry);
+		fsnotify_create(root->d_inode, dentry);
 	}
 
-	mutex_unlock(&devpts_root->d_inode->i_mutex);
+	mutex_unlock(&root->d_inode->i_mutex);
 
 	return 0;
 }
 
 struct tty_struct *devpts_get_tty(int number)
 {
-	struct dentry *dentry = get_node(number);
+	struct dentry *root;
+	struct dentry *dentry;
 	struct tty_struct *tty;
 
+	root = init_pts_ns.mnt->mnt_root;
+	dentry = get_node(root, number);
+
 	tty = NULL;
 	if (!IS_ERR(dentry)) {
 		if (dentry->d_inode)

@@ -256,14 +283,18 @@ struct tty_struct *devpts_get_tty(int nu
 		dput(dentry);
 	}
 
-	mutex_unlock(&devpts_root->d_inode->i_mutex);
+	mutex_unlock(&root->d_inode->i_mutex);
 
 	return tty;
 }
 
 void devpts_pty_kill(int number)
 {
-	struct dentry *dentry = get_node(number);
+	struct dentry *root;
+	struct dentry *dentry;
+
+	root = init_pts_ns.mnt->mnt_root;
+	dentry = get_node(root, number);
 
 	if (!IS_ERR(dentry)) {
 		struct inode *inode = dentry->d_inode;

@@ -274,16 +305,21 @@ void devpts_pty_kill(int number)
 		}
 		dput(dentry);
 	}
-	mutex_unlock(&devpts_root->d_inode->i_mutex);
+	mutex_unlock(&root->d_inode->i_mutex);
 }
 
 static int __init init_devpts_fs(void)
 {
-	int err = register_filesystem(&devpts_fs_type);
+	struct vfsmount *mnt;
+	int err;
+
+	err = register_filesystem(&devpts_fs_type);
 	if (!err) {
-		devpts_mnt = kern_mount(&devpts_fs_type);
-		if (IS_ERR(devpts_mnt))
-			err = PTR_ERR(devpts_mnt);
+		mnt = kern_mount(&devpts_fs_type);
+		if (IS_ERR(mnt))
+			err = PTR_ERR(mnt);
+		else
+			init_pts_ns.mnt = mnt;
 	}
 	return err;
 }

@@ -291,7 +327,7 @@ static int __init init_devpts_fs(void)
 static void __exit exit_devpts_fs(void)
 {
 	unregister_filesystem(&devpts_fs_type);
-	mntput(devpts_mnt);
+	mntput(init_pts_ns.mnt);
 }
 
 module_init(init_devpts_fs)

include/linux/devpts_fs.h +10 -0

Index: 2.6.25-rc8-mm1/include/linux/devpts_fs.h
===================================================================
@@ -14,6 +14,16 @@
 #define _LINUX_DEVPTS_FS_H
 
 #include <linux/errno.h>
+#include <linux/kref.h>
+#include <linux/idr.h>
+
+struct pts_namespace {
+	struct kref kref;
+	struct idr allocated_ptys;
+	struct vfsmount *mnt;
+};
+
+extern struct pts_namespace init_pts_ns;
 
 #ifdef CONFIG_UNIX98_PTYS

[email protected]

author

2008-04-12 17:34 UTC #4 lore raw

PATCH 4/4 : Enable multiple mounts of /dev/pts

In reply to [email protected]

From: Sukadev Bhattiprolu <[email protected]> Subject:[PATCH 4/4]: Enable multiple mounts of /dev/pts

To support multiple PTY namespaces, allow multiple mounts of /dev/pts, once within each PTY namespace.

This patch removes the get_sb_single() in devpts_get_sb() and uses test and set sb interfaces to allow remounting /dev/pts.

Changelog [v4]:
- Split-off the simpler changes of moving global=variables into 'pts_namespace' to previous patch.

Changelog [v3]:
- Removed some unnecessary comments from devpts_set_sb()

Changelog [v2]:

- (Pavel Emelianov/Serge Hallyn) Remove reference to pts_ns from sb->s_fs_info to fix the circular reference (/dev/pts is not unmounted unless the pts_ns is destroyed, so we don't need a reference to the pts_ns).

Signed-off-by Sukadev Bhattiprolu <[email protected]>

Signed-off-by Serge Hallyn <[email protected]>

Signed-off-by Matt Helsley <[email protected]>

---
 fs/devpts/inode.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 59 insertions(+), 3 deletions(-)

fs/devpts/inode.c +59 -3

Index: 2.6.25-rc8-mm1/fs/devpts/inode.c
===================================================================
@@ -154,17 +154,73 @@ fail:
 	return -ENOMEM;
 }
 
+/*
+ * We use test and set super-block operations to help determine whether we
+ * need a new super-block for this namespace. get_sb() walks the list of
+ * existing devpts supers, comparing them with the @data ptr. Since we
+ * passed 'current's namespace as the @data pointer we can compare the
+ * namespace pointer in the super-block's 's_fs_info'.  If the test is
+ * TRUE then get_sb() returns a new active reference to the super block.
+ * Otherwise, it helps us build an active reference to a new one.
+ */
+
+static int devpts_test_sb(struct super_block *sb, void *data)
+{
+	return sb->s_fs_info == data;
+}
+
+static int devpts_set_sb(struct super_block *sb, void *data)
+{
+	sb->s_fs_info = data;
+	return set_anon_super(sb, NULL);
+}
+
 static int devpts_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data, struct vfsmount *mnt)
 {
-	return get_sb_single(fs_type, flags, data, devpts_fill_super, mnt);
+	struct super_block *sb;
+	struct pts_namespace *ns;
+	int err;
+
+	/* hereafter we're very similar to proc_get_sb */
+	if (flags & MS_KERNMOUNT)
+		ns = data;
+	else
+		ns = &init_pts_ns;
+
+	/* hereafter we're very simlar to get_sb_nodev */
+	sb = sget(fs_type, devpts_test_sb, devpts_set_sb, ns);
+	if (IS_ERR(sb))
+		return PTR_ERR(sb);
+
+	if (sb->s_root)
+		return simple_set_mnt(mnt, sb);
+
+	sb->s_flags = flags;
+	err = devpts_fill_super(sb, data, flags & MS_SILENT ? 1 : 0);
+	if (err) {
+		up_write(&sb->s_umount);
+		deactivate_super(sb);
+		return err;
+	}
+
+	sb->s_flags |= MS_ACTIVE;
+	ns->mnt = mnt;
+
+	return simple_set_mnt(mnt, sb);
+}
+
+static void devpts_kill_sb(struct super_block *sb)
+{
+	sb->s_fs_info = NULL;
+	kill_anon_super(sb);
 }
 
 static struct file_system_type devpts_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "devpts",
 	.get_sb		= devpts_get_sb,
-	.kill_sb	= kill_anon_super,
+	.kill_sb	= devpts_kill_sb,
 };
 
 /*

@@ -315,7 +371,7 @@ static int __init init_devpts_fs(void)
 
 	err = register_filesystem(&devpts_fs_type);
 	if (!err) {
-		mnt = kern_mount(&devpts_fs_type);
+		mnt = kern_mount_data(&devpts_fs_type, &init_pts_ns);
 		if (IS_ERR(mnt))
 			err = PTR_ERR(mnt);
 		else

A message that is not in the archive (sent off-list or not yet received): 20080412172933.GA19295-r/[email protected]

Subrata Modak

2008-04-12 17:39 UTC #6 lore raw

Sukadev,

Any corresponding test cases for LTP. We just have UTS, PID & SYSVIPC Namespace till now.

Regards--
Subrata

Quoted 17 lines · On Sat, Apr 12, 2008 at 10:59 PM, <sukadev-r/[email protected].…

Some simple helper patches to enable implementation of multiple PTY (or device) namespaces.
[PATCH 1/4]: Propagate error code from devpts_pty_new [PATCH 2/4]: Factor out PTY index allocation [PATCH 3/4]: Move devpts globals into init_pts_ns [PATCH 3/4]: Enable multiple mounts of /dev/pts
This patchset is based on earlier versions developed by Serge Hallyn and Matt Helsley.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines

Regards & Thanks--
Subrata

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

_______________________________________________
Ltp-list mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

A message that is not in the archive (sent off-list or not yet received): bb33bcf20804121039u409ae37q2dead849ca24dfa-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

sukadev-r/[email protected]

2008-04-12 18:05 UTC #8 lore raw

Subrata Modak [[email protected]] wrote:

| Sukadev,
| 
| Any corresponding test cases for LTP. We just have UTS, PID & SYSVIPC
| Namespace till now.

I had some unit-tests that I used with the clone-pts-ns patchset. But the patches in this set are just helpers and should not change existing functionality.

I can send the tests I used (they are not in LTP format) when I tested the clone-pts-ns patchset.

Thanks,

Sukadev

A message that is not in the archive (sent off-list or not yet received): 20080412180533.GE19449-r/[email protected]

Subrata Modak

2008-04-14 11:20 UTC #10 lore raw

Quoted 6 lines · On Sat, 2008-04-12 at 11:05 -0700, sukadev-r/[email protected].…

Subrata Modak [[email protected]] wrote:

| Sukadev,
| 
| Any corresponding test cases for LTP. We just have UTS, PID & SYSVIPC
| Namespace till now.

Quoted 6 lines

I had some unit-tests that I used with the clone-pts-ns patchset. But the patches in this set are just helpers and should not change existing functionality.
I can send the tests I used (they are not in LTP format) when I tested the clone-pts-ns patchset.

Looping in Veerandra. He may be able to help us here.

Regards--
Subrata

Quoted 14 lines

Thanks,
Sukadev
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Ltp-list mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

A message that is not in the archive (sent off-list or not yet received): 1208172006.5762.21.camel-NRFfyExJdYpgXGGE5LP+UZlqa2bBAFbm0E9HWUfgJXw@public.gmane.org

Veerendra Chandrappa

2008-04-14 12:31 UTC #12 lore raw

Hi Sukadev,

That sounds nice, I will integrate them with the LTP.
Of course it depends on the order of priority, and as I am learning the tricks of container/ltp .

Regards, Veerendra C
________________________________________________________
Linux Technology Center, India Software Labs, Bangalore, Ph: 080-4177 6428

Subrata Modak
<[email protected]
et.ibm.com> To
sukadev-r/[email protected], Veerendra
04/14/2008 04:50 Chandrappa/India/IBM@IBMIN
PM cc
Subrata Modak
<[email protected]>, Andrew
Please respond to Morton <[email protected]>, ltp-list
[email protected] <[email protected]>,
t.ibm.com "Eric W. Biederman"
<ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>, Rishikesh
K Rajak/India/IBM@IBMIN, matthltc-r/[email protected], [email protected], Containers
<[email protected]>, Pavel
Emelyanov <[email protected]>
Subject
Re: [LTP] [PATCH 0/4] Helper
patches for PTY namespaces

Quoted 6 lines · On Sat, 2008-04-12 at 11:05 -0700, sukadev-r/[email protected].…

Subrata Modak [[email protected]] wrote:

| Sukadev,
|
| Any corresponding test cases for LTP. We just have UTS, PID & SYSVIPC
| Namespace till now.

Quoted 6 lines

I had some unit-tests that I used with the clone-pts-ns patchset. But the patches in this set are just helpers and should not change existing functionality.
I can send the tests I used (they are not in LTP format) when I tested the clone-pts-ns patchset.

Looping in Veerandra. He may be able to help us here.

Regards--
Subrata

Quoted 10 lines

Thanks,
Sukadev
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.

http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Quoted 4 lines

_______________________________________________
Ltp-list mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

A message that is not in the archive (sent off-list or not yet received): OF766ECBB8.8B7DAFCE-ON6525742B.00437346-6525742B.0043FD8A-xthvdsQ13ZrQT0dZR+AlfA@public.gmane.org

Subrata Modak

2008-04-14 12:31 UTC #14 lore raw

Quoted 5 lines · Veerendra Chandrappa

Hi Sukadev,
That sounds nice, I will integrate them with the LTP.
Of course it depends on the order of priority, and as I am learning the tricks of container/ltp .

That is cool Veerandra. Now, for this to move forward, Suka needs to hand over those test cases to you ;-)

Regards--
Subrata

Quoted 59 lines

Regards, Veerendra C
________________________________________________________
Linux Technology Center, India Software Labs, Bangalore, Ph: 080-4177 6428
Subrata Modak
<[email protected]
et.ibm.com> To
sukadev-r/[email protected], Veerendra
04/14/2008 04:50 Chandrappa/India/IBM@IBMIN
PM cc
Subrata Modak
<[email protected]>, Andrew
Please respond to Morton <[email protected]>, ltp-list
[email protected] <[email protected]>,
t.ibm.com "Eric W. Biederman"
<ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>, Rishikesh
K Rajak/India/IBM@IBMIN, matthltc-r/[email protected], [email protected], Containers
<[email protected]>, Pavel
Emelyanov <[email protected]>
Subject
Re: [LTP] [PATCH 0/4] Helper
patches for PTY namespaces
Quoted 6 lines · On Sat, 2008-04-12 at 11:05 -0700, sukadev-r/[email protected].…
Subrata Modak [[email protected]] wrote:
| Sukadev,
|
| Any corresponding test cases for LTP. We just have UTS, PID & SYSVIPC
| Namespace till now.
Quoted 6 lines
I had some unit-tests that I used with the clone-pts-ns patchset. But the patches in this set are just helpers and should not change existing functionality.
I can send the tests I used (they are not in LTP format) when I tested the clone-pts-ns patchset.
Looping in Veerandra. He may be able to help us here.
Regards--
Subrata
Quoted 3 lines
Thanks,

Quoted 16 lines

Quoted 7 lines
Sukadev
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Quoted 4 lines
_______________________________________________
Ltp-list mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

A message that is not in the archive (sent off-list or not yet received): 1208176260.5762.36.camel-NRFfyExJdYpgXGGE5LP+UZlqa2bBAFbm0E9HWUfgJXw@public.gmane.org

Subrata Modak

2008-04-22 07:11 UTC #16 lore raw

Quoted 9 lines · Subrata Modak

Quoted 5 lines · Veerendra Chandrappa
Hi Sukadev,
That sounds nice, I will integrate them with the LTP.
Of course it depends on the order of priority, and as I am learning the tricks of container/ltp .
That is cool Veerandra. Now, for this to move forward, Suka needs to hand over those test cases to you ;-)

Veerandra,

Have you already received those test cases from Suka ?

Regards--
Subrata

Quoted 81 lines

Regards--
Subrata
Quoted 59 lines
Regards, Veerendra C
________________________________________________________
Linux Technology Center, India Software Labs, Bangalore, Ph: 080-4177 6428
Subrata Modak
<[email protected]
et.ibm.com> To
sukadev-r/[email protected], Veerendra
04/14/2008 04:50 Chandrappa/India/IBM@IBMIN
PM cc
Subrata Modak
<[email protected]>, Andrew
Please respond to Morton <[email protected]>, ltp-list
[email protected] <[email protected]>,
t.ibm.com "Eric W. Biederman"
<ebiederm-aS9lmoZGLiVWk0Htik3J/[email protected]>, Rishikesh
K Rajak/India/IBM@IBMIN, matthltc-r/[email protected], [email protected], Containers
<[email protected]>, Pavel
Emelyanov <[email protected]>
Subject
Re: [LTP] [PATCH 0/4] Helper
patches for PTY namespaces
Quoted 6 lines · On Sat, 2008-04-12 at 11:05 -0700, sukadev-r/[email protected].…
Subrata Modak [[email protected]] wrote:
| Sukadev,
|
| Any corresponding test cases for LTP. We just have UTS, PID & SYSVIPC
| Namespace till now.
Quoted 6 lines
I had some unit-tests that I used with the clone-pts-ns patchset. But the patches in this set are just helpers and should not change existing functionality.
I can send the tests I used (they are not in LTP format) when I tested the clone-pts-ns patchset.
Looping in Veerandra. He may be able to help us here.
Regards--
Subrata
Quoted 3 lines
Thanks,
Quoted 16 lines
Quoted 7 lines
Sukadev
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Quoted 4 lines
_______________________________________________
Ltp-list mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Quoted 9 lines

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Ltp-list mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

A message that is not in the archive (sent off-list or not yet received): 1208848319.4847.16.camel-NRFfyExJdYpgXGGE5LP+UZlqa2bBAFbm0E9HWUfgJXw@public.gmane.org

Serge E. Hallyn

2008-04-22 12:25 UTC #18 lore raw

Quoted 8 lines · Subrata Modak

Quoted 7 lines · Subrata Modak
Quoted 5 lines · Veerendra Chandrappa
Hi Sukadev,
That sounds nice, I will integrate them with the LTP.
Of course it depends on the order of priority, and as I am learning the tricks of container/ltp .

Quoted 6 lines

Quoted 2 lines
That is cool Veerandra. Now, for this to move forward, Suka needs to hand over those test cases to you ;-)
Veerandra,
Have you already received those test cases from Suka ?

Subrata,

pty namespaces as such are not going to happen. We'll be pursuing full-scale device namespaces instead.

thanks,
-serge

A message that is not in the archive (sent off-list or not yet received): 20080422122530.GB8331-6s5zFf/[email protected]

Subrata Modak

2008-04-22 15:08 UTC #20 lore raw

Quoted 12 lines · Serge E. Hallyn

Quoted 11 lines · Subrata Modak
Quoted 9 lines · Subrata Modak
Quoted 5 lines · Veerendra Chandrappa
Hi Sukadev,
That sounds nice, I will integrate them with the LTP.
Of course it depends on the order of priority, and as I am learning the tricks of container/ltp .
That is cool Veerandra. Now, for this to move forward, Suka needs to hand over those test cases to you ;-)

Quoted 9 lines

Quoted 3 lines
Veerandra,
Have you already received those test cases from Suka ?
Subrata,
pty namespaces as such are not going to happen. We'll be pursuing full-scale device namespaces instead.

Oops.

device namespaces test cases are also welcome in future.

Regards--
Subrata

Quoted 2 lines

thanks,
-serge

H. Peter Anvin

2008-04-23 18:01 UTC #21 lore raw

In reply to a message not in the archive

Quoted 6 lines · Serge E. Hallyn

Subrata,
pty namespaces as such are not going to happen. We'll be pursuing full-scale device namespaces instead.

Again, either that, or tie Unix98 pty's closer into devpts (which would have other advantages, in particular avoiding the double lookup) which would permit the ordinary filesystem namespace mechanisms to be used.

-hpa

H. Peter Anvin

2008-04-12 18:09 UTC #22 lore raw

In reply to [email protected]

Quoted 10 lines · sukadev-r/[email protected] wrote:

Some simple helper patches to enable implementation of multiple PTY (or device) namespaces.
[PATCH 1/4]: Propagate error code from devpts_pty_new [PATCH 2/4]: Factor out PTY index allocation [PATCH 3/4]: Move devpts globals into init_pts_ns [PATCH 3/4]: Enable multiple mounts of /dev/pts
This patchset is based on earlier versions developed by Serge Hallyn and Matt Helsley.

Any measurable performance impact when not using these kinds of namespaces?

-hpa

Al Viro

2008-04-12 18:35 UTC #23 lore raw

In reply to [email protected]

Quoted 11 lines · On Sat, Apr 12, 2008 at 10:29:33AM -0700, [email protected] wrote:

Some simple helper patches to enable implementation of multiple PTY (or device) namespaces.
[PATCH 1/4]: Propagate error code from devpts_pty_new [PATCH 2/4]: Factor out PTY index allocation [PATCH 3/4]: Move devpts globals into init_pts_ns [PATCH 3/4]: Enable multiple mounts of /dev/pts
This patchset is based on earlier versions developed by Serge Hallyn and Matt Helsley.

*boggle*

Care to explain how that "namespace" is different from devpts instance? IOW, why the devil do you guys ignore Occam's Razor?

Frankly, this nonsense has gone far enough; I can buy the need to compensate for shitty APIs (sockets, non-fs-based-IPC, etc.), but devpts *is* *a* *fucking* *filesystem*. Already. And as such it's already present in normal, real, we-really-shouldn't-have-any-other-if-not-for-ancient-stupidity namespace.

Why not simply allow independent instances of devpts and be done with that?

H. Peter Anvin

2008-04-12 18:54 UTC #24 lore raw

Multiple instances of devpts

Quoted 6 lines · Al Viro

*boggle*
Care to explain how that "namespace" is different from devpts instance? IOW, why the devil do you guys ignore Occam's Razor?

Quoted 7 lines

Frankly, this nonsense has gone far enough; I can buy the need to compensate for shitty APIs (sockets, non-fs-based-IPC, etc.), but devpts *is* *a* *fucking* *filesystem*. Already. And as such it's already present in normal, real, we-really-shouldn't-have-any-other-if-not-for-ancient-stupidity namespace.
Why not simply allow independent instances of devpts and be done with that?

In particular:

/dev/ptmx can be a symlink ptmx -> pts/ptmx, and we add a ptmx instance inside the devpts filesystem. Each devpts filesystem is responsible for its own pool of ptys, with own numbering, etc.

This does mean that entries in /dev/pts are more than just plain device nodes, which they are now (you can cp -a a device node from /dev/pts into another filesystem and it will still "just work"), but I doubt this actually matters to anyone. If anyone cares, now I guess would be a good time to speak up.

-hpa

Eric W. Biederman

2008-04-12 19:15 UTC #25 lore raw

Re: Multiple instances of devpts

Quoted 10 lines · H. Peter Anvin

Quoted 2 lines · Al Viro
Why not simply allow independent instances of devpts and be done with that?
In particular:
/dev/ptmx can be a symlink ptmx -> pts/ptmx, and we add a ptmx instance inside the devpts filesystem. Each devpts filesystem is responsible for its own pool of ptys, with own numbering, etc.

Quoted 5 lines

This does mean that entries in /dev/pts are more than just plain device nodes, which they are now (you can cp -a a device node from /dev/pts into another filesystem and it will still "just work"), but I doubt this actually matters to anyone. If anyone cares, now I guess would be a good time to speak up.

Agreed. That is another legitimate path. And if all you care about is
isolation and not dealing with the general class of problems with the global device number to device mapping that is sane. I know we have several other virtual devices that we tend to care about but ptys are the real world pain point.

Further I don't see any conflict with the generalizing devpts in this manner (so you only see a subset of the ptys) and then later adding a namespace that deals with the whole device number to device mapping.

Eric

H. Peter Anvin

2008-04-12 19:24 UTC #26 lore raw

Re: Multiple instances of devpts

Quoted 11 lines · Eric W. Biederman

Quoted 10 lines
/dev/ptmx can be a symlink ptmx -> pts/ptmx, and we add a ptmx instance inside the devpts filesystem. Each devpts filesystem is responsible for its own pool of ptys, with own numbering, etc.
This does mean that entries in /dev/pts are more than just plain device nodes, which they are now (you can cp -a a device node from /dev/pts into another filesystem and it will still "just work"), but I doubt this actually matters to anyone. If anyone cares, now I guess would be a good time to speak up.

Quoted 6 lines

Agreed. That is another legitimate path. And if all you care about is
isolation and not dealing with the general class of problems with the global device number to device mapping that is sane. I know we have several other virtual devices that we tend to care about but ptys are the real world pain point.

Thinking about it further, allowing this restriction would also allow a whole lot of cleanups inside the pty setup, since it would eliminate the need to do a separate lookup to find the corresponding devpts entry in pty_open(). The benefit here comes from the closer coupling between the pty and the devpts filesystem and isn't at all related to namespaces, but it's a very nice side benefit.

-hpa

H. Peter Anvin

2008-04-12 19:30 UTC #27 lore raw

Re: Multiple instances of devpts

Quoted 8 lines · H. Peter Anvin

Thinking about it further, allowing this restriction would also allow a whole lot of cleanups inside the pty setup, since it would eliminate the need to do a separate lookup to find the corresponding devpts entry in pty_open(). The benefit here comes from the closer coupling between the pty and the devpts filesystem and isn't at all related to namespaces, but it's a very nice side benefit.

Minor correction: the lookup is actually in init_dev() in tty_io.c; I'm specifically referring to devpts_get_tty().

-hpa

Eric W. Biederman

2008-04-12 19:06 UTC #28 lore raw

In reply to [email protected]

Quoted 10 lines · On Sat, 2008-04-12 at 10:29 -0700, [email protected] wrote:

Some simple helper patches to enable implementation of multiple PTY (or device) namespaces.
[PATCH 1/4]: Propagate error code from devpts_pty_new [PATCH 2/4]: Factor out PTY index allocation [PATCH 3/4]: Move devpts globals into init_pts_ns [PATCH 3/4]: Enable multiple mounts of /dev/pts
This patchset is based on earlier versions developed by Serge Hallyn and Matt Helsley.

Suka Stop.

The first two patches appear to just be cleanups and as such should be able to stand on their own. Mentioning that you found these opportunities while working on your pts namespace is fine. Justifying the cleanups this way is not.

When you mentioned you intended to just resend the cleanups this is not at all what I thought you were about. So please just get the first two patches in a form that stands by themselves.

The pts namespace as designed is not acceptable.

The problem you are trying to solve with the pts namespace is real.

So what we need is a device namespace and possibly and incremental path to get there. A device namespace would abstract the device number to device mapping for all devices. A safe incremental path would disable all device number to device mappings for process in that namespace. So all functionality would be gone, then it would enable certain mappings and certain pieces of the functionality if the code in the kernel is not clean enough that we can do it all in one go.

We need to see that path. Only then can we take patches that add namespace specific goo. The pattern I am proposing has worked quite well for the network namespace. Meanwhile the uid namespace which follows the pattern you seem to be following now seems does not look to be completed any time soon.

So send your clean up patches and then let's architect this thing so we are really talking about a namespace. Then we can update devpts to capture the device namespace on mount and do it's work in a namespace specific manner.

Eric

Serge E. Hallyn

2008-04-13 00:59 UTC #29 lore raw

Quoted 14 lines · Eric W. Biederman

Quoted 10 lines · On Sat, 2008-04-12 at 10:29 -0700, [email protected] wrote:
Some simple helper patches to enable implementation of multiple PTY (or device) namespaces.
[PATCH 1/4]: Propagate error code from devpts_pty_new [PATCH 2/4]: Factor out PTY index allocation [PATCH 3/4]: Move devpts globals into init_pts_ns [PATCH 3/4]: Enable multiple mounts of /dev/pts
This patchset is based on earlier versions developed by Serge Hallyn and Matt Helsley.
Suka Stop.

Quoted 8 lines

The first two patches appear to just be cleanups and as such should be able to stand on their own. Mentioning that you found these opportunities while working on your pts namespace is fine. Justifying the cleanups this way is not.
When you mentioned you intended to just resend the cleanups this is not at all what I thought you were about. So please just get the first two patches in a form that stands by themselves.

It took me a minute to figure out what you were offended by, but I see, the patches still introduce a "pts ns", which shouldn't exist at all.

Quoted 12 lines

The pts namespace as designed is not acceptable.
The problem you are trying to solve with the pts namespace is real.
So what we need is a device namespace and possibly and incremental path to get there. A device namespace would abstract the device number to device mapping for all devices. A safe incremental path would disable all device number to device mappings for process in that namespace. So all functionality would be gone, then it would enable certain mappings and certain pieces of the functionality if the code in the kernel is not clean enough that we can do it all in one go.

Quoted 5 lines

We need to see that path. Only then can we take patches that add namespace specific goo. The pattern I am proposing has worked quite well for the network namespace. Meanwhile the uid namespace which follows the pattern you seem to be following now seems does not look to be completed any time soon.

(you know perfectly well that we're holding off on any further real uidns work until netns is complete and you have time to partake in the design - it wouldn't be any fun without you :)

Quoted 4 lines

So send your clean up patches and then let's architect this thing so we are really talking about a namespace. Then we can update devpts to capture the device namespace on mount and do it's work in a namespace specific manner.

Sounds reasonable to me.

thanks,
-serge

H. Peter Anvin

2008-08-01 18:12 UTC #30 lore raw

Per-instance devpts

In reply to Eric W. Biederman

Since the issue of PTY namespaces came up (and was rejected) back in April, I have thought a little bit about changing ptys to be tied directly into a devpts instance. devpts would then be a "normal" filesystem, which can be mounted multiple times (or not at all). pty's would then become private to a devpts instance.

This is what it would appear would have to change, and I'd like to get people's feeing for the user-space impact:

1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.
2. Permissions on /dev/ptmx would not be persistent, and would have to be set via devpts mount options (unless they're 0666 root.tty, which would presumably be the default.)
3. The /proc/sys/kernel/pty limit would be global; a per-filesystem limit could be added on top or instead (presumably via a filesystem mount options.)

I worry #1 would have substantial user-space impact, but I don't see a way around it, since there would be no obvious way to associate /dev/ptmx with a filesystem.

-hpa

Dave Hansen

2008-08-01 19:23 UTC #31 lore raw

Re: Per-instance devpts

Quoted 1 line · H. Peter Anvin

1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.

...

Quoted 3 lines

I worry #1 would have substantial user-space impact, but I don't see a way around it, since there would be no obvious way to associate /dev/ptmx with a filesystem.

Are your worries just about replacing what is now a normal file with a symlink, and the behavioral changes that come with that?

I wonder if using a bind mount for the file would be more robust. We wouldn't, of course, be able to do it persistently, but I bet it would be something we could count on udev to do for us.

-- Dave

Al Viro

2008-08-01 19:35 UTC #32 lore raw

Re: Per-instance devpts

Quoted 7 lines · Dave Hansen

Quoted 1 line · H. Peter Anvin
1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.
...
Quoted 3 lines
I worry #1 would have substantial user-space impact, but I don't see a way around it, since there would be no obvious way to associate /dev/ptmx with a filesystem.

Quoted 6 lines

Are your worries just about replacing what is now a normal file with a symlink, and the behavioral changes that come with that?
I wonder if using a bind mount for the file would be more robust. We wouldn't, of course, be able to do it persistently, but I bet it would be something we could count on udev to do for us.

Not on my boxen. However, devpts tends to be mounted by initscripts, which is where one would naturally do that.

H. Peter Anvin

2008-08-01 19:37 UTC #33 lore raw

Re: Per-instance devpts

In reply to Dave Hansen

Quoted 11 lines · Dave Hansen

Quoted 1 line · H. Peter Anvin
1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.
...
Quoted 1 line
I worry #1 would have substantial user-space impact, but I don't see a way around it, since there would be no obvious way to associate /dev/ptmx with a filesystem.
Are your worries just about replacing what is now a normal file with a symlink, and the behavioral changes that come with that?
I wonder if using a bind mount for the file would be more robust. We wouldn't, of course, be able to do it persistently, but I bet it would be something we could count on udev to do for us.

No, I'm concerned about the changes needed for udev and setup scripts.

-hpa

A message that is not in the archive (sent off-list or not yet received): [email protected]

Kyle Moffett

2008-08-02 07:06 UTC #35 lore raw

Re: Per-instance devpts

My apologies, I accidentally hit "reply" instead of "reply all" and this got sent only to Peter.

Quoted 10 lines · H. Peter Anvin

This is what it would appear would have to change, and I'd like to get people's feeing for the user-space impact:
1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.
2. Permissions on /dev/ptmx would not be persistent, and would have to be set via devpts mount options (unless they're 0666 root.tty, which would presumably be the default.)
3. The /proc/sys/kernel/pty limit would be global; a per-filesystem limit could be added on top or instead (presumably via a filesystem mount options.)

Here's my suggestion:

By default, without any mount options, use the current "legacy" behavior. The devpts filesystem would point to a "global" instance on the whole box, controlled by the traditional /dev/ptmx device node. There would *NOT* be a /dev/pts/ptmx node.

If the devpts filesystem is mounted with a special option ("permount"? "noglobal"?), then it will create a new devpts instance associated with the filesystem. A devpts mounted that way *WILL* have a magic /dev/pts/ptmx node.

If the kernel is built with CONFIG_DEVPTS_FORCE_PERMOUNT then the traditional /dev/ptmx device node will be neutered (IE: always return -ENODEV) and the "permount" option will be forced for all devpts mounts. This will also remove the static global devpts instance.

Once distros add the ptmx=>pts/ptmx symlink and validate their software they can turn on that config option and be able to safely virtualize /dev/pts.

For the distros which don't, sysadmins can easily patch their own udev and init scripts to turn on the "permount" option and set up the ptmx symlink, although child namespaces will still theoretically be able to get outside of their namespace through the mostly-unused global devpts instance.

Cheers,
Kyle Moffett

H. Peter Anvin

2008-08-02 15:33 UTC #36 lore raw

Re: Per-instance devpts

Quoted 13 lines · Kyle Moffett

Here's my suggestion:
By default, without any mount options, use the current "legacy" behavior. The devpts filesystem would point to a "global" instance on the whole box, controlled by the traditional /dev/ptmx device node. There would *NOT* be a /dev/pts/ptmx node.
If the devpts filesystem is mounted with a special option ("permount"? "noglobal"?), then it will create a new devpts instance associated with the filesystem. A devpts mounted that way *WILL* have a magic /dev/pts/ptmx node.

Quoted 5 lines

If the kernel is built with CONFIG_DEVPTS_FORCE_PERMOUNT then the traditional /dev/ptmx device node will be neutered (IE: always return -ENODEV) and the "permount" option will be forced for all devpts mounts. This will also remove the static global devpts instance.

Hm. This might work if we can get the mount behaviour to work right. I'll think about it. It definitely seems like a reasonable way to get from A to B.

-hpa

Bastian Blank

2008-08-02 08:54 UTC #37 lore raw

Re: Per-instance devpts

In reply to H. Peter Anvin

Quoted 3 lines · H. Peter Anvin

2. Permissions on /dev/ptmx would not be persistent, and would have to be set via devpts mount options (unless they're 0666 root.tty, which would presumably be the default.)

On Debian based systems /dev/ptmx is 0666 root.root. But the gid of tty is already supplied for devpts.

Quoted 3 lines

I worry #1 would have substantial user-space impact, but I don't see a way around it, since there would be no obvious way to associate /dev/ptmx with a filesystem.

Hmm. Several possibilities:
- Change the filesysteme name and the old name remains usable with /dev/ptmx.
Problem: You could mount the filesystem with the old name within a container.
- Make the first mounted one special.
Problem: Will not survive a umount/mount cycle. But this would be not the case anyway.

But you are sure, none of them is a pretty solution.

Bastian

Signature · 2 lines

I'm a soldier, not a diplomat.  I can only tell the truth.
		-- Kirk, "Errand of Mercy", stardate 3198.9

[email protected]

author

2008-08-03 05:08 UTC #38 lore raw

Re: Per-instance devpts

In reply to H. Peter Anvin

Quoted 5 lines · H. Peter Anvin [[email protected]] wrote:

Since the issue of PTY namespaces came up (and was rejected) back in April, I have thought a little bit about changing ptys to be tied directly into a devpts instance. devpts would then be a "normal" filesystem, which can be mounted multiple times (or not at all). pty's would then become private to a devpts instance.

Sorry, I thought we were going with a complete device namespace - since that would address other devices as well and would avoid the following user-space issue.

I guess this issue came up in OLS recently and have been looking into this again. I have some helper patches to explore multiple mounts of devpts without namespace stuff and can send them out in a couple of days.

Quoted 5 lines

This is what it would appear would have to change, and I'd like to get people's feeing for the user-space impact:
1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.

IIRC, /dev/tty also needs a similar symlink.

Quoted 10 lines

2. Permissions on /dev/ptmx would not be persistent, and would have to be set via devpts mount options (unless they're 0666 root.tty, which would presumably be the default.)
3. The /proc/sys/kernel/pty limit would be global; a per-filesystem limit could be added on top or instead (presumably via a filesystem mount options.)
I worry #1 would have substantial user-space impact, but I don't see a way around it, since there would be no obvious way to associate /dev/ptmx with a filesystem.

Sukadev

H. Peter Anvin

2008-08-03 11:31 UTC #39 lore raw

Re: Per-instance devpts

Quoted 3 lines · sukadev-r/[email protected] wrote:

IIRC, /dev/tty also needs a similar symlink.

Why? I do not believe that is correct.

-hpa

Alan Cox

2008-08-03 12:04 UTC #40 lore raw

Re: Per-instance devpts

In reply to [email protected]

Quoted 3 lines

Quoted 1 line
1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.
IIRC, /dev/tty also needs a similar symlink.

Making them symlinks is asking for trouble because some code does go around using stat and the like and tools like MAKEDEV have definite ideas.

For /dev/tty the definition is precisely that it is your controlling tty. No reference to namespace and a task whose controlling tty is in a different namespace should still open the controlling tty not some parallel in another universe when you open /dev/tty.

If you want to make sure the controlling tty is in the right namespace that can be done in userspace when transferring control into a namespace. In many cases I doubt that is even what is wanted.

Quoted 10 lines

Quoted 10 lines
2. Permissions on /dev/ptmx would not be persistent, and would have to be set via devpts mount options (unless they're 0666 root.tty, which would presumably be the default.)
3. The /proc/sys/kernel/pty limit would be global; a per-filesystem limit could be added on top or instead (presumably via a filesystem mount options.)
I worry #1 would have substantial user-space impact, but I don't see a way around it, since there would be no obvious way to associate /dev/ptmx with a filesystem.

/dev/tty and /dev/ptmx already primarily operate by identifying a device and switching the work to that. Actually putting a bit of namespace logic in the driver code so the actual files stay as expected (device nodes etc) seems a *lot* simpler than trying to do symlink hacks.

Alan

[email protected]

author

2008-08-03 17:46 UTC #41 lore raw

Re: Per-instance devpts

Alan Cox [[email protected]] wrote:

| > > 1. /dev/ptmx would have to change to a symlink, ptmx -> pts/ptmx.
| > 
| > IIRC, /dev/tty also needs a similar symlink.
| 
| Making them symlinks is asking for trouble because some code does go
| around using stat and the like and tools like MAKEDEV have definite ideas.
| 
| For /dev/tty the definition is precisely that it is your controlling
| tty. No reference to namespace and a task whose controlling tty is in a
| different namespace should still open the controlling tty not some
| parallel in another universe when you open /dev/tty.

Well, I thought the problem was something like this:

If /dev/pts/1 is the controlling terminal and there are multiple mounts of devpts, when we open /dev/tty, kernel would somehow have to find the right instance of devpts.

When init_dev() calls devpts_get_tty(), it would need to specify the devpts instance. So tty_open() of "/dev/tty" would somehow have to find it based on the /dev/tty inode (which could be in ext3).

(I thought the issue was similar with /dev/ptmx, ptmx allocates a new index, /dev/tty accesses an existing index, but both need to somehow find the right pts instance -no ?)

| 
| If you want to make sure the controlling tty is in the right namespace
| that can be done in userspace when transferring control into a namespace.
| In many cases I doubt that is even what is wanted.
| 
| > > 2. Permissions on /dev/ptmx would not be persistent, and would have to
| > >    be set via devpts mount options (unless they're 0666 root.tty, which
| > >    would presumably be the default.)
| > > 3. The /proc/sys/kernel/pty limit would be global; a per-filesystem
| > >    limit could be added on top or instead (presumably via a filesystem
| > >    mount options.)
| > >
| > > I worry #1 would have substantial user-space impact, but I don't see a way 
| > > around it, since there would be no obvious way to associate /dev/ptmx with 
| > > a filesystem.
| 
| /dev/tty and /dev/ptmx already primarily operate by identifying a device
| and switching the work to that. Actually putting a bit of namespace logic
| in the driver code so the actual files stay as expected (device nodes
| etc) seems a *lot* simpler than trying to do symlink hacks.
| 
| Alan

Alan Cox

2008-08-03 17:54 UTC #42 lore raw

Re: Per-instance devpts

Quoted 3 lines

If /dev/pts/1 is the controlling terminal and there are multiple mounts of devpts, when we open /dev/tty, kernel would somehow have to find the right instance of devpts.

current->tty - it is a direct internal reference.

Alan