Report a creation time from the simple example - #747
Conversation
Implements Filesystem::statx(), added in e039dae, which differs from getattr() in carrying a creation time. That is the reason to answer it: fuse_attr has a field for one on macOS alone, so on Linux the kernel has had nothing to report and statx(2) answered with whatever it last cached. InodeAttributes gains a created time, set once when the inode is made and never touched again. It also fills FileAttr::crtime, which was hardcoded to the epoch, so macOS gets a real creation time through fuse_attr as well rather than only Linux through statx. generic/528 now runs and passes. It asks for a creation time through _require_btime and was skipped for want of one: without this the reply mask comes back as 0x17ff, missing STATX_BTIME, and with it 0x1fff. The STATX_ATTR_* bits are filled in from the inode flags that chattr sets, which is what the field is for, but the kernel discards them: fuse_do_statx() takes the creation time and the basic stats out of the reply and nothing else. That is noted where they are set, so the example does not read as a way to make chattr +i visible to statx(2), which it is not - generic/424 stays excluded for exactly that reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4hiZwHE9fEYdn7DK3ZrV2
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11252eef57
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pub last_modified: (i64, u32), | ||
| pub last_metadata_changed: (i64, u32), | ||
| /// Time of creation, which `statx(2)` reports as `stx_btime` and nothing ever changes | ||
| pub created: (i64, u32), |
There was a problem hiding this comment.
Make created deserialization backward-compatible
When simple is started with a data_dir written by the previous version (including the default /tmp/fuser), the serialized inode records do not contain this new required field. init() immediately calls get_inode(ROOT), and get_inode unwraps rmp_serde::from_read, so those existing filesystems panic instead of mounting; please give created a migration/default path, such as deriving it from the existing timestamps on old records.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same answer as on #744, where this came up for the flags field and the PR merged as it was.
The example's data directory carries no format compatibility guarantee, and this has happened twice before without ceremony: 4c3e859 added rdev as a plain required field and 5ce9092 added flags, both mid-struct, both with no #[serde(default)] and no migration.
The suggested default would also not work here, for the same reason it would not have there. rmp_serde encodes structs as arrays, so what matters is field position, not field presence. created sits sixth of fifteen, between last_metadata_changed and kind, so an old record's kind lands in its slot and fails on the type rather than falling back to a default. Making this survive an upgrade means putting the field last, adding #[serde(default)], and writing down that ordering constraint so the next field addition does not silently undo it.
Deriving it from the existing timestamps is a good idea for a filesystem that wants this, and worth doing deliberately if the example should keep its data dir readable across versions - for every field rather than this one, and probably with a version in the superblock it already writes. Not as a side effect of adding a creation time.
Generated by Claude Code
Implements
Filesystem::statx(), added in e039dae, which differs fromgetattr()in carrying a creation time. That is the reason to answer it:
fuse_attrhas afield for one on macOS alone, so on Linux the kernel has had nothing to report and
statx(2)answered with whatever it last cached.InodeAttributesgains acreatedtime, set once when the inode is made andnever touched again. It also fills
FileAttr::crtime, which was hardcoded to theepoch, so macOS gets a real creation time through
fuse_attras well rather thanonly Linux through statx.
Effect
generic/528 runs and passes. It asks for a creation time through
_require_btimeand was skipped for want of one. Measured against the parent commit rather than
assumed:
stx_btime0x17ff- noSTATX_BTIME0x1fffThat is one of the two tests I expected
FUSE_STATXto reach. The other,generic/424, stays excluded: it reads the
chattrflags back through statx, andthe kernel discards the attributes from a FUSE reply, as #746 documents.
The
STATX_ATTR_*bits are filled in anyway from the flagschattrsets, sincethat is what the field is for and a filesystem doing so is correct today and needs
no change if the kernel starts reading it. It is noted where they are set, so the
example does not read as a way to make
chattr +ivisible tostatx(2).Testing
Full suite, green: 175 tests run, 0 failures, no filesystem panics - up from 174,
the difference being 528.
Worth saying why the whole suite mattered here rather than the usual targeted
subset: glibc routes
stat(2)throughstatx(2)on a current kernel, so this onemethod now answers essentially every attribute lookup the suite makes. A mistake
in it would not have been subtle.
Behavior checked directly as well:
STATX_BTIMEis in the result mask, the timematches creation, it does not move when the file is written while
mtimedoes,the basic stats still arrive, and directories report it too.
Both gates clean, the macOS one now with
--all-targetsso that it builds tests -which is what #746 needed a second push for.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Q4hiZwHE9fEYdn7DK3ZrV2
Generated by Claude Code