From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adnan Ali Date: Thu, 14 Mar 2013 11:42:19 +0000 Subject: [U-Boot] [PATCH v6] Introduced btrfs file-system with btrload command In-Reply-To: References: <1363175303-6230-1-git-send-email-adnan.ali@codethink.co.uk> <1363175303-6230-2-git-send-email-adnan.ali@codethink.co.uk> Message-ID: <5141B79B.30500@codethink.co.uk> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi On 13/03/13 22:03, Simon Glass wrote: > Hi Adnan, > > On Wed, Mar 13, 2013 at 4:48 AM, 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. >> >> v6: patch re-formated. >> v5: merged with master. >> v4: btrls command added. >> v3: patch re-formated. >> v2: patch error removed. >> >> Signed-off-by: Adnan Ali > Sorry there are just a few more nits here, otherwise it looks fine to me. Thanks for reply, below are my comments > >> --- >> Makefile | 1 + >> common/Makefile | 1 + >> common/cmd_btr.c | 65 +++ >> fs/btrfs/Makefile | 51 ++ >> fs/btrfs/btrfs.c | 1355 ++++++++++++++++++++++++++++++++++++++++++++ >> fs/btrfs/crc32_c.c | 40 ++ >> fs/fs.c | 10 + >> include/btrfs.h | 416 ++++++++++++++ >> include/config_fallbacks.h | 4 + >> include/fs.h | 1 + >> 10 files changed, 1944 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 >> > [...] >> diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c >> new file mode 100644 >> index 0000000..19db3a3 >> --- /dev/null >> +++ b/fs/btrfs/btrfs.c >> @@ -0,0 +1,1355 @@ >> +/* >> + * (C) Copyright 2013 Codethink Limited >> + * Btrfs port to Uboot by >> + * Adnan Ali >> + >> + * btrfs.c -- readonly btrfs support for syslinux >> + * Some data structures are derivated from btrfs-tools-0.19 ctree.h >> + * Copyright 2009 Intel Corporation; author: alek.du at intel.com >> + * >> + * 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, Inc., 53 Temple Place Ste 330, >> + * Boston MA 02111-1307, USA; either version 2 of the License, or >> + * (at your option) any later version; incorporated herein by reference. >> + * >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +unsigned long btr_part_offset; >> +static u16 build_crc_table=1; > Better as u8 I think, or just char (since I don't think we have our > bool type yet). Also if you change it around to 'crc_table_built' then > it can start as 0 (i.e. you don't need to assign it) and this avoid > using the data section. Ack (Even though that looks fine) > > Note also that you have some lines ending with space, including this one. Ack i will to remove spaces from top where variables are declared before btrfs code is this fine with you. > >> +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"); > Here I think you need two tabs. Ack > >> + >> + return -1; >> + } >> + >> + 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) >> + ; > I think this ; should be intented Sure this is. I think you missed my comments in previous mail where i mentioned that readdir() prints contents on media. It loops until the last item on media. This ; completes while loop format. I hope this answered your question. > >> + >> + return 0; >> +} >> + >> +/* >> + * umount btrfs file-system >> + */ >> +void btrfs_close(void) >> +{ >> +} >> diff --git a/fs/btrfs/crc32_c.c b/fs/btrfs/crc32_c.c >> new file mode 100644 >> index 0000000..b97c98a >> --- /dev/null >> +++ b/fs/btrfs/crc32_c.c > What do you think about moving this into lib/ ? I think i have to move it to lib/. Even though you told me to move it to fs/btrfs/ earlier mails. Ack Going to send these changes in v7. > >> @@ -0,0 +1,40 @@ >> +/* >> + * Copied from Linux kernel crypto/crc32c.c >> + * Copyright (c) 2004 Cisco Systems, Inc. >> + * Copyright (c) 2008 Herbert Xu >> + * >> + * 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. >> + * >> + */ > Regards, > Simon