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

[Hans de Goede <hdegoede@redhat.com>]

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