Re: [PATCH v3 1/3] NFSD: Added fault injection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 
>> 
>>> - 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.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com




--
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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux