Incorrect error message from erofs "backed by file" in 6.12-rc

[Christian Brauner]

On Sat, Oct 05, 2024 at 10:41:10PM GMT, Gao Xiang wrote:
> Hi Allison,
> 
> (try to +Cc Christian)
> 
> On 2024/10/2 20:58, Allison Karlitskaya wrote:
> > hi,
> > 
> > In context of my work on composefs/bootc I've been testing the new support for directly mounting files with erofs (ie: without a loopback device) and it's working well.  Thanks for adding this feature --- it's a huge quality of life improvement for us.
> > 
> > I've observed a strange behaviour, though: when mounting a file as an erofs, if you read() the filesystem context fd, you always get the following error message reported: Can't lookup blockdev.
> > 
> > That's caused by the code in erofs_fc_get_tree() trying to call get_tree_bdev() and recovering from the error in case it was ENOTBLK and CONFIG_EROFS_FS_BACKED_BY_FILE.  Unfortunately, get_tree_bdev() logs the error directly on the fs_context, so you get the error message even on successful mounts.
> > > It looks something like this at the syscall level:
> > 
> > fsopen("erofs", FSOPEN_CLOEXEC)         = 3
> > fsconfig(3, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0
> > fsconfig(3, FSCONFIG_SET_STRING, "source", "/home/lis/src/mountcfs/cfs", 0) = 0
> > fsconfig(3, FSCONFIG_CMD_CREATE, NULL, NULL, 0) = 0
> > fsmount(3, FSMOUNT_CLOEXEC, 0)          = 5
> > move_mount(5, "", AT_FDCWD, "/tmp/composefs.upper.KuT5aV", MOVE_MOUNT_F_EMPTY_PATH) = 0
> > read(3, "e /home/lis/src/mountcfs/cfs: Can't lookup blockdev\n", 1024) = 52
> > 
> > This is kernel 6.12.0-0.rc0.20240926git11a299a7933e.13.fc42.x86_64 from Fedora Rawhide.
> > 
> > It's a pretty minor issue, but it sent me on a wild goose chase for an hour or two, so probably it should get fixed before the final release.
> > 
> 
> Sorry for late response. I'm on vacation recently.
> 
> Yes, I also observed this message, but I'm not sure
> how to handle it better.  Indeed, the message itself
> is out of get_tree_bdev() as you said.
> 
> Yet I tend to avoid unnecessary extra lookup_bdev()
> likewise to confirm the type of the source in advance,
> since the majority mount type of EROFS is still
> bdev-based instead file-based so I tend to make
> file-based mount as a fallback...
> 
> Hi Christian, if possible, could you give some other
> idea to handle this case better? Many thanks!

(1) Require that the path be qualified like:

    fsconfig(<fd>, FSCONFIG_SET_STRING, "source", "file:/home/lis/src/mountcfs/cfs", 0)

    and match on it in either erofs_*_get_tree() or by adding a custom
    function for the Opt_source/"source" parameter.

(2) Add a erofs specific "source-file" mount option. IOW, check that
    either "source-file" or "source" was specified but not both. You
    could even set fc->source to "source-file" value and fail if
    fc->source is already set. You get the idea.

?