-
Notifications
You must be signed in to change notification settings - Fork 808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add lfs_fs_stat for access to filesystem status/configuration #838
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently this includes: - minor_version - on-disk minor version - block_usage - estimated number of in-use blocks - name_max - configurable name limit - file_max - configurable file limit - attr_max - configurable attr limit These are currently the only configuration operations that need to be written to disk. Other configuration is either needed to mount, such as block_size, or does not change the on-disk representation, such as read/prog_size. This also includes the current block usage, which is common in other filesystems, though a more expensive to find in littlefs. I figure it's not unreasonable to make lfs_fs_stat no worse than block allocation, hopefully this isn't a mistake. It may be worth caching the current usage after the most recent lookahead scan. More configuration may be added to this struct in the future.
LFS_VERSION -> LFS_DISK_VERSION These tests shouldn't depend on LFS_VERSION. It's a bit subtle, but LFS_VERSION versions the API, and LFS_DISK_VERSION versions the on-disk format, which is what test_compat should be testing.
Needed for minor version reporting in lfs_fs_stat to work correctly.
This function naturally doesn't exist in the previous version. We should eventually add these calls when we can expect the previous version to support this function, though it's a bit unclear when that should happen. Or maybe not! Maybe this is testing more of the previous version than we really care about.
Valgrind output is very verbose but useful, with a default limit of 5 lines the output usually doesn't contain much useful info.
Version are now returned with major/minor packed into 32-bits, so 0x00020001 is the current disk version, for example. 1. This needed to change to use a disk_* prefix for consistency with the defines that already exist for LFS_VERSION/LFS_DISK_VERSION. 2. Encoding the version this way has the nice side effect of making 0 an invalid value. This is useful for adding a similar config option that needs to have reasonable default behavior for backwards compatibility. In theory this uses more space, but in practice most other config/status is 32-bits in littlefs. We would be wasting this space for alignment anyways.
Tests passed ✓, Code: 16682 B (+0.8%), Stack: 1432 B (+0.0%), Structs: 792 B (+2.6%)
|
2 tasks
geky
added
the
needs minor version
new functionality only allowed in minor versions
label
Jun 14, 2023
geky
added
next minor
and removed
needs minor version
new functionality only allowed in minor versions
labels
Jun 26, 2023
In terms of ease-of-use, a user familiar with other filesystems expects block_usage in fsinfo. But in terms of practicality, block_usage can be expensive to find in littlefs, so if it's not needed in the resulting fsinfo, that operation is wasteful. It's not clear to me what the best course of action is, but since block_usage can always be added to fsinfo later, but not removed without breaking backwards compatibility, I'm leaving this out for now. Block usage can still be found by explicitly calling lfs_fs_size.
Tests passed ✓, Code: 16674 B (+0.7%), Stack: 1432 B (+0.0%), Structs: 788 B (+2.1%)
|
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is more-or-less cherry-picked from #753, which is paused for now.
lfs_fs_stat
introduces a way to access some of the littlefs-specific status/configuration that hasn't been available yet. The intention is to be analogous the POSIX statvfs:The immediate benefit of this function is a way to get the minor version on disk, which @TjoBitDk noted was oddly missing in the recent release. This should resolve #826.
Currently
lfs_fs_stat
includes:These are currently the only configuration operations that need to be written to disk. Other configuration is either needed to mount, such as block_size, or does not change the on-disk representation, such as read/prog_size.
This also includes the current block usage, which is common in other filesystems, though a more expensive to find in littlefs. I figure it's not unreasonable to make lfs_fs_stat no worse than block allocation, hopefully this isn't a mistake. It may be worth caching the current usage after the most recent lookahead scan.EDIT: Removed due to concerns about runtime cost. May add back to fsinfo in the future.More configuration may be added to this struct in the future.