[Ocfs2-tools-devel] [PATCH 6/9] ocfs2-tools: add xattr support in lib

[Tiger Yang]

Hi, Joel,
thanks for quick review.

Joel Becker wrote:
> 	Do we really need swap_header/entries in ocfs2.h?  Why doesn't
> swap_block work for all callers.  I see in other code you call them for
> swapping buckets, but it would seem that we could have an API for
> swapping a bucket.
> 	Maybe this belongs with a later bucket discussion, though.  Not
> a show-stopper for inclusion.

Swap_block doesn't work when xattr in inode or bucket, because them 
doesn't start with struct ocfs2_xattr_block, them just start with 
ocfs2_xattr_header.
I am going to introduce a function called ocfs2_swap_xattrs_to/from_cpu 
(struct ocfs2_xattr_header *xh) which swap xattrs in inode or bucket.
Then I can remove swap_header/entries from ocfs2.h.

  > 	This is ocfs2_xattr_recs_per_xb() in ocfs2_fs.h.  No need to
> duplicate.

OK. will remove it in next version.

> 
>> +static void ocfs2_swap_xattr_entry(struct ocfs2_xattr_entry *xe)
>> +{
>> +	xe->xe_name_offset	= bswap_16(xe->xe_name_offset);
>> +	xe->xe_value_size	= bswap_64(xe->xe_value_size);
>> +}
> 
> 	Why don't we swap xe_name_hash?  Is that always calculated and
> compared in little-endian?
Sorry, miss it.

> 
>> +static void ocfs2_swap_block_check(struct ocfs2_block_check *xk)
>> +{
>> +	xk->bc_crc32e		= bswap_32(xk->bc_crc32e);
>> +	xk->bc_ecc		= bswap_16(xk->bc_ecc);
>> +}
> 
> 	Don't swap the block_check structure.  Only the ecc code touches
> it.
OK. got it.

> 
> 	If these patches go in after the metaecc branch, they need to
> compute and validate eccs.  Thus, the validate_meta_ecc call needs to be
> before the signature check here.
> 	And compute_meta_ecc() needs to be before io_write_block() here.
> 	Also, with no bucket API, there is no validation of the bucket
> ecc.
OK. I see.

Thanks,
tiger