On Wed, Sep 15, 2021 at 01:45:23PM -0700, Eric Biggers wrote: > On Tue, Sep 14, 2021 at 11:34:29AM -0700, Boris Burkov wrote: > > > Okay, so it is used. (Due to the macro, it didn't show up when grepping.) > > > > > > Doesn't it defeat the purpose of a ro_compat inode flag if the whole filesystem > > > is marked with a ro_compat feature flag, though? I thought that the point of > > > the ro_compat inode flag is to allow old kernels to mount the filesystem > > > read-write, with only verity files being forced to read-only. That would be > > > more flexible than ext4's implementation of fs-verity which forces the whole > > > filesystem to read-only. But it seems you're forcing the whole filesystem to > > > read-only anyway? > > > > > > - Eric > > > > I was thinking of it in terms of "RO compat is the goal" and having new > > inode flags totally broke that and was treated as a corruption of the > > inode regardless of the fs being ro/rw. I think a check on a live fs > > would just flip the fs ro, which was the goal anyway, but a check that > > happened during mount would fail the mount, even for a read-only fs. > > > > Making it fully per file would be pretty cool! The only thing > > really missing as far as I can tell is a way to mark a file read only > > with the same semantics fsverity uses from within btrfs. > > I don't understand. Why are you bothering with the ro_compat inode flag at all > if it doesn't actually work? > > - Eric Sorry I explained that really badly. My first try was ro-compat bit only, that failed because btrfs couldn't add an inode flag in a ro-compat way before my changes, as it could fail to mount. To fix that, I had to work on the inode flag compatibility, which evolved into this notion of inode ro-compat flags, which does work as expected: if you see a file with an unknown ro-compat flag it's an error if you aren't read-only. Read-only mount will never fail. I think changing the semantics of the ro-compat inodes from: "an unknown ro inode flag -> fs ro" to "an unknown ro inode flag -> file ro" could be a big win. I don't think there is a rush to do that, though? If I add it now, on top of the existing code, then you might go back to a kernel that can only mount the fs read-only or you might go back to one which is clever enough to only force the file read-only. Hope I'm making a bit more sense, now.