Add extended attributes to ext2/3
RFC PATCH 3/4 Add extended attributes to ext2/3
This is the third of four patches which add extended attribute support to the ext2 and ext3 filesystems. Please comment and bleed.
This patch adds extended attribute support to the ext3 filesystem. This uses the generic extended attribute patch which was developed by Andreas Gruenbacher and the XFS team. As a result, the user space utilities which work for XFS will also work with these patches.
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
#
# fs/Config.help | 24 # fs/Config.in | 5 # fs/ext3/Makefile | 4 # fs/ext3/file.c | 5 # fs/ext3/ialloc.c | 2 # fs/ext3/inode.c | 35 - # fs/ext3/namei.c | 22 # fs/ext3/super.c | 30 + # fs/ext3/symlink.c | 16 # fs/ext3/xattr.c | 1233 +++++++++++++++++++++++++++++++++++++++++++++ # fs/ext3/xattr_user.c | 113 ++++ # include/linux/ext3_fs.h | 31 - # include/linux/ext3_jbd.h | 8 # include/linux/ext3_xattr.h | 157 +++++
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/10/05 [email protected] 1.667
# Port of 0.8.50 xattr-ext3 to 2.5.
#
# Enable EA's by default.
# --------------------------------------------
#
fs/Config.help +24 -0
@@ -154,6 +154,30 @@ of your root partition (the one containing the directory /) cannot be compiled as a module, and so this may be dangerous. +Ext3 extended attributes +CONFIG_EXT3_FS_XATTR + Extended attributes are name:value pairs associated with inodes by + the kernel or by users (see the attr(5) manual page, or visit + <http://acl.bestbits.at/> for details). + + If unsure, say N. + +Ext3 extended attribute block sharing +CONFIG_EXT3_FS_XATTR_SHARING + This options enables code for sharing identical extended attribute + blocks among multiple inodes. + + Usually, say Y. + +Ext3 extended user attributes +CONFIG_EXT3_FS_XATTR_USER + This option enables extended user attributes on ext3. Processes can + associate extended user attributes with inodes to store additional + information such as the character encoding of files, etc. (see the + attr(5) manual page, or visit <http://acl.bestbits.at/> for details). + + If unsure, say N. + CONFIG_JBD This is a generic journaling layer for block devices. It is currently used by the ext3 file system, but it could also be used to
fs/Config.in +5 -0
@@ -27,6 +27,11 @@ dep_tristate 'BFS file system support (EXPERIMENTAL)' CONFIG_BFS_FS $CONFIG_EXPERIMENTAL tristate 'Ext3 journalling file system support' CONFIG_EXT3_FS +if [ "$CONFIG_EXT3_FS" != "n" ] ; then + define_tristate CONFIG_EXT3_FS_XATTR $CONFIG_EXT3_FS + define_bool CONFIG_EXT3_FS_XATTR_SHARING y + define_bool CONFIG_EXT3_FS_XATTR_USER y +fi # CONFIG_JBD could be its own option (even modular), but until there are # other users than ext3, we will simply make it be the same as CONFIG_EXT3_FS # dep_tristate ' Journal Block Device support (JBD for ext3)' CONFIG_JBD $CONFIG_EXT3_FS
fs/ext3/Makefile +4 -0
@@ -7,4 +7,8 @@ ext3-objs := balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \ ioctl.o namei.o super.o symlink.o +export-objs += xattr.o +obj-$(CONFIG_EXT3_FS_XATTR) += xattr.o +obj-$(CONFIG_EXT3_FS_XATTR_USER) += xattr_user.o + include $(TOPDIR)/Rules.make
fs/ext3/file.c +5 -0
@@ -22,6 +22,7 @@#include <linux/fs.h> #include <linux/jbd.h> #include <linux/ext3_fs.h> +#include <linux/ext3_xattr.h> #include <linux/ext3_jbd.h> #include <linux/smp_lock.h>
@@ -92,5 +93,9 @@struct inode_operations ext3_file_inode_operations = { .truncate = ext3_truncate, .setattr = ext3_setattr, + .setxattr = ext3_setxattr, + .getxattr = ext3_getxattr, + .listxattr = ext3_listxattr, + .removexattr = ext3_removexattr, };
fs/ext3/ialloc.c +2 -0
@@ -17,6 +17,7 @@#include <linux/jbd.h> #include <linux/ext3_fs.h> #include <linux/ext3_jbd.h> +#include <linux/ext3_xattr.h> #include <linux/stat.h> #include <linux/string.h> #include <linux/quotaops.h>
@@ -118,6 +119,7 @@* as writing the quota to disk may need the lock as well. */ DQUOT_INIT (inode); +ext3_xattr_delete_inode (handle, inode);DQUOT_FREE_INODE (inode);DQUOT_DROP (inode);
fs/ext3/inode.c +22 -13
@@ -42,6 +42,18 @@*/ #undef SEARCH_FROM_ZERO +/* +* Test whether an inode is a fast symlink. +*/ +static inline int ext3_inode_is_fast_symlink (struct inode *inode) +{ +int ea_blocks =EXT3_I (inode)->i_file_acl ? + (inode->i_sb->s_blocksize >>9 ) :0 ; + +return (S_ISLNK (inode->i_mode) && + inode->i_blocks - ea_blocks ==0 ); +} +/* The ext3 forget function must perform a revoke if we are freeing data * which has been journaled. Metadata (eg. indirect blocks) must be * revoked in all cases.
@@ -51,7 +63,7 @@* still needs to be revoked. */ -static int ext3_forget (handle_t *handle,int is_metadata, +int ext3_forget (handle_t *handle,int is_metadata,struct inode *inode,struct buffer_head *bh,int blocknr) {
@@ -167,9 +179,7 @@ {handle_t *handle; -if (is_bad_inode (inode) || - inode->i_ino ==EXT3_ACL_IDX_INO || - inode->i_ino ==EXT3_ACL_DATA_INO ) +if (is_bad_inode (inode)) goto no_delete;lock_kernel ();
@@ -1985,6 +1995,8 @@if (!(S_ISREG (inode->i_mode) ||S_ISDIR (inode->i_mode) ||S_ISLNK (inode->i_mode)))return ; +if (ext3_inode_is_fast_symlink (inode)) +return ;if (IS_APPEND (inode) ||IS_IMMUTABLE (inode))return ;
@@ -2136,8 +2148,6 @@struct ext3_group_desc * gdp;if ((inode->i_ino !=EXT3_ROOT_INO && - inode->i_ino !=EXT3_ACL_IDX_INO && - inode->i_ino !=EXT3_ACL_DATA_INO && inode->i_ino !=EXT3_JOURNAL_INO && inode->i_ino <EXT3_FIRST_INO (inode->i_sb)) || inode->i_ino > le32_to_cpu(
@@ -2269,10 +2279,7 @@brelse (iloc.bh); -if (inode->i_ino ==EXT3_ACL_IDX_INO || - inode->i_ino ==EXT3_ACL_DATA_INO ) -/* Nothing to do */ ; -else if (S_ISREG (inode->i_mode)) { +if (S_ISREG (inode->i_mode)) { inode->i_op = &ext3_file_inode_operations; inode->i_fop = &ext3_file_operations;if (ext3_should_writeback_data (inode))
@@ -2283,18 +2290,20 @@ inode->i_op = &ext3_dir_inode_operations; inode->i_fop = &ext3_dir_operations; }else if (S_ISLNK (inode->i_mode)) { -if (!inode->i_blocks) +if (ext3_inode_is_fast_symlink (inode)) inode->i_op = &ext3_fast_symlink_inode_operations;else { - inode->i_op = &page_symlink_inode_operations; + inode->i_op = &ext3_symlink_inode_operations;if (ext3_should_writeback_data (inode)) inode->i_mapping->a_ops = &ext3_writeback_aops;else inode->i_mapping->a_ops = &ext3_aops; } - }else + }else { + inode->i_op = &ext3_special_inode_operations;init_special_inode (inode, inode->i_mode,le32_to_cpu (iloc.raw_inode->i_block[0 ])); + }if (ei->i_flags &EXT3_SYNC_FL ) inode->i_flags |=S_SYNC ;if (ei->i_flags &EXT3_APPEND_FL )
fs/ext3/namei.c +16 -6
@@ -23,6 +23,7 @@#include <linux/time.h> #include <linux/ext3_fs.h> #include <linux/ext3_jbd.h> +#include <linux/ext3_xattr.h> #include <linux/fcntl.h> #include <linux/stat.h> #include <linux/string.h>
@@ -572,7 +573,7 @@if (IS_DIRSYNC (dir)) handle->h_sync =1 ; - inode =ext3_new_inode (handle, dir,S_IFDIR ); + inode =ext3_new_inode (handle, dir,S_IFDIR | mode); err =PTR_ERR (inode);if (IS_ERR (inode)) goto out_stop;
@@ -580,7 +581,6 @@ inode->i_op = &ext3_dir_inode_operations; inode->i_fop = &ext3_dir_operations; inode->i_size =EXT3_I (inode)->i_disksize = inode->i_sb->s_blocksize; - inode->i_blocks =0 ; dir_block =ext3_bread (handle, inode,0 ,1 , &err);if (!dir_block) { inode->i_nlink--;/* is this nlink == 0? */
@@ -607,9 +607,6 @@BUFFER_TRACE (dir_block,"call ext3_journal_dirty_metadata" );ext3_journal_dirty_metadata (handle, dir_block);brelse (dir_block); - inode->i_mode =S_IFDIR | mode; -if (dir->i_mode &S_ISGID ) - inode->i_mode |=S_ISGID ;ext3_mark_inode_dirty (handle, inode); err =ext3_add_entry (handle, dentry, inode);if (err)
@@ -987,7 +984,7 @@ goto out_stop;if (l >sizeof (EXT3_I (inode)->i_data)) { - inode->i_op = &page_symlink_inode_operations; + inode->i_op = &ext3_symlink_inode_operations;if (ext3_should_writeback_data (inode)) inode->i_mapping->a_ops = &ext3_writeback_aops;else
@@ -1203,4 +1200,17 @@ .rmdir = ext3_rmdir, .mknod = ext3_mknod, .rename = ext3_rename, + .setxattr = ext3_setxattr, + .getxattr = ext3_getxattr, + .listxattr = ext3_listxattr, + .removexattr = ext3_removexattr, }; + +struct inode_operations ext3_special_inode_operations = { + .setxattr = ext3_setxattr, + .getxattr = ext3_getxattr, + .listxattr = ext3_listxattr, + .removexattr = ext3_removexattr, +}; + +
fs/ext3/super.c +28 -2
@@ -24,6 +24,7 @@#include <linux/jbd.h> #include <linux/ext3_fs.h> #include <linux/ext3_jbd.h> +#include <linux/ext3_xattr.h> #include <linux/slab.h> #include <linux/init.h> #include <linux/blkdev.h>
@@ -405,6 +406,7 @@struct ext3_super_block *es = sbi->s_es;int i; +ext3_xattr_put_super (sb);journal_destroy (sbi->s_journal);if (!(sb->s_flags &MS_RDONLY )) {EXT3_CLEAR_INCOMPAT_FEATURE (sb,EXT3_FEATURE_INCOMPAT_RECOVER );
@@ -554,6 +556,7 @@ int is_remount) {unsigned long *mount_options = &sbi->s_mount_opt; +uid_t *resuid = &sbi->s_resuid;gid_t *resgid = &sbi->s_resgid;char * this_char;
@@ -566,6 +569,13 @@continue ;if ((value =strchr (this_char,'=' )) !=NULL ) *value++ =0 ; +#ifdef CONFIG_EXT3_FS_XATTR_USER +if (!strcmp (this_char,"user_xattr" )) +set_opt (*mount_options,XATTR_USER ); +else if (!strcmp (this_char,"nouser_xattr" )) +clear_opt (*mount_options,XATTR_USER ); + else +#endif if (!strcmp (this_char,"bsddf" ))clear_opt (*mount_options,MINIX_DF );else if (!strcmp (this_char,"nouid32" )) {
@@ -979,6 +989,12 @@ sbi->s_mount_opt =0 ; sbi->s_resuid =EXT3_DEF_RESUID ; sbi->s_resgid =EXT3_DEF_RESGID ; + +/* Default extended attribute flags */ +#ifdef CONFIG_EXT3_FS_XATTR_USER +set_opt (sbi->s_mount_opt,XATTR_USER ); +#endif +if (!parse_options ((char *) data, &sb_block, sbi, &journal_inum,0 )) goto out_fail;
@@ -1814,7 +1830,13 @@static int __init init_ext3_fs (void ) { -int err =init_inodecache (); +int err =init_ext3_xattr (); +if (err) +return err; + err =init_ext3_xattr_user (); +if (err) + goto out2; + err =init_inodecache ();if (err) goto out1; err =register_filesystem (&ext3_fs_type);
@@ -1824,13 +1846,17 @@ out:destroy_inodecache (); out1: +exit_ext3_xattr_user (); +out2: +exit_ext3_xattr ();return err; } static void __exit exit_ext3_fs(void) {unregister_filesystem (&ext3_fs_type); -destroy_inodecache (); +exit_ext3_xattr_user (); +exit_ext3_xattr (); }MODULE_AUTHOR ("Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others" );
fs/ext3/symlink.c +15 -1
@@ -20,6 +20,7 @@#include <linux/fs.h> #include <linux/jbd.h> #include <linux/ext3_fs.h> +#include <linux/ext3_xattr.h> static int ext3_readlink (struct dentry *dentry,char *buffer,int buflen) {
@@ -33,7 +34,20 @@return vfs_follow_link (nd, (char *)ei->i_data); } +struct inode_operations ext3_symlink_inode_operations = { + .readlink = page_readlink, + .follow_link = page_follow_link, + .setxattr = ext3_setxattr, + .getxattr = ext3_getxattr, + .listxattr = ext3_listxattr, + .removexattr = ext3_removexattr, +}; +struct inode_operations ext3_fast_symlink_inode_operations = { - .readlink = ext3_readlink,/* BKL not held. Don't need */ + .readlink = ext3_readlink,/* BKL not held. Don't need */ .follow_link = ext3_follow_link,/* BKL not held. Don't need */ + .setxattr = ext3_setxattr, + .getxattr = ext3_getxattr, + .listxattr = ext3_listxattr, + .removexattr = ext3_removexattr, };
fs/ext3/xattr.c new file +1233 -0
1,233 lines not on this page · raw message
fs/ext3/xattr_user.c new file +113 -0
113 lines not on this page · raw message
include/linux/ext3_fs.h +5 -26
@@ -58,8 +58,6 @@*/ #define EXT3_BAD_INO 1/* Bad blocks inode */ #define EXT3_ROOT_INO 2/* Root inode */ -#define EXT3_ACL_IDX_INO 3/* ACL inode */ -#define EXT3_ACL_DATA_INO 4/* ACL inode */ #define EXT3_BOOT_LOADER_INO 5/* Boot loader inode */ #define EXT3_UNDEL_DIR_INO 6/* Undelete directory inode */ #define EXT3_RESIZE_INO 7/* Reserved group descriptors inode */
@@ -89,7 +87,6 @@#else # define EXT3_BLOCK_SIZE (s) (EXT3_MIN_BLOCK_SIZE << (s)->s_log_block_size)#endif -#define EXT3_ACLE_PER_BLOCK (s) (EXT3_BLOCK_SIZE(s) / sizeof (struct ext3_acl_entry))#define EXT3_ADDR_PER_BLOCK (s) (EXT3_BLOCK_SIZE(s) / sizeof (__u32))#ifdef __KERNEL__# define EXT3_BLOCK_SIZE_BITS (s) ((s)->s_blocksize_bits)
@@ -124,28 +121,6 @@#endif /* -* ACL structures -*/ -struct ext3_acl_header /* Header of Access Control Lists */ -{ -__u32 aclh_size; -__u32 aclh_file_count; -__u32 aclh_acle_count; -__u32 aclh_first_acle; -}; - -struct ext3_acl_entry /* Access Control List Entry */ -{ -__u32 acle_size; -__u16 acle_perms;/* Access permissions */ -__u16 acle_type;/* Type of entry */ -__u16 acle_tag;/* User or group identity */ -__u16 acle_pad1; -__u32 acle_next;/* Pointer on next entry for the */ -/* same inode or on next free entry */ -}; - -/* * Structure of a blocks group descriptor */ struct ext3_group_desc
@@ -341,6 +316,7 @@#define EXT3_MOUNT_WRITEBACK_DATA 0x0C00/* No data ordering */ #define EXT3_MOUNT_UPDATE_JOURNAL 0x1000/* Update the journal format */ #define EXT3_MOUNT_NO_UID32 0x2000/* Disable 32-bit UIDs */ +#define EXT3_MOUNT_XATTR_USER 0x4000/* Extended user attributes */ /* Compatibility, for having both ext2_fs.h and ext3_fs.h included at once */ #ifndef _LINUX_EXT2_FS_H
@@ -520,7 +496,7 @@#define EXT3_FEATURE_INCOMPAT_RECOVER 0x0004/* Needs recovery */ #define EXT3_FEATURE_INCOMPAT_JOURNAL_DEV 0x0008/* Journal device */ -#define EXT3_FEATURE_COMPAT_SUPP 0 +#define EXT3_FEATURE_COMPAT_SUPP EXT2_FEATURE_COMPAT_EXT_ATTR#define EXT3_FEATURE_INCOMPAT_SUPP (EXT3_FEATURE_INCOMPAT_FILETYPE| \ EXT3_FEATURE_INCOMPAT_RECOVER)#define EXT3_FEATURE_RO_COMPAT_SUPP (EXT3_FEATURE_RO_COMPAT_SPARSE_SUPER| \
@@ -636,6 +612,7 @@extern unsigned long ext3_count_free (struct buffer_head *,unsigned );/* inode.c */ +extern int ext3_forget (handle_t *,int ,struct inode *,struct buffer_head *,int );extern struct buffer_head *ext3_getblk (handle_t *,struct inode *,long ,int ,int *);extern struct buffer_head *ext3_bread (handle_t *,struct inode *,int ,int ,int *);
@@ -702,8 +679,10 @@/* namei.c */ extern struct inode_operations ext3_dir_inode_operations; +extern struct inode_operations ext3_special_inode_operations;/* symlink.c */ +extern struct inode_operations ext3_symlink_inode_operations;extern struct inode_operations ext3_fast_symlink_inode_operations;
include/linux/ext3_jbd.h +7 -1
@@ -30,13 +30,19 @@#define EXT3_SINGLEDATA_TRANS_BLOCKS 8 +/* Extended attributes may touch two data buffers, two bitmap buffers, +* and two group and summaries. */ + +#define EXT3_XATTR_TRANS_BLOCKS 8 +/* Define the minimum size for a transaction which modifies data. This * needs to take into account the fact that we may end up modifying two * quota files too (one for the group, one for the user quota). The * superblock only gets updated once, of course, so don't bother * counting that again for the quota updates. */ -#define EXT3_DATA_TRANS_BLOCKS (3 * EXT3_SINGLEDATA_TRANS_BLOCKS - 2) +#define EXT3_DATA_TRANS_BLOCKS (3 * EXT3_SINGLEDATA_TRANS_BLOCKS + \ + EXT3_XATTR_TRANS_BLOCKS - 2)extern int ext3_writepage_trans_blocks (struct inode *inode);
include/linux/ext3_xattr.h new file +157 -0
157 lines not on this page · raw message
Quoted 8 lines · On Tue, Oct 08, 2002 at 02:08:20PM -0400, [email protected] wrote:
This is the third of four patches which add extended attribute support to the ext2 and ext3 filesystems. Please comment and bleed.
This patch adds extended attribute support to the ext3 filesystem. This uses the generic extended attribute patch which was developed by Andreas Gruenbacher and the XFS team. As a result, the user space utilities which work for XFS will also work with these patches.
JFS also already supports that API..
fs/Config.help line 163
Quoted 6 lines
fs/Config.help +6
-0+CONFIG_EXT3_FS_XATTR + Extended attributes are name:value pairs associated with inodes by + the kernel or by users (see the attr(5) manual page, or visit + <http://acl.bestbits.at/> for details). + + If unsure, say N.
No need for a help text if you set it always to true. Even better: remove the option if it's always set..
Quoted 7 lines
fs/Config.help +7
-0+ +Ext3 extended attribute block sharing +CONFIG_EXT3_FS_XATTR_SHARING + This options enables code for sharing identical extended attribute + blocks among multiple inodes. + + Usually, say Y.
fs/Config.help line 180
Quoted 10 lines
fs/Config.help +10
-0+ +Ext3 extended user attributes +CONFIG_EXT3_FS_XATTR_USER + This option enables extended user attributes on ext3. Processes can + associate extended user attributes with inodes to store additional + information such as the character encoding of files, etc. (see the + attr(5) manual page, or visit <http://acl.bestbits.at/> for details). + + If unsure, say N. +
I think user xattrs should be implied automatically like xfs and jfs do.
Quoted 7 lines
CONFIG_JBD
This is a generic journaling layer for block devices. It is currently used by the ext3 file system, but it could also be used to
diff -Nru a/fs/Config.in b/fs/Config.in
fs/Config.in line 34
Quoted 8 lines
fs/Config.in +5
-0@@ -27,6 +27,11 @@ dep_tristate 'BFS file system support (EXPERIMENTAL)' CONFIG_BFS_FS $CONFIG_EXPERIMENTAL tristate 'Ext3 journalling file system support' CONFIG_EXT3_FS +if [ "$CONFIG_EXT3_FS" != "n" ] ; then + define_tristate CONFIG_EXT3_FS_XATTR $CONFIG_EXT3_FS + define_bool CONFIG_EXT3_FS_XATTR_SHARING y + define_bool CONFIG_EXT3_FS_XATTR_USER y +fi
Have you actually tried to compile this modular? AFACIS this would make fs/ext3/xattr.o it's own module which can't work.
Just remove the CONFIG_EXT3_FS_XATTR option and use three-space indents in Config.in's while we're at it :)
fs/ext3/Makefile line 12
Quoted 10 lines
diff -Nru a/fs/ext3/Makefile b/fs/ext3/Makefile
fs/ext3/Makefile +3
-0@@ -7,4 +7,8 @@ ext3-objs := balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o \ ioctl.o namei.o super.o symlink.o +export-objs += xattr.o +obj-$(CONFIG_EXT3_FS_XATTR) += xattr.o +obj-$(CONFIG_EXT3_FS_XATTR_USER) += xattr_user.o
I think this should be
ifneq($(CONFIG_EXT3_FS_XATTR))
ext3-objs += xattr.o
endif
ifneq($(CONFIG_EXT3_FS_XATTR_USER))
ext3-objs += xattr_user.o
endif
Why do I get the feeling you blindly took a 2.4 patch and just got it compile built-in? :)
fs/ext3/Makefile
Quoted 3 lines
fs/ext3/Makefile +3
-0+#ifdef CONFIG_EXT3_FS_XATTR_USER + if (!strcmp (this_char, "user_xattr")) + set_opt (*mount_options, XATTR_USER);
If we really want a user_xattr mount option I'd suggest taking it into VFS. But IMHO it's rather useless, just don't access them if you don't want to.
fs/ext3/Makefile
Quoted 3 lines
fs/ext3/Makefile +3
-0+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,0) +# define mark_buffer_dirty(bh) mark_buffer_dirty(bh, 1) +#endif
2.2 compat code in new 2.5 filesystem features?
fs/ext3/Makefile
Quoted 13 lines
fs/ext3/Makefile +13
-0+DECLARE_MUTEX(ext3_xattr_sem); + +static inline void +ext3_xattr_lock(void) +{ + down(&ext3_xattr_sem); +} + +static inline void +ext3_xattr_unlock(void) +{ + up(&ext3_xattr_sem); +}
What about just directly taking that lock?
Quoted 1 line
+#ifdef
OLD_QUOTAS
Do you really want to keep around the praehistoric quota code?
Quoted 1 line
+#if
LINUX_VERSION_CODE<KERNEL_VERSION(2,4,18)
More old compat code - not even usefull for any recent 2.4
Quoted 1 line
+#ifdef
__KERNEL__
I thought the time of shared headers was long gone. Do you use an exact copy of this in e2fsprogs or is this just junk?
fs/ext3/Makefile
Quoted 11 lines
fs/ext3/Makefile +11
-0+extern int ext3_setxattr(struct dentry *, const char *, void *, size_t, int); +extern ssize_t ext3_getxattr(struct dentry *, const char *, void *, size_t); +extern ssize_t ext3_listxattr(struct dentry *, char *, size_t); +extern int ext3_removexattr(struct dentry *, const char *); + +extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t); +extern int ext3_xattr_list(struct inode *, char *, size_t); +extern int ext3_xattr_set(handle_t *handle, struct inode *, int, const char *, const void *, size_t, int); + +extern void ext3_xattr_delete_inode(handle_t *, struct inode *); +extern void ext3_xattr_put_super(struct super_block *);
All other function prototypes inside ext3 are in ext3_fs.h, do we really need a new header for xattr ones?
Also please get rid of the registration API for xattr handlers - this is inside a single module so hardconding them in the inode operations won't hurt. the additional lock for the registration OTOH may hurt and it looks really overengineered.
Quoted 7 lines · Christoph Hellwig
fs/ext3/super.c line 574
Quoted 3 lines
fs/ext3/super.c +3
-0+#ifdef CONFIG_EXT3_FS_XATTR_USER +if (!strcmp (this_char,"user_xattr" )) +set_opt (*mount_options,XATTR_USER );If we really want a
user_xattrmount option I'd suggest taking it into VFS. But IMHO it's rather useless, just don't access them if you don't want to.
Users might just fill up all xattr space leaving no space for ACLs (or similar). If user xattrs are disabled this can no longer occur, so some administrators might be happy to have a choice.
Quoted 4 lines
Also please get rid of the registration API for xattr handlers - this is inside a single module so hardconding them in the inode operations won't hurt. the additional lock for the registration OTOH may hurt and it looks really overengineered.
With the registration API modules doing HSM/LSM/... can just register handlers without having to modify the file system code. Otherwise we would have to hand code additional hooks for independently loadable modules.
--Andreas.
Quoted 3 lines · Andreas Gruenbacher
Users might just fill up all xattr space leaving no space for ACLs (or similar). If user xattrs are disabled this can no longer occur, so some administrators might be happy to have a choice.
Umm, that's why we have quota..
Quoted 3 lines
With the registration API modules doing HSM/LSM/... can just register handlers without having to modify the file system code. Otherwise we would have to hand code additional hooks for independently loadable modules.
a) if the HSM/whatever is so standalone that it should patch ext2 code it is truely generic and thus above the filesystem.
b) you don;t even export the register/unregister to modules, not mention other ext2/ext3 core functionality that it would need.
c) looks at the 'would' there is no such code currently and if it gets a real consern it still could be added. Don't bloat the kernel more than you really need to..
Hi,
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Andreas Gruenbacher
Users might just fill up all xattr space leaving no space for ACLs (or similar). If user xattrs are disabled this can no longer occur, so some administrators might be happy to have a choice.
Umm, that's why we have quota..
It's the per-inode extended attribute space that's at risk here, quotas don't help.
--Stephen
Quoted 4 lines · Stephen C. Tweedie
Hi,
Quoted 1 line · Christoph Hellwig
On Tue, Oct 08, 2002 at 09:14:00PM +0200, Andreas Gruenbacher wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Users might just fill up all xattr space leaving no space for ACLs (or similar). If user xattrs are disabled this can no longer occur, so some administrators might be happy to have a choice.
Umm, that's why we have quota..
It's the per-inode extended attribute space that's at risk here, quotas don't help.
Well, that's a more important problem. But I doubt a hack to just turn off user xattrs is the right fix then. A static reservation for ACLs or just totally separating them (like in XFS) seems more måture.
Quoted 7 lines · Christoph Hellwig
Quoted 6 lines · Stephen C. Tweedie
Quoted 5 lines · Christoph Hellwig
Quoted 3 lines · Andreas Gruenbacher
Users might just fill up all xattr space leaving no space for ACLs (or similar). If user xattrs are disabled this can no longer occur, so some administrators might be happy to have a choice.
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Umm, that's why we have quota..
It's the per-inode extended attribute space that's at risk here, quotas don't help.
Well, that's a more important problem. But I doubt a hack to just turn off user xattrs is the right fix then. A static reservation for ACLs or just totally separating them (like in XFS) seems more m?ture.
Yes, we have made proposals in the past to change the way ext2/3 EAs are stored on disk, but nobody has had time to work on it yet. The current limitations of 4kB of EA per inode (user+system) is bad, as is the fact that you need a full block for each inode if you have any unique per-inode data (basically anything other than ACLs).
That said, the benefits of the current EA implementation far outweigh the limitations, and since this is an internal-to-ext2/3 issue, we can change it at some later date without affecting the rest of the kernel.
Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/
Quoted 11 lines · Stephen C. Tweedie
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Andreas Gruenbacher
Users might just fill up all xattr space leaving no space for ACLs (or similar). If user xattrs are disabled this can no longer occur, so some administrators might be happy to have a choice.
Umm, that's why we have quota..
It's the per-inode extended attribute space that's at risk here, quotas don't help.
Well, how about this as a compromise? We define a new superblock field which reserves a certain amount of space in the EA block for "system" attributes. The default will be zero, but if we want to reserve space for the ACL, we can do that by adjusting the superblock field. (I'll add support into tune2fs for that purpose.)
I'll then remove the CONFIG #ifdef's and the user_xattr mount options. (I hate having too many mount options, and this sort of thing should be a per-filesystem run-time decision, not a compile-time option.)
Andreas, does that sound good to you?
- Ted
Quoted 18 lines · Theodore Ts'o
Quoted 10 lines · Stephen C. Tweedie
Quoted 6 lines · Christoph Hellwig
Quoted 3 lines · Andreas Gruenbacher
Users might just fill up all xattr space leaving no space for ACLs (or similar). If user xattrs are disabled this can no longer occur, so some administrators might be happy to have a choice.
Umm, that's why we have quota..
It's the per-inode extended attribute space that's at risk here, quotas don't help.
Well, how about this as a compromise? We define a new superblock field which reserves a certain amount of space in the EA block for "system" attributes. The default will be zero, but if we want to reserve space for the ACL, we can do that by adjusting the superblock field. (I'll add support into tune2fs for that purpose.)
Quoted 5 lines
I'll then remove the CONFIG #ifdef's and the
user_xattrmount options. (I hate having too many mount options, and this sort of thing should be a per-filesystem run-time decision, not a compile-time option.)Andreas, does that sound good to you?
I'd rather not store such policy things on the file system permanently, and make it a mount option. I'm wondering how many installations this would affect; to me it seems that it's not worth a super block flag.
Eventually we will hopefully move to a more flexible xattr storage mechanism; that hack will then just become baggage.
--Andreas.
Hi,
Quoted 10 lines · Andreas Gruenbacher
Quoted 5 lines · Theodore Ts'o
Well, how about this as a compromise? We define a new superblock field which reserves a certain amount of space in the EA block for "system" attributes.
Andreas, does that sound good to you?
I'd rather not store such policy things on the file system permanently, and make it a mount option. I'm wondering how many installations this would affect; to me it seems that it's not worth a super block flag.
We already have a lot of similar places where we do this in ext2/3. The standard procedure is to store the default in the superblock but to allow the user to override at mount time.
--Stephen
Quoted 5 lines · Christoph Hellwig
Also please get rid of the registration API for xattr handlers - this is inside a single module so hardconding them in the inode operations won't hurt. the additional lock for the registration OTOH may hurt and it looks really overengineered.
The registration API is also used for the ACL routines. That's why it's there.
That being said, I'm not at all convinced that allowing things like the EA routines and the ACL routines to be separate modules is a good idea. Life would be much simpler if everything was compiled into a single ext3 subsystem, and we eliminate all of the extra overhead of having separate modules for specific filesystem features. Certainly the XFS and JFS folks didn't go down this path.
(That's why I took out to the questions in Config.in; I really felt that it was wrong. I however didn't want to go as far as to remove the #ifdef's and removing the registration API's without at least having some discussion on the topic.)
- Ted
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 8 lines · Andreas Dilger
Well, for people who are using these features, it is irrelevant whether you can configure them separately or not. However, for people who do not use these features, it is just more "bloat" that is being added to their kernel. Certainly the EA and ACL code have a noticable size, so having a config option can only be good for those users. Sure - make it default to "on" for people who don't know any better, but it is still nice to be able to tune things as needed (e.g. for a small box which needs a journaling fs, but not fancy features).
Well, the ACL code would be depend on CONFIG_FS_POSIX_ACL as in XFS and JFS. We might want one XATTR config option for ext2/ext3 if this is a real concern for people (it might be a good idea after thinking a bit more about it). IMHO we should end up with one option for xattr support in ext2, one for xattrs in ext3 and one for all filesystems for ACLs.