From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from e2i340.smtp2go.com (e2i340.smtp2go.com [103.2.141.84]) (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 EED0C34028F for ; Wed, 26 Nov 2025 20:32:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.2.141.84 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764189127; cv=none; b=udmBnRvaCDYndFxxQaGZ/2OMhfTQ+FedKzQcVbjt17XhWyitTnMWlkPGhvb/jojWsUVnAyA1jkfQ6HXPV8/paQbniWxGcE55o04JOP74qpKM2iaxmBeZwlcGBVgxItQjYz1XctX+GWY85UJHUSTqUNubXYCzT4BF7JP8mWJOktE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764189127; c=relaxed/simple; bh=kt0GqCOQ8nKevzgl1DYg31XjX+YvCvRrK1pWPVUWsdc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=e+8YJAMbJgEBMOhhHF+MNjUgbnQnqquDVNirBc8NRrk8mUEeJbrrfrIhwoYp4OWVVFk2QPipjhRYtly97aZfN3nFZw4u4OBE8JeNq7q8PLhF//e7EXxvGpx1rBWX40Gu117GFwEAY1LdKS6OHm0oEn12a9Gq6knF7EV+Tu4Th4w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=triplefau.lt; spf=pass smtp.mailfrom=em510616.triplefau.lt; dkim=fail (0-bit key) header.d=smtpservice.net header.i=@smtpservice.net header.b=KHFegGCo reason="unknown key version"; dkim=pass (2048-bit key) header.d=triplefau.lt header.i=@triplefau.lt header.b=hGFLIFKJ; arc=none smtp.client-ip=103.2.141.84 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=triplefau.lt Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=em510616.triplefau.lt Authentication-Results: smtp.subspace.kernel.org; dkim=fail reason="unknown key version" (0-bit key) header.d=smtpservice.net header.i=@smtpservice.net header.b="KHFegGCo"; dkim=pass (2048-bit key) header.d=triplefau.lt header.i=@triplefau.lt header.b="hGFLIFKJ" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=smtpservice.net; s=maxzs0.a1-4.dyn; x=1764190025; h=Feedback-ID: X-Smtpcorp-Track:Message-ID:Subject:To:From:Date:Reply-To:Sender: List-Unsubscribe:List-Unsubscribe-Post; bh=oo0M4ydtK5Fa6pnW+MiV3ZxPH6O6l9/HYBAD3BWh0eE=; b=KHFegGCoU3JmwBzWs0dnlSFHl2 jCNuqzKnRT1NVCZSC2WMC11zm0XGk6Tdt6L8mfsXt1nlf+hrAbYeO6lgTJZdxN7BtuheSqxsCJgNh fG0yMnWVPyQfrENsyD3YEeuyqZLIfCXAPhFgehagaae5v5iyfJWgiSwngeGV/Wj3jmnFbdVAXsJGD HwT0nSHwc5zyHjYRFGWbjeUSJqRmmMSPfip5Y8zjNLiCDq4nu2ehHaPq0BWoX4d1o5IrvmCW5avWA 9eWdX0egEVn8s+XkC6INRNVJ/HkN8EuutePk5ClrTTvDxWK0sbB3VmYElvi7mSJj9msTdRLeSSsoW jsVrRWYA==; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=triplefau.lt; i=@triplefau.lt; q=dns/txt; s=s510616; t=1764189125; h=from : subject : to : message-id : date; bh=oo0M4ydtK5Fa6pnW+MiV3ZxPH6O6l9/HYBAD3BWh0eE=; b=hGFLIFKJcV32VR/8pjI4D4tLTjQ86eABgHBvoteu0+zA9MBtSGi3CIdqgqECeUv2Ma3nF ftbu4JJqKv2xsYTZi0IvLoPbxvMW/bwVJpHxjsGB55AFvicah7Xkn64XGiJkn02XcqRWAIp xP6taw1e+G6Udz6Bam62grAzG1w8jfyf4qnC1nOfouoeaxk4y9Z4aghK333ZS9PaCtlTDDe NpmnCaHn+CHOTBNhpkG9ZkGTwT6xCUtZKME1u0QqMKTShHSrM3TyOxLsJ8p1UQA+QsVlX9c Pt9OyHJZ5K/yrmQfE54BXtSLruPZNktyehdMcPKHs8caYLwGoaB4OzgU5ZMw== Received: from [10.139.162.187] (helo=SmtpCorp) by smtpcorp.com with esmtpsa (TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.94.2-S2G) (envelope-from ) id 1vOMAr-TRk23d-DV; Wed, 26 Nov 2025 20:31:37 +0000 Received: from [10.12.239.196] (helo=localhost) by smtpcorp.com with esmtpsa (TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.98.1-S2G) (envelope-from ) id 1vOMAq-4o5NDgrnPxr-mjHh; Wed, 26 Nov 2025 20:31:36 +0000 Date: Wed, 26 Nov 2025 21:16:14 +0100 From: Remi Pommarel To: Dominique Martinet Cc: Eric Sandeen , v9fs@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, ericvh@kernel.org, lucho@ionkov.net, linux_oss@crudebyte.com, eadavis@qq.com Subject: Re: [PATCH V3 4/4] 9p: convert to the new mount API Message-ID: References: <20251010214222.1347785-1-sandeen@redhat.com> <20251010214222.1347785-5-sandeen@redhat.com> Precedence: bulk X-Mailing-List: linux-kernel@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: X-Smtpcorp-Track: qCyjUTQcITFx.Aiez4odjWQI7.aFleOwev4cn Feedback-ID: 510616m:510616apGKSTK:510616sykM-zowXn X-Report-Abuse: Please forward a copy of this message, including all headers, to Hi Eric, Dominique, On Mon, Oct 13, 2025 at 07:26:35PM +0900, Dominique Martinet wrote: > Hi Eric, > > Thanks for this V3! > > I find it much cleaner, hopefully will be easier to debug :) > ... Which turned out to be needed right away, trying with qemu's 9p > export "mount -t 9p -o trans=virtio tmp /mnt" apparently calls > p9_virtio_create() with fc->source == NULL, instead of the expected > "tmp" string > (FWIW I tried '-o trans=tcp 127.0.0.1' and I got the same problem in > p9_fd_create_tcp(), might be easier to test with diod if that's what you > used) > > Looking at other filesystems (e.g. fs/nfs/fs_context.c but others are > the same) it looks like they all define a fsparam_string "source" option > explicitly?... > > Something like this looks like it works to do (+ probably make the error > more verbose? nothing in dmesg hints at why mount returns EINVAL...) > ----- > diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c > index 6c07635f5776..999d54a0c7d9 100644 > --- a/fs/9p/v9fs.c > +++ b/fs/9p/v9fs.c > @@ -34,6 +34,8 @@ struct kmem_cache *v9fs_inode_cache; > */ > > enum { > + /* Mount-point source */ > + Opt_source, > /* Options that take integer arguments */ > Opt_debug, Opt_dfltuid, Opt_dfltgid, Opt_afid, > /* String options */ > @@ -82,6 +84,7 @@ static const struct constant_table p9_cache_mode[] = { > * the client, and all the transports. > */ > const struct fs_parameter_spec v9fs_param_spec[] = { > + fsparam_string ("source", Opt_source), > fsparam_u32hex ("debug", Opt_debug), > fsparam_uid ("dfltuid", Opt_dfltuid), > fsparam_gid ("dfltgid", Opt_dfltgid), > @@ -210,6 +213,14 @@ int v9fs_parse_param(struct fs_context *fc, struct fs_parameter *param) > } > > switch (opt) { > + case Opt_source: > + if (fc->source) { > + pr_info("p9: multiple sources not supported\n"); > + return -EINVAL; > + } > + fc->source = param->string; > + param->string = NULL; > + break; > case Opt_debug: > session_opts->debug = result.uint_32; > #ifdef CONFIG_NET_9P_DEBUG > ----- 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. 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? 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. ---- diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c index a58abe69ec7a..e4e8304e5934 100644 --- a/fs/9p/v9fs.c +++ b/fs/9p/v9fs.c @@ -120,6 +120,21 @@ const struct fs_parameter_spec v9fs_param_spec[] = { {} }; +static char const *p9_cache_to_str(enum p9_cache_shortcuts sc) +{ + char const *cache = "none"; + int i; + + for (i = 0; i < ARRAY_SIZE(p9_cache_mode); ++i) { + if (p9_cache_mode[i].value == sc) { + cache = p9_cache_mode[i].name; + break; + } + } + + return cache; +} + /* * Display the mount options in /proc/mounts. */ @@ -144,7 +159,7 @@ int v9fs_show_options(struct seq_file *m, struct dentry *root) if (v9ses->nodev) seq_puts(m, ",nodevmap"); if (v9ses->cache) - seq_printf(m, ",cache=%x", v9ses->cache); + seq_printf(m, ",cache=%s", p9_cache_to_str(v9ses->cache)); #ifdef CONFIG_9P_FSCACHE if (v9ses->cachetag && (v9ses->cache & CACHE_FSCACHE)) seq_printf(m, ",cachetag=%s", v9ses->cachetag); ---- 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? Thanks, -- Remi