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

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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to