[U-Boot] [PATCH v2 3/4] ubifs: Add generic fs support

Hans de Goede
2015-09-14T17:35:30Z

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 mount volume 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).

Regards,

Hans