Re: [PATCH 1/1] mount_util: fix mount_flags entries for MS_RDONLY

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, May 08, 2026 at 06:44:25PM +0200, Bernd Schubert wrote:
> 
> 
> On 5/8/26 17:44, Darrick J. Wong wrote:
> > On Fri, May 08, 2026 at 12:43:14PM +0200, Bernd Schubert wrote:
> >>
> >>
> >> On 5/8/26 00:13, Darrick J. Wong wrote:
> >>> From: Darrick J. Wong <djwong@kernel.org>
> >>>
> >>> MS_RDONLY maps to MOUNT_ATTR_RDONLY in the new fsmount API, but the
> >>> table omitted that.  Fix that.
> >>>
> >>> Fixes: 0d7e72541564a5 ("Unify mount flag structures and remove redundant is_mount_attr field")
> >>> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> >>> ---
> >>>  lib/mount_util.c |    4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>>
> >>> diff --git a/lib/mount_util.c b/lib/mount_util.c
> >>> index 1a0aec9bfe1a70..7ab1dba91e28e3 100644
> >>> --- a/lib/mount_util.c
> >>> +++ b/lib/mount_util.c
> >>> @@ -110,8 +110,8 @@
> >>>  
> >>>  const struct mount_flags mount_flags[] = {
> >>>  /* opt            flag             on  safe  mount_attr */
> >>> -{"rw",           MS_RDONLY,        0,  1,    0},                      /* fsconfig */
> >>> -{"ro",           MS_RDONLY,        1,  1,    0},                      /* fsconfig */
> >>> +{"rw",           MS_RDONLY,        0,  1,    MOUNT_ATTR_RDONLY},     /* fsconfig */
> >>> +{"ro",           MS_RDONLY,        1,  1,    MOUNT_ATTR_RDONLY},     /* fsconfig */
> >>>  {"suid",         MS_NOSUID,        0,  0,    MOUNT_ATTR_NOSUID},     /* fsmount  */
> >>>  {"nosuid",       MS_NOSUID,        1,  1,    MOUNT_ATTR_NOSUID},     /* fsmount  */
> >>>  {"dev",          MS_NODEV,         0,  1,    MOUNT_ATTR_NODEV},      /* fsmount  */
> >>>
> >>
> >> Hi Darrick,
> >>
> >> I had already pulled this the night before and it fails the new tests as
> >> well. I think issue is that ro needs to be done via fsmount(...,
> >> MOUNT_ATTR_RDONLY) *and* fsconfig(SET_FLAG, "ro"). I'm going to add a
> >> new column into const struct mount_flags mount_flags[].
> > 
> > Ewww, you're right.  util-linux also special-cases this, and in a weird
> > way:
> > 
> >        Since version 2.41, libmount has the ability to use optional
> >        arguments vfs and fs (e.g. ro=fs) to specify where the read-only
> >        setting should be applied. For example, using the command:
> > 
> >           mount -o ro=vfs /dev/sdc1 /A
> > 
> >        will mount the filesystem as read-write on the superblock level,
> >        but the /A node will be set as read-only. In previous versions,
> >        this required an additional "-o bind,remount,ro" operation to
> >        achieve the same result.
> > 
> > "ro=vfs" means apply it to the mount but don't tell the filesystem;
> > "ro=fs" means apply tell it to the filesystem, but not the mount;
> > and plain "ro" means apply it to both?  Apparently??
> > 
> 
> I just sent out a new patch series that fixes plain "ro". Thanks for
> looking it up in util-linux, I see "ro=vfs/fs" as new feature and

Yeah, I know.  TBH I wonder how much of the fsdevel community knows that
this was done -- I certainly would have remained ignorant if you hadn't
noticed this. :(

> already spent the entire (public holi)day on this series (there was
> another critical sync-init issue, when sync-init was actually not used).
> I.e. I'm going to ignore the "weird ways" for now.

<nod> I'm a little preoccupied with cleaning up the fuse-iomap patchsets
right now, but I'll have time to look at your fixpatches after lunch.

--D

> 
> Thanks,
> Bernd