* [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not used.
@ 2024-04-16 21:23 NeilBrown
2024-04-17 5:25 ` Petr Vorel
0 siblings, 1 reply; 5+ messages in thread
From: NeilBrown @ 2024-04-16 21:23 UTC (permalink / raw)
To: Chuck Lever, Jeff Layton, Dai Ngo, Tom Talpey; +Cc: linux-nfs, Petr Vorel
When CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set, the virtual file
/proc/fs/nfsd/nfsv4recoverydir
is created but responds EINVAL to any access.
This is not useful, is somewhat surprising, and it causes ltp to
complain.
The only known user of this file is in nfs-utils, which handles
non-existence and read-failure equally well. So there is nothing to
gain from leaving the file present but inaccessible.
So this patch removes the file when its content is not available - i.e.
when that config option is not selected.
Also remove the #ifdef which hides some of the enum values when
CONFIG_NFSD_V$ not selection. simple_fill_super() quietly ignores array
entries that are not present, so having slots in the array that don't
get used is perfectly acceptable. So there is no value in this #ifdef.
Reported-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Fixes: 74fd48739d04 ("nfsd: new Kconfig option for legacy client tracking")
Signed-off-by: NeilBrown <neilb@suse.de>
---
fs/nfsd/nfsctl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 93c87587e646..340c5d61f199 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -48,12 +48,10 @@ enum {
NFSD_MaxBlkSize,
NFSD_MaxConnections,
NFSD_Filecache,
-#ifdef CONFIG_NFSD_V4
NFSD_Leasetime,
NFSD_Gracetime,
NFSD_RecoveryDir,
NFSD_V4EndGrace,
-#endif
NFSD_MaxReserved
};
@@ -1360,7 +1358,9 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc)
#ifdef CONFIG_NFSD_V4
[NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR},
[NFSD_Gracetime] = {"nfsv4gracetime", &transaction_ops, S_IWUSR|S_IRUSR},
+#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
[NFSD_RecoveryDir] = {"nfsv4recoverydir", &transaction_ops, S_IWUSR|S_IRUSR},
+#endif
[NFSD_V4EndGrace] = {"v4_end_grace", &transaction_ops, S_IWUSR|S_IRUGO},
#endif
/* last one */ {""}
--
2.44.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not used.
2024-04-16 21:23 [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not used NeilBrown
@ 2024-04-17 5:25 ` Petr Vorel
2024-04-17 16:28 ` Chuck Lever
0 siblings, 1 reply; 5+ messages in thread
From: Petr Vorel @ 2024-04-17 5:25 UTC (permalink / raw)
To: NeilBrown; +Cc: Chuck Lever, Jeff Layton, Dai Ngo, Tom Talpey, linux-nfs
Hi Neil, all,
> When CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set, the virtual file
> /proc/fs/nfsd/nfsv4recoverydir
> is created but responds EINVAL to any access.
> This is not useful, is somewhat surprising, and it causes ltp to
> complain.
> The only known user of this file is in nfs-utils, which handles
> non-existence and read-failure equally well. So there is nothing to
> gain from leaving the file present but inaccessible.
> So this patch removes the file when its content is not available - i.e.
> when that config option is not selected.
> Also remove the #ifdef which hides some of the enum values when
> CONFIG_NFSD_V$ not selection. simple_fill_super() quietly ignores array
> entries that are not present, so having slots in the array that don't
> get used is perfectly acceptable. So there is no value in this #ifdef.
> Reported-by: Petr Vorel <pvorel@suse.cz>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> Fixes: 74fd48739d04 ("nfsd: new Kconfig option for legacy client tracking")
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfsctl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 93c87587e646..340c5d61f199 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -48,12 +48,10 @@ enum {
> NFSD_MaxBlkSize,
> NFSD_MaxConnections,
> NFSD_Filecache,
> -#ifdef CONFIG_NFSD_V4
> NFSD_Leasetime,
> NFSD_Gracetime,
> NFSD_RecoveryDir,
> NFSD_V4EndGrace,
> -#endif
> NFSD_MaxReserved
> };
> @@ -1360,7 +1358,9 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc)
> #ifdef CONFIG_NFSD_V4
> [NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR},
> [NFSD_Gracetime] = {"nfsv4gracetime", &transaction_ops, S_IWUSR|S_IRUSR},
> +#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
> [NFSD_RecoveryDir] = {"nfsv4recoverydir", &transaction_ops, S_IWUSR|S_IRUSR},
> +#endif
LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
> [NFSD_V4EndGrace] = {"v4_end_grace", &transaction_ops, S_IWUSR|S_IRUGO},
> #endif
> /* last one */ {""}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not used.
2024-04-17 5:25 ` Petr Vorel
@ 2024-04-17 16:28 ` Chuck Lever
2024-04-17 18:33 ` Petr Vorel
2024-04-17 18:34 ` Petr Vorel
0 siblings, 2 replies; 5+ messages in thread
From: Chuck Lever @ 2024-04-17 16:28 UTC (permalink / raw)
To: NeilBrown; +Cc: Petr Vorel, Jeff Layton, Dai Ngo, Tom Talpey, linux-nfs
On Wed, Apr 17, 2024 at 07:25:16AM +0200, Petr Vorel wrote:
> Hi Neil, all,
>
> > When CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set, the virtual file
> > /proc/fs/nfsd/nfsv4recoverydir
> > is created but responds EINVAL to any access.
> > This is not useful, is somewhat surprising, and it causes ltp to
> > complain.
>
> > The only known user of this file is in nfs-utils, which handles
> > non-existence and read-failure equally well. So there is nothing to
> > gain from leaving the file present but inaccessible.
>
> > So this patch removes the file when its content is not available - i.e.
> > when that config option is not selected.
>
> > Also remove the #ifdef which hides some of the enum values when
> > CONFIG_NFSD_V$ not selection. simple_fill_super() quietly ignores array
> > entries that are not present, so having slots in the array that don't
> > get used is perfectly acceptable. So there is no value in this #ifdef.
>
> > Reported-by: Petr Vorel <pvorel@suse.cz>
> > Reviewed-by: Jeff Layton <jlayton@kernel.org>
> > Fixes: 74fd48739d04 ("nfsd: new Kconfig option for legacy client tracking")
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> > fs/nfsd/nfsctl.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index 93c87587e646..340c5d61f199 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -48,12 +48,10 @@ enum {
> > NFSD_MaxBlkSize,
> > NFSD_MaxConnections,
> > NFSD_Filecache,
> > -#ifdef CONFIG_NFSD_V4
> > NFSD_Leasetime,
> > NFSD_Gracetime,
> > NFSD_RecoveryDir,
> > NFSD_V4EndGrace,
> > -#endif
> > NFSD_MaxReserved
> > };
>
> > @@ -1360,7 +1358,9 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc)
> > #ifdef CONFIG_NFSD_V4
> > [NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR},
> > [NFSD_Gracetime] = {"nfsv4gracetime", &transaction_ops, S_IWUSR|S_IRUSR},
> > +#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
> > [NFSD_RecoveryDir] = {"nfsv4recoverydir", &transaction_ops, S_IWUSR|S_IRUSR},
> > +#endif
>
> LGTM.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr
>
> > [NFSD_V4EndGrace] = {"v4_end_grace", &transaction_ops, S_IWUSR|S_IRUGO},
> > #endif
> > /* last one */ {""}
Thanks to you both! Applied to nfsd-next (for v6.10), unless you'd
like this to go in sooner.
--
Chuck Lever
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not used.
2024-04-17 16:28 ` Chuck Lever
@ 2024-04-17 18:33 ` Petr Vorel
2024-04-17 18:34 ` Petr Vorel
1 sibling, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2024-04-17 18:33 UTC (permalink / raw)
To: Chuck Lever; +Cc: NeilBrown, Jeff Layton, Dai Ngo, Tom Talpey, linux-nfs
> On Wed, Apr 17, 2024 at 07:25:16AM +0200, Petr Vorel wrote:
> > Hi Neil, all,
> > > When CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set, the virtual file
> > > /proc/fs/nfsd/nfsv4recoverydir
> > > is created but responds EINVAL to any access.
> > > This is not useful, is somewhat surprising, and it causes ltp to
> > > complain.
> > > The only known user of this file is in nfs-utils, which handles
> > > non-existence and read-failure equally well. So there is nothing to
> > > gain from leaving the file present but inaccessible.
> > > So this patch removes the file when its content is not available - i.e.
> > > when that config option is not selected.
> > > Also remove the #ifdef which hides some of the enum values when
> > > CONFIG_NFSD_V$ not selection. simple_fill_super() quietly ignores array
> > > entries that are not present, so having slots in the array that don't
> > > get used is perfectly acceptable. So there is no value in this #ifdef.
> > > Reported-by: Petr Vorel <pvorel@suse.cz>
> > > Reviewed-by: Jeff Layton <jlayton@kernel.org>
> > > Fixes: 74fd48739d04 ("nfsd: new Kconfig option for legacy client tracking")
> > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > ---
> > > fs/nfsd/nfsctl.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index 93c87587e646..340c5d61f199 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> > > @@ -48,12 +48,10 @@ enum {
> > > NFSD_MaxBlkSize,
> > > NFSD_MaxConnections,
> > > NFSD_Filecache,
> > > -#ifdef CONFIG_NFSD_V4
> > > NFSD_Leasetime,
> > > NFSD_Gracetime,
> > > NFSD_RecoveryDir,
> > > NFSD_V4EndGrace,
> > > -#endif
> > > NFSD_MaxReserved
> > > };
> > > @@ -1360,7 +1358,9 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc)
> > > #ifdef CONFIG_NFSD_V4
> > > [NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR},
> > > [NFSD_Gracetime] = {"nfsv4gracetime", &transaction_ops, S_IWUSR|S_IRUSR},
> > > +#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
> > > [NFSD_RecoveryDir] = {"nfsv4recoverydir", &transaction_ops, S_IWUSR|S_IRUSR},
> > > +#endif
> > LGTM.
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Kind regards,
> > Petr
> > > [NFSD_V4EndGrace] = {"v4_end_grace", &transaction_ops, S_IWUSR|S_IRUGO},
> > > #endif
> > > /* last one */ {""}
> Thanks to you both! Applied to nfsd-next (for v6.10), unless you'd
> like this to go in sooner.
OK for me, I guess LTP testers can wait.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not used.
2024-04-17 16:28 ` Chuck Lever
2024-04-17 18:33 ` Petr Vorel
@ 2024-04-17 18:34 ` Petr Vorel
1 sibling, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2024-04-17 18:34 UTC (permalink / raw)
To: Chuck Lever; +Cc: NeilBrown, Jeff Layton, Dai Ngo, Tom Talpey, linux-nfs
> On Wed, Apr 17, 2024 at 07:25:16AM +0200, Petr Vorel wrote:
> > Hi Neil, all,
> > > When CONFIG_NFSD_LEGACY_CLIENT_TRACKING is not set, the virtual file
> > > /proc/fs/nfsd/nfsv4recoverydir
> > > is created but responds EINVAL to any access.
> > > This is not useful, is somewhat surprising, and it causes ltp to
> > > complain.
> > > The only known user of this file is in nfs-utils, which handles
> > > non-existence and read-failure equally well. So there is nothing to
> > > gain from leaving the file present but inaccessible.
> > > So this patch removes the file when its content is not available - i.e.
> > > when that config option is not selected.
> > > Also remove the #ifdef which hides some of the enum values when
> > > CONFIG_NFSD_V$ not selection. simple_fill_super() quietly ignores array
> > > entries that are not present, so having slots in the array that don't
> > > get used is perfectly acceptable. So there is no value in this #ifdef.
> > > Reported-by: Petr Vorel <pvorel@suse.cz>
> > > Reviewed-by: Jeff Layton <jlayton@kernel.org>
> > > Fixes: 74fd48739d04 ("nfsd: new Kconfig option for legacy client tracking")
> > > Signed-off-by: NeilBrown <neilb@suse.de>
> > > ---
> > > fs/nfsd/nfsctl.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > > index 93c87587e646..340c5d61f199 100644
> > > --- a/fs/nfsd/nfsctl.c
> > > +++ b/fs/nfsd/nfsctl.c
> > > @@ -48,12 +48,10 @@ enum {
> > > NFSD_MaxBlkSize,
> > > NFSD_MaxConnections,
> > > NFSD_Filecache,
> > > -#ifdef CONFIG_NFSD_V4
> > > NFSD_Leasetime,
> > > NFSD_Gracetime,
> > > NFSD_RecoveryDir,
> > > NFSD_V4EndGrace,
> > > -#endif
> > > NFSD_MaxReserved
> > > };
> > > @@ -1360,7 +1358,9 @@ static int nfsd_fill_super(struct super_block *sb, struct fs_context *fc)
> > > #ifdef CONFIG_NFSD_V4
> > > [NFSD_Leasetime] = {"nfsv4leasetime", &transaction_ops, S_IWUSR|S_IRUSR},
> > > [NFSD_Gracetime] = {"nfsv4gracetime", &transaction_ops, S_IWUSR|S_IRUSR},
> > > +#ifdef CONFIG_NFSD_LEGACY_CLIENT_TRACKING
> > > [NFSD_RecoveryDir] = {"nfsv4recoverydir", &transaction_ops, S_IWUSR|S_IRUSR},
> > > +#endif
> > LGTM.
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Kind regards,
> > Petr
> > > [NFSD_V4EndGrace] = {"v4_end_grace", &transaction_ops, S_IWUSR|S_IRUGO},
> > > #endif
> > > /* last one */ {""}
> Thanks to you both! Applied to nfsd-next (for v6.10), unless you'd
> like this to go in sooner.
Also thanks to all for a very quick fix.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-17 18:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-16 21:23 [PATCH] nfsd: don't create nfsv4recoverydir in nfsdfs when not used NeilBrown
2024-04-17 5:25 ` Petr Vorel
2024-04-17 16:28 ` Chuck Lever
2024-04-17 18:33 ` Petr Vorel
2024-04-17 18:34 ` Petr Vorel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).