linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] get/set FAT filesystem attribute bits
@ 2005-01-03 20:57 H. Peter Anvin
  2005-01-03 21:24 ` Nicholas Miell
  2005-01-04  8:34 ` OGAWA Hirofumi
  0 siblings, 2 replies; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-03 20:57 UTC (permalink / raw)
  To: hirofumi, linux-kernel, Andrew Morton

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

This patch adds a set of ioctls to get and set the FAT filesystem native 
attribute bits, including the unused bits (6 and 7.)

It also includes some very minor code cleanups; mostly by macroizing a 
few idioms.

	-hpa

Signed-Off-By: H. Peter Anvin <hpa@zytor.com>

[-- Attachment #2: fat-filesystem-attributes.patch --]
[-- Type: text/x-patch, Size: 8224 bytes --]

Index: linux-2.5/fs/fat/dir.c
===================================================================
RCS file: /home/hpa/kernel/bkcvs/linux-2.5/fs/fat/dir.c,v
retrieving revision 1.28
diff -u -r1.28 dir.c
--- linux-2.5/fs/fat/dir.c	1 Nov 2004 00:18:37 -0000	1.28
+++ linux-2.5/fs/fat/dir.c	3 Jan 2005 15:06:16 -0000
@@ -663,7 +663,7 @@
 		both = 1;
 		break;
 	default:
-		return -EINVAL;
+		return fat_generic_ioctl(inode, filp, cmd, arg);
 	}
 
 	d1 = (struct dirent __user *)arg;
Index: linux-2.5/fs/fat/file.c
===================================================================
RCS file: /home/hpa/kernel/bkcvs/linux-2.5/fs/fat/file.c,v
retrieving revision 1.26
diff -u -r1.26 file.c
--- linux-2.5/fs/fat/file.c	24 Aug 2004 18:43:21 -0000	1.26
+++ linux-2.5/fs/fat/file.c	3 Jan 2005 15:06:41 -0000
@@ -19,6 +19,7 @@
 	.read		= generic_file_read,
 	.write		= fat_file_write,
 	.mmap		= generic_file_mmap,
+	.ioctl		= fat_generic_ioctl,
 	.fsync		= file_fsync,
 	.readv		= generic_file_readv,
 	.writev		= generic_file_writev,
Index: linux-2.5/fs/fat/inode.c
===================================================================
RCS file: /home/hpa/kernel/bkcvs/linux-2.5/fs/fat/inode.c,v
retrieving revision 1.108
diff -u -r1.108 inode.c
--- linux-2.5/fs/fat/inode.c	20 Oct 2004 15:16:56 -0000	1.108
+++ linux-2.5/fs/fat/inode.c	3 Jan 2005 20:50:25 -0000
@@ -562,7 +562,6 @@
 	MSDOS_I(inode)->i_attrs = 0;
 	inode->i_mtime.tv_sec = inode->i_atime.tv_sec = inode->i_ctime.tv_sec = 0;
 	inode->i_mtime.tv_nsec = inode->i_atime.tv_nsec = inode->i_ctime.tv_nsec = 0;
-	MSDOS_I(inode)->i_ctime_ms = 0;
 	inode->i_nlink = fat_subdirs(inode)+2;
 
 	return 0;
@@ -1218,8 +1217,7 @@
 		MSDOS_SB(sb)->options.isvfat
 		? date_dos2unix(le16_to_cpu(de->ctime), le16_to_cpu(de->cdate))
 		: inode->i_mtime.tv_sec;
-	inode->i_ctime.tv_nsec = de->ctime_ms * 1000000;
-	MSDOS_I(inode)->i_ctime_ms = de->ctime_ms;
+	inode->i_ctime.tv_nsec = de->ctime_ms * 1000000U;
 
 	return 0;
 }
@@ -1255,21 +1253,18 @@
 	raw_entry = &((struct msdos_dir_entry *) (bh->b_data))
 	    [i_pos & (sbi->dir_per_block - 1)];
 	if (S_ISDIR(inode->i_mode)) {
-		raw_entry->attr = ATTR_DIR;
 		raw_entry->size = 0;
 	}
 	else {
-		raw_entry->attr = ATTR_NONE;
 		raw_entry->size = cpu_to_le32(inode->i_size);
 	}
-	raw_entry->attr |= MSDOS_MKATTR(inode->i_mode) |
-	    MSDOS_I(inode)->i_attrs;
+	raw_entry->attr = msdos_attr(inode);
 	raw_entry->start = cpu_to_le16(MSDOS_I(inode)->i_logstart);
 	raw_entry->starthi = cpu_to_le16(MSDOS_I(inode)->i_logstart >> 16);
 	fat_date_unix2dos(inode->i_mtime.tv_sec, &raw_entry->time, &raw_entry->date);
 	if (sbi->options.isvfat) {
 		fat_date_unix2dos(inode->i_ctime.tv_sec,&raw_entry->ctime,&raw_entry->cdate);
-		raw_entry->ctime_ms = MSDOS_I(inode)->i_ctime_ms; /* use i_ctime.tv_nsec? */
+		raw_entry->ctime_ms = inode->i_ctime.tv_nsec / 1000000U;
 	}
 	spin_unlock(&sbi->inode_hash_lock);
 	mark_buffer_dirty(bh);
@@ -1328,4 +1323,105 @@
 	unlock_kernel();
 	return error;
 }
+
+int fat_generic_ioctl(struct inode * inode, struct file * filp,
+		      unsigned int cmd, unsigned long arg)
+{
+	struct super_block *sb = inode->i_sb;
+	struct msdos_sb_info *sbi = MSDOS_SB(sb);
+
+	switch (cmd) {
+	case FAT_IOCTL_GET_ATTRIBUTES:
+	{
+		u8 attr;
+		
+		/* msdos_attr() isn't atomic */
+		down(&inode->i_sem);
+
+		if ( inode->i_ino == MSDOS_ROOT_INO )
+			attr = ATTR_DIR;
+		else
+			attr = msdos_attr(inode);
+
+		up(&inode->i_sem);
+
+		return put_user(attr, (u8 __user *)arg);
+	}
+	case FAT_IOCTL_SET_ATTRIBUTES:
+	{
+		u8 attr, oldattr;
+		int err;
+		int is_dir;
+		struct iattr ia;
+
+		if ( (err = get_user(attr, (u8 __user *)arg)) )
+			return err;
+
+		if (IS_RDONLY(inode))
+			return -EROFS;
+
+		/* ATTR_VOLUME and ATTR_DIR cannot be changed; this also
+		   prevents the user from turning us into a VFAT
+		   longname entry. */
+		attr &= ~(ATTR_VOLUME|ATTR_DIR);
+
+		down(&inode->i_sem);
+
+		/* This is equivalent to an fchmod */
+		ia.ia_valid = ATTR_MODE|ATTR_CTIME;
+		ia.ia_ctime = CURRENT_TIME;
+
+		is_dir = S_ISDIR(inode->i_mode);
+
+		/* Merge in ATTR_VOLUME and ATTR_DIR */
+		attr |= (MSDOS_I(inode)->i_attrs & ATTR_VOLUME) |
+			(is_dir ? ATTR_DIR : 0);
+
+		if (is_dir) {
+			ia.ia_mode = MSDOS_MKMODE(attr,
+				S_IRWXUGO & ~sbi->options.fs_dmask)
+				| S_IFDIR;
+		} else {
+			ia.ia_mode = MSDOS_MKMODE(attr,
+				(S_IRUGO|S_IWUGO|(inode->i_mode & S_IXUGO))
+				& ~sbi->options.fs_fmask)
+				| S_IFREG;
+		}
+
+		if ( (err = inode_change_ok(inode, &ia)) )
+			goto out;
+		
+		/* The root directory has no attributes */
+		if ( inode->i_ino == MSDOS_ROOT_INO && attr != ATTR_DIR ) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		oldattr = msdos_attr(inode);
+		if (sbi->options.sys_immutable) {
+			if ((attr^oldattr) & ATTR_SYS) {
+				if (!capable(CAP_LINUX_IMMUTABLE)) {
+					err = -EPERM;
+					goto out;
+				}
+				
+				if ( attr & ATTR_SYS )
+					inode->i_flags |= S_IMMUTABLE;
+				else
+					inode->i_flags &= S_IMMUTABLE;
+			}
+		}
+
+		MSDOS_I(inode)->i_attrs = attr & ATTR_UNUSED;
+		
+		err = inode_setattr(inode, &ia);
+	out:
+		up(&inode->i_sem);
+		return err;
+	}
+	default:
+		return -ENOTTY;	/* Inappropriate ioctl for device */
+	}
+}
+
 MODULE_LICENSE("GPL");
Index: linux-2.5/include/linux/compat_ioctl.h
===================================================================
RCS file: /home/hpa/kernel/bkcvs/linux-2.5/include/linux/compat_ioctl.h,v
retrieving revision 1.35
diff -u -r1.35 compat_ioctl.h
--- linux-2.5/include/linux/compat_ioctl.h	16 Nov 2004 04:09:40 -0000	1.35
+++ linux-2.5/include/linux/compat_ioctl.h	3 Jan 2005 19:15:21 -0000
@@ -571,6 +571,9 @@
 COMPATIBLE_IOCTL(DEVFSDIOC_SET_EVENT_MASK)
 COMPATIBLE_IOCTL(DEVFSDIOC_RELEASE_EVENT_QUEUE)
 COMPATIBLE_IOCTL(DEVFSDIOC_SET_DEBUG_MASK)
+/* FAT */
+COMPATIBLE_IOCTL(FAT_IOCTL_GET_ATTRIBUTES)
+COMPATIBLE_IOCTL(FAT_IOCTL_SET_ATTRIBUTES)
 /* Raw devices */
 COMPATIBLE_IOCTL(RAW_SETBIND)
 COMPATIBLE_IOCTL(RAW_GETBIND)
Index: linux-2.5/include/linux/msdos_fs.h
===================================================================
RCS file: /home/hpa/kernel/bkcvs/linux-2.5/include/linux/msdos_fs.h,v
retrieving revision 1.41
diff -u -r1.41 msdos_fs.h
--- linux-2.5/include/linux/msdos_fs.h	20 Oct 2004 15:16:45 -0000	1.41
+++ linux-2.5/include/linux/msdos_fs.h	3 Jan 2005 20:35:36 -0000
@@ -95,6 +95,9 @@
  */
 #define	VFAT_IOCTL_READDIR_BOTH		_IOR('r', 1, struct dirent [2])
 #define	VFAT_IOCTL_READDIR_SHORT	_IOR('r', 2, struct dirent [2])
+/* Badness: 1..6 conflict with linux/videotext.h, despite ioctl-numbers.txt */
+#define FAT_IOCTL_GET_ATTRIBUTES	_IOR('r', 0x10, __u8)
+#define FAT_IOCTL_SET_ATTRIBUTES	_IOW('r', 0x11, __u8)
 
 /* 
  * vfat shortname flags
@@ -202,6 +205,13 @@
 	return container_of(inode, struct msdos_inode_info, vfs_inode);
 }
 
+static inline u8 msdos_attr(struct inode *inode)
+{
+	return MSDOS_MKATTR(inode->i_mode) |
+		MSDOS_I(inode)->i_attrs |
+		(S_ISDIR(inode->i_mode) ? ATTR_DIR : 0);
+}
+
 static inline void fat16_towchar(wchar_t *dst, const __u8 *src, size_t len)
 {
 #ifdef __BIG_ENDIAN
@@ -267,6 +277,8 @@
 int fat_fill_super(struct super_block *sb, void *data, int silent,
 		   struct inode_operations *fs_dir_inode_ops, int isvfat);
 extern int fat_notify_change(struct dentry * dentry, struct iattr * attr);
+extern int fat_generic_ioctl(struct inode * inode, struct file * filp,
+			     unsigned int cmd, unsigned long arg);
 
 /* fat/misc.c */
 extern void fat_fs_panic(struct super_block *s, const char *fmt, ...);
Index: linux-2.5/include/linux/msdos_fs_i.h
===================================================================
RCS file: /home/hpa/kernel/bkcvs/linux-2.5/include/linux/msdos_fs_i.h,v
retrieving revision 1.11
diff -u -r1.11 msdos_fs_i.h
--- linux-2.5/include/linux/msdos_fs_i.h	20 Oct 2004 15:15:45 -0000	1.11
+++ linux-2.5/include/linux/msdos_fs_i.h	3 Jan 2005 20:37:05 -0000
@@ -20,7 +20,6 @@
 	int i_start;	/* first cluster or 0 */
 	int i_logstart;	/* logical first cluster */
 	int i_attrs;	/* unused attribute bits */
-	int i_ctime_ms;	/* unused change time in milliseconds */
 	loff_t i_pos;	/* on-disk position of directory entry or 0 */
 	struct hlist_node i_fat_hash;	/* hash by i_location */
 	struct inode vfs_inode;

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 20:57 [PATCH] get/set FAT filesystem attribute bits H. Peter Anvin
@ 2005-01-03 21:24 ` Nicholas Miell
  2005-01-03 21:35   ` H. Peter Anvin
  2005-01-04  8:34 ` OGAWA Hirofumi
  1 sibling, 1 reply; 17+ messages in thread
From: Nicholas Miell @ 2005-01-03 21:24 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: hirofumi, linux-kernel, Andrew Morton

On Mon, 2005-01-03 at 12:57 -0800, H. Peter Anvin wrote:
> This patch adds a set of ioctls to get and set the FAT filesystem native 
> attribute bits, including the unused bits (6 and 7.)
> 

Instead of adding another ioctl, wouldn't an xattr be more appropriate?
For instance, system.fatattrs containing a text representation of the
attribute bits.

i.e. 
	a = archive
	d = directory
	h = hidden
	r = read only
	s = system
	v = volume
	6 = unused bit 6
	7 = unused bit 7

and

bash-3.00$ getfattr -n system.fatattrs dosfile.txt
# file: dosfile.txt
system.fatattrs="ar"

bash-3.00$
-- 
Nicholas Miell <nmiell@comcast.net>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 21:24 ` Nicholas Miell
@ 2005-01-03 21:35   ` H. Peter Anvin
  2005-01-03 21:46     ` Nicholas Miell
  0 siblings, 1 reply; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-03 21:35 UTC (permalink / raw)
  To: Nicholas Miell; +Cc: hirofumi, linux-kernel, Andrew Morton

Nicholas Miell wrote:
> On Mon, 2005-01-03 at 12:57 -0800, H. Peter Anvin wrote:
> 
>>This patch adds a set of ioctls to get and set the FAT filesystem native 
>>attribute bits, including the unused bits (6 and 7.)
>>
> 
> 
> Instead of adding another ioctl, wouldn't an xattr be more appropriate?
> For instance, system.fatattrs containing a text representation of the
> attribute bits.
> 

This really worries me, because it's not clear to me that Microsoft 
isn't going to add NTFS-style xattrs to FAT in the future.  There is a 
very specific reason why they might want to do that: since they want to 
keep NTFS secret and proprietary, FAT is the published interchange 
format that other devices can use to exchange data with MS operating 
systems.  If we then have overloaded the xattr mechanism, that would be 
very ugly.

	-hpa

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 21:35   ` H. Peter Anvin
@ 2005-01-03 21:46     ` Nicholas Miell
  2005-01-03 22:02       ` H. Peter Anvin
  0 siblings, 1 reply; 17+ messages in thread
From: Nicholas Miell @ 2005-01-03 21:46 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: hirofumi, linux-kernel, Andrew Morton

On Mon, 2005-01-03 at 13:35 -0800, H. Peter Anvin wrote:
> Nicholas Miell wrote:
> > Instead of adding another ioctl, wouldn't an xattr be more appropriate?
> > For instance, system.fatattrs containing a text representation of the
> > attribute bits.
> > 
> 
> This really worries me, because it's not clear to me that Microsoft 
> isn't going to add NTFS-style xattrs to FAT in the future.  There is a 
> very specific reason why they might want to do that: since they want to 
> keep NTFS secret and proprietary, FAT is the published interchange 
> format that other devices can use to exchange data with MS operating 
> systems.  If we then have overloaded the xattr mechanism, that would be 
> very ugly.
> 
> 	-hpa

That's why I put fatattrs in the system namespace, which is wholly owned
by the Linux kernel. Any theoretical FAT-with-xattrs variant would put
those xattrs in the user namespace.

On another note, NTFS-style xattrs (aka named streams) are unrelated to
Linux xattrs. A named stream is a separate file with a funny name, while
a Linux xattr is a named extension to struct stat.
-- 
Nicholas Miell <nmiell@comcast.net>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 21:46     ` Nicholas Miell
@ 2005-01-03 22:02       ` H. Peter Anvin
  2005-01-03 22:10         ` Nicholas Miell
  2005-01-04 10:09         ` Anton Altaparmakov
  0 siblings, 2 replies; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-03 22:02 UTC (permalink / raw)
  To: Nicholas Miell; +Cc: hirofumi, linux-kernel, Andrew Morton

Nicholas Miell wrote:
> 
> That's why I put fatattrs in the system namespace, which is wholly owned
> by the Linux kernel. Any theoretical FAT-with-xattrs variant would put
> those xattrs in the user namespace.
> 
> On another note, NTFS-style xattrs (aka named streams) are unrelated to
> Linux xattrs. A named stream is a separate file with a funny name, while
> a Linux xattr is a named extension to struct stat.
 >

OK, that does make it more sensible.  I do note, however, that ext2/ext3 
do not seem to export their attributes (chattr/lsattr) in this way; I do 
also note that the xattr code wherever it has been implemented is just 
painfully complex.

I'll see if I can weed it down to some kind of sane size.

	-hpa


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 22:02       ` H. Peter Anvin
@ 2005-01-03 22:10         ` Nicholas Miell
  2005-01-03 22:25           ` H. Peter Anvin
  2005-01-04 10:09         ` Anton Altaparmakov
  1 sibling, 1 reply; 17+ messages in thread
From: Nicholas Miell @ 2005-01-03 22:10 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: hirofumi, linux-kernel, Andrew Morton

On Mon, 2005-01-03 at 14:02 -0800, H. Peter Anvin wrote:
> Nicholas Miell wrote:
> > 
> > That's why I put fatattrs in the system namespace, which is wholly owned
> > by the Linux kernel. Any theoretical FAT-with-xattrs variant would put
> > those xattrs in the user namespace.
> > 
> > On another note, NTFS-style xattrs (aka named streams) are unrelated to
> > Linux xattrs. A named stream is a separate file with a funny name, while
> > a Linux xattr is a named extension to struct stat.
>  >
> 
> OK, that does make it more sensible.  I do note, however, that ext2/ext3 
> do not seem to export their attributes (chattr/lsattr) in this way; I do 
> also note that the xattr code wherever it has been implemented is just 
> painfully complex.
> 
> I'll see if I can weed it down to some kind of sane size.
> 
> 	-hpa

Yeah, I contemplated adding system.fattattrs, system.ntfsattrs, and
system.linuxattrs (for the ext2 attrs that have popped up in several
other filesystems) a while ago, but xattrs seem to be the red-headed
left-handed stepchild of the Linux VFS and I lost interest in the
project.

Nice to see someone else interested in it, though.
-- 
Nicholas Miell <nmiell@comcast.net>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 22:10         ` Nicholas Miell
@ 2005-01-03 22:25           ` H. Peter Anvin
  2005-01-03 23:16             ` Nicholas Miell
  0 siblings, 1 reply; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-03 22:25 UTC (permalink / raw)
  To: Nicholas Miell; +Cc: hirofumi, linux-kernel, Andrew Morton

Nicholas Miell wrote:
> 
> Yeah, I contemplated adding system.fattattrs, system.ntfsattrs, and
> system.linuxattrs (for the ext2 attrs that have popped up in several
> other filesystems) a while ago, but xattrs seem to be the red-headed
> left-handed stepchild of the Linux VFS and I lost interest in the
> project.
> 
> Nice to see someone else interested in it, though.
 >

I'm honestly not sure that using an ASCII string in an xattr is the sane 
way of doing this.  Even a binary byte in an xattr would make more sense 
in some ways.

I think the xattr mechanism is ignored largely because it's painfully 
complex.

A plus with using xattr is that in theory (but of course not in 
practice!) it would let one store a copy of a DOS filesystem on an ext3 
(or xfs, or...) filesystem and have it restored, all using standard (but 
by necessity, xattr-aware) tools.  However, the splitting of xattr into 
namespaces may very well make that impossible, since what's a "system" 
attribute to one filesystem is a "user" attribute to another.  Classic 
design flaw, by the way.

Anyway, I'm going to send out something to the various maintainers of 
DOS-based filesystems (FAT, CIFS, NTFS) and see what they think.

	-hpa

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 22:25           ` H. Peter Anvin
@ 2005-01-03 23:16             ` Nicholas Miell
  2005-01-03 23:22               ` H. Peter Anvin
  0 siblings, 1 reply; 17+ messages in thread
From: Nicholas Miell @ 2005-01-03 23:16 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: hirofumi, linux-kernel, Andrew Morton

On Mon, 2005-01-03 at 14:25 -0800, H. Peter Anvin wrote:
> I'm honestly not sure that using an ASCII string in an xattr is the sane 
> way of doing this.  Even a binary byte in an xattr would make more sense 
> in some ways.

ASCII strings require no special tools to manipulate from shell scripts.

> I think the xattr mechanism is ignored largely because it's painfully 
> complex.
> 
> A plus with using xattr is that in theory (but of course not in 
> practice!) it would let one store a copy of a DOS filesystem on an ext3 
> (or xfs, or...) filesystem and have it restored, all using standard (but 
> by necessity, xattr-aware) tools.  However, the splitting of xattr into 
> namespaces may very well make that impossible, since what's a "system" 
> attribute to one filesystem is a "user" attribute to another.  Classic 
> design flaw, by the way.

The design does allow users to store whatever they want as an xattr
without having to worry about how the kernel chooses to interpret it,
though. (i.e. the user namespace is just a byte array that the kernel
stores for you, while the system/security namespaces are probably
generated and interpreted on demand.)

> Anyway, I'm going to send out something to the various maintainers of 
> DOS-based filesystems (FAT, CIFS, NTFS) and see what they think.
> 
> 	-hpa
-- 
Nicholas Miell <nmiell@comcast.net>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 23:16             ` Nicholas Miell
@ 2005-01-03 23:22               ` H. Peter Anvin
  2005-01-03 23:46                 ` Nicholas Miell
  0 siblings, 1 reply; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-03 23:22 UTC (permalink / raw)
  To: Nicholas Miell; +Cc: linux-kernel

[Pruning the Cc: list.]

Nicholas Miell wrote:
> On Mon, 2005-01-03 at 14:25 -0800, H. Peter Anvin wrote:
> 
>>I'm honestly not sure that using an ASCII string in an xattr is the sane 
>>way of doing this.  Even a binary byte in an xattr would make more sense 
>>in some ways.
> 
> ASCII strings require no special tools to manipulate from shell scripts.
> 

You need some kind of special tool anyway, i.e. getfattr/setfattr.  What 
tool you use isn't really important.

The fact that getfattr/setfattr can't deal with attributes that aren't 
ASCII strings seem like flaws in these tools.

> 
>>I think the xattr mechanism is ignored largely because it's painfully 
>>complex.
>>
>>A plus with using xattr is that in theory (but of course not in 
>>practice!) it would let one store a copy of a DOS filesystem on an ext3 
>>(or xfs, or...) filesystem and have it restored, all using standard (but 
>>by necessity, xattr-aware) tools.  However, the splitting of xattr into 
>>namespaces may very well make that impossible, since what's a "system" 
>>attribute to one filesystem is a "user" attribute to another.  Classic 
>>design flaw, by the way.
> 
> The design does allow users to store whatever they want as an xattr
> without having to worry about how the kernel chooses to interpret it,
> though. (i.e. the user namespace is just a byte array that the kernel
> stores for you, while the system/security namespaces are probably
> generated and interpreted on demand.)
 >

Exactly, and that's a total screwup.  It makes something that would 
otherwise be possible -- for some filesystems to have an attribute (call 
it "system.dosattrib") which is used, and for others which is stored. 
The problem is that with the current design, that won't happen.

Encoding this in the namespace, therefore preventing this kind of 
compatiblity, is daft.  From the looks of it, the CIFS people were 
planning to do the "put everything in user.*" workaround for this design 
error.

	-hpa

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 23:22               ` H. Peter Anvin
@ 2005-01-03 23:46                 ` Nicholas Miell
  0 siblings, 0 replies; 17+ messages in thread
From: Nicholas Miell @ 2005-01-03 23:46 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: linux-kernel

On Mon, 2005-01-03 at 15:22 -0800, H. Peter Anvin wrote:
> [Pruning the Cc: list.]
> 
> Nicholas Miell wrote:
> > On Mon, 2005-01-03 at 14:25 -0800, H. Peter Anvin wrote:
> > 
> >>I'm honestly not sure that using an ASCII string in an xattr is the sane 
> >>way of doing this.  Even a binary byte in an xattr would make more sense 
> >>in some ways.
> > 
> > ASCII strings require no special tools to manipulate from shell scripts.
> > 
> 
> You need some kind of special tool anyway, i.e. getfattr/setfattr.  What 
> tool you use isn't really important.

I was talking about getdosattr and setdosattr (and the corresponding
pair for every other filesystem with it's own set of special
attributes).

getfattr and setfattr are standard tools already provided with the
distro.

> The fact that getfattr/setfattr can't deal with attributes that aren't 
> ASCII strings seem like flaws in these tools.

They can. Non-ASCII xattrs are either base64 encoded or octal escaped.

Try

getfattr -n system.posix_acl_access some_file_with_an_acl
getfattr -e text -n system.posix_acl_access some_file_with_an_acl

for a quick example.

> > The design does allow users to store whatever they want as an xattr
> > without having to worry about how the kernel chooses to interpret it,
> > though. (i.e. the user namespace is just a byte array that the kernel
> > stores for you, while the system/security namespaces are probably
> > generated and interpreted on demand.)
>  >
> 
> Exactly, and that's a total screwup.  It makes something that would 
> otherwise be possible -- for some filesystems to have an attribute (call 
> it "system.dosattrib") which is used, and for others which is stored. 
> The problem is that with the current design, that won't happen.
> 

I responded to this in the other thread already.

> Encoding this in the namespace, therefore preventing this kind of 
> compatiblity, is daft.  From the looks of it, the CIFS people were 
> planning to do the "put everything in user.*" workaround for this design 
> error.
> 



> 	-hpa
-- 
Nicholas Miell <nmiell@comcast.net>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 20:57 [PATCH] get/set FAT filesystem attribute bits H. Peter Anvin
  2005-01-03 21:24 ` Nicholas Miell
@ 2005-01-04  8:34 ` OGAWA Hirofumi
  2005-01-04  9:41   ` H. Peter Anvin
  1 sibling, 1 reply; 17+ messages in thread
From: OGAWA Hirofumi @ 2005-01-04  8:34 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: linux-kernel, Andrew Morton

"H. Peter Anvin" <hpa@zytor.com> writes:

> -	inode->i_ctime.tv_nsec = de->ctime_ms * 1000000;
> -	MSDOS_I(inode)->i_ctime_ms = de->ctime_ms;
> +	inode->i_ctime.tv_nsec = de->ctime_ms * 1000000U;

Actually, the ->ctime_ms is not mili seconds. The valid range is 0-199 (*10ms).
(And ->ctime is started from 2 seconds)

> -		raw_entry->ctime_ms = MSDOS_I(inode)->i_ctime_ms; /* use i_ctime.tv_nsec? */
> +		raw_entry->ctime_ms = inode->i_ctime.tv_nsec / 1000000U;

Ditto


BTW, do you already have any plan to use this ioctls?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-04  8:34 ` OGAWA Hirofumi
@ 2005-01-04  9:41   ` H. Peter Anvin
  0 siblings, 0 replies; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-04  9:41 UTC (permalink / raw)
  To: OGAWA Hirofumi; +Cc: linux-kernel, Andrew Morton

OGAWA Hirofumi wrote:

> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
> 
>>-	inode->i_ctime.tv_nsec = de->ctime_ms * 1000000;
>>-	MSDOS_I(inode)->i_ctime_ms = de->ctime_ms;
>>+	inode->i_ctime.tv_nsec = de->ctime_ms * 1000000U;
> 
> Actually, the ->ctime_ms is not mili seconds. The valid range is 0-199 (*10ms).
> (And ->ctime is started from 2 seconds)
> 

D'oh!  They probably should be renamed _cs (centiseconds ;)

> 
>>-		raw_entry->ctime_ms = MSDOS_I(inode)->i_ctime_ms; /* use i_ctime.tv_nsec? */
>>+		raw_entry->ctime_ms = inode->i_ctime.tv_nsec / 1000000U;
> 
> 
> BTW, do you already have any plan to use this ioctls?
> 

Yes, I wanted to use them for the syslinux installer.

	-hpa


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-03 22:02       ` H. Peter Anvin
  2005-01-03 22:10         ` Nicholas Miell
@ 2005-01-04 10:09         ` Anton Altaparmakov
  2005-01-04 21:45           ` Nicholas Miell
  1 sibling, 1 reply; 17+ messages in thread
From: Anton Altaparmakov @ 2005-01-04 10:09 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Nicholas Miell, hirofumi, lkml, Andrew Morton

> Nicholas Miell wrote:
> > On another note, NTFS-style xattrs (aka named streams) are unrelated to
> > Linux xattrs. A named stream is a separate file with a funny name, while
> > a Linux xattr is a named extension to struct stat.

This is incorrect.  NTFS has two different beasts:

- Extended Attributes (EAs) which are just like Linux xattrs and in
Windows a very similar API exists as in Linux for accessing them (on
NTFS volumes only - VFAT does not support EAs in Windows).

- Named streams for which you are pretty much correct that they are like
a file with a funny name but note that named streams share the same
inode as their parent (unnamed) stream.  And this is not just the inode
number, it is the same on-disk inode.  This means that the ACLs, dos
attributes, etc, are all valid for both the unnamed and all named
streams.  So you cannot for example allow user A to access the unnamed
stream but not some named stream and vice versa you cannot allow user B
to access some named stream but not the unnamed stream.  In Windows
these beasts are accessed as filename::namedstreamname and in Linux
there is no existing API to access named streams at all that I am aware
off.  Again, as EAs, in Windows named streams only are supported on NTFS
volumes - VFAT does not support them.

The major difference is that access to named streams is much faster on a
per-stream basis as they are separate entities, i.e. stream A and stream
B share the same inode but are otherwise accessed independently and are
stored independently on disk (and as such can be read/written
simultaneously).  EAs on the other hand are all linked together and are
in fact just one large blob of data.  Resizing one EA modifies
potentially all of them since all EAs are stored in a single byte
stream, not very dissimilar to a named stream in fact.  So EAs are more
suited for storage of constant length structures while named streams are
better suited for variable length data (e.g. icons for executables, name
of document author, etc).

One interesting bit of trivia is that Windows uses named streams very
extensively while it _never_ uses EAs.  In fact I have never seen a
Windows OS or application that uses EAs.  They were added to be
compatible with OS/2 EAs when it came out but since OS/2 died they now
just seem like old baggage/backwards compatibility in Windows that is no
longer used.  (If anyone knows of a Windows application that uses EAs
please let me know.  I would be most interested in knowing about it!)

Hope this clears things up a bit as far as NTFS is concerned...

I don't know what API would be best for accessing named streams on NTFS
but an xattrs like interface is not suitable IMO.  You really want to be
able to open them and access them like normal files.  An interface
similar to the Solaris openat() system call (see
http://docs.sun.com/app/docs/doc/816-0212/6m6nd4nc7?a=view) that has
been discussed on LKML before seems like a good way to deal with this
but I am more interested in getting normal write support into NTFS at
present than I am in fancy features like EAs and named streams...

Best regards,

        Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-04 10:09         ` Anton Altaparmakov
@ 2005-01-04 21:45           ` Nicholas Miell
  0 siblings, 0 replies; 17+ messages in thread
From: Nicholas Miell @ 2005-01-04 21:45 UTC (permalink / raw)
  To: Anton Altaparmakov; +Cc: H. Peter Anvin, hirofumi, lkml, Andrew Morton

On Tue, 2005-01-04 at 10:09 +0000, Anton Altaparmakov wrote:
> > Nicholas Miell wrote:
> > > On another note, NTFS-style xattrs (aka named streams) are unrelated to
> > > Linux xattrs. A named stream is a separate file with a funny name, while
> > > a Linux xattr is a named extension to struct stat.
> 
> This is incorrect.  NTFS has two different beasts:
> 
[ why this is incorrect omitted for brevity ]
> 
> One interesting bit of trivia is that Windows uses named streams very
> extensively while it _never_ uses EAs.  In fact I have never seen a
> Windows OS or application that uses EAs.  They were added to be
> compatible with OS/2 EAs when it came out but since OS/2 died they now
> just seem like old baggage/backwards compatibility in Windows that is no
> longer used.  (If anyone knows of a Windows application that uses EAs
> please let me know.  I would be most interested in knowing about it!)
> 

This would probably explain why I've never heard of them. In my brief
perusal of the MSDN Library in search of more information about these
beasts, the only hint I could find related to their existence is in
parameters to ZwCreateFile/NtCreateFile (which are themselves mostly
undocumented). The high level file API certainly doesn't support their
use, AFAICT.

I think it's reasonably safe to assume that hpa's worry that FAT might
get EAs in the future is unfounded. (Named streams is still a
possibility, but I don't think Microsoft is all that interested in
making improvements to a filesystem that people use so that they don't
have to license NTFS.)

> Hope this clears things up a bit as far as NTFS is concerned...
> 
> I don't know what API would be best for accessing named streams on NTFS
> but an xattrs like interface is not suitable IMO.  You really want to be
> able to open them and access them like normal files.  An interface
> similar to the Solaris openat() system call (see
> http://docs.sun.com/app/docs/doc/816-0212/6m6nd4nc7?a=view) that has
> been discussed on LKML before seems like a good way to deal with this
> but I am more interested in getting normal write support into NTFS at
> present than I am in fancy features like EAs and named streams...
> 

Good luck with that.

> Best regards,
> 
>         Anton
-- 
Nicholas Miell <nmiell@comcast.net>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-06  0:07   ` Bodo Eggert
@ 2005-01-06  1:35     ` H. Peter Anvin
  0 siblings, 0 replies; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-06  1:35 UTC (permalink / raw)
  To: 7eggert; +Cc: linux-kernel

Bodo Eggert wrote:
> H. Peter Anvin wrote:
> 
>>By author:    Bodo Eggert <7eggert@gmx.de>
> 
> 
>>>>a = archive
>>>
>>>Should be the "dump" attribute
> 
> 
>>What dump attribute?
> 
> 
> The one described in man chattr.

... which describes ext[23]-specific attributes?

	-hpa


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
  2005-01-04 11:57   ` Bodo Eggert
@ 2005-01-04 21:26     ` H. Peter Anvin
  0 siblings, 0 replies; 17+ messages in thread
From: H. Peter Anvin @ 2005-01-04 21:26 UTC (permalink / raw)
  To: linux-kernel

Followup to:  <E1ClnK3-0000TB-00@be1.7eggert.dyndns.org>
By author:    Bodo Eggert <7eggert@gmx.de>
In newsgroup: linux.dev.kernel
> 
> > a = archive
> 
> Should be the "dump" attribute
> 

What dump attribute?

> > h = hidden
> > r = read only
> 
> Should be reflected by the write bits. (Maybe there should be an option to
> set the file mode for "read only" files to something different than
> $rw_mode and not 0222.)

It is.

> > s = system
> 
> Should be made "immutable", IMO

This is a filesystem mount option, but it's really unpleasant to set
it.  It's one of those things that seems to make sense, but really
doesn't.

	-hpa

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] get/set FAT filesystem attribute bits
       [not found] ` <fa.lub44op.a2ec2d@ifi.uio.no>
@ 2005-01-04 11:57   ` Bodo Eggert
  2005-01-04 21:26     ` H. Peter Anvin
  0 siblings, 1 reply; 17+ messages in thread
From: Bodo Eggert @ 2005-01-04 11:57 UTC (permalink / raw)
  To: Nicholas Miell, hirofumi, linux-kernel, Andrew Morton

Nicholas Miell wrote:

> Instead of adding another ioctl, wouldn't an xattr be more appropriate?
> For instance, system.fatattrs containing a text representation of the
> attribute bits.
> 
> i.e.
> a = archive

Should be the "dump" attribute

> d = directory

It's the file type. Redundant.

> h = hidden
> r = read only

Should be reflected by the write bits. (Maybe there should be an option to
set the file mode for "read only" files to something different than
$rw_mode and not 0222.)

> s = system

Should be made "immutable", IMO

> v = volume

The volume bit of a file will never be set, since no file is a volume name.
Setting a volume attribute should be disallowed, too, since it would cause
fs corruption. This leaves us with no need to know or set 'v'.

You should rather make the volume name be similar to the e2fs volume label.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2005-01-06  1:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-03 20:57 [PATCH] get/set FAT filesystem attribute bits H. Peter Anvin
2005-01-03 21:24 ` Nicholas Miell
2005-01-03 21:35   ` H. Peter Anvin
2005-01-03 21:46     ` Nicholas Miell
2005-01-03 22:02       ` H. Peter Anvin
2005-01-03 22:10         ` Nicholas Miell
2005-01-03 22:25           ` H. Peter Anvin
2005-01-03 23:16             ` Nicholas Miell
2005-01-03 23:22               ` H. Peter Anvin
2005-01-03 23:46                 ` Nicholas Miell
2005-01-04 10:09         ` Anton Altaparmakov
2005-01-04 21:45           ` Nicholas Miell
2005-01-04  8:34 ` OGAWA Hirofumi
2005-01-04  9:41   ` H. Peter Anvin
     [not found] <fa.ea9o20r.kje5qn@ifi.uio.no>
     [not found] ` <fa.lub44op.a2ec2d@ifi.uio.no>
2005-01-04 11:57   ` Bodo Eggert
2005-01-04 21:26     ` H. Peter Anvin
     [not found] <fa.i537e7s.1d6m90c@ifi.uio.no>
     [not found] ` <fa.ihdqkec.1i5umji@ifi.uio.no>
2005-01-06  0:07   ` Bodo Eggert
2005-01-06  1:35     ` H. Peter Anvin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).