Re: [PATCH 1/1] mount_service: use the mount_flags table instead of declaring our own

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

On Fri, May 08, 2026 at 10:59:45AM -0700, Darrick J. Wong wrote:
> On Fri, May 08, 2026 at 06:51:45PM +0200, Bernd Schubert wrote:
> > 
> > 
> > On 5/8/26 00:14, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Use the canonical mount flags table in mount_util.c instead of
> > > opencoding our own version in mount_service.c.
> > > 
> > > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > > ---
> > >  util/mount_service.c |   51 +++++++++++++-------------------------------------
> > >  1 file changed, 13 insertions(+), 38 deletions(-)
> > > 
> > > 
> > > diff --git a/util/mount_service.c b/util/mount_service.c
> > > index 8c9747b3928a51..79e5b060ba3356 100644
> > > --- a/util/mount_service.c
> > > +++ b/util/mount_service.c
> > > @@ -1457,73 +1457,48 @@ struct ms_to_mount_map {
> > >  	unsigned int mount_attr_flag;
> > >  };
> > >  
> > > -static const struct ms_to_mount_map attrs[] = {
> > > -	{ MS_RDONLY,		MOUNT_ATTR_RDONLY },
> > > -	{ MS_NOSUID,		MOUNT_ATTR_NOSUID },
> > > -	{ MS_NODEV,		MOUNT_ATTR_NODEV },
> > > -	{ MS_NOEXEC,		MOUNT_ATTR_NOEXEC },
> > > -	{ MS_RELATIME,		MOUNT_ATTR_RELATIME },
> > > -	{ MS_NOATIME,		MOUNT_ATTR_NOATIME },
> > > -	{ MS_STRICTATIME,	MOUNT_ATTR_STRICTATIME },
> > > -	{ MS_NODIRATIME,	MOUNT_ATTR_NODIRATIME },
> > > -#ifdef MOUNT_ATTR_NOSYMFOLLOW
> > > -	{ MS_NOSYMFOLLOW,	MOUNT_ATTR_NOSYMFOLLOW },
> > > -#endif
> > > -	{ 0, 0 },
> > > -};
> > > -
> > >  static void get_mount_attr_flags(const struct fuse_service_mount_command *oc,
> > >  				 unsigned int *attr_flags,
> > >  				 unsigned long *leftover_ms_flags)
> > >  {
> > > -	const struct ms_to_mount_map *i;
> > > +	const struct mount_flags *i;
> > >  	unsigned int ms_flags = ntohl(oc->ms_flags);
> > >  	unsigned int mount_attr_flags = 0;
> > >  
> > > -	for (i = attrs; i->ms_flag != 0; i++) {
> > > -		if (ms_flags & i->ms_flag)
> > > -			mount_attr_flags |= i->mount_attr_flag;
> > > -		ms_flags &= ~i->ms_flag;
> > > +	for (i = mount_flags; i->opt != NULL; i++) {
> > > +		if (!i->on || !(ms_flags & i->flag))
> > > +			continue;
> > > +
> > > +		mount_attr_flags |= i->mount_attr;
> > > +		ms_flags &= ~i->flag;
> > >  	}
> > >  
> > >  	*leftover_ms_flags = ms_flags;
> > >  	*attr_flags = mount_attr_flags;
> > >  }
> > >  
> > > -struct ms_to_str_map {
> > > -	unsigned long ms_flag;
> > > -	const char *string;
> > > -};
> > > -
> > > -static const struct ms_to_str_map strflags[] = {
> > > -	{ MS_SYNCHRONOUS,	"sync" },
> > > -	{ MS_DIRSYNC,		"dirsync" },
> > > -	{ MS_LAZYTIME,		"lazytime" },
> > > -	{ 0, 0 },
> > > -};
> > > -
> > >  static int set_ms_flags(struct mount_service *mo, unsigned long ms_flags)
> > >  {
> > > -	const struct ms_to_str_map *i;
> > > +	const struct mount_flags *i;
> > >  	int ret;
> > >  
> > > -	for (i = strflags; i->ms_flag != 0; i++) {
> > > -		if (!(ms_flags & i->ms_flag))
> > > +	for (i = mount_flags; i->opt != NULL; i++) {
> > > +		if (!i->on || !(ms_flags & i->flag))
> > >  			continue;
> > >  
> > > -		ret = fsconfig(mo->fsopenfd, FSCONFIG_SET_FLAG, i->string,
> > > +		ret = fsconfig(mo->fsopenfd, FSCONFIG_SET_FLAG, i->opt,
> > >  			       NULL, 0);
> > >  		if (ret) {
> > >  			int error = errno;
> > >  
> > >  			fprintf(stderr, "%s: set %s option: %s\n",
> > > -				mo->msgtag, i->string, strerror(error));
> > > +				mo->msgtag, i->opt, strerror(error));
> > >  			emit_fsconfig_messages(mo);
> > >  
> > >  			errno = error;
> > >  			return -1;
> > >  		}
> > > -		ms_flags &= ~i->ms_flag;
> > > +		ms_flags &= ~i->flag;
> > >  	}
> > >  
> > >  	/*
> > > 
> > > 
> > 
> > 
> > Hi Darrick,
> > 
> > wouldn't it make sense to use the functions from mount_fsmount.c? I can
> > also clean this up later if you want.
> 
> Oh, hah!  Yes, I'll revise this patch to use them.

Update: I tried this, but ran into a problem: the two helpers are in
lib/mount_fsmount.c, which means the object code ends up in libfuse.so.
Unfortunately, that means that util/mount_service.c can't use them
unless they're exported from the .so.

I don't mind doing that, but I feel that I should check with you if it's
ok to add that to the public libfuse API before doing that?

Though I think a better solution is to move ms_flags_to_mount_attrs and
set_ms_flags to mount_util.c because that's compiled into the util/
programs.

How about I do that second thing on Monday?

--D

> > I guess this doesn't handle the case for "ro" going into fsmount() and
> > fsconfig() yet - update now or later?
> 
> That'll become another fix patch for mount_service.c.
> 
> --D
> 
> > 
> > Thanks,
> > Bernd
> > 
> > 
> > 
> >