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

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

On Mon, May 11, 2026 at 05:58:36PM +0200, Bernd Schubert wrote:
> 
> 
> On 5/11/26 17:22, Darrick J. Wong wrote:
> > On Sat, May 09, 2026 at 09:42:12AM +0200, Bernd Schubert wrote:
> >>
> >>
> >> On 5/9/26 02:48, Darrick J. Wong wrote:
> >>> 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?
> >>
> >>
> >> I would prefer the 3rd option, like for fusermount3
> >>
> >> executable('fusermount3', ['fusermount.c', '../lib/mount_util.c',
> >> '../lib/mount_fsmount.c', '../lib/util.c'],
> >>
> >>
> >> I.e. add lib/mount_fsmount.c to mount_service_sources.
> > 
> > That works, will put together a patch to do that. :)
> 
> Hi Darrick,
> 
> I didn't manage yesterday anymore to work on that, the container branch
> is in a total merge conflict with libfuse master. If you want you could
> continue on the iomap and I would solve the merge conflicts. Though,
> probably not today or tomorrow.

I don't mind trying to resolving the conflict, and will work on that
this afternoon.  I'll report back whatever the outcome of that is.

--D