Re: [PATCH v2 31/34] block: use file->f_op to indicate restricted writes^[

Christian Brauner
2024-01-29T17:09:44Z

On Mon, Jan 29, 2024 at 05:49:34PM +0100, Christoph Hellwig wrote:
> On Tue, Jan 23, 2024 at 02:26:48PM +0100, Christian Brauner wrote:
> > Make it possible to detected a block device that was opened with
> > restricted write access solely based on its file operations that it was
> > opened with. This avoids wasting an FMODE_* flag.
> >
> > def_blk_fops isn't needed to check whether something is a block device
> > checking the inode type is enough for that. And def_blk_fops_restricted
> > can be kept private to the block layer.
>
> I agree with not wasting a FMODE_* flag, but I also really hate
> duplicating the file operations.

I don't think it's that bad and is temporary until we can
unconditionally disable writing to mounted block devices. Until then we
can place all of this under #if IS_ENABLED(CONFIG_BLK_DEV_WRITE_MOUNTED)
in a single location in block/fops.c so its nicely encapsulated and
confined.

> I went to search for a good place to stash this information and ended up
> at the f_version field in struct file. That one is never touched by the
> VFS proper but just the file system and a few lseek helpers. The
> latter currently happen to be used by block devices unfortunately,
> but it seem like moving the f_version clearing into the few filesystem
> actualy using it would be good code hygiene anyway.

Kinda like choosing between pest and cholera. I think that the f_op
solution is nicer. Overloading f_version is not something I feel I have
the stomach for. The cleanup itself might still be worth it ofc.