[U-Boot] [PATCH] sandbox: restore ability to access host fs through standard commands

[U-Boot] [PATCH] sandbox: restore ability to access host fs through standard commands

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Commit 95fac6ab4589 "sandbox: Use os functions to read host device tree"
removed the ability for get_device_and_partition() to handle the "host"
device type, and redirect accesses to it to the host filesystem. This
broke some unit tests that use this feature. So, revert that change. The
code added back by this patch is slightly different to pacify checkpatch.

However, we're then left with "host" being both:
- A pseudo device that accesses the hosts real filesystem.
- An emulated block device, which accesses "sectors" inside a file stored
  on the host.

In order to resolve this discrepancy, rename the pseudo device from host
to hostfs, and adjust the unit-tests for this change.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 disk/part.c       | 19 +++++++++++++++++++
 test/command_ut.c |  8 ++++----
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index b3097e32f0eb..068be06c3baf 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -510,6 +510,25 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,
 	int part;
 	disk_partition_t tmpinfo;
 
+	/*
+	 * Special-case a psuedo block device "hostfs", to allow access to the
+	 * host's own filesystem.
+	 */
+	if (0 == strcmp(ifname, "hostfs")) {
+		*dev_desc = NULL;
+		info->start = = 0;
+		info->size = 0;
+		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, "-"))
diff --git a/test/command_ut.c b/test/command_ut.c
index b2666bfc182b..ae6466d0ed83 100644
--- a/test/command_ut.c
+++ b/test/command_ut.c
@@ -165,12 +165,12 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 #ifdef CONFIG_SANDBOX
 	/* File existence */
-	HUSH_TEST(e, "-e host - creating_this_file_breaks_uboot_unit_test", n);
-	run_command("sb save host - creating_this_file_breaks_uboot_unit_test 0 1", 0);
-	HUSH_TEST(e, "-e host - creating_this_file_breaks_uboot_unit_test", y);
+	HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", n);
+	run_command("sb save hostfs - creating_this_file_breaks_uboot_unit_test 0 1", 0);
+	HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", y);
 	/* Perhaps this could be replaced by an "rm" shell command one day */
 	assert(!os_unlink("creating_this_file_breaks_uboot_unit_test"));
-	HUSH_TEST(e, "-e host - creating_this_file_breaks_uboot_unit_test", n);
+	HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", n);
 #endif
 #endif
 
-- 
1.8.1.5

[U-Boot] [PATCH] sandbox: restore ability to access host fs through standard commands

[Josh Wu]

Hi, Stephen

On 6/11/2014 6:43 AM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> Commit 95fac6ab4589 "sandbox: Use os functions to read host device tree"
> removed the ability for get_device_and_partition() to handle the "host"
> device type, and redirect accesses to it to the host filesystem. This
> broke some unit tests that use this feature. So, revert that change. The
> code added back by this patch is slightly different to pacify checkpatch.
>
> However, we're then left with "host" being both:
> - A pseudo device that accesses the hosts real filesystem.
> - An emulated block device, which accesses "sectors" inside a file stored
>    on the host.
>
> In order to resolve this discrepancy, rename the pseudo device from host
> to hostfs, and adjust the unit-tests for this change.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>   disk/part.c       | 19 +++++++++++++++++++
>   test/command_ut.c |  8 ++++----
>   2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/disk/part.c b/disk/part.c
> index b3097e32f0eb..068be06c3baf 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -510,6 +510,25 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str,
>   	int part;
>   	disk_partition_t tmpinfo;
>   
> +	/*
> +	 * Special-case a psuedo block device "hostfs", to allow access to the
> +	 * host's own filesystem.
> +	 */
Do we need to change the sb ls help message from 'host' to 'hostfs' as well?

> +	if (0 == strcmp(ifname, "hostfs")) {
> +		*dev_desc = NULL;
> +		info->start = = 0;
a typo. one additional '='.


> +		info->size = 0;
> +		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, "-"))
> diff --git a/test/command_ut.c b/test/command_ut.c
> index b2666bfc182b..ae6466d0ed83 100644
> --- a/test/command_ut.c
> +++ b/test/command_ut.c
> @@ -165,12 +165,12 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   
>   #ifdef CONFIG_SANDBOX
>   	/* File existence */
> -	HUSH_TEST(e, "-e host - creating_this_file_breaks_uboot_unit_test", n);
> -	run_command("sb save host - creating_this_file_breaks_uboot_unit_test 0 1", 0);
> -	HUSH_TEST(e, "-e host - creating_this_file_breaks_uboot_unit_test", y);
> +	HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", n);

There still has a odd behavior:
at first, when we run 'sb load host 0 200000 /home/env.sh', it will show 
'** Bad device host 0 **'
but after use 'sb bind 0 test.img', then above command can work well.

IMHO, we need use 'host' and 'hostfs' for different usage. suck like:
'host' interface means host block device that we use 'sb bind'.
'hostfs' interface means host file system


> +	run_command("sb save hostfs - creating_this_file_breaks_uboot_unit_test 0 1", 0);
> +	HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", y);
>   	/* Perhaps this could be replaced by an "rm" shell command one day */
>   	assert(!os_unlink("creating_this_file_breaks_uboot_unit_test"));
> -	HUSH_TEST(e, "-e host - creating_this_file_breaks_uboot_unit_test", n);
> +	HUSH_TEST(e, "-e hostfs - creating_this_file_breaks_uboot_unit_test", n);
>   #endif
>   #endif
>   
Best Regards,
Josh Wu

[U-Boot] [PATCH] sandbox: restore ability to access host fs through standard commands

[Stephen Warren]

On 06/10/2014 09:29 PM, Josh Wu wrote:
> On 6/11/2014 6:43 AM, Stephen Warren wrote:
>> Commit 95fac6ab4589 "sandbox: Use os functions to read host device tree"
>> removed the ability for get_device_and_partition() to handle the "host"
>> device type, and redirect accesses to it to the host filesystem. This
>> broke some unit tests that use this feature. So, revert that change. The
>> code added back by this patch is slightly different to pacify checkpatch.
>>
>> However, we're then left with "host" being both:
>> - A pseudo device that accesses the hosts real filesystem.
>> - An emulated block device, which accesses "sectors" inside a file stored
>>    on the host.
>>
>> In order to resolve this discrepancy, rename the pseudo device from host
>> to hostfs, and adjust the unit-tests for this change.

>> diff --git a/disk/part.c b/disk/part.c

>>  +    /*
>> +     * Special-case a psuedo block device "hostfs", to allow access
>> to the
>> +     * host's own filesystem.
>> +     */
>
> Do we need to change the sb ls help message from 'host' to 'hostfs' as
> well?

Yes.

>> +    if (0 == strcmp(ifname, "hostfs")) {
>> +        *dev_desc = NULL;
>> +        info->start = = 0;
>
> a typo. one additional '='.

Indeed. I guess I forgot to recompile after fixing checkpatch:-/

>> diff --git a/test/command_ut.c b/test/command_ut.c

>> @@ -165,12 +165,12 @@ static int do_ut_cmd(cmd_tbl_t *cmdtp, int flag,
>> int argc, char * const argv[])
>>     #ifdef CONFIG_SANDBOX
>>       /* File existence */
>> -    HUSH_TEST(e, "-e host -
>> creating_this_file_breaks_uboot_unit_test", n);
>> -    run_command("sb save host -
>> creating_this_file_breaks_uboot_unit_test 0 1", 0);
>> -    HUSH_TEST(e, "-e host -
>> creating_this_file_breaks_uboot_unit_test", y);
>> +    HUSH_TEST(e, "-e hostfs -
>> creating_this_file_breaks_uboot_unit_test", n);
> 
> There still has a odd behavior:
> at first, when we run 'sb load host 0 200000 /home/env.sh', it will show
> '** Bad device host 0 **'
> but after use 'sb bind 0 test.img', then above command can work well.

"sb load" works fine for me on *hostfs*:

=> sb ls hostfs - /boot
...
        176764 memtest86+.bin
...
=> sb load hostfs - 0 /boot/memtest86+.bin
176764 bytes read in 29 ms (5.8 MiB/s)

I'm not sure if "sb load" on "host" is expected to work; "host" is an
emulated block device that works just like any other block device, i.e.
without using the "sb" comamnd, so I'd expect it to be used with plain
old ls/fatls/extls, load/fatload/ext2load/, ...

> IMHO, we need use 'host' and 'hostfs' for different usage. suck like:
> 'host' interface means host block device that we use 'sb bind'.
> 'hostfs' interface means host file system

That's what this patch should provide.