Mbed-os: MBRBlockDevice: cannot mount multiple partitions

[netanelgonen]

Description

• Type: Bug
• Priority: | Major

Bug

cannot create mulipal partitions with the new API in mbed-os 5.5 receiving this error:
“ mbed assertation failed: is_valid_read(addr, size), file: ../..//mbed-os/features/filesystem/bd/MBRBlockDevice.cpp, line 234”

This is my init code for the partiotions (in global scope)

    int partitonoGlobal1 = MBRBlockDevice::partition(&sd, 1, 0x00, 0, 512*1024);
    int partitonGlobal2 = MBRBlockDevice::partition(&sd, 2, 0x00, 0, 512*1024);

    // Create the block device for each partition
    MBRBlockDevice part1(&sd, 1);
    MBRBlockDevice part2(&sd, 2);

    // Format each partition with a FAT filesystem

    // Create the FAT filesystem instance for each filesystem
    FATFileSystem fat1("sd1", &part1);
    FATFileSystem fat2("sd2", &part2);

What am I doing wrong?
Thank you for your help

Some more info:
Mbed-os tag mbed-os-5.5
My code:
Repo: mbed-client-pal
Branch: format_partiotions
Init file : https://github.com/ARMmbed/mbed-client-pal/blob/format_partitions/Examples/PlatformBSP/MK64F_mbedOS/FileSystemInit.cpp#L46

[0xc0170]

cc @geky @c1728p9

[0xc0170]

The assert is fails because of this check

    /** Convenience function for checking block read validity
     *
     *  @param addr     Address of block to begin reading from
     *  @param size     Size to read in bytes
     *  @return         True if read is valid for underlying block device
     */
    bool is_valid_read(bd_addr_t addr, bd_size_t size) const
    {
        return (
            addr % get_read_size() == 0 &&
            size % get_read_size() == 0 &&
            addr + size <= this->size());
    }

When is this assert triggered (the call stack), what is the addr, size ?

[netanelgonen]

@0xc0170 who are you asking?

[0xc0170]

@netanelgonen You, to get more details about the failure (its an assertions so some assumptions are not done correctly). @sg- pointed out the address for both partition is the same, shouldn't be?

[JanneKiiskila]

@netanelgonen - I think you have the start address wrong. Please check the header.

 *  // Partition into two partitions with ~half the blocks
 *  MBRBlockDevice::partition(&mem, 1, 0x83, 0*512, 32*512);
 *  MBRBlockDevice::partition(&mem, 2, 0x83, 32*512);

It has an example - notice how the STARTING point for the 2nd partitioin is not zero. It can't be, as it would otherwise overlap with the 1st partition, right?

So, the 2nd partition can start after the point where the 1st partition ends.

int partitonoGlobal1 = MBRBlockDevice::partition(&sd, 1, 0x00, 0, 512*1024);
int partitonGlobal2 = MBRBlockDevice::partition(&sd, 2, 0x00, 512*1024, 512*1024);

Typically you would want these to come from some definition/API, no hard codings.

[netanelgonen]

Hi @JanneKiiskila thanks, this helped a bit but now I'm getting the same assert in a later stage.
I'm getting this assert while trying to open a directory that not exists. (hoping to get an error not collapsing)

what should I put in the type argument? i don't see any info about this

@0xc0170

[JanneKiiskila]

@netanelgonen - some sample code etc. would help, now this is just guesswork. You didn't specify the function call that works, what you do prior to that nor what assert you're seeing.

[geky]

Thanks, @JanneKiiskila was right on this one, perhaps an assert in the partition function would be useful.

I'm assuming the new error your seeing is because of the zero type? The init function should have been returning an error then (here). The zero type is used by the MBR as an empty entry.

The full list of partition types is here:
https://en.wikipedia.org/wiki/Partition_type

For the most part you can just use 0x83 and forget about it.

[netanelgonen]

I'm getting the same assert as before.

I'm failing in https://github.com/ARMmbed/mbed-client-pal/blob/format_partitions/Source/Port/Reference-Impl/OS_Specific/mbedOS/FileSystem/pal_plat_fileSystem.cpp#L267

in this case i actually know the directory does not exists.

while the back trace is in this image:

[JanneKiiskila]

Nope, your traceback is showing a different assert, it's not the same (as in the beginning).

https://github.com/ARMmbed/mbed-os/blob/master/features/filesystem/bd/MBRBlockDevice.cpp#L232

It's hitting that assert there. What are the parameters you're giving in?

[netanelgonen]

Hi @JanneKiiskila @geky @amirpjm ,
thanks you all after the changes i was able to create partition trough mbed-os 5.5 APIs
but, i cant use them... for some reason the same senario that pass on other OS (even FreeRTOS that uses the same fatFS) are failing.

if I'm trying to use SDcard with predefined (fat in linux trough gnome-disks) or by using the mbedOS 5.5 APIs.

I'm failing while I'm Teing to use them
init: https://github.com/ARMmbed/mbed-client-pal/blob/format_partitions/Examples/PlatformBSP/MK64F_mbedOS/FileSystemInit.cpp
use of pal api:
https://github.com/ARMmbed/mbed-client-pal/blob/format_partitions/Test/Unitest/PAL_Modules/FileSystem/pal_fileSystem_test.c

the pal_* calling stright to the pal_plat_* that uses mbed-os API
https://github.com/ARMmbed/mbed-client-pal/blob/format_partitions/Source/Port/Reference-Impl/OS_Specific/mbedOS/FileSystem/pal_plat_fileSystem.cpp

is there any tests that i can look at to for fs system on multi partitions?

[geky]

There is this test, although it doesn't go through the pal layer:
https://github.com/ARMmbed/mbed-os/blob/master/features/TESTS/filesystem/multipart_fat_filesystem/main.cpp

Use:

mbed test --compile -t GCC_ARM -m K64F -DMBED_EXTENDED_TESTS -n features-tests-filesystem-multipart_fat_filesystem
mbed test --run -t GCC_ARM -m K64F -n features-tests-filesystem-multipart_fat_filesystem

I'm assuming you updated the code to use the non-overlapping regions.

It looks like the code doesn't call FATFileSystem::format. Are you using preformated storage? If the storage already has an MBR (from being formated somewhere else) you should not need to call MBRBlockDevice::partition. If you are formatting from scratch you will need to call FATFileSystem::format.

If you want to just brute force your way to a working fat filesystem (possibly destroying whats on storage in the process), the following should work:

MBRBlockDevice part1(&sd, 1);
MBRBlockDevice part2(&sd, 2);
FATFileSystem fs1;
FATFileSystem fs2;

int mountboth() {
    int err = fs1.mount(&part1);
    if (err) {
        return err;
    }

    err = fs2.mount(&part2);
    if (err) {
        fs1.unmount();
        return err;
    }

     return 0;
}

int formatboth() {
    int err = MBRBlockDevice::partition(&sd, 1, 0x83, 0, 512*1024);
    if (err) {
        return err;
    }

    err = MBRBlockDevice::partition(&sd, 1, 0x83, 512*1024);
    if (err) {
        return err;
    }

    err = FATFileSystem::format(&part1);
    if (err) {
        return err;
    }

    err = FATFileSystem::format(&part2);
    if (err) {
        return err;
    }

    return 0;
}

int forcemountboth() {
    int err = mountboth();
    if (err) {
        err = formatboth();
        if (err) {
             return err;
        }

        return mountboth();
    }

    return 0;
}

And no you won't be able to continue to take advantage of static initializer function calls.

[netanelgonen]

I assumption is currect i just didn't push the code yet to git.

I have successeded to mount and format.
The problem starts when I'm trying to create directories and files on the file system.

Do you an example for this?

Sorry for typos

נשלח מ-BlueMailhttp://www.bluemail.me/r?b=9856
ב-13 ביוני 2017, ב-Christopher Haster <[email protected][e-mail]>, 19:07 כתב:

There is this test, although it doesn't go through the pal layer:
https://github.com/ARMmbed/mbed-os/blob/master/features/TESTS/filesystem/multipart_fat_filesystem/main.cpp

Use:

mbed test -t GCC_ARM -m K64F -DMBED_EXTENDED_TESTS -n features-tests-filesystem-multipart_fat_filesystem

I'm assuming you updated the code to use the non-overlapping regions.

It looks like the code doesn't call FATFileSystem::format. Are you using preformated storage? If the storage already has an MBR (from being formated somewhere else) you should not need to call MBRBlockDevice::partition. If you are formatting from scratch you will need to call FATFileSystem::format.

If you want to just brute force your way to a working fat filesystem (possibly destroying whats on storage in the process), the following should work:

MBRBlockDevice part1(&sd, 1);
MBRBlockDevice part2(&sd, 2);
FATFileSystem fs1;
FATFileSystem fs2;

int mountboth() {
int err = fs1.mount(&part1);
if (err) {
return err;
}

err = fs2.mount(&part2);
if (err) {
    fs1.unmount();
    return err;
}

 return 0;

}

int formatboth() {
int err = MBRBlockDevice::partition(&sd, 1, 0x83, 0, 512*1024);
if (err) {
return err;
}

err = MBRBlockDevice::partition(&sd, 1, 0x83, 512*1024);
if (err) {
    return err;
}

err = FATFileSystem::format(&part1);
if (err) {
    return err;
}

err = FATFileSystem::format(&part2);
if (err) {
    return err;
}

return 0;

}

int forcemountboth() {
int err = mountboth();
if (err) {
err = formatboth();
if (err) {
return err;
}
}

return mountboth();

}

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/ARMmbed/mbed-os/issues/4526#issuecomment-308167098, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AQOJRLEThSOb_qSGwD5zUDOjFgSr4bE9ks5sDrPEgaJpZM4N3C8t.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[geky]

I've reproduced this locally and it seems that there are a few more subtle issues that crept in:

1. The partitions need to be numbered correctly. Currently partition number 1 is be overwritten repeatedly.

2. Turns out the block device initialization sequence is a bit more hairy than I thought. Most of the utility block devices initialize children block devices for you, but this is not the case for the FATFileSystem. This may be worth changing in mbed OS to help usability, but I'm not sure it's a possible change right now. You will need to manually initialize block devices in use by the FAT filesystem.

Here's a fully working example that brute forces two filesystems on an sd card:
https://gist.github.com/geky/55d96b7c857fbe2932784ac4e9de4e29

You will need to import mbed-os and sd-driver. I tested it locally with a K64F, GCC_ARM, and the ci-test-shield. I'll turn it into a test case when I have the time.

[netanelgonen]

Thanks i will try this tommorow morning

נשלח מ-BlueMailhttp://www.bluemail.me/r?b=9856
ב-14 ביוני 2017, ב-Christopher Haster <[email protected][e-mail]>, 0:35 כתב:

I've reproduced this locally and it seems that there are a few more subtle issues that crept in:

1. The partitions need to be numbered correctly. Currently partition number 1 is be overwritten repeatedly.

2. Turns out the block device initialization sequence is a bit more hairy than I thought. Most of the utility block devices initialize children block devices for you, but this is not the case for the FATFileSystem. This may be worth changing in mbed OS to help usability, but I'm not sure it's a possible change right now. You will need to manually initialize block devices in use by the FAT filesystem.

Here's a fully working example that brute forces two filesystems on an sd card:
https://gist.github.com/geky/55d96b7c857fbe2932784ac4e9de4e29

You will need to import mbed-os and sd-driver. I tested it locally with a K64F, GCC_ARM, and the ci-test-shield. I'll turn it into a test case when I have the time.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/ARMmbed/mbed-os/issues/4526#issuecomment-308255610, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AQOJRCaCnRPpNsqj-O-lEiC51hswUnq0ks5sDwCSgaJpZM4N3C8t.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[netanelgonen]

I have encountered a new issue when using directories.
when creating directory it will be always created inside the 1st partition regardless to the path

i used this example as a base
https://gist.github.com/geky/55d96b7c857fbe2932784ac4e9de4e29

and created the test as following:
int testing_fs2()
{
printf("go\n");

int err = destroy();
printf("destroy %d\n", err);

err = forcemountboth();
printf("forcemountboth %d\n", err);

err = mkdir("/fs1/dir1", 0600);
printf("mkdir fs1/dir1 %d\n", err);

err = mkdir("/fs2/dir2", 0600);
printf("mkdir fs2/dir2 %d\n", err);

FILE *f = fopen("/fs1/dir1/hi", "w");
printf("fopen fs1 %p %d\n", f, errno);

err = fwrite("hello", 1, 5, f);
printf("fwrite fs1 %d\n", err);

err = fclose(f);
printf("fclose fs1 %d\n", err);

f = fopen("/fs2/dir2/hi", "w");
printf("fopen fs2 %p %d\n", f, errno);

err = fwrite("hello", 1, 5, f);
printf("fwrite fs2 %d\n", err);

err = fclose(f);
printf("fclose fs2 %d\n", err);

err = unmountboth();
printf("unmountboth %d\n", err);

printf("done\n");
return 0;

}

both directories created under the 1st partition.

thanks

[geky]

Thanks for the example code. Looks like you're right. There is apparently an issue in how the current fatfs prefixes paths. This is kinda a messy step needed to interface with the chanfs implementation. Unfortunately we haven't ran into this before since multiple fatfss have never been used simultaneously.

I put up a patch that should fix this issue: https://github.com/ARMmbed/mbed-os/pull/4561

With that patch I was able to get your example working, let me know if it doesn't.

[netanelgonen]

still mounting pre-created partition isn't working properly,
scenario:
create 2 partition for fat using gnome-disks
mountboth() is failing only forcemount() is working

in addition why format performing unmount to the drive?

[geky]

You aren't still calling destroy are you?

int main() {
    printf("go\n");

-   int err = destroy();
-   printf("destroy %d\n", err);
-
    err = forcemountboth();
    printf("forcemountboth %d\n", err);

I'll have to see if I can reproduce with gnome-disks.

As for why format doesn't leave the drive mounted, the format function is a stateless function (it's a static member function). You don't need to declare a FATFileSystem to call format. If you want to mount and format you can just call mount after you call format.

[yogpan01]

@geky Did you make any progress on this ?

[geky]

Sorry, I didn't know if there was still an issue here or not, I didn't get to checking parted until today.

It looks like there was an issue with the initialization of the MBR's child block device that only happens with the SD card, here's the fix https://github.com/ARMmbed/mbed-os/pull/4607. With https://github.com/ARMmbed/mbed-os/pull/4607 I didn't have any issues with using parted to format the SD card.

Here's the steps I followed with parted:

# Make sure sdb is the sd card!
sudo parted /dev/sdb mklabel msdos
sudo parted /dev/sdb mkpart primary fat32 0 1GB
sudo parted /dev/sdb mkpart primary fat32 1GB 100%
sudo mkfs.fat /dev/sdb2
sudo mkfs.fat /dev/sdb2

mkdir mnt
sudo mount /dev/sdb1 mnt
sudo echo "hello" > mnt/hello.txt
sudo umount mnt
sudo mount /dev/sdb2 mnt
sudo echo "hello" > mnt/hello.txt
sudo umount mnt
rmdir mnt

From there I was able to read the sd card on the device side using the forcemountboth function from above. The sd card went unmodified when I remounted on host computer.

Let me know if you have any issues with #4561, #4562, and https://github.com/ARMmbed/mbed-os/pull/4607 merged.

[JanneKiiskila]

@sg- @0xc0170 - please add label OSCA.

[theotherjimmy]

@JanneKiiskila What would OSCA stand for?

[JanneKiiskila]

(mbed) OS Client Alignment.

[theotherjimmy]

Ah, so it's the client version of needs: CLA (Contributer license agreement)?

[JanneKiiskila]

IOTMORF-1018 raised internally.

[geky]

I think this issue has been resolved now, @netanelgonen I'm going to go ahead and close this. Feel free to reopen if I'm mistaken.