[U-Boot] [PATCH v3] Introduced btrfs file-system with btrload command

[Adnan Ali]

On 25/02/13 15:04, Tom Rini wrote:
> On Mon, Feb 25, 2013 at 12:24:37PM +0000, Adnan Ali wrote:
>
>> Introduces btrfs file-system to read file
>> from volume/sub-volumes with btrload command. This
>> implementation has read-only support.
>> This btrfs implementation is based on syslinux btrfs
>> code, commit 269ebc845ebc8b46ef4b0be7fa0005c7fdb95b8d.
>>
>> Signed-off-by: Adnan Ali <adnan.ali at codethink.co.uk>
> A few things:
> - In general in fs/btrfs/btrfs.c I see some coding style problems (lack
>    of spacing, non-printf's longer than 80-wide).  Do these come from
>    syslinux and thus will make any re-syncs easier?
      Most of the ported code is unmodified so its coding style should
    be same as syslinux.
> - It looks like you added support for CONFIG_CMD_FS_GENERIC, if so did
>    you test that?
      This command wasn't enabled in my configs. I haven't added
  any command i.e btrls for this as this feature is not supported yet.

> - Can you please enable this support code on at least one platform,
>    preferably the one you've tested and developed with?
Even if do enable support for this, it will also debug
'Unsupported filesystem type.'

For the rest of changes you proposed i will change them and
send as v4 patch.



>
> [snip]
>> diff --git a/fs/fs.c b/fs/fs.c
> [snip]
>> +        //file handle is valid get the size of the file
> /* Style comments only */
>
>> +        len=filedata.size;
> And spacing between variable and assignment.
>
>> @@ -178,7 +248,6 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype)
>>   	for (i = 0; i < ARRAY_SIZE(fstypes); i++) {
>>   		if ((fstype != FS_TYPE_ANY) && (fstype != fstypes[i].fstype))
>>   			continue;
>> -
>>   		if (!fstypes[i].probe()) {
>>   			fs_type = fstypes[i].fstype;
>>   			return 0;
> [snip]
>> @@ -208,7 +280,6 @@ static void fs_close(void)
>>   int fs_ls(const char *dirname)
>>   {
>>   	int ret;
>> -
>>   	switch (fs_type) {
>>   	case FS_TYPE_FAT:
>>   		ret = fs_ls_fat(dirname);
> Unrelated, please drop.
>
>> @@ -237,11 +311,13 @@ int fs_read(const char *filename, ulong addr, int offset, int len)
>>   	case FS_TYPE_EXT:
>>   		ret = fs_read_ext(filename, addr, offset, len);
>>   		break;
>> +        case FS_TYPE_BTR:
>> +		ret = fs_read_btr(filename, addr, offset, len);
>> +		break;
>>   	default:
>>   		ret = fs_read_unsupported(filename, addr, offset, len);
>>   		break;
>>   	}
>> -
>>   	fs_close();
> And unrelated whitespace changes here as well.
>
>> diff --git a/include/btrfs.h b/include/btrfs.h
> [snip]
>> +/*
>> + * Extent structure: contains the mapping of some chunk of a file
>> + * that is contiguous on disk.
>> + */
>> +struct extent {
>> +    //sector_t    pstart;         /* Physical start sector */
>> +    __le64   pstart;
> Fix please.
>
>> +/*
>> + * Our definition of "not whitespace"
>> + */
>> +static inline char not_whitespace(char c)
>> +{
>> +  return (unsigned char)c > ' ';
>> +}
> Can't you just use isspace from <linux/ctypes.h> ?
>
>> diff --git a/include/crc32c.h b/include/crc32c.h
>> new file mode 100644
>> index 0000000..d04916e
>> --- /dev/null
>> +++ b/include/crc32c.h
>> @@ -0,0 +1,48 @@
>> +/*
>> + * Copied from Linux kernel crypto/crc32c.c
>> + * Copyright (c) 2004 Cisco Systems, Inc.
>> + * Copyright (c) 2008 Herbert Xu <herbert at gondor.apana.org.au>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License as published by the Free
>> + * Software Foundation; either version 2 of the License, or (at your option)
>> + * any later version.
>> + *
>> + */
>> +
>> +/*
>> + * This is the CRC-32C table
>> + * Generated with:
>> + * width = 32 bits
>> + * poly = 0x1EDC6F41
>> + * reflect input bytes = true
>> + * reflect output bytes = true
>> + */
>> +
>> +/*
>> + * Steps through buffer one byte at at time, calculates reflected
>> + * crc using table.
>> + */
>> +
>> +static inline u32 crc32c_cal(u32 crc, const char *data, size_t length, u32 *crc32c_table)
>> +{
>> +	while (length--)
>> +		crc = crc32c_table[(u8)(crc ^ *data++)] ^ (crc >> 8);
>> +
>> +	return crc;
>> +}
>> +
>> +static inline void crc32c_init(u32 *crc32c_table, u32 pol)
>> +{
>> +	int i, j;
>> +	u32 v;
>> +	const u32 poly = pol; /* Bit-reflected CRC32C polynomial */
>> +
>> +	for (i = 0; i < 256; i++) {
>> +		v = i;
>> +		for (j = 0; j < 8; j++) {
>> +			v = (v >> 1) ^ ((v & 1) ? poly : 0);
>> +		}
>> +		crc32c_table[i] = v;
>> +	}
>> +}
> Simon, since you've just been over all the crc32 recently, do we have
> these functions somewhere already that can be easily tapped in to?
> Thanks!
>
Thanks

Adnan