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

Adnan Ali adnan.ali at codethink.co.uk
Tue Mar 12 15:53:26 CET 2013


Hi

On 12/03/13 14:15, Simon Glass wrote:
> Hi Adnan,
>
> On Mon, Mar 11, 2013 at 6:18 AM, Adnan Ali <adnan.ali at codethink.co.uk> 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.
>>
>> v5:     merged with master.
>> v4:     btrls command added.
>> v3:     patch re-formated.
>> v2:     patch error removed.
>>
>> Signed-off-by: Adnan Ali <adnan.ali at codethink.co.uk>
>> ---
>>   Makefile                   |    1 +
>>   common/Makefile            |    1 +
>>   common/cmd_btr.c           |   65 +++
>>   fs/btrfs/Makefile          |   51 ++
>>   fs/btrfs/btrfs.c           | 1348 ++++++++++++++++++++++++++++++++++++++++++++
>>   fs/btrfs/crc32_c.c         |   54 ++
>>   fs/fs.c                    |   10 +
>>   include/btrfs.h            |  416 ++++++++++++++
>>   include/config_fallbacks.h |    4 +
>>   include/fs.h               |    1 +
>>   10 files changed, 1951 insertions(+)
>>   create mode 100644 common/cmd_btr.c
>>   create mode 100644 fs/btrfs/Makefile
>>   create mode 100644 fs/btrfs/btrfs.c
>>   create mode 100644 fs/btrfs/crc32_c.c
>>   create mode 100644 include/btrfs.h
>>
> I have ignored the code before this point in btrfs.c since it is
> imported and you want to keep the code style as is, but if you don't
> mind I will make a few comments after that point>
     Here are my comments
>
>> +struct btrfs_info fs;
>> +
>> +/*
>> + *  mount btrfs file-system
>> + */
>> +int btrfs_probe(block_dev_desc_t *rbdd, disk_partition_t *info)
>> +{
>> +        btrfs_block_dev_desc = rbdd;
>> +        part_info = info;
>> +        btr_part_offset = info->start;
>> +       if (btrfs_fs_init(&fs) < 0 ) {
>> +          debug("btrfs probe failed\n");
>> +          return -1;
>> +       }
> You should use tabs for intend (some of this bit uses spaces, some uses tabs).
    Some how i missed will check it again. Ack
>
>> +
>> +       return 0;
>> +}
>> +
>> +/*
>> + *  Read file data
>> + */
>> +int btrfs_read_file(const char *filename, void *buf, int offset, int len)
>> +{
>> +       int file_len=0;
>> +        int len_read;
>> +        struct com32_filedata filedata;
>> +        int handle;
>> +        if (offset != 0) {
>> +                debug("** Cannot support non-zero offset **\n");
>> +                return -1;
>> +        }
>> +
>> +        handle=btrfs_open_file(filename, &filedata);
>> +        if (handle < 0) {
>> +                debug("** File not found %s Invalid handle**\n", filename);
>> +                return -1;
>> +        }
>> +
>> +        /*file handle is valid get the size of the file*/
>> +        len = filedata.size;
>> +        if (len == 0)
>> +                len = file_len;
>> +
>> +        len_read = getfssec(&filedata, (char *)buf);
>> +        if (len_read != len) {
>> +                debug("** Unable to read file %s **\n", filename);
>> +                return -1;
>> +        }
>> +
>> +        return len_read;
>> +
>> +}
>> +
>> +/*
>> + * Show directory entries
>> + */
>> +char btrfs_ls(char *dirname)
>> +{
>> +  struct btrfs_dirent *de;
>> +  struct _DIR_ *dir;
>> +
>> +  if(*dirname == '/' && *(dirname+1) == 0)
>> +     *dirname = '.';
>> +
>> +  dir = opendir(dirname);
>> +  if (dir == NULL)
>> +        return -1;
>> +
>> +  while ((de = readdir(dir)) != NULL)
>> +        ;
> This doesn't seem to print anything.
      readdir->btrfs_read, prints contents on media.
>
>> +
>> +  return 0;
>> +}
>> +
>> +/*
>> + *  umount btrfs file-system
>> + */
>> +void btrfs_close(void)
>> +{
>> +
> Remove blank line
    Ack
>
>> +}
>> diff --git a/fs/btrfs/crc32_c.c b/fs/btrfs/crc32_c.c
>> new file mode 100644
>> index 0000000..78d0447
>> --- /dev/null
>> +++ b/fs/btrfs/crc32_c.c
>> @@ -0,0 +1,54 @@
>> +/*
>> + * 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
>> + */
> Old comment?
    this crc was part of port
>
>> +
>> +/*
>> + * Steps through buffer one byte at at time, calculates reflected
>> + * crc using table.
>> + */
>> +#include <linux/stat.h>
>> +#include <command.h>
>> +#include <asm/byteorder.h>
>> +#include <linux/compiler.h>
>> +#include <common.h>
>> +#include <config.h>
>> +
>> +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;
>> +}
>> +
>> +inline void crc32c_init(u32 *crc32c_table, u32 pol)
> The 'inline' isn't needed.
     Ack
>
>> +{
>> +       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++) {
> Can remove this inner {} since you only have one line of code.
      Ack
>> +                       v = (v >> 1) ^ ((v & 1) ? poly : 0);
>> +               }
>> +               crc32c_table[i] = v;
>> +       }
>> +}
> I suggest you move your crc_32.c file to lib/ since it might be more
> generally useful. You should probably include the function prototypes
> in include/crc.h.
   The crc32_c.c was part of port as header file and you previously said
to move to btrfs folder and make it c file.
> Also you seem to recalculate the crc table on every operation. Is is
> possible to only do this ones on startup?
   Yes every time prob is called its calculated. It can be calculated at
boot time. Can you point me to file and function that it can be called.
before that can we make decision on crc32_c.c.


Plus once i make all these changes will it be accepted than ;).
>
> Regards,
> Simon
Regards
Adnan



More information about the U-Boot mailing list