From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D4C303A5E83 for ; Mon, 30 Mar 2026 21:47:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774907265; cv=none; b=IkCKUxiDsakcBhKrTOZt/CHibBTgOYFmKwEG25SJtCfCweRKf6j/iZ1eDoWfWYt8PhO0J6X/FTdFu5aiNzVUUOwx1Pe8LrnD91rI6+FGO3fy6SwzTnPMl6CyQqxthmmJIIP5OWcdYrehmzexigjpG016Vznh7HtLrKEQffqScXk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774907265; c=relaxed/simple; bh=VtpZTbNUreFYTm7KrEyMVyaOj0zSrfKFgAPOEkfI36A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=AXnTgMYKoliCBoZZZNqqNdsHYSaDdIm9TH8W0qeXGyfP3h6zW0JqujFPiS2+UzWZ8CpMnqcyRFHUIkjpVori5iJKbNeIJSaN+ZGkgl2/2ueQXXctu8e1QIRhBkm1izWu4Q7dVKwff5GFktLPvv1pswwadzIMNKvHbmEnqTdYQGY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=rlkFlzOi; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="rlkFlzOi" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 77001C4CEF7; Mon, 30 Mar 2026 21:47:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1774907265; bh=VtpZTbNUreFYTm7KrEyMVyaOj0zSrfKFgAPOEkfI36A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rlkFlzOinoXXtx7nO5DCYsZ4ynDVVFX55lg+JoG0oaVeGNZnED2fxVSbVlW9U7GdD PBKR5d22H+8b6TVEz/cj491ZYExeP4pOGl8lBHtMczcm8VX4eRkBIjeCKoPSkWUKJ3 kqFVzE5RXCLDNmtllRDDFlbivyQ7LS9GH/VVSgKIaPYaIr2dzke9DGDk0tH81xGOVH TEZjMVp6KGXoly9h2suEPbJDUoa3hjMwO2z9RUXtHz2UVqeQBi0+gZz2GTND+XsWI5 eFidL48PaArzKYV01tB1zS65RBLNekBtZ2JoEkL1RpMtXxuUEX9wFg6WjxOmoVJYFK 6Wbj5pCNcDSbg== Date: Mon, 30 Mar 2026 14:47:44 -0700 From: "Darrick J. Wong" To: Bernd Schubert Cc: Bernd Schubert , "linux-fsdevel@vger.kernel.org" , "miklos@szeredi.hu" , "neal@gompa.dev" , "joannelkoong@gmail.com" Subject: Re: [PATCH 04/17] mount_service: use the new mount api for the mount service Message-ID: <20260330214744.GH6202@frogsfrogsfrogs> References: <177457463048.1008428.11432672970504238251.stgit@frogsfrogsfrogs> <177457463189.1008428.16697718096512582220.stgit@frogsfrogsfrogs> <2a4afa9c-050e-4867-9dac-e9e87f693f23@ddn.com> <20260330211836.GE6202@frogsfrogsfrogs> Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Mar 30, 2026 at 11:40:22PM +0200, Bernd Schubert wrote: > > > On 3/30/26 23:18, Darrick J. Wong wrote: > > On Mon, Mar 30, 2026 at 09:06:05PM +0000, Bernd Schubert wrote: > >> On 3/27/26 02:25, Darrick J. Wong wrote: > >>> From: Darrick J. Wong > >>> > >>> Use the new fsopen/fsmount system calls to mount the filesystem so that > >>> we get somewhat better diagnostics if something gets screwed up. > >>> > >>> Signed-off-by: "Darrick J. Wong" > >>> --- > >>> meson.build | 15 +++ > >>> util/mount_service.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >>> 2 files changed, 314 insertions(+) > >>> > >>> > >>> diff --git a/meson.build b/meson.build > >>> index d6ba8740effd5c..b0988548bf806a 100644 > >>> --- a/meson.build > >>> +++ b/meson.build > >>> @@ -130,6 +130,21 @@ special_funcs = { > >>> int main(int argc, char *argv[]) { > >>> return SD_LISTEN_FDS_START; > >>> } > >>> + ''', > >>> + 'new_mount_api': ''' > >>> + #define _GNU_SOURCE > >>> + #include > >>> + #include > >>> + #include > >>> + #include > >>> + > >>> + int main(void) { > >>> + int fsfd = fsopen("fuse", FSOPEN_CLOEXEC); > >>> + int res = fsconfig(fsfd, FSCONFIG_SET_STRING, "source", "test", 0); > >>> + int mntfd = fsmount(fsfd, FSMOUNT_CLOEXEC, 0); > >>> + res = move_mount(mntfd, "", AT_FDCWD, "/mnt", MOVE_MOUNT_F_EMPTY_PATH); > >>> + return 0; > >>> + } > >>> ''' > >>> } > >>> > >>> diff --git a/util/mount_service.c b/util/mount_service.c > >>> index ae078e537dc560..4a2c1111b66b91 100644 > >>> --- a/util/mount_service.c > >>> +++ b/util/mount_service.c > >>> @@ -65,6 +65,9 @@ struct mount_service { > >>> > >>> /* O_PATH fd for mount point */ > >>> int mountfd; > >>> + > >>> + /* fd for fsopen */ > >>> + int fsopenfd; > >>> }; > >>> > >>> /* Filter out the subtype of the filesystem (e.g. fuse.Y -> Y) */ > >>> @@ -87,6 +90,7 @@ static int mount_service_init(struct mount_service *mo, int argc, char *argv[]) > >>> mo->argvfd = -1; > >>> mo->fusedevfd = -1; > >>> mo->mountfd = -1; > >>> + mo->fsopenfd = -1; > >>> > >>> for (i = 0; i < argc; i++) { > >>> if (!strcmp(argv[i], "-t") && i + 1 < argc) { > >>> @@ -690,9 +694,50 @@ static int mount_service_handle_fsopen_cmd(struct mount_service *mo, > >>> return mount_service_send_reply(mo, error); > >>> } > >>> > >>> +#ifdef HAVE_NEW_MOUNT_API > >>> + /* If this fails we fall back on mount() */ > >>> + mo->fsopenfd = fsopen(oc->value, FSOPEN_CLOEXEC); > >>> +#endif > >>> + > >>> return mount_service_send_reply(mo, 0); > >>> } > >>> > >>> +#ifdef HAVE_NEW_MOUNT_API > >>> +/* callers must preserve errno */ > >>> +static void emit_fsconfig_messages(const struct mount_service *mo) > >>> +{ > >>> + uint8_t buf[BUFSIZ]; > >>> + ssize_t sz; > >>> + > >>> + while ((sz = read(mo->fsopenfd, buf, sizeof(buf) - 1)) != -1) { > >>> + if (sz <= 0) > >>> + continue; > >>> + if (buf[sz - 1] == '\n') > >>> + buf[--sz] = '\0'; > >>> + else > >>> + buf[sz] = '\0'; > >>> + > >>> + if (!*buf) > >>> + continue; > >>> + > >>> + switch (buf[0]) { > >>> + case 'e': > >>> + fprintf(stderr, "Error: %s\n", buf + 2); > >>> + break; > >>> + case 'w': > >>> + fprintf(stderr, "Warning: %s\n", buf + 2); > >>> + break; > >>> + case 'i': > >>> + fprintf(stderr, "Info: %s\n", buf + 2); > >>> + break; > >>> + default: > >>> + fprintf(stderr, " %s\n", buf); > >>> + break; > >>> + } > >>> + } > >>> +} > >>> +#endif > >> > >> How about unifying that with my series and then use log_fsconfig_kmsg()? > >> We can also do it independently, but should not forget to use a common > >> function later on. That applies to all these functions for the new mount > >> API. Part of the headache I have with my series is that fusermount.c > >> duplicates quite some code. > >> For me it looks a bit like we can unify most of util/mount_service.c and > >> lib/mount_fsmount.c. > > > > The mount option string parsing (i.e. all that MS_ -> MOUNT_ATTR > > translation) and error reporting parts look trivially shareable. > > fuse_kern_fsmount isn't so easy to share since the fuse server sends > > over all the relevant pieces in separate commands. > > > >>> + > >>> static int mount_service_handle_source_cmd(struct mount_service *mo, > >>> const struct fuse_service_packet *p) > >>> { > >>> @@ -714,6 +759,21 @@ static int mount_service_handle_source_cmd(struct mount_service *mo, > >>> return mount_service_send_reply(mo, error); > >>> } > >>> > >>> +#ifdef HAVE_NEW_MOUNT_API > >>> + if (mo->fsopenfd >= 0) { > >>> + int ret = fsconfig(mo->fsopenfd, FSCONFIG_SET_STRING, "source", > >>> + oc->value, 0); > >>> + if (ret) { > >>> + int error = errno; > >>> + > >>> + fprintf(stderr, "%s: fsconfig source: %s\n", > >>> + mo->msgtag, strerror(error)); > >>> + emit_fsconfig_messages(mo); > >>> + return mount_service_send_reply(mo, error); > >>> + } > >>> + } > >>> +#endif > >>> + > >>> return mount_service_send_reply(mo, 0); > >>> } > >>> > >>> @@ -722,6 +782,8 @@ static int mount_service_handle_mntopts_cmd(struct mount_service *mo, > >>> { > >>> struct fuse_service_string_command *oc = > >>> container_of(p, struct fuse_service_string_command, p); > >>> + char *tokstr = oc->value; > >>> + char *tok, *savetok; > >>> > >>> if (mo->mntopts) { > >>> fprintf(stderr, "%s: mount options respecified!\n", > >>> @@ -738,6 +800,45 @@ static int mount_service_handle_mntopts_cmd(struct mount_service *mo, > >>> return mount_service_send_reply(mo, error); > >>> } > >>> > >>> + /* strtok_r mutates tokstr aka oc->value */ > >>> + while ((tok = strtok_r(tokstr, ",", &savetok)) != NULL) { > >>> + char *equals = strchr(tok, '='); > >>> + char oldchar = 0; > >>> + > >>> + if (equals) { > >>> + oldchar = *equals; > >>> + *equals = 0; > >>> + } > >>> + > >>> +#ifdef HAVE_NEW_MOUNT_API > >>> + if (mo->fsopenfd >= 0) { > >>> + int ret; > >>> + > >>> + if (equals) > >>> + ret = fsconfig(mo->fsopenfd, > >>> + FSCONFIG_SET_STRING, tok, > >>> + equals + 1, 0); > >>> + else > >>> + ret = fsconfig(mo->fsopenfd, > >>> + FSCONFIG_SET_FLAG, tok, > >>> + NULL, 0); > >>> + if (ret) { > >>> + int error = errno; > >>> + > >>> + fprintf(stderr, "%s: set mount option: %s\n", > >>> + mo->msgtag, strerror(error)); > >>> + emit_fsconfig_messages(mo); > >>> + return mount_service_send_reply(mo, error); > >>> + } > >>> + } > >>> +#endif > >>> + > >>> + if (equals) > >>> + *equals = oldchar; > >>> + > >>> + tokstr = NULL; > >>> + } > >>> + > >>> return mount_service_send_reply(mo, 0); > >>> } > >>> > >>> @@ -875,6 +976,196 @@ static int mount_service_regular_mount(struct mount_service *mo, > >>> return mount_service_send_reply(mo, 0); > >>> } > >>> > >>> +#ifdef HAVE_NEW_MOUNT_API > >>> +struct ms_to_mount_map { > >>> + unsigned long ms_flag; > >>> + 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; > >>> + 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; > >>> + } > >>> + > >>> + *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; > >>> + int ret; > >>> + > >>> + for (i = strflags; i->ms_flag != 0; i++) { > >>> + if (!(ms_flags & i->ms_flag)) > >>> + continue; > >>> + > >>> + ret = fsconfig(mo->fsopenfd, FSCONFIG_SET_FLAG, i->string, > >>> + NULL, 0); > >>> + if (ret) { > >>> + int error = errno; > >>> + > >>> + fprintf(stderr, "%s: set %s option: %s\n", > >>> + mo->msgtag, i->string, strerror(error)); > >>> + emit_fsconfig_messages(mo); > >>> + > >>> + errno = error; > >>> + return -1; > >>> + } > >>> + ms_flags &= ~i->ms_flag; > >>> + } > >>> + > >>> + /* > >>> + * We can't translate all the supplied MS_ flags into MOUNT_ATTR_ flags > >>> + * or string flags! Return a magic code so the caller will fall back > >>> + * to regular mount(2). > >>> + */ > >>> + return ms_flags ? -2 : 0; > >>> +} > >> > >> Let's please not forget later on to use the new define. > > > > Er... which new define? HAVE_NEW_MOUNT_API? I think we're still inside > > the #ifdef for that at this point. > > > > > > > Sorry, I meant FUSE_MOUNT_FALLBACK_NEEDED. Fixed. --D