On Tue 09-12-14 13:22:25, Li Xi wrote:
This patch adds a new internal field of ext4 inode to save project
identifier. Also a new flag EXT4_INODE_PROJINHERIT is added for
inheriting project ID from parent directory.
I have noticed one thing you apparently changed in v7 of the patch set.
See below.
quoted hunk
Signed-off-by: Li Xi <redacted>
Reviewed-by: Jan Kara <redacted>
---
fs/ext4/ext4.h | 21 +++++++++++++++++----
fs/ext4/ialloc.c | 6 ++++++
fs/ext4/inode.c | 29 +++++++++++++++++++++++++++++
fs/ext4/namei.c | 17 +++++++++++++++++
fs/ext4/super.c | 1 +
include/uapi/linux/fs.h | 1 +
6 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 29c43e7..8bd1da9 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -377,16 +377,18 @@ struct flex_groups {
#define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
#define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
#define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
+#define EXT4_PROJINHERIT_FL FS_PROJINHERIT_FL /* Create with parents projid */
How did FS_PROJINHERIT_FL get here? There used to be 0x20000000 in older
version of the patch set which is correct - this definition is defining
ext4 on-disk format. As such it is an ext4 specific flag and should be
definined to a fixed constant independed of any other filesystem. It seems
you are somewhat mixing what is an on-disk format flag value and what is a
flag value passed from userspace. These two may be different things and you
need to convert between the values when getting / setting flags...
Honza
--
Jan Kara [off-list ref]
SUSE Labs, CR
On Jan 8, 2015, at 1:26 AM, Jan Kara [off-list ref] wrote:
On Tue 09-12-14 13:22:25, Li Xi wrote:
quoted
This patch adds a new internal field of ext4 inode to save project
identifier. Also a new flag EXT4_INODE_PROJINHERIT is added for
inheriting project ID from parent directory.
I have noticed one thing you apparently changed in v7 of the patch set.
See below.
quoted
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 29c43e7..8bd1da9 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -377,16 +377,18 @@ struct flex_groups {#define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
#define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
#define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
+#define EXT4_PROJINHERIT_FL FS_PROJINHERIT_FL /* Create with parents projid */
How did FS_PROJINHERIT_FL get here? There used to be 0x20000000 in older
version of the patch set which is correct - this definition is defining
ext4 on-disk format. As such it is an ext4 specific flag and should be
definined to a fixed constant independed of any other filesystem. It seems
you are somewhat mixing what is an on-disk format flag value and what is a
flag value passed from userspace. These two may be different things and
you need to convert between the values when getting / setting flags...
Currently the EXT4_*_FL and FS_*_FL values are all identical, and there
is no reason to change that before it is actually needed. Since the
FS_PROJINHERIT_FL is used via chattr/lsattr from userspace, this value
must also be kept the same in the future to avoid API breakage, so there
is no reason to worry about incompatibilities.
See also the [v8 5/5] patch, which is changing the EXT4_*_FL values to
use FS_*_FL constants, where applicable, so that it is more clear that
these values need to be the same.
Cheers, Andreas
On Thu 08-01-15 15:20:21, Andreas Dilger wrote:
On Jan 8, 2015, at 1:26 AM, Jan Kara [off-list ref] wrote:
quoted
On Tue 09-12-14 13:22:25, Li Xi wrote:
quoted
This patch adds a new internal field of ext4 inode to save project
identifier. Also a new flag EXT4_INODE_PROJINHERIT is added for
inheriting project ID from parent directory.
I have noticed one thing you apparently changed in v7 of the patch set.
See below.
quoted
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 29c43e7..8bd1da9 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -377,16 +377,18 @@ struct flex_groups {#define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
#define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
#define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
+#define EXT4_PROJINHERIT_FL FS_PROJINHERIT_FL /* Create with parents projid */
How did FS_PROJINHERIT_FL get here? There used to be 0x20000000 in older
version of the patch set which is correct - this definition is defining
ext4 on-disk format. As such it is an ext4 specific flag and should be
definined to a fixed constant independed of any other filesystem. It seems
you are somewhat mixing what is an on-disk format flag value and what is a
flag value passed from userspace. These two may be different things and
you need to convert between the values when getting / setting flags...
Currently the EXT4_*_FL and FS_*_FL values are all identical, and there
is no reason to change that before it is actually needed. Since the
FS_PROJINHERIT_FL is used via chattr/lsattr from userspace, this value
must also be kept the same in the future to avoid API breakage, so there
is no reason to worry about incompatibilities.
Agreed. I was somewhat worried about having on-disk flag defined through
the external non-ext4 define but you are right that neither can really
change once we ship a kernel with it.
See also the [v8 5/5] patch, which is changing the EXT4_*_FL values to
use FS_*_FL constants, where applicable, so that it is more clear that
these values need to be the same.
OK, I've missed that. So if things will be consistent again, I'm fine
with the change.
Honza
--
Jan Kara [off-list ref]
SUSE Labs, CR