diff mbox

[06/36,v4] libext2fs: add data structures for inline data feature

Message ID 1343735309-30579-7-git-send-email-wenqing.lz@taobao.com
State Superseded, archived
Headers show

Commit Message

Zheng Liu July 31, 2012, 11:47 a.m. UTC 
From: Zheng Liu <wenqing.lz@taobao.com>

Add ext2_ext_attr_ibody_heaer to check extend attribute.  Add inline_data
to indicate the position of inline data in extend attribute and the size
of inline data.

Signed-off-by: Zheng Liu <wenqing.lz@taobao.com>
---
 lib/ext2fs/ext2_ext_attr.h |    4 ++++
 lib/ext2fs/ext2_fs.h       |    7 +++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

Comments

Theodore Ts'o Aug. 7, 2012, 6:50 p.m. UTC | #1 
On Tue, Jul 31, 2012 at 07:47:59PM +0800, Zheng Liu wrote:
> +struct ext2_ext_attr_ibody_header {
> +	__u32	h_magic;
> +};
> +

I've searched through the entire patch series, and I don't find any
usage of h_magic, and in fact the only place this structure definition
is used is in ext2fs_get_inline_xattr_pos().

So that's a bit worrying; if this is a magic number, then it should be
checked (and an error returned if the magic number is not what we
expect it tobe).  Add checks into e2fsck would also be a really good
idea.  Also, what is the value that h_magic is epxected to be?  It
needs to be defined here.

It's also clear from looking at this function that this patch
significantly changes the layout of the extended attribute block of
data.  It would be a really good idea to add some ascii art to
document exactly what is going on.  A diagram so it's obvious to
future developers about the data layout is really needed.

Regards,

						- 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
Zheng Liu Aug. 13, 2012, 8:10 a.m. UTC | #2 
On Tue, Aug 07, 2012 at 02:50:53PM -0400, Theodore Ts'o wrote:
> On Tue, Jul 31, 2012 at 07:47:59PM +0800, Zheng Liu wrote:
> > +struct ext2_ext_attr_ibody_header {
> > +	__u32	h_magic;
> > +};
> > +
> 
> I've searched through the entire patch series, and I don't find any
> usage of h_magic, and in fact the only place this structure definition
> is used is in ext2fs_get_inline_xattr_pos().
> 
> So that's a bit worrying; if this is a magic number, then it should be
> checked (and an error returned if the magic number is not what we
> expect it tobe).  Add checks into e2fsck would also be a really good
> idea.  Also, what is the value that h_magic is epxected to be?  It
> needs to be defined here.
> 
> It's also clear from looking at this function that this patch
> significantly changes the layout of the extended attribute block of
> data.  It would be a really good idea to add some ascii art to
> document exactly what is going on.  A diagram so it's obvious to
> future developers about the data layout is really needed.

Thanks, I will fix it.

Regards,
Zheng
--
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/lib/ext2fs/ext2_ext_attr.h b/lib/ext2fs/ext2_ext_attr.h
index bbb0aaa..5c7c715 100644
--- a/lib/ext2fs/ext2_ext_attr.h
+++ b/lib/ext2fs/ext2_ext_attr.h
@@ -25,6 +25,10 @@  struct ext2_ext_attr_header {
 	__u32	h_reserved[3];	/* zero right now */
 };
 
+struct ext2_ext_attr_ibody_header {
+	__u32	h_magic;
+};
+
 struct ext2_ext_attr_entry {
 	__u8	e_name_len;	/* length of name */
 	__u8	e_name_index;	/* attribute name index */
diff --git a/lib/ext2fs/ext2_fs.h b/lib/ext2fs/ext2_fs.h
index aa3e808..7e05183 100644
--- a/lib/ext2fs/ext2_fs.h
+++ b/lib/ext2fs/ext2_fs.h
@@ -904,4 +904,11 @@  struct mmp_struct {
  */
 #define EXT4_MMP_MIN_CHECK_INTERVAL     5
 
+struct inline_data {
+	__u16	inline_off;
+	__u16	inline_size;
+};
+
+#define EXT4_MIN_INLINE_DATA_SIZE	((sizeof(__u32) * EXT2_N_BLOCKS))
+
 #endif	/* _LINUX_EXT2_FS_H */