sandbox: block driver using host file/device as backing store

A simple "host" block driver using any host file/device as backing store.

The mapping is established using the "sb bind" command

Signed-off-by Henrik Nordstrom <[email protected]>

---
 common/cmd_sandbox.c       |  10 +++-
 disk/part.c                |  20 ++-----
 drivers/block/Makefile     |   1 +
 drivers/block/sandbox.c    | 130 +++++++++++++++++++++++++++++++++++++++++++++
 include/config_fallbacks.h |   3 +-
 include/configs/sandbox.h  |   2 +
 include/part.h             |   3 ++
 7 files changed, 150 insertions(+), 19 deletions(-)
 create mode 100644 drivers/block/sandbox.c

Some high-level comments:

Configuring for sandbox board...
sandbox.c: In function ?host_dev_bind?:

- should not have any warnings

=>sb bind
Segmentation fault (core dumped)

- should print a nice error if an arg is missing

=>sb bind fred
=>

- should print an error if the file is not found

- should create an example of how to use this

- suggest a test script in test/sandbox which sets up a loopback device containing a partition table and ext2 filesystem (for example, then runs U-Boot sandbox and lists and reads a file. Example test scripts you might copy are:

http://patchwork.ozlabs.org/patch/228876/

and this rather more complicated one:

http://patchwork.ozlabs.org/patch/211049/

Also please enable partitions and EFI support so we get more functionality:

#define CONFIG_PARTITION_UUIDS
#define CONFIG_CMD_PART
#define CONFIG_EFI_PARTITION

Version 2 of this patch addressing the code comments by Simon and adding some small missing pieces.

left on the to-do is documentation and adding test suite tests.

---
A simple "host" block driver using any host file/device as backing store.

Adds two sb subcommands for managing host device bindings:
sb bind <dev> [<filename>] - bind "host" device to file
sb info [<dev>] - show device binding & info

Signed-off-by Henrik Nordstrom <[email protected]>

---
 common/cmd_sandbox.c       |  54 +++++++++++++++++++
 disk/part.c                |  23 +++-----
 drivers/block/Makefile     |   1 +
 drivers/block/sandbox.c    | 127 +++++++++++++++++++++++++++++++++++++++++++++
 include/config_fallbacks.h |   3 +-
 include/configs/sandbox.h  |   6 +++
 include/part.h             |   3 ++
 include/sandboxblockdev.h  |  29 +++++++++++
 8 files changed, 228 insertions(+), 18 deletions(-)
 create mode 100644 drivers/block/sandbox.c
 create mode 100644 include/sandboxblockdev.h

common/cmd_sandbox.c +2 -0

index a28a844..59fbc97 100644
@@ -19,6 +19,8 @@
 
 #include <common.h>
 #include <fs.h>
+#include <part.h>
+#include <sandboxblockdev.h>
 
 static int do_sandbox_load(cmd_tbl_t *cmdtp, int flag, int argc,
                           char * const argv[])

@@ -38,10 +40,60 @@ static int do_sandbox_save(cmd_tbl_t *cmdtp, int flag, int argc,
        return do_save(cmdtp, flag, argc, argv, FS_TYPE_SANDBOX, 16);
 }

common/cmd_sandbox.c +10 -0

+               printf("%12lu %s\n", (unsigned long)blk_dev->lba,
+                      host_dev->filename);
+       }
+       return 0;
+}
+
 static cmd_tbl_t cmd_sandbox_sub[] = {
        U_BOOT_CMD_MKENT(load, 7, 0, do_sandbox_load, "", ""),
        U_BOOT_CMD_MKENT(ls, 3, 0, do_sandbox_ls, "", ""),
        U_BOOT_CMD_MKENT(save, 6, 0, do_sandbox_save, "", ""),
+       U_BOOT_CMD_MKENT(bind, 3, 0, do_sandbox_bind, "", ""),
+       U_BOOT_CMD_MKENT(info, 3, 0, do_sandbox_info, "", ""),
 };
 
 static int do_sandbox(cmd_tbl_t *cmdtp, int flag, int argc,

@@ -70,4 +122,6 @@ U_BOOT_CMD(
        "sb ls host <filename>                      - list files on host\n"
        "sb save host <dev> <filename> <addr> <bytes> [<offset>] - "
                "save a file to host\n"
+       "sb bind <dev> [<filename>] - bind \"host\" device to file\n"
+       "sb info [<dev>]            - show device binding & info"
 );

disk/part.c +6 -17

index d73625c..5906aef 100644
@@ -59,6 +59,9 @@ static const struct block_drvr block_drvr[] = {
 #if defined(CONFIG_SYSTEMACE)
        { .name = "ace", .get_dev = systemace_get_dev, },
 #endif
+#if defined(CONFIG_SANDBOX)
+       { .name = "host", .get_dev = host_get_dev, },
+#endif
        { },
 };
 

@@ -302,6 +305,9 @@ static void print_part_header (const char *type, block_dev_desc_t * dev_desc)
        case IF_TYPE_MMC:
                puts ("MMC");
                break;
+       case IF_TYPE_HOST:
+               puts("HOST");
+               break;
        default:
                puts ("UNKNOWN");
                break;

@@ -462,23 +468,6 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,
        int part;
        disk_partition_t tmpinfo;
 
-       /*
-        * For now, we have a special case for sandbox, since there is no
-        * real block device support.
-        */
-       if (0 == strcmp(ifname, "host")) {
-               *dev_desc = NULL;
-               info->start = info->size =  info->blksz = 0;
-               info->bootable = 0;
-               strcpy((char *)info->type, BOOT_PART_TYPE);
-               strcpy((char *)info->name, "Sandbox host");
-#ifdef CONFIG_PARTITION_UUIDS
-               info->uuid[0] = 0;
-#endif
-
-               return 0;
-       }
-
        /* If no dev_part_str, use bootdevice environment variable */
        if (!dev_part_str || !strlen(dev_part_str) ||
            !strcmp(dev_part_str, "-"))

drivers/block/Makefile +0 -0

index f1ebdcc..2d2fb55 100644

drivers/block/Makefile +0 -0

 COBJS  := $(COBJS-y)
 SRCS   := $(COBJS:.o=.c)

drivers/block/sandbox.c new file +18 -0

new file mode 100644
index 0000000..90f7dca
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 2013 Henrik Nordstrom <[email protected]>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software

drivers/block/sandbox.c new file +48 -0

+       return NULL;
+}
+
+static unsigned long host_block_read(int dev, unsigned long start,
+                                    lbaint_t blkcnt, void *buffer)
+{
+       struct host_block_dev *host_dev = find_host_device(dev);
+       if (os_lseek(host_dev->fd,
+                    start * host_dev->blk_dev.blksz,
+                    OS_SEEK_SET) == -1) {
+               printf("ERROR: Invalid position\n");
+               return -1;
+       }
+       ssize_t len = os_read(host_dev->fd, buffer,
+                             blkcnt * host_dev->blk_dev.blksz);
+       if (len >= 0)
+               return len / host_dev->blk_dev.blksz;
+       return -1;
+}
+
+static unsigned long host_block_write(int dev, unsigned long start,
+                                     lbaint_t blkcnt, const void *buffer)
+{
+       struct host_block_dev *host_dev = find_host_device(dev);
+       if (os_lseek(host_dev->fd,
+                    start * host_dev->blk_dev.blksz,
+                    OS_SEEK_SET) == -1) {
+               printf("ERROR: Invalid position\n");
+               return -1;
+       }
+       ssize_t len = os_write(host_dev->fd, buffer, blkcnt *
+                              host_dev->blk_dev.blksz);
+       if (len >= 0)
+               return len / host_dev->blk_dev.blksz;
+       return -1;
+}
+
+int host_dev_bind(int dev, char *filename)
+{
+       struct host_block_dev *host_dev = find_host_device(dev);
+       if (host_dev->blk_dev.priv) {
+               os_close(host_dev->fd);
+               host_dev->blk_dev.priv = NULL;
+       }
+       if (host_dev->filename)
+               free(host_dev->filename);
+       if (filename && *filename) {
+               host_dev->filename = strdup(filename);

drivers/block/sandbox.c new file +31 -0

+       }
+
+       block_dev_desc_t *blk_dev = &host_dev->blk_dev;
+       blk_dev->if_type = IF_TYPE_HOST;
+       blk_dev->priv = host_dev;
+       blk_dev->blksz = 512;
+       blk_dev->lba = os_lseek(host_dev->fd, 0, OS_SEEK_END) / blk_dev->blksz;
+       blk_dev->block_read = host_block_read;
+       blk_dev->block_write = host_block_write;
+       blk_dev->dev = dev;
+       blk_dev->part_type = PART_TYPE_UNKNOWN;
+       init_part(blk_dev);
+
+       return 0;
+}
+
+block_dev_desc_t *host_get_dev(int dev)
+{
+       struct host_block_dev *host_dev = find_host_device(dev);
+
+       if (!host_dev)
+               return NULL;
+
+       if (!host_dev->blk_dev.priv) {
+               printf("Not bound to a backing file\n");
+               return NULL;
+       }
+
+       block_dev_desc_t *blk_dev = &host_dev->blk_dev;
+       return blk_dev;
+}

include/config_fallbacks.h +2 -1

index e59ee96..9ea8c09 100644
@@ -49,7 +49,8 @@
        defined(CONFIG_CMD_USB) || \
        defined(CONFIG_CMD_PART) || \
        defined(CONFIG_MMC) || \
-       defined(CONFIG_SYSTEMACE)
+       defined(CONFIG_SYSTEMACE) || \
+       defined(CONFIG_SANDBOX)
 #define HAVE_BLOCK_DEVICE
 #endif
 

include/configs/sandbox.h +6 -0

index 788207d..844a49f 100644
@@ -38,6 +38,11 @@
 #define CONFIG_CMD_FAT
 #define CONFIG_CMD_EXT4
 #define CONFIG_CMD_EXT4_WRITE
+#define CONFIG_CMD_PART
+#define CONFIG_DOS_PARTITION
+#define CONFIG_PARTITION_UUIDS
+#define CONFIG_EFI_PARTITION
+#define CONFIG_HOST_MAX_DEVICES 4
 
 #define CONFIG_SYS_VSNPRINTF
 

@@ -45,6 +50,7 @@
 #define CONFIG_SANDBOX_GPIO
 #define CONFIG_SANDBOX_GPIO_COUNT      20
 
+
 /*
  * Size of malloc() pool, although we don't actually use this yet.
  */

include/part.h +3 -0

index f7c7cc5..a29d615 100644
@@ -74,6 +74,7 @@ typedef struct block_dev_desc {
 #define IF_TYPE_MMC            6
 #define IF_TYPE_SD             7
 #define IF_TYPE_SATA           8
+#define IF_TYPE_HOST           9
 
 /* Part types */
 #define PART_TYPE_UNKNOWN      0x00

@@ -118,6 +119,7 @@ block_dev_desc_t* usb_stor_get_dev(int dev);
 block_dev_desc_t* mmc_get_dev(int dev);
 block_dev_desc_t* systemace_get_dev(int dev);
 block_dev_desc_t* mg_disk_get_dev(int dev);
+block_dev_desc_t *host_get_dev(int dev);
 
 /* disk/part.c */
 int get_partition_info (block_dev_desc_t * dev_desc, int part, disk_partition_t *info);

@@ -139,6 +141,7 @@ static inline block_dev_desc_t* usb_stor_get_dev(int dev) { return NULL; }
 static inline block_dev_desc_t* mmc_get_dev(int dev) { return NULL; }
 static inline block_dev_desc_t* systemace_get_dev(int dev) { return NULL; }
 static inline block_dev_desc_t* mg_disk_get_dev(int dev) { return NULL; }
+static inline block_dev_desc_t *host_get_dev(int dev) { return NULL; }
 
 static inline int get_partition_info (block_dev_desc_t * dev_desc, int part,
        disk_partition_t *info) { return -1; }

include/sandboxblockdev.h new file +17 -0

new file mode 100644
index 0000000..8723062
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2013, Henrik Nordstrom <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+

include/sandboxblockdev.h new file +2 -1

+
+#endif
--

1.8.1.4

Hi Henrik,

On Wed, May 15, 2013 at 2:54 PM, Henrik Nordstr?m
<[email protected]> wrote:

Quoted 2 lines

Version 2 of this patch addressing the code comments by Simon and adding some small missing pieces.

Looks good.

For change log, you should follow the standard approach - also instead of 'comments by Simon' it is good to list the things you changed. You might find patman useful for preparing and sending patches.

blank line here, please fix globally (a blank line between declarations and code).

common/cmd_sandbox.c static int do_sandbox_save(cmd_tbl_t *cmdtp, int flag, int argc, · line 63

Quoted 5 lines

common/cmd_sandbox.c +5 -0

+static int do_sandbox_info(cmd_tbl_t *cmdtp, int flag, int argc,
+                          char * const argv[])
+{
+       if (argc < 1 || argc > 2)
+               return CMD_RET_USAGE;

Probably best to put this after the declarations

Move to top of function. Try to collect your declarations within a block when it's easy to do so.

common/cmd_sandbox.c static int do_sandbox_save(cmd_tbl_t *cmdtp, int flag, int argc, · line 83

Quoted 6 lines

common/cmd_sandbox.c +6 -0

+       printf("%3s %12s %s\n", "dev", "blocks", "path");
+       for (dev = min_dev; dev <= max_dev; dev++) {
+               printf("%3d ", dev);
+               block_dev_desc_t *blk_dev = host_get_dev(dev);
+               if (!blk_dev)
+                       continue;

Does this case ever happen? If so you don't print \n.

Quoted 1 line

+ struct host_block_dev *host_dev = blk_dev->priv;

Maybe leave the assignment here but put the declaration at the start of the block.

Quoted 4 lines

COBJS-$(CONFIG_IDE_SIL680) += sil680.o
COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o
COBJS-$(CONFIG_SYSTEMACE) += systemace.o
+COBJS-$(CONFIG_SANDBOX) += sandbox.o

Alphabetic order so this should go up two.

Quoted 1 line

+#include <sandboxblockdev.h>

How about sandbox-blockdev.h

puts() when you don't have format args. Please fix globally.

drivers/block/sandbox.c line 96

Quoted 4 lines

drivers/block/sandbox.c new file +4 -0

+       if (host_dev->fd == -1) {
+               printf("Failed to access host backing file '%s'\n",
+                      host_dev->filename);
+               return 1;

-ENOENT might be better (include errno.h)

Quoted 1 line

+int host_dev_bind(int dev, char *filename);

Please add a comment as to what this function does, describing the meaning and valid values for each argument.

=>ext4load host 0:3
Segmentation fault (core dumped)

ons 2013-05-15 klockan 15:20 -0700 skrev Simon Glass:

Quoted 4 lines

Hi Henrik,

On Wed, May 15, 2013 at 2:54 PM, Henrik Nordstr?m
<[email protected]> wrote:

Quoted 8 lines

Quoted 2 lines

Version 2 of this patch addressing the code comments by Simon and adding some small missing pieces.

Looks good.

For change log, you should follow the standard approach - also instead of 'comments by Simon' it is good to list the things you changed. You might find patman useful for preparing and sending patches.

Just looked into it and looks nice. Giving it a try for next version.

Took a little while to get started, mostly because it crashes & burns when not finding an matching alias for sandbox.

Quoted 2 lines

blank line here, please fix globally (a blank line between declarations and code).

Ok.

Quoted 7 lines

common/cmd_sandbox.c static int do_sandbox_save(cmd_tbl_t *cmdtp, int flag, int argc, · line 63

Quoted 5 lines

common/cmd_sandbox.c +5 -0

+static int do_sandbox_info(cmd_tbl_t *cmdtp, int flag, int argc,
+                          char * const argv[])
+{
+       if (argc < 1 || argc > 2)
+               return CMD_RET_USAGE;

Probably best to put this after the declarations

Ok. Also restrucured do_sandbox_bind a little to match this. There one of the declarations depends on this being checked first.

Quoted 2 lines

Move to top of function. Try to collect your declarations within a block when it's easy to do so.

Ok.

Quoted 8 lines

common/cmd_sandbox.c static int do_sandbox_save(cmd_tbl_t *cmdtp, int flag, int argc, · line 83

Quoted 6 lines

common/cmd_sandbox.c +6 -0

+       printf("%3s %12s %s\n", "dev", "blocks", "path");
+       for (dev = min_dev; dev <= max_dev; dev++) {
+               printf("%3d ", dev);
+               block_dev_desc_t *blk_dev = host_get_dev(dev);
+               if (!blk_dev)
+                       continue;

Does this case ever happen? If so you don't print \n.

Yes. And it then relies on the driver to print an error.

Quoted 4 lines

Quoted 1 line

+ struct host_block_dev *host_dev = blk_dev->priv;

Maybe leave the assignment here but put the declaration at the start of the block.

Yes.

Quoted 6 lines

Quoted 4 lines

COBJS-$(CONFIG_IDE_SIL680) += sil680.o
COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o
COBJS-$(CONFIG_SYSTEMACE) += systemace.o
+COBJS-$(CONFIG_SANDBOX) += sandbox.o

Alphabetic order so this should go up two.

up seven. sorted on filename here not CONFIG_..

Quoted 3 lines

Quoted 1 line

+#include <sandboxblockdev.h>

How about sandbox-blockdev.h

Yee.

Quoted 1 line

puts() when you don't have format args. Please fix globally.

Ok.

Quoted 6 lines

drivers/block/sandbox.c line 96

Quoted 4 lines

drivers/block/sandbox.c new file +4 -0

+       if (host_dev->fd == -1) {
+               printf("Failed to access host backing file '%s'\n",
+                      host_dev->filename);
+               return 1;

-ENOENT might be better (include errno.h)

or maybe just -errno?

and handle the error in do_sandbox_bind().

Quoted 4 lines

Quoted 1 line

+int host_dev_bind(int dev, char *filename);

Please add a comment as to what this function does, describing the meaning and valid values for each argument.

Ok.

Quoted 2 lines

=>ext4load host 0:3
Segmentation fault (core dumped)

Doesn't ext2load expect a address & filename to load?

Regads
Henrik

Sorry I meant:

=>ext4ls host 0:3
Segmentation fault (core dumped)

It may not be your code, but I think the segfault is there.

tor 2013-05-16 klockan 21:53 -0700 skrev Simon Glass:

Quoted 6 lines

Sorry I meant:

=>ext4ls host 0:3
Segmentation fault (core dumped)

It may not be your code, but I think the segfault is there.

It's crashing in ext4 for me.

#0 ext4fs_set_blk_dev (rbdd=0x6434a0 <host_devices>,

    info=info at entry=0x643980 <fs_partition>) at dev.c:56

#0 ext4fs_set_blk_dev (rbdd=0x6434a0 <host_devices>,

    info=info at entry=0x643980 <fs_partition>) at dev.c:56

56 get_fs()->total_sect = (info->size * info->blksz) >>
57 get_fs()->dev_desc->log2blksz;
58 get_fs()->dev_desc = rbdd;

p(gdb) p get_fs()
$2 = (struct ext_filesystem *) 0x644410 <ext_fs>
(gdb) p $2->dev_desc
$5 = (block_dev_desc_t *) 0x0

Looks like a generic ext4 bug in the new block size handling code.

I think it just needs to be shuffled around a bit so the dev_desc is assigned before, not after.

Regards
Henrik

Hi Henrik,

On Wed, May 15, 2013 at 6:09 PM, Henrik Nordstr?m
<[email protected]> wrote:

Quoted 6 lines

ons 2013-05-15 klockan 15:20 -0700 skrev Simon Glass:

Quoted 5 lines

Hi Henrik,

Quoted 1 line · Henrik Nordstr?m

Version 2 of this patch addressing the code comments by Simon and

adding

Quoted 10 lines

Quoted 7 lines

Quoted 1 line

some small missing pieces.

Looks good.

For change log, you should follow the standard approach - also instead of 'comments by Simon' it is good to list the things you changed. You might find patman useful for preparing and sending patches.

Just looked into it and looks nice. Giving it a try for next version.

Did you send a new version? I might have missed it.

s?n 2013-06-16 klockan 07:50 -0700 skrev Simon Glass:

Quoted 1 line

Did you send a new version? I might have missed it.

Not yet. Work got in a bit of a panic mode and then been ill for a while. Not forgotten.