ext4: yet another project quota

This patch adds generic vfs interface for project id and related super-block methods.

Two new ioctls:
int ioctl(fd, FS_IOC_GETPROJECT, unsigned *project); int ioctl(fd, FS_IOC_SETPROJECT, unsigned *project);

Signed-off-by Konstantin Khlebnikov <[email protected]>

---

Documentation/filesystems/Locking |    4 +++
Documentation/filesystems/vfs.txt |    8 +++++
fs/compat_ioctl.c                 |    2 +
fs/ioctl.c                        |   58 +++++++++++++++++++++++++++++++++++++
include/linux/fs.h                |    2 +
include/uapi/linux/fs.h           |    3 ++
6 files changed, 77 insertions(+)

Documentation/filesystems/Locking +2 -0

index f91926f2f482..7a01e2b606d0 100644
@@ -126,6 +126,8 @@ prototypes:
   ssize_t (*quota_read)(struct super_block *, int, char *, size_t, loff_t);
   ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
   int (*bdev_try_to_free_page)(struct super_block*, struct page*, gfp_t);
+    kprojid_t (*get_project) (struct inode *);
+    int (*set_project) (struct inode *, kprojid_t);
 

locking rules:
All may block [not true, see below]

Documentation/filesystems/Locking +2 -1

@@ -147,6 +149,8 @@ show_options:        no        (namespace_sem)
 quota_read:        no        (see below)
 quota_write:        no        (see below)
 bdev_try_to_free_page:    no        (see below)
+get_project        no
+set_project        no        (i_mutex)
 
->statfs() has s_umount (shared) when called by ustat(2) (native or

compat), but that's an accident of bad API; s_umount is used to pin

Documentation/filesystems/vfs.txt +2 -0

index 966b22829f3b..aeff3c54021b 100644
@@ -230,6 +230,8 @@ struct super_operations {
        ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
   int (*nr_cached_objects)(struct super_block *);
   void (*free_cached_objects)(struct super_block *, int);
+    kprojid_t (*get_project)(struct inode *);
+    int (*set_project)(struct inode *, kprojid_t);
 };
 

All methods are called without any locks being held, unless otherwise

Documentation/filesystems/vfs.txt +6 -0

@@ -319,6 +321,12 @@ or bottom half).
   implementations will cause holdoff problems due to large scan batch
   sizes.
 
+  get_project: called by the ioctl and quota code to get project id of an inode.
+    Method should return INVALID_PROJID if feature isn't supported.
+
+  set_project: called by the ioctl to set project id for an inode.
+    This is called with i_mutex held.
+

Whoever sets up the inode is responsible for filling in the "i_op" field. This is a pointer to a "struct inode_operations" which describes the methods that can be performed on individual inodes.

fs/compat_ioctl.c +0 -0

index afec6450450f..9a24e4038377 100644
@@ -889,6 +889,8 @@ COMPATIBLE_IOCTL(FIOASYNC)
 COMPATIBLE_IOCTL(FIONBIO)

COMPATIBLE_IOCTL(FIONREAD) /* This is also TIOCINQ */ COMPATIBLE_IOCTL(FS_IOC_FIEMAP)

fs/compat_ioctl.c +2 -0

+COMPATIBLE_IOCTL(FS_IOC_GETPROJECT)
+COMPATIBLE_IOCTL(FS_IOC_SETPROJECT)

/* 0x00 */
COMPATIBLE_IOCTL(FIBMAP)
COMPATIBLE_IOCTL(FIGETBSZ)

fs/ioctl.c +58 -0

index 5d01d2638ca5..d351576d95c8 100644
@@ -9,6 +9,7 @@
 #include <linux/capability.h>
 #include <linux/file.h>
 #include <linux/fs.h>
+#include <linux/mount.h>
 #include <linux/security.h>
 #include <linux/export.h>
 #include <linux/uaccess.h>

@@ -545,6 +546,57 @@ static int ioctl_fsthaw(struct file *filp)
   return thaw_super(sb);
 }
 
+static int ioctl_getproject(struct file *filp, projid_t __user *argp)
+{
+    struct user_namespace *ns = current_user_ns();
+    struct inode *inode = file_inode(filp);
+    struct super_block *sb = inode->i_sb;
+    kprojid_t kprojid;
+    projid_t projid;
+
+    if (!sb->s_op->get_project)
+        return -EOPNOTSUPP;
+    kprojid = sb->s_op->get_project(inode);
+    if (!projid_valid(kprojid))
+        return -EOPNOTSUPP;
+    projid = from_kprojid(ns, kprojid);
+    if (projid == (projid_t)-1)
+        return -EACCES;
+    return put_user(projid, argp);
+}
+
+static int ioctl_setproject(struct file *filp, projid_t __user *argp)
+{
+    struct user_namespace *ns = current_user_ns();
+    struct inode *inode = file_inode(filp);
+    struct super_block *sb = inode->i_sb;
+    kprojid_t kprojid;
+    projid_t projid;
+    int ret;
+
+    if (!sb->s_op->set_project)
+        return -EOPNOTSUPP;
+    if (ns != &init_user_ns)
+        return -EPERM;
+    ret = get_user(projid, argp);
+    if (ret)
+        return ret;
+    kprojid = make_kprojid(ns, projid);
+    if (!projid_valid(kprojid))
+        return -EACCES;
+    ret = mnt_want_write_file(filp);
+    if (ret)
+        return ret;
+    mutex_lock(&inode->i_mutex);
+    if (inode_owner_or_capable(inode))
+        ret = sb->s_op->set_project(inode, kprojid);
+    else
+        ret = -EPERM;
+    mutex_unlock(&inode->i_mutex);
+    mnt_drop_write_file(filp);
+    return ret;
+}
+
 /*
 * When you add any new common ioctls to the switches above and below
 * please update compat_sys_ioctl() too.

@@ -597,6 +649,12 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
   case FS_IOC_FIEMAP:
       return ioctl_fiemap(filp, arg);
 
+    case FS_IOC_GETPROJECT:
+        return ioctl_getproject(filp, argp);
+
+    case FS_IOC_SETPROJECT:
+        return ioctl_setproject(filp, argp);
+
   case FIGETBSZ:
       return put_user(inode->i_sb->s_blocksize, argp);
 

include/linux/fs.h +2 -0

index b4d71b5e1ff2..42156801739e 100644
@@ -1655,6 +1655,8 @@ struct super_operations {
                 struct shrink_control *);
   long (*free_cached_objects)(struct super_block *,
                   struct shrink_control *);
+    kprojid_t (*get_project)(struct inode *);
+    int (*set_project)(struct inode *, kprojid_t);
 };
 
 /*

include/uapi/linux/fs.h +3 -1

index 9b964a5920af..144426326e15 100644
@@ -170,6 +170,9 @@ struct inodes_stat_t {
 #define FS_IOC32_GETVERSION        _IOR('v', 1, int)
 #define FS_IOC32_SETVERSION        _IOW('v', 2, int)
 
+#define FS_IOC_GETPROJECT        _IOR('f', 20, unsigned)
+#define FS_IOC_SETPROJECT        _IOW('f', 21, unsigned)
+
 /*
 * Inode flags (FS_IOC_GETFLAGS / FS_IOC_SETFLAGS)
 */
 
--

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

How do your patches relate to the project quota patches from Li Xi?

Cheers, Andreas

Quoted 11 lines · Konstantin Khlebnikov

This patch adds generic vfs interface for project id and related super-block methods.

Two new ioctls:
int ioctl(fd, FS_IOC_GETPROJECT, unsigned *project); int ioctl(fd, FS_IOC_SETPROJECT, unsigned *project);

Signed-off-by Konstantin Khlebnikov <[email protected]>

---

Historically XFS project id doesn't have any permission control: file owner is able to set any project id. Later they was sealed with user-namespace:
XFS allows to change it only from init user-ns. That works fine for isolated containers or if user doesn't have direct access to the filesystem (NFS/FTP).

This patch adds sysctl fs.protected_projects which makes changing project id privileged operation which requires CAP_SYS_RESOURCE in current user-namespace. Thus there are two levels of protection: project id mapping in user-ns defines set of permitted projects and capability protects operations within this set.

Quoted 5 lines · Konstantin Khlebnikov

Historically XFS project id doesn't have any permission control: file owner is able to set any project id. Later they was sealed with user-namespace:
XFS allows to change it only from init user-ns. That works fine for isolated containers or if user doesn't have direct access to the filesystem (NFS/FTP).

Quoted 4 lines

This patch adds sysctl fs.protected_projects which makes changing project id privileged operation which requires CAP_SYS_RESOURCE in current user-namespace. Thus there are two levels of protection: project id mapping in user-ns defines set of permitted projects and capability protects operations within this set.

If I understand this right, this doesn't work. If I lack CAP_SYS_RESOURCE but I have two projids mapped, then I can create a new userns, map both projids, and get CAP_SYS_RESOURCE.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 7 lines · Andy Lutomirski

Quoted 5 lines · Konstantin Khlebnikov

Historically XFS project id doesn't have any permission control: file owner is able to set any project id. Later they was sealed with user-namespace:
XFS allows to change it only from init user-ns. That works fine for isolated containers or if user doesn't have direct access to the filesystem (NFS/FTP).

Quoted 8 lines

Quoted 4 lines

This patch adds sysctl fs.protected_projects which makes changing project id privileged operation which requires CAP_SYS_RESOURCE in current user-namespace. Thus there are two levels of protection: project id mapping in user-ns defines set of permitted projects and capability protects operations within this set.

If I understand this right, this doesn't work. If I lack CAP_SYS_RESOURCE but I have two projids mapped, then I can create a new userns, map both projids, and get CAP_SYS_RESOURCE.

Setting project id mapping for nested user-namespace also requires this capability in parent namespace. The same as for setting uid/gid mapping but without special case for mapping current uid/gid because task has no "current" project id.

This is mentioned in cover letter but I forget it here. Sorry.

Quoted 7 lines

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Projects quota allows to enforce disk quota for several subtrees or even individual files on the filesystem. Each inode is marked with project-id (independently from uid and gid) and accounted into corresponding project quota. New files inherits project id from directory where they are created.

This is must-have feature for deploying lightweight containers. Also project quota can tell size of subtree without doing recursive 'du'.

This patchset adds project id and quota into ext4.

This time I've prepared patches also for e2fsprogs and quota-tools.

All patches are available at github:
https://github.com/koct9i/linux --branch project
https://github.com/koct9i/e2fsprogs --branch project
https://github.com/koct9i/quota-tools --branch project

Porposed behavior is similar to project quota in XFS:

* all inode are marked with project id
* new files inherits project id from parent directory
* project quota accounts inodes and enforces limits
* cross-project link and rename operations are restricted

Differences:

There is no flag similar to XFS_XFLAG_PROJINHERIT (which allows to disable project id inheritance), instead of that project which userspace sees as '0' (in nested user-name space that might be actually non-zero project) acts as default project where restrictions for link/rename are ignored. (also see below, in "why new ioctl" part)

This implementation adds shortcut for moving files from one project into
another: non-directory inodes with n_link == 1 are transferred without copying (XFS in this case returns -EXDEV and userspace have to copy file).

In XFS file owner (or process with CAP_FOWNER) can set set any project id, XFS permits changing project id only from init user-namespace.

This patchset adds sysctl fs.protected_projects. By default it's 0 and project id acts as XFS project. Setting it to 1 makes chaning project id priviliged operation which requires CAP_SYS_RESOURCE in current user-namespace, changing project id mapping for nested user-namespace also requires that capability. Thus there are two levels of control: project id mapping in user-ns defines set of permitted projects and capability protects operations within this set.

I see no problems with supporting all this in XFS, all difference in interface.

Ext4 layout
-----------

Project id introduce ro-compatible feature 'project'.

Inode project id is stored in place of obsolete field 'i_faddr' (that trick was suggested by Darrick J. Wong in previous discussions of project quota). Linux never used that field and present fsck checks that it contains zero.

Quota information is stored in special inode №11 (by default only 10 inodes are reserved for special usage, I've add option to resize2fs to reserve more). (see e2fsprogs patches for details) For symmetry with other quotas inode number is stored in superblock.

Project quota supports only modern 'hidden' journaled mode.

Interface
---------

Interface for changing limits / query current usage is common vfs quotactl() where quotatype = PRJQUOTA = 2. User can query current state of any project mapped into user-ns, changing of limits requires CAP_SYS_ADMIN in init user-ns.

Two new ioctls for getting / changing inode project id:
int ioctl(fd, FS_IOC_GETPROJECT, unsigned *project); int ioctl(fd, FS_IOC_SETPROJECT, unsigned *project);

They acts as interface for super-block methods get_project / set_project Generic code checks permissions, does project id translation in user-namespace mapping, grabs write-access to the filesystem, locks i_mutex for set opetaion. Filesystem method only updates inode and transfers project quota.

No new mount options added. Disk usage tracking is enabled at mount. Limits are enabeld later by "quotaon".

(BTW why journalled quota doesn't enable limits right at the time of mounting?)

Why new ioctls?
---------------

XFS has it's own interface for that: XFS_IOC_FSGETXATTR / XFS_IOC_FSSETXATTR. But it has several flaws and doesn't fit for a role of generic interface.

It contains a lot of xfs-specific flags and racy by design: set operation commits all fields at once thus it's used in sequence get-change-set without any lock, Concurrent updates from user space will collide.

Also xfs has flag XFS_XFLAG_PROJINHERIT which tells should new files inherit project id from parent directory or not. This flag is barely useful and only makes everything complicated. Even tools in xfsprogs don't use it: they always set it together with project id and clears when set project id back to zero.

And the main reason: this compatibility gives nothing. The only user of xfs ioctl which I've found is the xfsprogs. And these tools check filesystem name and don't work anywhere except 'xfs'.

Links
-----

[1] 2014-12-09 ext4: add project quota support by Li Xi
http://marc.info/?l=linux-fsdevel&m=141810265603565&w=2

[2] 2014-01-28 A draft for making ext4 support project quota by Zheng Liu
http://marc.info/?l=linux-ext4&m=139089109605795&w=2

[3] 2012-07-09 introduce extended inode owner identifier v10 by Dmitry Monakhov
http://thread.gmane.org/gmane.linux.file-systems/65752

[4] 2010-02-08 Introduce subtree quota support by Dmitry Monakhov
http://thread.gmane.org/gmane.comp.file-systems.ext4/17530

---

Konstantin Khlebnikov (6):
fs: vfs ioctls for managing project id
fs: protected project id
quota: generic project quota
ext4: support project id and project quota
ext4: add shortcut for moving files across projects
ext4: mangle statfs results accourding to project quota usage and limits

 Documentation/filesystems/Locking |    4 +
 Documentation/filesystems/vfs.txt |    8 +++
 Documentation/sysctl/fs.txt       |   16 ++++++
 fs/compat_ioctl.c                 |    2 +
 fs/ext4/ext4.h                    |   15 ++++-
 fs/ext4/ialloc.c                  |    3 +
 fs/ext4/inode.c                   |   15 +++++
 fs/ext4/namei.c                   |  102 ++++++++++++++++++++++++++++++++++++-
 fs/ext4/super.c                   |   61 ++++++++++++++++++++--
 fs/ioctl.c                        |   62 ++++++++++++++++++++++
 fs/quota/dquot.c                  |   96 +++++++++++++++++++++++++++++++++--
 fs/quota/quota.c                  |    8 ++-
 fs/quota/quotaio_v2.h             |    6 +-
 include/linux/fs.h                |    3 +
 include/linux/quota.h             |    1 
 include/linux/quotaops.h          |   16 ++++++
 include/uapi/linux/capability.h   |    1 
 include/uapi/linux/fs.h           |    3 +
 include/uapi/linux/quota.h        |    6 +-
 kernel/sysctl.c                   |    9 +++
 kernel/user_namespace.c           |    4 +
 21 files changed, 416 insertions(+), 25 deletions(-)

--
Signature

Hello,

Quoted 17 lines · Konstantin Khlebnikov

Projects quota allows to enforce disk quota for several subtrees or even individual files on the filesystem. Each inode is marked with project-id (independently from uid and gid) and accounted into corresponding project quota. New files inherits project id from directory where they are created.

This is must-have feature for deploying lightweight containers. Also project quota can tell size of subtree without doing recursive 'du'.

This patchset adds project id and quota into ext4.

This time I've prepared patches also for e2fsprogs and quota-tools.

All patches are available at github:
https://github.com/koct9i/linux --branch project
https://github.com/koct9i/e2fsprogs --branch project
https://github.com/koct9i/quota-tools --branch project

Quoted 6 lines

Porposed behavior is similar to project quota in XFS:

* all inode are marked with project id
* new files inherits project id from parent directory
* project quota accounts inodes and enforces limits
* cross-project link and rename operations are restricted

Thanks for the patches and sorry for taking a long time to look into
them. I like your patches and they looks mostly fine to me but can we *please* start with making things completely compatible with how XFS works? I understand that may seem broken / not useful for your usecase but consistency among filesystems is really important.

I was talking with Dave Chinner last week and we agreed that the direction you are changing things makes sense but getting things compatible with how they were for 15 years in XFS is an important first step (because there may be people with other usecases depending on the old behavior and they would get confused by ext4 behaving differently). After we have compatible code in, we can add your fs.protected_projects thing on top of that (and also support it in XFS to stay compatible).

Quoted 56 lines

Differences:

There is no flag similar to XFS_XFLAG_PROJINHERIT (which allows to disable project id inheritance), instead of that project which userspace sees as '0' (in nested user-name space that might be actually non-zero project) acts as default project where restrictions for link/rename are ignored. (also see below, in "why new ioctl" part)

This implementation adds shortcut for moving files from one project into
another: non-directory inodes with n_link == 1 are transferred without copying (XFS in this case returns -EXDEV and userspace have to copy file).

In XFS file owner (or process with CAP_FOWNER) can set set any project id, XFS permits changing project id only from init user-namespace.

This patchset adds sysctl fs.protected_projects. By default it's 0 and project id acts as XFS project. Setting it to 1 makes chaning project id priviliged operation which requires CAP_SYS_RESOURCE in current user-namespace, changing project id mapping for nested user-namespace also requires that capability. Thus there are two levels of control: project id mapping in user-ns defines set of permitted projects and capability protects operations within this set.

I see no problems with supporting all this in XFS, all difference in interface.

Ext4 layout
-----------

Project id introduce ro-compatible feature 'project'.

Inode project id is stored in place of obsolete field 'i_faddr' (that trick was suggested by Darrick J. Wong in previous discussions of project quota). Linux never used that field and present fsck checks that it contains zero.

Quota information is stored in special inode №11 (by default only 10 inodes are reserved for special usage, I've add option to resize2fs to reserve more). (see e2fsprogs patches for details) For symmetry with other quotas inode number is stored in superblock.

Project quota supports only modern 'hidden' journaled mode.

Interface
---------

Interface for changing limits / query current usage is common vfs quotactl() where quotatype = PRJQUOTA = 2. User can query current state of any project mapped into user-ns, changing of limits requires CAP_SYS_ADMIN in init user-ns.

Two new ioctls for getting / changing inode project id:
int ioctl(fd, FS_IOC_GETPROJECT, unsigned *project); int ioctl(fd, FS_IOC_SETPROJECT, unsigned *project);

They acts as interface for super-block methods get_project / set_project Generic code checks permissions, does project id translation in user-namespace mapping, grabs write-access to the filesystem, locks i_mutex for set opetaion. Filesystem method only updates inode and transfers project quota.

Quoted 4 lines

No new mount options added. Disk usage tracking is enabled at mount. Limits are enabeld later by "quotaon".

(BTW why journalled quota doesn't enable limits right at the time of mounting?)

Well, mostly historical reasons :) The biggest probably was that to
properly initialize quotas (or fix them if quota file gets corrupted) you have to still run quotacheck and for that kernel mustn't touch the files. At the time when I was writing journaled quotas I didn't feel like adding quotacheck support into e2fsck... We eventually did that anyway for support of quota in system files but how we did that was actually based on the experience I had from the journaled quota lesson ;).

Quoted 6 lines

Why new ioctls?
---------------

XFS has it's own interface for that: XFS_IOC_FSGETXATTR / XFS_IOC_FSSETXATTR. But it has several flaws and doesn't fit for a role of generic interface.

Quoted 3 lines

It contains a lot of xfs-specific flags and racy by design: set operation commits all fields at once thus it's used in sequence get-change-set without any lock, Concurrent updates from user space will collide.

Sure but that's the case for generic GETFLAGS / SETFLAGS interface as
well.

Quoted 5 lines

Also xfs has flag XFS_XFLAG_PROJINHERIT which tells should new files inherit project id from parent directory or not. This flag is barely useful and only makes everything complicated. Even tools in xfsprogs don't use it: they always set it together with project id and clears when set project id back to zero.

I think this is about balance - adding the support for the PROJINHERIT flag
costs us close to nothing and we get a compatibility with XFS. So even though the usefulness of that flag is doubtful, I think the additional compatibility is worth it.

Quoted 4 lines

And the main reason: this compatibility gives nothing. The only user of xfs ioctl which I've found is the xfsprogs. And these tools check filesystem name and don't work anywhere except 'xfs'.

The fact that you didn't find any other user doesn't mean there isn't
any. And historically if we though "nobody could be relying on this", we were sometimes proven wrong - a few years later when it was *much* more painful to make things compatible.

Here we speak about new feature for the filesystem so compatibility requirements aren't that strong but still I find the case that the same ioctl will just work on ext4 as it does on xfs more appealing than creating a new simpler generic ioctl. That way userspace doesn't have to care on which filesystem it is working or whether the current kernel already supports the new ioctl for XFS. So please use the XFS interface is Li Xi does in his patches.

Honza

Quoted 50 lines

Links
-----

[1] 2014-12-09 ext4: add project quota support by Li Xi
http://marc.info/?l=linux-fsdevel&m=141810265603565&w=2

[2] 2014-01-28 A draft for making ext4 support project quota by Zheng Liu
http://marc.info/?l=linux-ext4&m=139089109605795&w=2

[3] 2012-07-09 introduce extended inode owner identifier v10 by Dmitry Monakhov
http://thread.gmane.org/gmane.linux.file-systems/65752

[4] 2010-02-08 Introduce subtree quota support by Dmitry Monakhov
http://thread.gmane.org/gmane.comp.file-systems.ext4/17530

---

Konstantin Khlebnikov (6):
fs: vfs ioctls for managing project id
fs: protected project id
quota: generic project quota
ext4: support project id and project quota
ext4: add shortcut for moving files across projects
ext4: mangle statfs results accourding to project quota usage and limits

 Documentation/filesystems/Locking |    4 +
 Documentation/filesystems/vfs.txt |    8 +++
 Documentation/sysctl/fs.txt       |   16 ++++++
 fs/compat_ioctl.c                 |    2 +
 fs/ext4/ext4.h                    |   15 ++++-
 fs/ext4/ialloc.c                  |    3 +
 fs/ext4/inode.c                   |   15 +++++
 fs/ext4/namei.c                   |  102 ++++++++++++++++++++++++++++++++++++-
 fs/ext4/super.c                   |   61 ++++++++++++++++++++--
 fs/ioctl.c                        |   62 ++++++++++++++++++++++
 fs/quota/dquot.c                  |   96 +++++++++++++++++++++++++++++++++--
 fs/quota/quota.c                  |    8 ++-
 fs/quota/quotaio_v2.h             |    6 +-
 include/linux/fs.h                |    3 +
 include/linux/quota.h             |    1
 include/linux/quotaops.h          |   16 ++++++
 include/uapi/linux/capability.h   |    1
 include/uapi/linux/fs.h           |    3 +
 include/uapi/linux/quota.h        |    6 +-
 kernel/sysctl.c                   |    9 +++
 kernel/user_namespace.c           |    4 +
 21 files changed, 416 insertions(+), 25 deletions(-)

Quoted 2 lines

--
Signature

--
Jan Kara <[email protected]>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 6 lines · Jan Kara

Quoted 6 lines

Why new ioctls?
---------------

XFS has it's own interface for that: XFS_IOC_FSGETXATTR / XFS_IOC_FSSETXATTR. But it has several flaws and doesn't fit for a role of generic interface.

Quoted 6 lines

Quoted 3 lines

It contains a lot of xfs-specific flags and racy by design: set operation commits all fields at once thus it's used in sequence get-change-set without any lock, Concurrent updates from user space will collide.

Sure but that's the case for generic GETFLAGS / SETFLAGS interface as
well.

In this case we care only about project id. We can make interface clear, non-racy and do it in just one syscall instead of two.

Quoted 9 lines

Quoted 5 lines

Also xfs has flag XFS_XFLAG_PROJINHERIT which tells should new files inherit project id from parent directory or not. This flag is barely useful and only makes everything complicated. Even tools in xfsprogs don't use it: they always set it together with project id and clears when set project id back to zero.

I think this is about balance - adding the support for the PROJINHERIT flag
costs us close to nothing and we get a compatibility with XFS. So even though the usefulness of that flag is doubtful, I think the additional compatibility is worth it.

There're only few free bits left. This bit gives nothing, xfsprogs can change it but project quota management doesn't uses it. It gives nothing but complication.

Also this bit makes difference only for accounting directories itself and it's internal structures. This is pretty fs-specific thing.

Quoted 9 lines

Quoted 4 lines

And the main reason: this compatibility gives nothing. The only user of xfs ioctl which I've found is the xfsprogs. And these tools check filesystem name and don't work anywhere except 'xfs'.

The fact that you didn't find any other user doesn't mean there isn't
any. And historically if we though "nobody could be relying on this", we were sometimes proven wrong - a few years later when it was *much* more painful to make things compatible.

Quoted 7 lines

Here we speak about new feature for the filesystem so compatibility requirements aren't that strong but still I find the case that the same ioctl will just work on ext4 as it does on xfs more appealing than creating a new simpler generic ioctl. That way userspace doesn't have to care on which filesystem it is working or whether the current kernel already supports the new ioctl for XFS. So please use the XFS interface is Li Xi does in his patches.

Please consider this as _new_ feature which has compatible behaviour from user's point of view. That's perfect time to fix its birth defects and redesign behavior to satisfy expectations of new users. I do not care about migration from xfs to ext4, that's not my case. I do not care about mythical closed source software which might use this interface.

As I already said ioctl compatibility gives nothing but legacy problems. Unexpected apperance of partial compatibility might be a nightmare. And I've seen a lot when I worked with mixed containerized evironments. it emerges untested and unexpected combinations of old and new software. But, of course not in this case because nobody uses this interface except xfsprogs. And as I said without update it works only with XFS.

Also that's ioctl is so poorly designed for generic usage so resulting code looks like a shit. I just cannot add this into the kernel without any good reason.

I've implemented interface and behaviour xfs-compatible enough to add support of that into xfsprogs/xfstests in few lines. That's my next step after polishing support in quota-tools.