diff mbox

ext4: Reserve FEATURE_RO_COMPAT and INO flag

Message ID 1366900760-18899-1-git-send-email-lczerner@redhat.com
State New, archived
Headers show

Commit Message

Lukas Czerner April 25, 2013, 2:39 p.m. UTC 
EXT4_FEATURE_RO_COMPAT_REPLICA and EXT4_REPLICA_INO are reserved in
e2fsprogs for non-upstream feature. However it has never been reserved
in kernel, fix this. Thanks to Radek Pazdera to pointing it out.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 fs/ext4/ext4.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Andreas Dilger April 26, 2013, 11:47 a.m. UTC | #1 
I was looking in the maint and master branches, but I don't see these flags anywhere?

Cheers, Andreas

On 2013-04-25, at 8:39, Lukas Czerner <lczerner@redhat.com> wrote:

> EXT4_FEATURE_RO_COMPAT_REPLICA and EXT4_REPLICA_INO are reserved in
> e2fsprogs for non-upstream feature. However it has never been reserved
> in kernel, fix this. Thanks to Radek Pazdera to pointing it out.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
> fs/ext4/ext4.h |    3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 3b83cd6..6369c28 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -241,6 +241,7 @@ struct ext4_io_submit {
> #define EXT4_UNDEL_DIR_INO     6    /* Undelete directory inode */
> #define EXT4_RESIZE_INO         7    /* Reserved group descriptors inode */
> #define EXT4_JOURNAL_INO     8    /* Journal inode */
> +#define EXT4_REPLICA_INO     10    /* Used by non-upstream feature */
> 
> /* First non-reserved inode for old ext4 filesystems */
> #define EXT4_GOOD_OLD_FIRST_INO    11
> @@ -1481,6 +1482,8 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
>  * GDT_CSUM bits are mutually exclusive.
>  */
> #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM    0x0400
> +/* Reserved for non-upstream feature */
> +#define EXT4_FEATURE_RO_COMPAT_REPLICA        0x0800
> 
> #define EXT4_FEATURE_INCOMPAT_COMPRESSION    0x0001
> #define EXT4_FEATURE_INCOMPAT_FILETYPE        0x0002
> -- 
> 1.7.7.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukas Czerner April 29, 2013, 12:58 p.m. UTC | #2 
On Fri, 26 Apr 2013, Andreas Dilger wrote:

> Date: Fri, 26 Apr 2013 05:47:58 -0600
> From: Andreas Dilger <adilger@dilger.ca>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
>     Lukas Czerner <lczerner@redhat.com>
> Subject: Re: [PATCH] ext4: Reserve FEATURE_RO_COMPAT and INO flag
> 
> I was looking in the maint and master branches, but I don't see these flags anywhere?

That's odd. I am using git repository:

git://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git

and it is there added with the commit
991211f676f49c6cf30af368dade2f81287f1fa8 libext2fs, libe2p: Reserve
RO_COMPAT_REPLICA feature

-Lukas

> 
> Cheers, Andreas
> 
> On 2013-04-25, at 8:39, Lukas Czerner <lczerner@redhat.com> wrote:
> 
> > EXT4_FEATURE_RO_COMPAT_REPLICA and EXT4_REPLICA_INO are reserved in
> > e2fsprogs for non-upstream feature. However it has never been reserved
> > in kernel, fix this. Thanks to Radek Pazdera to pointing it out.
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > ---
> > fs/ext4/ext4.h |    3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index 3b83cd6..6369c28 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -241,6 +241,7 @@ struct ext4_io_submit {
> > #define EXT4_UNDEL_DIR_INO     6    /* Undelete directory inode */
> > #define EXT4_RESIZE_INO         7    /* Reserved group descriptors inode */
> > #define EXT4_JOURNAL_INO     8    /* Journal inode */
> > +#define EXT4_REPLICA_INO     10    /* Used by non-upstream feature */
> > 
> > /* First non-reserved inode for old ext4 filesystems */
> > #define EXT4_GOOD_OLD_FIRST_INO    11
> > @@ -1481,6 +1482,8 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
> >  * GDT_CSUM bits are mutually exclusive.
> >  */
> > #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM    0x0400
> > +/* Reserved for non-upstream feature */
> > +#define EXT4_FEATURE_RO_COMPAT_REPLICA        0x0800
> > 
> > #define EXT4_FEATURE_INCOMPAT_COMPRESSION    0x0001
> > #define EXT4_FEATURE_INCOMPAT_FILETYPE        0x0002
> > -- 
> > 1.7.7.6
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Dilger April 29, 2013, 6:35 p.m. UTC | #3 
On 2013-04-29, at 6:58 AM, Lukáš Czerner wrote:
> On Fri, 26 Apr 2013, Andreas Dilger wrote:
>> Date: Fri, 26 Apr 2013 05:47:58 -0600
>> From: Andreas Dilger <adilger@dilger.ca>
>> To: Lukas Czerner <lczerner@redhat.com>
>> Cc: "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
>>    Lukas Czerner <lczerner@redhat.com>
>> Subject: Re: [PATCH] ext4: Reserve FEATURE_RO_COMPAT and INO flag
>> 
>> I was looking in the maint and master branches, but I don't see these flags anywhere?
> 
> That's odd. I am using git repository:
> 
> git://git.kernel.org/pub/scm/fs/ext2/e2fsprogs.git
> 
> and it is there added with the commit
> 991211f676f49c6cf30af368dade2f81287f1fa8 libext2fs, libe2p: Reserve
> RO_COMPAT_REPLICA feature

Strange, I see it now.  I don't know what file I was looking at
that didn't have this included...

One real problem that I see is that EXT4_REPLICA_INO uses the
last reserved inode.  I recall in the past that Ted wanted to
make this last reserved inode a hidden directory, so that it
would be possible to put special inodes in there and access them
by name.  Not quite as easy as accessing a reserved inode by
number, but still preferable to not being able to have filesystem
internal inodes at all in the future.

Of the currently-reserved inodes, the only other one that might
be a potential candidate for re-use is EXT2_UNDEL_DIR_INO, which
has never been used in any upstream kernel (Googling shows there
was a patch developed by some students, but never released
AFAICS).  The BOOT_LOADED_INO has recently seen interest for use
by Grub, and real patches exist for the snapshot code .

That leaves us in a quandry - change the meaning of a reserved
inode which has been publicly available for many years but never
really used, or change the meaning of a private reserved inode
which is (presumably) in use privately, but will (presumably) never
be landed?

IMHO, since EXT2_UNDEL_DIR_INO was already supposed to be a
directory it might be reasonable to repurpose it to be an "internal
files" directory, possibly even linking the existing reserved
directories to it by name for consistency?  This concept is used
by ZFS to store all kinds of configuration data, mount parameters
and tunables, etc. and is very handy IMHO.

Cheers, Andreas

>> On 2013-04-25, at 8:39, Lukas Czerner <lczerner@redhat.com> wrote:
>> 
>>> EXT4_FEATURE_RO_COMPAT_REPLICA and EXT4_REPLICA_INO are reserved in
>>> e2fsprogs for non-upstream feature. However it has never been reserved
>>> in kernel, fix this. Thanks to Radek Pazdera to pointing it out.
>>> 
>>> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
>>> ---
>>> fs/ext4/ext4.h |    3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>> 
>>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>>> index 3b83cd6..6369c28 100644
>>> --- a/fs/ext4/ext4.h
>>> +++ b/fs/ext4/ext4.h
>>> @@ -241,6 +241,7 @@ struct ext4_io_submit {
>>> #define EXT4_UNDEL_DIR_INO     6    /* Undelete directory inode */
>>> #define EXT4_RESIZE_INO         7    /* Reserved group descriptors inode */
>>> #define EXT4_JOURNAL_INO     8    /* Journal inode */
>>> +#define EXT4_REPLICA_INO     10    /* Used by non-upstream feature */
>>> 
>>> /* First non-reserved inode for old ext4 filesystems */
>>> #define EXT4_GOOD_OLD_FIRST_INO    11
>>> @@ -1481,6 +1482,8 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
>>> * GDT_CSUM bits are mutually exclusive.
>>> */
>>> #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM    0x0400
>>> +/* Reserved for non-upstream feature */
>>> +#define EXT4_FEATURE_RO_COMPAT_REPLICA        0x0800
>>> 
>>> #define EXT4_FEATURE_INCOMPAT_COMPRESSION    0x0001
>>> #define EXT4_FEATURE_INCOMPAT_FILETYPE        0x0002
>>> -- 
>>> 1.7.7.6
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 


Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o May 3, 2013, 6:53 a.m. UTC | #4 
On Mon, Apr 29, 2013 at 12:35:18PM -0600, Andreas Dilger wrote:
> 
> One real problem that I see is that EXT4_REPLICA_INO uses the
> last reserved inode.  I recall in the past that Ted wanted to
> make this last reserved inode a hidden directory, so that it
> would be possible to put special inodes in there and access them
> by name.  Not quite as easy as accessing a reserved inode by
> number, but still preferable to not being able to have filesystem
> internal inodes at all in the future.

That wasn't my idea, but it's not a bad one.  I'm not too worried
about running out of file system internal inodes, though.  There are
two main benefits of having special inode numbers.  First, it's a low
numbered inode, so it's easy to recognize.  Second, it's easier to add
a new file system feature to an existing file system.  But either way,
it's not impossible.

First of all, "the first non-reserved inode" is actually a superblock
field, so we could actually change that for newer file systems, and
hence effectively create new reserved inodes.  Secondly, it's simple
enough just to use a superblock field to designate a particular inode
as being special.  This is how we originally dealt with the journal
inode, after all.

Cheers,

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 3b83cd6..6369c28 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -241,6 +241,7 @@  struct ext4_io_submit {
 #define EXT4_UNDEL_DIR_INO	 6	/* Undelete directory inode */
 #define EXT4_RESIZE_INO		 7	/* Reserved group descriptors inode */
 #define EXT4_JOURNAL_INO	 8	/* Journal inode */
+#define EXT4_REPLICA_INO	 10	/* Used by non-upstream feature */
 
 /* First non-reserved inode for old ext4 filesystems */
 #define EXT4_GOOD_OLD_FIRST_INO	11
@@ -1481,6 +1482,8 @@  static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
  * GDT_CSUM bits are mutually exclusive.
  */
 #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM	0x0400
+/* Reserved for non-upstream feature */
+#define EXT4_FEATURE_RO_COMPAT_REPLICA		0x0800
 
 #define EXT4_FEATURE_INCOMPAT_COMPRESSION	0x0001
 #define EXT4_FEATURE_INCOMPAT_FILETYPE		0x0002