fatfs: Add full support for multiple fs volume prefixes #4561
Conversation
|
|
||
| // Adds prefix needed internally by fatfs, this can be avoided for the first fatfs | ||
| // (id 0) otherwise a prefix of "id:/" is inserted in front of the string. | ||
| static Deferred<const char*> fat_path_prefix(int id, const char *path) |
There was a problem hiding this comment.
What happens when id > 256?
There was a problem hiding this comment.
- The fsid will break.
- The chanfs will fail to mount.
- MBR will fail with >4 partitions
- GPT will fail with >128 partitions.
- Linux won't recognize >255 partitions.
- Windows won't recognize >1 partition.
- You will need ~800KB of ram to mount all of the filesystems at once.
- With 160MB of storage you will be wasting 10% on metadata alone.
Oh and this code may break.
There was a problem hiding this comment.
Actually, reading tha GPT article, the format supports >128 partitions. The maximum should be 4,294,967,295 or 0xFFFFFFFF
There was a problem hiding this comment.
Ah looks like you're right, glad we won't have to reinvent partitioning to get your 300 fs cluster working.
There was a problem hiding this comment.
Could it just be uint8? we don't loose anything by doing that.
|
retest uvisor |
|
retest uvisor |
|
@geky could you address the issues raised by @theotherjimmy please ? |
|
We need this on a patch release as well (5.5.x). |
|
@adbridge That "review" was not blocking, and just a question |
|
/morph test-nightly |
|
@adbridge, @theotherjimmy, sorry, I've been out and unable to update the pr. Feel free to push another commit to change the id type if you'd like. |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
|
/morph test-nightly |
|
retest uvisor |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
|
/morph test-nightly |
Result: FAILUREYour command has finished executing! Here's what you wrote!
|
This is only an issue when multiple fatfss are used simultaneously. Repeated use of a single fatfs instance (even with different storages) do not show this issue. Full support requires path prefixes being applied for every function that takes a path. Note: this is only required filesystems after the first mounted filesystem. The first filesystem has no penalty.
geky
force-pushed
the
fat-fs-volume-prefixes
branch
from
4846541 to
1775e1f
Compare
|
merge conflict, took a bit of effort but is rebased now |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
|
I aborted this, as this requires one fix to be functional, waiting for CI to complete #4672 |
|
#4672 on master. /morph test-nightly |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
In the Chan implementation of the fatfs, paths can be prefixed with a drive number (ie "2:/hi/hello/filehere.txt" for drive 2). This is used internally to lookup the storage that is in use. Full support of multiple fatfs simultaneously requires path prefixes being applied for every function that takes a path.
Note: This is only an issue when multiple fatfss are used simultaneously. Repeated use of a single fatfs instance (even with different storages) do not show this issue.
Note: this is only required filesystems after the first mounted filesystem. The first filesystem has no penalty.
related issue #4526
cc @netanelgonen