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

Adnan Ali
2013-02-25T16:08:21Z

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@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@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