Re: A modest proposal -- We need a patch penguin

Re: A modest proposal -- We need a patch penguin

[Andi Kleen]

torvalds@transmeta.com (Linus Torvalds) writes:

> In article <200201292208.g0TM8ql17622@ns.caldera.de>,
> Christoph Hellwig  <hch@ns.caldera.de> wrote:
> >In article <p73aduwddni.fsf@oldwotan.suse.de> you wrote:
> >> "Most times". For example the EA patches have badly failed so far, just because
> >> Linus ignored all patches to add sys call numbers for a repeatedly discussed 
> >> and stable API and nobody else can add syscall numbers on i386. 
> >
> >There still seems to be a lot of discussion vs EAs and ACLs.
> >Setting the suboptimal XFS APIs in stone doesn't make the discussion
> >easier.
> 
> In fact, every time I thought that the extended attributes had reached
> some kind of consensus, somebody piped up with some apparently major
> complaint. 
> 
> I think last time it was Al Viro.  Admittedly (_very_ much admittedly),
> making Al happy is really really hard.  His perfectionism makes his
> patches very easy to accept, but they make it hard for others to try to
> make _him_ accept patches.  But since he effectively is the VFS
> maintainer whether he wants it to be written down in MAINTAINERS or not,
> a comment from him on VFS interfaces makes me jump. 
> 
> The last discussion over EA's in my mailbox was early-mid December, and
> there were worries from Al and Stephen Tweedie.  I never heard from the
> worried people whether their worries were calmed.

Stephen's objection was that the EA specification didn't define a standard
format for ACL EAs too. That's certainly true, but outside the EA spec
actually. First there are good reasons to have ACLs mapped to generic EAs.
There are also already other applications of EAs independent
of ACLs, for example there is an DMAPI implementation for XFS which needs
EAs, or HFS and NTFS need to map arbitary EAs defined by the foreign fs
to some common backupable format, and various extended security projects
need generic "blob" EAs too to store their per file security information.
Mapping the ACLs to EAs has the advantage that you can backup/archive/
network sync all these things with a single API, instead of teaching all
your backup tools about first ACLs and then generic EAs. 

About the common ACL spec: there are unfortunately different flavours of
ACLs. All the unix like implementations so far (ext2/3-acl and XFS acl)
use something very near the withdrawn POSIX draft, while NTFS/CIFS/NFSv4
have completely different ACLs, and AFS has another flavour. Trying to map 
these into a single ACL syscall seemed to hard and likely doesn't make 
sense. 

ext2/XFS currently rely on a single user space library that generates 
the POSIX ACLs. NTFS/NFSv4/etc. are out of scope because it's not clear
how to merge them. With generic EAs you could have a different user library
that manages NTFS ACLs (and another one for AFS etc.), it seems there will 
be no clean way around this.

For the ACL format used by the bestbits libacl there is afaik currently
no draft, just some source code, but the semantics are defined by the
POSIX draft and relatively fixed and also quite common in other Unixes. 

Al's objection seemed to be that the patch adding the new VFS functions
passed a struct inode * instead of a struct dentry * in one case. 
Fixing that is a trivial search'n'replace. I think it wasn't done at first
because it required changing the VFS to pass a dentry to i_op->permission() 
and they didn't want to bloat the patch with search-n-replace in every
filesystem yet. If you would integrate it that could be done of course.
[in short it is a non brainer, has nothing to do with the API but is 
just a small implementation detail that can be easily changed] 

Does that answer your questions? 
Would you look at a patch again? 

-Andi

Re: A modest proposal -- We need a patch penguin

[Linus Torvalds]

On 30 Jan 2002, Andi Kleen wrote:
> Does that answer your questions?
> Would you look at a patch again?

That answers the specific questions about Al and Stephen.

It does NOT address whether consensus has been reached in general, and
whether people are happy. Is that the case?

Also, obviously nobody actually took over maintainership of the patch,
because equally obviously nobody has been pinging me about it. For some
reason you seem to want _me_ to go out of my way to search for patches
that are over a month old that I don't know whether they are valid or not,
used or not, or even agreed upon or not.

But yes, it's so much easier to blame me.

			Linus

Re: A modest proposal -- We need a patch penguin

[Nathan Scott]

hi Linus,

On Tue, Jan 29, 2002 at 03:13:14PM -0800, Linus Torvalds wrote:
> 
> On 30 Jan 2002, Andi Kleen wrote:
> > Does that answer your questions?
> > Would you look at a patch again?
> 
> That answers the specific questions about Al and Stephen.

Al had several (additional) issues with the original patch, but I
think we progressively worked through them - Al stopped suggesting
changes at one point anyway, and the level of abuse died away ;),
so I guess he became more satisfied with them.

> It does NOT address whether consensus has been reached in general, and
> whether people are happy. Is that the case?

I believe so - I know that the ext2/ext3 EA/ACL maintainer 
(AndreasG, CCd) is satisfied with the current patches, from
an XFS point of view they satisfy all our needs, and Anton
has chimed in saying this interface will work well for NTFS
EA support (Anton sent me patches and several suggestions as
well, which were included in the patch at the time).

> Also, obviously nobody actually took over maintainership of the patch,
> because equally obviously nobody has been pinging me about it. For some
> reason you seem to want _me_ to go out of my way to search for patches
> that are over a month old that I don't know whether they are valid or not,
> used or not, or even agreed upon or not.
> 
> But yes, it's so much easier to blame me.
> 

Not much point apportioning blame - its as much my fault - I
hadn't heard back from you at all since day 1, so figured you
were just not interested in this stuff, so I stopped sending.

The two patches which seemed to satisfy the most people are
below - they are unchanged from 2.5.0 but should apply to any
2.5.x tree (they are fairly non-intrusive and the system call
table hasn't changed since last time).  A complete userspace
implementation for both EAs and POSIX ACLs exists above these
interfaces.

cheers.

-- 
Nathan


[Patch #1: reserve syscall numbers]

diff -Naur 2.5.0-pristine/arch/i386/kernel/entry.S 2.5.0-reserved/arch/i386/kernel/entry.S
--- 2.5.0-pristine/arch/i386/kernel/entry.S	Sat Nov  3 12:18:49 2001
+++ 2.5.0-reserved/arch/i386/kernel/entry.S	Tue Dec  4 11:57:32 2001
@@ -622,6 +622,18 @@
 	.long SYMBOL_NAME(sys_ni_syscall)	/* Reserved for Security */
 	.long SYMBOL_NAME(sys_gettid)
 	.long SYMBOL_NAME(sys_readahead)	/* 225 */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for setxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for lsetxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for fsetxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for getxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* 230 reserved for lgetxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for fgetxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for listxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for llistxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for flistxattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* 235 reserved for removexattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for lremovexattr */
+	.long SYMBOL_NAME(sys_ni_syscall)	/* reserved for fremovexattr */
 
 	.rept NR_syscalls-(.-sys_call_table)/4
 		.long SYMBOL_NAME(sys_ni_syscall)
diff -Naur 2.5.0-pristine/include/asm-i386/unistd.h 2.5.0-reserved/include/asm-i386/unistd.h
--- 2.5.0-pristine/include/asm-i386/unistd.h	Thu Oct 18 03:03:03 2001
+++ 2.5.0-reserved/include/asm-i386/unistd.h	Tue Dec  4 11:58:21 2001
@@ -230,6 +230,18 @@
 #define __NR_security		223	/* syscall for security modules */
 #define __NR_gettid		224
 #define __NR_readahead		225
+#define __NR_setxattr		226
+#define __NR_lsetxattr		227
+#define __NR_fsetxattr		228
+#define __NR_getxattr		229
+#define __NR_lgetxattr		230
+#define __NR_fgetxattr		231
+#define __NR_listxattr		232
+#define __NR_llistxattr		233
+#define __NR_flistxattr		234
+#define __NR_removexattr	235
+#define __NR_lremovexattr	236
+#define __NR_fremovexattr	237
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 


[Patch #2: VFS implementation]

diff -Naur 2.5.0-pristine/arch/i386/kernel/entry.S 2.5.0-xattr/arch/i386/kernel/entry.S
--- 2.5.0-pristine/arch/i386/kernel/entry.S	Sat Nov  3 12:18:49 2001
+++ 2.5.0-xattr/arch/i386/kernel/entry.S	Thu Dec  6 12:59:47 2001
@@ -622,6 +622,18 @@
 	.long SYMBOL_NAME(sys_ni_syscall)	/* Reserved for Security */
 	.long SYMBOL_NAME(sys_gettid)
 	.long SYMBOL_NAME(sys_readahead)	/* 225 */
+	.long SYMBOL_NAME(sys_setxattr)
+	.long SYMBOL_NAME(sys_lsetxattr)
+	.long SYMBOL_NAME(sys_fsetxattr)
+	.long SYMBOL_NAME(sys_getxattr)
+	.long SYMBOL_NAME(sys_lgetxattr)	/* 230 */
+	.long SYMBOL_NAME(sys_fgetxattr)
+	.long SYMBOL_NAME(sys_listxattr)
+	.long SYMBOL_NAME(sys_llistxattr)
+	.long SYMBOL_NAME(sys_flistxattr)
+	.long SYMBOL_NAME(sys_removexattr)	/* 235 */
+	.long SYMBOL_NAME(sys_lremovexattr)
+	.long SYMBOL_NAME(sys_fremovexattr)
 
 	.rept NR_syscalls-(.-sys_call_table)/4
 		.long SYMBOL_NAME(sys_ni_syscall)
diff -Naur 2.5.0-pristine/fs/Makefile 2.5.0-xattr/fs/Makefile
--- 2.5.0-pristine/fs/Makefile	Tue Nov 13 04:34:16 2001
+++ 2.5.0-xattr/fs/Makefile	Thu Dec  6 12:59:47 2001
@@ -14,7 +14,7 @@
 		super.o block_dev.o char_dev.o stat.o exec.o pipe.o namei.o \
 		fcntl.o ioctl.o readdir.o select.o fifo.o locks.o \
 		dcache.o inode.o attr.o bad_inode.o file.o iobuf.o dnotify.o \
-		filesystems.o namespace.o seq_file.o
+		filesystems.o namespace.o seq_file.o xattr.o
 
 ifeq ($(CONFIG_QUOTA),y)
 obj-y += dquot.o
diff -Naur 2.5.0-pristine/fs/xattr.c 2.5.0-xattr/fs/xattr.c
--- 2.5.0-pristine/fs/xattr.c	Thu Jan  1 10:00:00 1970
+++ 2.5.0-xattr/fs/xattr.c	Thu Dec  6 12:59:58 2001
@@ -0,0 +1,341 @@
+/*
+  File: fs/xattr.c
+
+  Extended attribute handling.
+
+  Copyright (C) 2001 by Andreas Gruenbacher <a.gruenbacher@computer.org>
+  Copyright (C) 2001 SGI - Silicon Graphics, Inc <linux-xfs@oss.sgi.com>
+ */
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/smp_lock.h>
+#include <linux/file.h>
+#include <linux/xattr.h>
+#include <asm/uaccess.h>
+
+/*
+ * Extended attribute memory allocation wrappers, originally
+ * based on the Intermezzo PRESTO_ALLOC/PRESTO_FREE macros.
+ * The vmalloc use here is very uncommon - extended attributes
+ * are supposed to be small chunks of metadata, and it is quite
+ * unusual to have very many extended attributes, so lists tend
+ * to be quite short as well.  The 64K upper limit is derived
+ * from the extended attribute size limit used by XFS.
+ * Intentionally allow zero @size for value/list size requests.
+ */
+static void *
+xattr_alloc(size_t size, size_t limit)
+{
+	void *ptr;
+
+	if (size > limit)
+		return ERR_PTR(-E2BIG);
+
+	if (!size)	/* size request, no buffer is needed */
+		return NULL;
+	else if (size <= PAGE_SIZE)
+		ptr = kmalloc((unsigned long) size, GFP_KERNEL);
+	else
+		ptr = vmalloc((unsigned long) size);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+	return ptr;
+}
+
+static void
+xattr_free(void *ptr, size_t size)
+{
+	if (!size)	/* size request, no buffer was needed */
+		return;
+	else if (size <= PAGE_SIZE)
+		kfree(ptr);
+	else
+		vfree(ptr);
+}
+
+/*
+ * Extended attribute SET operations
+ */
+static long
+setxattr(struct dentry *d, char *name, void *value, size_t size, int flags)
+{
+	int error;
+	void *kvalue;
+	char kname[XATTR_NAME_MAX + 1];
+
+	if (flags & ~(XATTR_CREATE|XATTR_REPLACE))
+		return -EINVAL;
+
+	if (copy_from_user(kname, name, XATTR_NAME_MAX))
+		return -EFAULT;
+	kname[XATTR_NAME_MAX] = '\0';
+
+	kvalue = xattr_alloc(size, XATTR_SIZE_MAX);
+	if (IS_ERR(kvalue))
+		return PTR_ERR(kvalue);
+
+	if (size > 0 && copy_from_user(kvalue, value, size)) {
+		xattr_free(kvalue, size);
+		return -EFAULT;
+	}
+
+	error = -EOPNOTSUPP;
+	if (d->d_inode->i_op && d->d_inode->i_op->setxattr) {
+		lock_kernel();
+		error = d->d_inode->i_op->setxattr(d, kname, kvalue, size, flags);
+		unlock_kernel();
+	}
+
+	xattr_free(kvalue, size);
+	return error;
+}
+
+asmlinkage long
+sys_setxattr(char *path, char *name, void *value, size_t size, int flags)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk(path, &nd);
+	if (error)
+		return error;
+	error = setxattr(nd.dentry, name, value, size, flags);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_lsetxattr(char *path, char *name, void *value, size_t size, int flags)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk_link(path, &nd);
+	if (error)
+		return error;
+	error = setxattr(nd.dentry, name, value, size, flags);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_fsetxattr(int fd, char *name, void *value, size_t size, int flags)
+{
+	struct file *f;
+	int error = -EBADF;
+
+	f = fget(fd);
+	if (!f)
+		return error;
+	error = setxattr(f->f_dentry, name, value, size, flags);
+	fput(f);
+	return error;
+}
+
+/*
+ * Extended attribute GET operations
+ */
+static long
+getxattr(struct dentry *d, char *name, void *value, size_t size)
+{
+	int error;
+	void *kvalue;
+	char kname[XATTR_NAME_MAX + 1];
+
+	if (copy_from_user(kname, name, XATTR_NAME_MAX))
+		return -EFAULT;
+	kname[XATTR_NAME_MAX] = '\0';
+
+	kvalue = xattr_alloc(size, XATTR_SIZE_MAX);
+	if (IS_ERR(kvalue))
+		return PTR_ERR(kvalue);
+
+	error = -EOPNOTSUPP;
+	if (d->d_inode->i_op && d->d_inode->i_op->getxattr) {
+		lock_kernel();
+		error = d->d_inode->i_op->getxattr(d, kname, kvalue, size);
+		unlock_kernel();
+	}
+
+	if (kvalue && error > 0)
+		if (copy_to_user(value, kvalue, size))
+			error = -EFAULT;
+	xattr_free(kvalue, size);
+	return error;
+}
+
+asmlinkage long
+sys_getxattr(char *path, char *name, void *value, size_t size)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk(path, &nd);
+	if (error)
+		return error;
+	error = getxattr(nd.dentry, name, value, size);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_lgetxattr(char *path, char *name, void *value, size_t size)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk_link(path, &nd);
+	if (error)
+		return error;
+	error = getxattr(nd.dentry, name, value, size);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_fgetxattr(int fd, char *name, void *value, size_t size)
+{
+	struct file *f;
+	int error = -EBADF;
+
+	f = fget(fd);
+	if (!f)
+		return error;
+	error = getxattr(f->f_dentry, name, value, size);
+	fput(f);
+	return error;
+}
+
+/*
+ * Extended attribute LIST operations
+ */
+static long
+listxattr(struct dentry *d, char *list, size_t size)
+{
+	int error;
+	char *klist;
+
+	klist = (char *)xattr_alloc(size, XATTR_LIST_MAX);
+	if (IS_ERR(klist))
+		return PTR_ERR(klist);
+
+	error = -EOPNOTSUPP;
+	if (d->d_inode->i_op && d->d_inode->i_op->listxattr) {
+		lock_kernel();
+		error = d->d_inode->i_op->listxattr(d, klist, size);
+		unlock_kernel();
+	}
+
+	if (klist && error > 0)
+		if (copy_to_user(list, klist, size))
+			error = -EFAULT;
+	xattr_free(klist, size);
+	return error;
+}
+
+asmlinkage long
+sys_listxattr(char *path, char *list, size_t size)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk(path, &nd);
+	if (error)
+		return error;
+	error = listxattr(nd.dentry, list, size);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_llistxattr(char *path, char *list, size_t size)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk_link(path, &nd);
+	if (error)
+		return error;
+	error = listxattr(nd.dentry, list, size);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_flistxattr(int fd, char *list, size_t size)
+{
+	struct file *f;
+	int error = -EBADF;
+
+	f = fget(fd);
+	if (!f)
+		return error;
+	error = listxattr(f->f_dentry, list, size);
+	fput(f);
+	return error;
+}
+
+/*
+ * Extended attribute REMOVE operations
+ */
+static long
+removexattr(struct dentry *d, char *name)
+{
+	int error;
+	char kname[XATTR_NAME_MAX + 1];
+
+	if (copy_from_user(kname, name, XATTR_NAME_MAX))
+		return -EFAULT;
+	kname[XATTR_NAME_MAX] = '\0';
+
+	error = -EOPNOTSUPP;
+	if (d->d_inode->i_op && d->d_inode->i_op->removexattr) {
+		lock_kernel();
+		error = d->d_inode->i_op->removexattr(d, kname);
+		unlock_kernel();
+	}
+	return error;
+}
+
+asmlinkage long
+sys_removexattr(char *path, char *name)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk(path, &nd);
+	if (error)
+		return error;
+	error = removexattr(nd.dentry, name);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_lremovexattr(char *path, char *name)
+{
+	struct nameidata nd;
+	int error;
+
+	error = user_path_walk_link(path, &nd);
+	if (error)
+		return error;
+	error = removexattr(nd.dentry, name);
+	path_release(&nd);
+	return error;
+}
+
+asmlinkage long
+sys_fremovexattr(int fd, char *name)
+{
+	struct file *f;
+	int error = -EBADF;
+
+	f = fget(fd);
+	if (!f)
+		return error;
+	error = removexattr(f->f_dentry, name);
+	fput(f);
+	return error;
+}
diff -Naur 2.5.0-pristine/include/asm-i386/unistd.h 2.5.0-xattr/include/asm-i386/unistd.h
--- 2.5.0-pristine/include/asm-i386/unistd.h	Thu Oct 18 03:03:03 2001
+++ 2.5.0-xattr/include/asm-i386/unistd.h	Thu Dec  6 12:59:47 2001
@@ -230,6 +230,18 @@
 #define __NR_security		223	/* syscall for security modules */
 #define __NR_gettid		224
 #define __NR_readahead		225
+#define __NR_setxattr		226
+#define __NR_lsetxattr		227
+#define __NR_fsetxattr		228
+#define __NR_getxattr		229
+#define __NR_lgetxattr		230
+#define __NR_fgetxattr		231
+#define __NR_listxattr		232
+#define __NR_llistxattr		233
+#define __NR_flistxattr		234
+#define __NR_removexattr	235
+#define __NR_lremovexattr	236
+#define __NR_fremovexattr	237
 
 /* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
 
diff -Naur 2.5.0-pristine/include/linux/fs.h 2.5.0-xattr/include/linux/fs.h
--- 2.5.0-pristine/include/linux/fs.h	Fri Nov 23 06:46:19 2001
+++ 2.5.0-xattr/include/linux/fs.h	Thu Dec  6 12:59:47 2001
@@ -851,6 +851,10 @@
 	int (*revalidate) (struct dentry *);
 	int (*setattr) (struct dentry *, struct iattr *);
 	int (*getattr) (struct dentry *, struct iattr *);
+	int (*setxattr) (struct dentry *, char *, void *, size_t, int);
+	int (*getxattr) (struct dentry *, char *, void *, size_t);
+	int (*listxattr) (struct dentry *, char *, size_t);
+	int (*removexattr) (struct dentry *, char *);
 };
 
 /*
diff -Naur 2.5.0-pristine/include/linux/limits.h 2.5.0-xattr/include/linux/limits.h
--- 2.5.0-pristine/include/linux/limits.h	Thu Jul 29 03:30:10 1999
+++ 2.5.0-xattr/include/linux/limits.h	Thu Dec  6 12:59:47 2001
@@ -13,6 +13,9 @@
 #define NAME_MAX         255	/* # chars in a file name */
 #define PATH_MAX        4095	/* # chars in a path name */
 #define PIPE_BUF        4096	/* # bytes in atomic write to a pipe */
+#define XATTR_NAME_MAX   255	/* # chars in an extended attribute name */
+#define XATTR_SIZE_MAX 65536	/* size of an extended attribute value (64k) */
+#define XATTR_LIST_MAX 65536	/* size of extended attribute namelist (64k) */
 
 #define RTSIG_MAX	  32
 
diff -Naur 2.5.0-pristine/include/linux/xattr.h 2.5.0-xattr/include/linux/xattr.h
--- 2.5.0-pristine/include/linux/xattr.h	Thu Jan  1 10:00:00 1970
+++ 2.5.0-xattr/include/linux/xattr.h	Thu Dec  6 12:59:47 2001
@@ -0,0 +1,15 @@
+/*
+  File: linux/xattr.h
+
+  Extended attributes handling.
+
+  Copyright (C) 2001 by Andreas Gruenbacher <a.gruenbacher@computer.org>
+  Copyright (C) 2001 SGI - Silicon Graphics, Inc <linux-xfs@oss.sgi.com>
+*/
+#ifndef _LINUX_XATTR_H
+#define _LINUX_XATTR_H
+
+#define XATTR_CREATE	0x1	/* set value, fail if attr already exists */
+#define XATTR_REPLACE	0x2	/* set value, fail if attr does not exist */
+
+#endif	/* _LINUX_XATTR_H */

Re: A modest proposal -- We need a patch penguin

[Linus Torvalds]

On Wed, 30 Jan 2002, Nathan Scott wrote:
>
> Al had several (additional) issues with the original patch, but I
> think we progressively worked through them - Al stopped suggesting
> changes at one point anyway, and the level of abuse died away ;),
> so I guess he became more satisfied with them.

I think you can safely assume that if Al doesn't curse you to hell, he can
be considered happy.

> Not much point apportioning blame - its as much my fault - I
> hadn't heard back from you at all since day 1, so figured you
> were just not interested in this stuff, so I stopped sending.

Basically, you should always consider email to me to be a unreliable
medium, with no explicit congestion control. So think of an email like a
TCP packet, with exponential backoff - except the times are different (in
TCP, the initial timeout is three seconds, and the max timeout is 2
minutes. In "Linus-lossy-network" it makes sense to use different
default and maximum values ;)

		Linus

Re: A modest proposal -- We need a patch penguin

[Stuart Young]

At 03:59 PM 29/01/02 -0800, Linus Torvalds wrote:
>Basically, you should always consider email to me to be a unreliable
>medium, with no explicit congestion control. So think of an email like a
>TCP packet, with exponential backoff - except the times are different (in
>TCP, the initial timeout is three seconds, and the max timeout is 2
>minutes. In "Linus-lossy-network" it makes sense to use different
>default and maximum values ;)

Actually it's more like UDP. *grin* Least with TCP we get an ACK that the 
connection is accepted, and some sort of status is kept. Not so sure we 
have that with you all the time.

But hey, lots of things run over UDP, just a matter of making sure everyone 
realizes it's not a guaranteed medium really, isn't it?


Stuart Young - sgy@amc.com.au
(aka Cefiar) - cefiar1@optushome.com.au

[All opinions expressed in the above message are my]
[own and not necessarily the views of my employer..]

VFS EA interface patch (was: A modest proposal...)

[Daniel Phillips]

On January 30, 2002 12:40 am, Nathan Scott wrote:
> hi Linus,
> 
> On Tue, Jan 29, 2002 at 03:13:14PM -0800, Linus Torvalds wrote:
> > 
> > On 30 Jan 2002, Andi Kleen wrote:
> > > Does that answer your questions?
> > > Would you look at a patch again?
> > 
> > That answers the specific questions about Al and Stephen.
> 
> Al had several (additional) issues with the original patch, but I
> think we progressively worked through them - Al stopped suggesting
> changes at one point anyway, and the level of abuse died away ;),
> so I guess he became more satisfied with them.
> 
> > It does NOT address whether consensus has been reached in general, and
> > whether people are happy. Is that the case?
> 
> I believe so - I know that the ext2/ext3 EA/ACL maintainer 
> (AndreasG, CCd) is satisfied with the current patches, from
> an XFS point of view they satisfy all our needs, and Anton
> has chimed in saying this interface will work well for NTFS
> EA support (Anton sent me patches and several suggestions as
> well, which were included in the patch at the time).
> 
> > Also, obviously nobody actually took over maintainership of the patch,
> > because equally obviously nobody has been pinging me about it. For some
> > reason you seem to want _me_ to go out of my way to search for patches
> > that are over a month old that I don't know whether they are valid or not,
> > used or not, or even agreed upon or not.
> > 
> > But yes, it's so much easier to blame me.
> > 
> 
> Not much point apportioning blame - its as much my fault - I
> hadn't heard back from you at all since day 1, so figured you
> were just not interested in this stuff, so I stopped sending.
> 
> The two patches which seemed to satisfy the most people are
> below - they are unchanged from 2.5.0 but should apply to any
> 2.5.x tree (they are fairly non-intrusive and the system call
> table hasn't changed since last time).  A complete userspace
> implementation for both EAs and POSIX ACLs exists above these
> interfaces.

It was a little odd to see this patch get submitted and accepted in the 
middle of the 'patch penguin' thread.  IMHO, this illustrates perfectly why
we need a standard place to submit patches.  There is a patchbot under
construction, which I hope will address this kind of problem without
introducing new ones.  We shall see if it really does when it's opened up for 
testing, EA about one month.

The idea is that we'll create a patch submission address for the patchbot 
itself, and see what it feels like submitting patchbot patches that way.  For 
anyone interest, the project page is here:

  http://killeri.net/patchbot.html

with links there already somewhat out of date, please see the mailing list 
archive.

My main problem with the EA interface patch itself is that there is no
way to specify the class of the EA, currenly 'system' or 'user'.
Instead, individual filesystems are expected to parse the attribute
name to determine the class.  I think this is inadequate, and has not
been discussed sufficiently.

What will happen is, user space utilities will start making assumptions
about the syntax of ea names (because the kernel interface provides no
other alternative) and the current, arbitrary, EA name syntax will become 
cast in stone for ever more.  Then attribute classes will start to multiply, 
we'll get attribute namespaces, and we'll get more arbitrary hacks added to 
the attribute name syntax to accomodate them.  Support for 'new, improved' 
attribute name syntax will be variable across filesystems.  This isn't going 
to be pretty.

-- 
Daniel

Re: A modest proposal -- We need a patch penguin

[Rob Landley]

On Tuesday 29 January 2002 06:13 pm, Linus Torvalds wrote:
> On 30 Jan 2002, Andi Kleen wrote:
> > Does that answer your questions?
> > Would you look at a patch again?
>
> That answers the specific questions about Al and Stephen.
>
> It does NOT address whether consensus has been reached in general, and
> whether people are happy. Is that the case?

Wouldn't it be nice if somebody could collect this sort of information for 
you?

> Also, obviously nobody actually took over maintainership of the patch,
> because equally obviously nobody has been pinging me about it.

I.E. patches get dropped, even when there's intense interest about them, due 
to sheer frustration burning out the patch's maintainer, and scaring away 
other potential maintainers.  (Is this NOT the case?  I could be wrong.  
Please point out the flaw in my logic...)

> For some
> reason you seem to want _me_ to go out of my way to search for patches
> that are over a month old that I don't know whether they are valid or not,
> used or not, or even agreed upon or not.

I thought I was actually proposing somebody else be formally tasked with the 
job of doing that...

> But yes, it's so much easier to blame me.

Maybe just to suggest that you could use a secretary?

> 			Linus

Rob

Re: A modest proposal -- We need a patch penguin

[Jeff Garzik]

On Tue, Jan 29, 2002 at 08:00:19PM -0500, Rob Landley wrote:
> On Tuesday 29 January 2002 06:13 pm, Linus Torvalds wrote:
> > On 30 Jan 2002, Andi Kleen wrote:
> > > Does that answer your questions?
> > > Would you look at a patch again?
> >
> > That answers the specific questions about Al and Stephen.
> >
> > It does NOT address whether consensus has been reached in general, and
> > whether people are happy. Is that the case?
> 
> Wouldn't it be nice if somebody could collect this sort of information for 
> you?

So, you think "somebody" is willing to track all kernel issues and patches?

I don't think that's possible for any person or automated system.  [it's possible with a
combination of -people- and -automation-, IMHO, though]


> > Also, obviously nobody actually took over maintainership of the patch,
> > because equally obviously nobody has been pinging me about it.
> 
> I.E. patches get dropped, even when there's intense interest about them, due 
> to sheer frustration burning out the patch's maintainer, and scaring away 
> other potential maintainers.  (Is this NOT the case?  I could be wrong.  
> Please point out the flaw in my logic...)

I would point out a flaw if I actually saw any logic...

Are you talking about the EA patch?  See Nathan's response and other responses in this
thread.


> > For some
> > reason you seem to want _me_ to go out of my way to search for patches
> > that are over a month old that I don't know whether they are valid or not,
> > used or not, or even agreed upon or not.
> 
> I thought I was actually proposing somebody else be formally tasked with the 
> job of doing that...

You clearly do not realize the enormity of the task.


> > But yes, it's so much easier to blame me.
> 
> Maybe just to suggest that you could use a secretary?

I suggest that you need a Valium.

	Jeff

Re: VFS EA interface patch (was: A modest proposal...)

[Anton Altaparmakov]

At 04:08 06/02/02, Daniel Phillips wrote:
[snip]
>My main problem with the EA interface patch itself is that there is no
>way to specify the class of the EA, currenly 'system' or 'user'.
>Instead, individual filesystems are expected to parse the attribute
>name to determine the class.  I think this is inadequate, and has not
>been discussed sufficiently.

Have you read those:
         http://acl.bestbits.at/man/extattr.5.html
         http://acl.bestbits.at/man/extattr.2.html

Why is this inadequate? The above specify perfectly well what the current 
defined EA namespaces are, and the API is extensible in that should people 
come up with other useful namespaces (I can't think of any) they can just 
be added without any modification to the generic EA interface in the kernel 
and that is good.

NTFS for example has no concept of different EA classes and I am planning 
to only support the "user." namespace and just adding "user." before 
returning an EA and removing it when writing an EA. All other classes will 
just return that such beasts are not supported.

>What will happen is, user space utilities will start making assumptions
>about the syntax of ea names (because the kernel interface provides no
>other alternative) and the current, arbitrary, EA name syntax will become
>cast in stone for ever more.

A binary interface is much worse. While with the current interface it is 
arbitrary, at least it is defined, so yes, it is set in stone for ever 
more, and that is a Good Thing, you don't want to be changing that in the 
future.

However you are missing a very important point and that is that this is the 
EA API for the kernel. There is nothing to stop glibc (or some EA library) 
defining whatever different interface they like and exposing that to user 
space.

>Then attribute classes will start to multiply, we'll get attribute 
>namespaces, and we'll get more arbitrary hacks added to the attribute name 
>syntax to accomodate them.  Support for 'new, improved' attribute name 
>syntax will be variable across filesystems.  This isn't going to be pretty.

I don't see why any of this should happen. The interface for EAs is well 
defined in the above referenced documents. And note that we already have 
attribute namespaces, that is the whole point of the "user.", "system.", 
etc. That is what is used to distinguish the various classes. Or did I miss 
your point completely?

Best regards,

Anton


-- 
   "I've not lost my mind. It's backed up on tape somewhere." - Unknown
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://linux-ntfs.sf.net/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/

Re: VFS EA interface patch (was: A modest proposal...)

[Daniel Phillips]

On February 6, 2002 11:35 am, Anton Altaparmakov wrote:
> At 04:08 06/02/02, Daniel Phillips wrote:
> [snip]
> >My main problem with the EA interface patch itself is that there is no
> >way to specify the class of the EA, currenly 'system' or 'user'.
> >Instead, individual filesystems are expected to parse the attribute
> >name to determine the class.  I think this is inadequate, and has not
> >been discussed sufficiently.
> 
> Have you read those:
>          http://acl.bestbits.at/man/extattr.5.html
>          http://acl.bestbits.at/man/extattr.2.html
> 
> Why is this inadequate? The above specify perfectly well what the current 
> defined EA namespaces are, and the API is extensible in that should people 
> come up with other useful namespaces (I can't think of any) they can just 
> be added without any modification to the generic EA interface in the kernel 
> and that is good.

There's always a namespace string and an attribute name string?  That sounds
like two parameters to me, why are they being passed as one.

> >What will happen is, user space utilities will start making assumptions
> >about the syntax of ea names (because the kernel interface provides no
> >other alternative) and the current, arbitrary, EA name syntax will become
> >cast in stone for ever more.
> 
> A binary interface is much worse. While with the current interface it is 
> arbitrary, at least it is defined, so yes, it is set in stone for ever 
> more, and that is a Good Thing, you don't want to be changing that in the 
> future.
>
> However you are missing a very important point and that is that this is the 
> EA API for the kernel. There is nothing to stop glibc (or some EA library) 
> defining whatever different interface they like and exposing that to user 
> space.

I'm did not miss that.  Why do you want glibc, or worse, user tools, doing
extra ascii smashing to build up the ea string in the format the kernel
interface wants.

> >Then attribute classes will start to multiply, we'll get attribute 
> >namespaces, and we'll get more arbitrary hacks added to the attribute name 
> >syntax to accomodate them.  Support for 'new, improved' attribute name 
> >syntax will be variable across filesystems.  This isn't going to be pretty.
> 
> I don't see why any of this should happen. The interface for EAs is well 
> defined in the above referenced documents. And note that we already have 
> attribute namespaces, that is the whole point of the "user.", "system.", 
> etc. That is what is used to distinguish the various classes. Or did I miss 
> your point completely?

Namespaces are good.  I don't like the syntax being built into the name.
This is every bit as wrong as the thing Al complained about, creating more
system calls by passing in a function number.

If there are two strings, pass two strings.

-- 
Daniel