Re: [PATCH v4] erofs: accept source file descriptor via fsconfig
Gao Xiang <[email protected]> Fri, 24 Jul 2026 06:38:20 +0800
| Newsgroups | org.ozlabs.lists.linux-erofs,org.kernel.vger.linux-fsdevel |
|---|---|
| Message-ID | <[email protected]> |
Hi Christian, On 2026/7/23 22:46, Christian Brauner wrote: >> I'm not quite sure if I catched the point, I think Giuseppe's patch here >> tried to record `file` into `sbi->dif0.file` (which indicates the primary >> "device" later.) >> >> And if `sbi->dif0.file` is set up by erofs_fc_parse_source(), >> erofs_fc_get_tree() will just use `sbi->dif0.file` instead of >> `fc->source` according to this patch. > > Oh, so you only do it for file-backed mounts. Do you only allow regular > files or do you also support block devices with > CONFIG_EROFS_FS_BACKED_BY_FILE? Block devices with CONFIG_EROFS_FS_BACKED_BY_FILE are supported, but with only `fc->source` (not this way.) That is the limitation I see in Giuseppe's patch. I'd hoped bdev-backed mounts could work the same way, but that would require changes to the VFS flow. Since this is a side improvement, I think it's fine as long as it's documented somewhere, and I do hope Giuseppe can at least address the documentation. I'd also like to make sure the way fc->source is filled out of fd passing follows common practice, so that if fd-based bdev-backed mounts land in the VFS later, they can keep the same fc->source convention, otherwise it will cause a userspace behavior change. > > Do you document the expected behavior for the file you're consuming? > Meaning, are concurrent modifications supported and what type of > behavior does this exhibit? As I perhaps mentioned, EROFS itself (or many EROFS) won't do any modification to the underlayfs bdev or files by design so the standard behavior is the blob devices / files won't get any change. Beyond that, both the on-disk format and the implementation are designed to tolerate unexpected external modifications (or storage media damage). Even in the worst case, where the underlying storage (block device or backing filesystem) is malicious, corrupted on-disk (meta)data will not lead to the kind of complex, hard-to-resolve inconsistencies you see in general-purpose writable filesystems, whose ondisk/in-memory cached metadata is much harder to reconcile. I'm not sure whether you'll agree, but I want to emphasize that again this is one of EROFS core design goals: the on-disk and implementation design ensure that. If there is any human bug, it will be addressed and fixed as long as it discloses: it won't be hard to fixed. But if you really want to avoid concurrent modifications or keep the image golden, I think dmverity or fsverify should be enforced to ensure the filesystem won't be modified unexpectedly or expectedly. > >> The reason why `fc->source` is set was discussed in the >> thread of the previous version suggested by Aleksa. >> >>> >>> If they close it before this means you can mount something completely >>> different. The other thing is even if they keep the fd open someone >>> could just rename the damn thing and fc->source ends up pointing >>> somwhere completely different. The could switch namespaces as well in >>> some circumstances and then it points again into wherever. >>> >>> I've played with that fd idea before. The only way to make this work >>> correctly is if you plumb this down into get_tree_nodev() >> >> fc->source in this case has no use in erofs_fc_get_tree() (`fc->source` >> is just used for mountinfo for example), `sbi->dif0.file` works instead >> I hope I don't misunderstand something. > > No, I misunderstood this. Thanks, Gao Xiang