Re: [PATCH V3 4/4] 9p: convert to the new mount API

[Dominique Martinet <asmadeus@codewreck.org>]

Hi Remi,

Remi Pommarel wrote on Wed, Nov 26, 2025 at 09:16:14PM +0100:
> While testing this series to mount a QEMU's 9p directory with
> trans=virtio, I encountered a few issues. The same fix as above was
> necessary, but further regressions were also observed.

Thanks for testing!
(FWIW that patch has been rolled into my 9p-next branch, so you shouldn't
have needed to fiddle with it if using linux-next)

> Previously, using msize=2048k would silently fail to parse the option,
> but the mount would still proceed. With this series, the parsing error
> now prevents the mount entirely. While I prefer the new behavior, I know
> there is a strict rule to not break userspace, so are we not breaking
> userspace here?

That's a good question, we had the same discussion about unknown options
which were causing errors in the previous version of this patch.

My personal opinion is that given it's easy enough to notice/fix and it
points at something that's obviously wrong, I think such breakage is a
necessary evil and are occasionally ok -- but it should be intentional,
so let's add some fallback for this version and we can make this break
at the same time as we make unknown options break

> Another more important issue is that I was not able to successfully
> mount a 9p as rootfs with the command line below:
>  'root=/dev/root rw rootfstype=9p rootflags=trans=virtio,cache=loose'
> 
> The issue arises because init systems typically remount root as
> read-only (mount -oremount,ro /). This process retrieves the current
> mount options via v9fs_show_options(), then attempts to remount with
> those options plus ro. However, v9fs_show_options() formats the cache
> option as an integer but v9fs_parse_param() expect cache option to be
> a string (fsparam_enum) causing remount to fail. The patch below fix the
> issue for the cache option, but pretty sure all fsparam_enum options
> should be fixed.

Oww. That's a bit more annoying, yes...

> However same question as above arise with this patch. Previously cat
> /proc/mounts would format cache as an hexadecimal value while now it is
> the enum value name string. Would this be considered userspace
> breakage?

Now these are most likely ok, it already changed when Eric (VH) made it
display caches as hex a while ago, I wouldn't fuss too much about it.

OTOH if the old code worked I assume it parsed the hex values too, so
that might be what we ought to do? Or was it just ignored?
I'll try to find some time to play with this and let's send a patch
before the merge window coming in fast... This was due for next
week-ish!
-- 
Dominique Martinet | Asmadeus