On 09/30/2011 10:57 AM, Chuck Lever wrote: > > On Sep 30, 2011, at 10:38 AM, Bryan Schumaker wrote: > >> On 09/30/2011 10:28 AM, Chuck Lever wrote: >>> >>> On Sep 29, 2011, at 4:14 PM, Bryan Schumaker wrote: >>> >>>> On 09/29/2011 03:06 PM, Chuck Lever wrote: >>>>> >>>>> On Sep 29, 2011, at 2:59 PM, bjschuma@xxxxxxxxxx wrote: >>>>> >>>>>> From: Bryan Schumaker <bjschuma@xxxxxxxxxx> >>>>>> >>>> ... >>>>>> diff --git a/fs/nfsd/Makefile b/fs/nfsd/Makefile >>>>>> index 9b118ee..69eae75 100644 >>>>>> --- a/fs/nfsd/Makefile >>>>>> +++ b/fs/nfsd/Makefile >>>>>> @@ -5,7 +5,8 @@ >>>>>> obj-$(CONFIG_NFSD) += nfsd.o >>>>>> >>>>>> nfsd-y := nfssvc.o nfsctl.o nfsproc.o nfsfh.o vfs.o \ >>>>>> - export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o >>>>>> + export.o auth.o lockd.o nfscache.o nfsxdr.o stats.o \ >>>>>> + fault_inject.o >>>>>> nfsd-$(CONFIG_NFSD_V2_ACL) += nfs2acl.o >>>>>> nfsd-$(CONFIG_NFSD_V3) += nfs3proc.o nfs3xdr.o >>>>>> nfsd-$(CONFIG_NFSD_V3_ACL) += nfs3acl.o >>>>>> diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c >>>>>> new file mode 100644 >>>>>> index 0000000..2139883 >>>>>> --- /dev/null >>>>>> +++ b/fs/nfsd/fault_inject.c >>>>>> @@ -0,0 +1,102 @@ >>>>>> +#ifdef CONFIG_NFSD_FAULT_INJECTION >>>>> >>>>> The usual practice is to conditionally compile this source file via a Makefile variable, not by wrapping the whole source file in an ifdef. You can see examples of this in the Makefile hunk above. >>>>> >>>> >>>> How's this? I moved the selectively compile bit to the makefile and created a fault_inject.h for the empty init() and cleanup() functions. I moved the rest of the fault injection function declarations there, too, to keep them all together. >>> >>> OK. How about adding documenting block comments for the new files? Does NetApp require you to add a copyright notice at the top of new files? >> >> I have no idea if I'm required, but I can if it's needed. > > Ask Trond or Beepy. > >>> Also, I wonder if it would be better if you added "default N" for the new CONFIG option. >> >> What would trigger the default option? Right now I control this by writing a number into the debugfs file. I suppose reading from the file could trigger the default case. > > See below. Oh! I get what you mean now. I thought you meant "n" as in the number of things to delete, and not default the feature to off. Yeah, I can change that. > >> >>> >>>> - Bryan >>>> >>>> Fault injection on the NFS server makes it easier to test the client's >>>> state manager and recovery threads. Simulating errors on the server is >>>> easier than finding the right conditions that cause them naturally. >>>> >>>> This patch uses debugfs to add a simple framework for fault injection to >>>> the server. This framework is a config option, and can be enabled >>>> through CONFIG_NFSD_FAULT_INJECTION. Assuming you have debugfs mounted >>>> to /sys/debug, a set of files will be created in /sys/debug/nfsd/. >>>> Writing to any of these files will cause the corresponding action and >>>> write a log entry to dmesg. >>>> >>>> Changes in v4: >>>> - Move fault injection function declarations to a new .h file >>>> - Use the Makefile to selectively compile fault_injection.c >>>> >>>> Changes in v3: >>>> - Code cleanup and better use of generic functions >>>> - Allow the user to input the number of state objects to delete >>>> - Remove "forget_everything()" since forgetting a client is basically >>>> the same thing >>>> >>>> Changes in v2: >>>> - Replaced "forget all state owners" with "forget all open owners" >>>> - Include fs/nfsd/fault_inject.c in the patch >>>> >>>> Signed-off-by: Bryan Schumaker <bjschuma@xxxxxxxxxx> >>>> --- >>>> fs/nfsd/Kconfig | 10 ++++ >>>> fs/nfsd/Makefile | 1 + >>>> fs/nfsd/fault_inject.c | 88 ++++++++++++++++++++++++++++++++++++ >>>> fs/nfsd/fault_inject.h | 22 +++++++++ >>>> fs/nfsd/nfs4state.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++ >>>> fs/nfsd/nfsctl.c | 7 +++ >>>> 6 files changed, 243 insertions(+), 0 deletions(-) >>>> create mode 100644 fs/nfsd/fault_inject.c >>>> create mode 100644 fs/nfsd/fault_inject.h >>>> >>>> diff --git a/fs/nfsd/Kconfig b/fs/nfsd/Kconfig >>>> index 10e6366..52fdd1c 100644 >>>> --- a/fs/nfsd/Kconfig >>>> +++ b/fs/nfsd/Kconfig >>>> @@ -80,3 +80,13 @@ config NFSD_V4 >>>> available from http://linux-nfs.org/. >>>> >>>> If unsure, say N. >>>> + >>>> +config NFSD_FAULT_INJECTION >>>> + bool "NFS server manual fault injection" >>>> + depends on NFSD_V4 && DEBUG_KERNEL > > default N > >>>> + help >>>> + This option enables support for manually injectiong faults >>>> + into the NFS server. This is intended to be used for >>>> + testing error recovery on the NFS client. >>>> + >>>> + If unsure, say N. > > I'm not sure that would even work. But the idea is to prevent "make oldconfig" from even asking. It should just be N unless it's explicitly set via the menu or an architecture config file. > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
