On 09/14/2015 11:35 AM, Hans de Goede wrote:
Hi,
On 01-09-15 22:03, Stephen Warren wrote:
On 08/22/2015 11:04 AM, Hans de Goede wrote:
Add generic fs support, so that commands like ls, load and test -e can be used on ubifs.
@@ -530,6 +531,28 @@ int get_device_and_partition(const char *ifname, const char *dev_part_str, return 0; }
+#ifdef CONFIG_CMD_UBIFS
- /*
* Special-case ubi, ubi goes through a mtd, rathen then through* a regular block device.*/- if (0 == strcmp(ifname, "ubi")) {
if (!ubifs_mounted) {printf("UBIFS not mounted, use ubifsmount to mountvolume first!\n");
return -1;}*dev_desc = NULL;memset(info, 0, sizeof(*info));strcpy((char *)info->type, BOOT_PART_TYPE);strcpy((char *)info->name, "UBI");+#ifdef CONFIG_PARTITION_UUIDS
info->uuid[0] = 0;+#endif
return 0;- }
+#endif
We now have two paths through this function that can "Return" a NULL dev_desc. This makes it impossible for sandbox and ubifs to successfully co-exist in the same U-Boot binary, since the sandbox and ubifs fs probe functions won't be able to tell if "hostfs" or "ubifs" was passed to get_device_and_partition(). Perhaps there's no ubifs support in sandbox right now, so there's no issue?
Right, ubifs is for raw nand, sandbox is for access to a host filesystem in a sandbox build. I basically never expect both CONFIG_CMD_UBIFS and CONFIG_SANDBOX to be set at the same time. I'll add a pre-processor check + #error to enforce this in the next version.
If this is an issue that needs to be solved now, I think the best solution would be for the two special cases in get_device_and_partition() to "return" a real dev_desc rather than NULL. Since there's nothing meaningful to put there, how about returning a hard-coded value that can then be checked in the fs probe functions to make sure it matches:
get_device_and_partition():
if (hostfs) { ... *dev_desc = &hostfs_fake_dev_desc; ... return 0; } if (ubi) { ... *dev_desc = &ubifs_fake_dev_desc; ... return 0; }ubifs_set_blk_dev():
if (rbdd != &ubifs_fake_dev_desc) return -1; ... return 0;... that said, I wonder if the ubifs special case in get_device_and_partition() shouldn't actually perform the ubifs_mount() call itself, based on the user-supplied parameters?
That is not possible as the supplied parameter for a generic fs call is a device index + partition number, where as ubi volumes use names (strings).
I don't think that matters.
The "generic fs" plumbing added in this series doesn't require a volume name to be passed from the "generic fs" layer to the ubifs layer; it assumes that a ubifs volume is already mounted and so no volume identity is required. I wasn't implying that should be changed. Given that, the disparity between parameters doesn't matter, since there's no need to translate the ubifs_fake_dev_dec to a ubifs volume name at all; the only thing it'd be used for is as an identity check to dispatch from the generic fs layer to the right underlying filesystem.
Still, the current solution you have should be fine for now. We only need to fix this if someone implements a raw NAND emulator for sandbox.