Regression tests for CIFS client

Regression tests for CIFS client

[Suresh Jayaraman]

Hi everyone,

The CIFS client never had proper regression tests and we try to tweak
the connectathon tests a little bit and run to avoid regressions
creeping in. I think a testsuite that checks for regressions would be
useful.

I spent sometime hacking up a set of tests. These tests use Python and
PyUnit framework which I think might help quickly adding newer tests.
Tests are written based on the past bug reports and experiences.

The primary intent of the tests is to provide some basic infrastructure
upon which tests can be added easily in the future and these tests are
by no means comprehensive. I have tried to avoid duplicating tests
already done by Connectathon and other tests but there could still be a
few duplicates in there. The tests are only lightly tested.

Currently cifstests is hosted here:

   https://github.com/sureshjayaram/cifstests

Feel free to try out and let me know your feedback or any comments and
suggestions.

Here's are the failures seen with 3.1 based kernel. I think the open()
with O_DIRECT is expected to fail since cifs doesn't support it (I'd be
interesting in knowing exact details). But, I've not dig into the xattr
tests, not sure why setattr is failing (even if CIFS_XATTR is set, fs
mounted with user_xattr).

Test Output
============

======================================================================
ERROR: test_directIO (__main__.OpenTests)
open a file with O_DIRECT
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./testcifs.py", line 144, in test_directIO
    raise e
OSError: [Errno 22] Invalid argument: 'testfile'

======================================================================
ERROR: test_dir_attr (__main__.XattrTests)
set attrs, get attrs and remove attrs for a dir
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./testcifs.py", line 364, in test_dir_attr
    raise e
IOError: [Errno 95] Operation not supported: 'test'

======================================================================
ERROR: test_file_attr (__main__.XattrTests)
set attrs, get attrs and remove attrs for a file
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./testcifs.py", line 343, in test_file_attr
    raise e
IOError: [Errno 95] Operation not supported: 'testfile'

----------------------------------------------------------------------
Ran 22 tests in 0.072s

FAILED (errors=3)


What do you think? Is it be a good idea?

I know I have barely scratched the surface, but any suggestion on having
a working regression test is welcome. And of course, regression tests
are supposed to evolve over time. Would this be a convenient way to add
more tests?


Thanks
Suresh

Re: Regression tests for CIFS client

[Jeff Layton]

On Wed, 08 Aug 2012 17:31:09 +0530
Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:

> Hi everyone,
> 
> The CIFS client never had proper regression tests and we try to tweak
> the connectathon tests a little bit and run to avoid regressions
> creeping in. I think a testsuite that checks for regressions would be
> useful.
> 
> I spent sometime hacking up a set of tests. These tests use Python and
> PyUnit framework which I think might help quickly adding newer tests.
> Tests are written based on the past bug reports and experiences.
> 
> The primary intent of the tests is to provide some basic infrastructure
> upon which tests can be added easily in the future and these tests are
> by no means comprehensive. I have tried to avoid duplicating tests
> already done by Connectathon and other tests but there could still be a
> few duplicates in there. The tests are only lightly tested.
> 
> Currently cifstests is hosted here:
> 
>    https://github.com/sureshjayaram/cifstests
> 
> Feel free to try out and let me know your feedback or any comments and
> suggestions.
> 
> Here's are the failures seen with 3.1 based kernel. I think the open()
> with O_DIRECT is expected to fail since cifs doesn't support it (I'd be
> interesting in knowing exact details). But, I've not dig into the xattr
> tests, not sure why setattr is failing (even if CIFS_XATTR is set, fs
> mounted with user_xattr).
> 
> Test Output
> ============
> 
> ======================================================================
> ERROR: test_directIO (__main__.OpenTests)
> open a file with O_DIRECT
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "./testcifs.py", line 144, in test_directIO
>     raise e
> OSError: [Errno 22] Invalid argument: 'testfile'
> 

With all of the recent changes to the read/write code, I think we can
reasonably do O_DIRECT now. Just make sure that you don't request an
oplock on open and ensure that you're not using cache=loose codepaths.

In point of fact, we do some bounce buffering under the covers, but it
does avoid the pagecache.

> ======================================================================
> ERROR: test_dir_attr (__main__.XattrTests)
> set attrs, get attrs and remove attrs for a dir
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "./testcifs.py", line 364, in test_dir_attr
>     raise e
> IOError: [Errno 95] Operation not supported: 'test'
> 
> ======================================================================
> ERROR: test_file_attr (__main__.XattrTests)
> set attrs, get attrs and remove attrs for a file
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "./testcifs.py", line 343, in test_file_attr
>     raise e
> IOError: [Errno 95] Operation not supported: 'testfile'
> 
> ----------------------------------------------------------------------
> Ran 22 tests in 0.072s
> 
> FAILED (errors=3)
> 

The above I'm not sure about -- maybe depends on which attributes
you're trying to set?

> 
> What do you think? Is it be a good idea?
> 
> I know I have barely scratched the surface, but any suggestion on having
> a working regression test is welcome. And of course, regression tests
> are supposed to evolve over time. Would this be a convenient way to add
> more tests?
> 

Sounds like a very worthwhile endeavor. The key to any testing
infrastructure is to make it very easy to run the tests. Any hassle in
setting it up is a reason not to do so, so you want to make sure there
are no such barriers.

You also want to make sure that you have the ability to drill down into
a single test failure without needing to run a bunch of goop around it.
The cthon suite is good for this since most of the tests are written in
C. That makes it easy to strace them to track down problems. I assume
we'll be able to write tests in C and just have the python framework
call them?

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: Regression tests for CIFS client

[Steve French]

On Wed, Aug 8, 2012 at 7:41 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Wed, 08 Aug 2012 17:31:09 +0530
> Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:
>
>> Hi everyone,
>>
>> The CIFS client never had proper regression tests and we try to tweak
>> the connectathon tests a little bit and run to avoid regressions
>> creeping in. I think a testsuite that checks for regressions would be
>> useful.
>>
>> I spent sometime hacking up a set of tests. These tests use Python and
>> PyUnit framework which I think might help quickly adding newer tests.
>> Tests are written based on the past bug reports and experiences.
>>
>> The primary intent of the tests is to provide some basic infrastructure
>> upon which tests can be added easily in the future and these tests are
>> by no means comprehensive. I have tried to avoid duplicating tests
>> already done by Connectathon and other tests but there could still be a
>> few duplicates in there. The tests are only lightly tested.
>>
>> Currently cifstests is hosted here:
>>
>>    https://github.com/sureshjayaram/cifstests
>>
>> Feel free to try out and let me know your feedback or any comments and
>> suggestions.
>>
>> Here's are the failures seen with 3.1 based kernel. I think the open()
>> with O_DIRECT is expected to fail since cifs doesn't support it (I'd be
>> interesting in knowing exact details). But, I've not dig into the xattr
>> tests, not sure why setattr is failing (even if CIFS_XATTR is set, fs
>> mounted with user_xattr).
>>
>> Test Output
>> ============
>>
>> ======================================================================
>> ERROR: test_directIO (__main__.OpenTests)
>> open a file with O_DIRECT
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "./testcifs.py", line 144, in test_directIO
>>     raise e
>> OSError: [Errno 22] Invalid argument: 'testfile'
>>
>
> With all of the recent changes to the read/write code, I think we can
> reasonably do O_DIRECT now. Just make sure that you don't request an
> oplock on open and ensure that you're not using cache=loose codepaths.
>
> In point of fact, we do some bounce buffering under the covers, but it
> does avoid the pagecache.
>
>> ======================================================================
>> ERROR: test_dir_attr (__main__.XattrTests)
>> set attrs, get attrs and remove attrs for a dir
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "./testcifs.py", line 364, in test_dir_attr
>>     raise e
>> IOError: [Errno 95] Operation not supported: 'test'
>>
>> ======================================================================
>> ERROR: test_file_attr (__main__.XattrTests)
>> set attrs, get attrs and remove attrs for a file
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "./testcifs.py", line 343, in test_file_attr
>>     raise e
>> IOError: [Errno 95] Operation not supported: 'testfile'
>>
>> ----------------------------------------------------------------------
>> Ran 22 tests in 0.072s
>>
>> FAILED (errors=3)
>>
>
> The above I'm not sure about -- maybe depends on which attributes
> you're trying to set?
>
>>
>> What do you think? Is it be a good idea?
>>
>> I know I have barely scratched the surface, but any suggestion on having
>> a working regression test is welcome. And of course, regression tests
>> are supposed to evolve over time. Would this be a convenient way to add
>> more tests?
>>
>
> Sounds like a very worthwhile endeavor. The key to any testing
> infrastructure is to make it very easy to run the tests. Any hassle in
> setting it up is a reason not to do so, so you want to make sure there
> are no such barriers.
>
> You also want to make sure that you have the ability to drill down into
> a single test failure without needing to run a bunch of goop around it.
> The cthon suite is good for this since most of the tests are written in
> C. That makes it easy to strace them to track down problems. I assume
> we'll be able to write tests in C and just have the python framework
> call them?

I like the idea of something expandable that we can add tests to (e.g.
for cifs specific mount options, or to add tests to cover recently
reported bugs).

I have patches to run xfstests over cifs, but I mostly run
connectathon and dbench and fsx.  Ideally running regression tests
could be triggered by commits to a staging tree (or branch of
cifs-2.6.git) on git.samba.org although not certain how easy this
would be to setup (unlike Ganesha and Samba we may have to reboot a VM
to load an updated kernel).


-- 
Thanks,

Steve

Re: Regression tests for CIFS client

[Suresh Jayaraman]

On 08/08/2012 07:20 PM, Steve French wrote:
> On Wed, Aug 8, 2012 at 7:41 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On Wed, 08 Aug 2012 17:31:09 +0530
>> Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:

>>>
>>> The primary intent of the tests is to provide some basic infrastructure
>>> upon which tests can be added easily in the future and these tests are
>>> by no means comprehensive. I have tried to avoid duplicating tests
>>> already done by Connectathon and other tests but there could still be a
>>> few duplicates in there. The tests are only lightly tested.
>>>
>>> Currently cifstests is hosted here:
>>>
>>>    https://github.com/sureshjayaram/cifstests
>>>
>>
>> Sounds like a very worthwhile endeavor. The key to any testing
>> infrastructure is to make it very easy to run the tests. Any hassle in
>> setting it up is a reason not to do so, so you want to make sure there
>> are no such barriers.
>>
>> You also want to make sure that you have the ability to drill down into
>> a single test failure without needing to run a bunch of goop around it.
>> The cthon suite is good for this since most of the tests are written in
>> C. That makes it easy to strace them to track down problems. I assume
>> we'll be able to write tests in C and just have the python framework
>> call them?
> 
> I like the idea of something expandable that we can add tests to (e.g.
> for cifs specific mount options, or to add tests to cover recently
> reported bugs).

Yeah, one of the reasons to keep these tests separate and not adding to
xfstests or other is it, it will give us more flexibility to add CIFS
specific tests without doing a lot of testing on other filesystems.

> I have patches to run xfstests over cifs, but I mostly run
> connectathon and dbench and fsx.  Ideally running regression tests
> could be triggered by commits to a staging tree (or branch of
> cifs-2.6.git) on git.samba.org although not certain how easy this

While that would be ideal, I think if we could run such tests atleast
twice during a release cycle (let's say once during -rc1 and once during
final rc), it would still help us to catch regressions.


Thanks
Suresh

Re: Regression tests for CIFS client

[Suresh Jayaraman]

On 08/08/2012 06:11 PM, Jeff Layton wrote:
> On Wed, 08 Aug 2012 17:31:09 +0530
> Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:
> 
>> Here's are the failures seen with 3.1 based kernel. I think the open()
>> with O_DIRECT is expected to fail since cifs doesn't support it (I'd be
>> interesting in knowing exact details). But, I've not dig into the xattr
>> tests, not sure why setattr is failing (even if CIFS_XATTR is set, fs
>> mounted with user_xattr).
>>
>> Test Output
>> ============
>>
>> ======================================================================
>> ERROR: test_directIO (__main__.OpenTests)
>> open a file with O_DIRECT
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "./testcifs.py", line 144, in test_directIO
>>     raise e
>> OSError: [Errno 22] Invalid argument: 'testfile'
>>
> 
> With all of the recent changes to the read/write code, I think we can
> reasonably do O_DIRECT now. Just make sure that you don't request an
> oplock on open and ensure that you're not using cache=loose codepaths.
> 

Hmm, I'm not sure yet what is going on wrong.. I disabled oplocks by doing

   echo N > /sys/module/cifs/parameters/enable_oplocks

(Is there a way for an application to not request oplocks?)
and mounted with cache=strict and ran the directIO test alone.

strace output (snip)
--------------
stat("/mnt/cifstests", 0x7fff317e68f0)  = -1 ENOENT (No such file or
directory)
mkdir("cifstests", 0755)                = 0
chdir("cifstests")                      = 0
open("testfile", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
close(3)                                = 0
open("testfile", O_RDONLY|O_DIRECT)     = -1 EINVAL (Invalid argument)
write(1, "[Errno 22] Invalid argument: 'te"..., 40) = 40

But I see from dmesg oplocks are being granted.

> In point of fact, we do some bounce buffering under the covers, but it
> does avoid the pagecache.
> 
>> ======================================================================
>> ERROR: test_dir_attr (__main__.XattrTests)
>> set attrs, get attrs and remove attrs for a dir
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "./testcifs.py", line 364, in test_dir_attr
>>     raise e
>> IOError: [Errno 95] Operation not supported: 'test'
>>
>> ======================================================================
>> ERROR: test_file_attr (__main__.XattrTests)
>> set attrs, get attrs and remove attrs for a file
>> ----------------------------------------------------------------------
>> Traceback (most recent call last):
>>   File "./testcifs.py", line 343, in test_file_attr
>>     raise e
>> IOError: [Errno 95] Operation not supported: 'testfile'
>>
>> ----------------------------------------------------------------------
>> Ran 22 tests in 0.072s
>>
>> FAILED (errors=3)
>>
> 
> The above I'm not sure about -- maybe depends on which attributes
> you're trying to set?

I was trying to set something like:

	user.mime_type = 'text/plain'

(the same worked on ext4)

>>
>> What do you think? Is it be a good idea?
>>
>> I know I have barely scratched the surface, but any suggestion on having
>> a working regression test is welcome. And of course, regression tests
>> are supposed to evolve over time. Would this be a convenient way to add
>> more tests?
>>
> Sounds like a very worthwhile endeavor. The key to any testing
> infrastructure is to make it very easy to run the tests. Any hassle in
> setting it up is a reason not to do so, so you want to make sure there
> are no such barriers.

One such hassle I see with the current tests is that the dependencies on
external modules such as xattr and acls. This could be overcome by
writing C extensions that could use the C apis. Any other hassles you
are noticing?

> You also want to make sure that you have the ability to drill down into
> a single test failure without needing to run a bunch of goop around it.
> The cthon suite is good for this since most of the tests are written in
> C. That makes it easy to strace them to track down problems. I assume

It is possible to run strace the same way with these tests (like for
e.g. the above direct IO strace output). The output does seem a little
more verbose. But, any other problems you are seeing with these tests
written in python?

> we'll be able to write tests in C and just have the python framework
> call them?
> 

I initially thought of writing these tests in C. But, later decided in
favor of Python because of ease of use, smaller code size and
availability of plenty of modules. Making it simpler (simpler here is
subjective) means that should be little or no excuse for not writing an
regression test when submitting a bug fix for a regression.

But, if the consensus is that we would prefer tests in C, I can redo the
tests in C. If we do it in C, I think we could use CUnit framework for
writing tests as it would be more natural.


Thanks
Suresh

Re: Regression tests for CIFS client

[Jeff Layton]

On Thu, 09 Aug 2012 20:07:56 +0530
Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:

> On 08/08/2012 06:11 PM, Jeff Layton wrote:
> > On Wed, 08 Aug 2012 17:31:09 +0530
> > Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:
> > 
> >> Here's are the failures seen with 3.1 based kernel. I think the open()
> >> with O_DIRECT is expected to fail since cifs doesn't support it (I'd be
> >> interesting in knowing exact details). But, I've not dig into the xattr
> >> tests, not sure why setattr is failing (even if CIFS_XATTR is set, fs
> >> mounted with user_xattr).
> >>
> >> Test Output
> >> ============
> >>
> >> ======================================================================
> >> ERROR: test_directIO (__main__.OpenTests)
> >> open a file with O_DIRECT
> >> ----------------------------------------------------------------------
> >> Traceback (most recent call last):
> >>   File "./testcifs.py", line 144, in test_directIO
> >>     raise e
> >> OSError: [Errno 22] Invalid argument: 'testfile'
> >>
> > 
> > With all of the recent changes to the read/write code, I think we can
> > reasonably do O_DIRECT now. Just make sure that you don't request an
> > oplock on open and ensure that you're not using cache=loose codepaths.
> > 
> 
> Hmm, I'm not sure yet what is going on wrong.. I disabled oplocks by doing
> 
>    echo N > /sys/module/cifs/parameters/enable_oplocks
> 
> (Is there a way for an application to not request oplocks?)
> and mounted with cache=strict and ran the directIO test alone.
> 
> strace output (snip)
> --------------
> stat("/mnt/cifstests", 0x7fff317e68f0)  = -1 ENOENT (No such file or
> directory)
> mkdir("cifstests", 0755)                = 0
> chdir("cifstests")                      = 0
> open("testfile", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
> fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> close(3)                                = 0
> open("testfile", O_RDONLY|O_DIRECT)     = -1 EINVAL (Invalid argument)
> write(1, "[Errno 22] Invalid argument: 'te"..., 40) = 40
> 
> But I see from dmesg oplocks are being granted.
> 

Sorry, I might not have been clear. I meant that in principle, we could
probably fix the cifs client to do O_DIRECT properly now that we have
the strictcache code. It doesn't do that currently, of course...


> > In point of fact, we do some bounce buffering under the covers, but it
> > does avoid the pagecache.
> > 
> >> ======================================================================
> >> ERROR: test_dir_attr (__main__.XattrTests)
> >> set attrs, get attrs and remove attrs for a dir
> >> ----------------------------------------------------------------------
> >> Traceback (most recent call last):
> >>   File "./testcifs.py", line 364, in test_dir_attr
> >>     raise e
> >> IOError: [Errno 95] Operation not supported: 'test'
> >>
> >> ======================================================================
> >> ERROR: test_file_attr (__main__.XattrTests)
> >> set attrs, get attrs and remove attrs for a file
> >> ----------------------------------------------------------------------
> >> Traceback (most recent call last):
> >>   File "./testcifs.py", line 343, in test_file_attr
> >>     raise e
> >> IOError: [Errno 95] Operation not supported: 'testfile'
> >>
> >> ----------------------------------------------------------------------
> >> Ran 22 tests in 0.072s
> >>
> >> FAILED (errors=3)
> >>
> > 
> > The above I'm not sure about -- maybe depends on which attributes
> > you're trying to set?
> 
> I was trying to set something like:
> 
> 	user.mime_type = 'text/plain'
> 
> (the same worked on ext4)
> 

Are you testing against samba? You might need to set "ea support = yes".

> >>
> >> What do you think? Is it be a good idea?
> >>
> >> I know I have barely scratched the surface, but any suggestion on having
> >> a working regression test is welcome. And of course, regression tests
> >> are supposed to evolve over time. Would this be a convenient way to add
> >> more tests?
> >>
> > Sounds like a very worthwhile endeavor. The key to any testing
> > infrastructure is to make it very easy to run the tests. Any hassle in
> > setting it up is a reason not to do so, so you want to make sure there
> > are no such barriers.
> 
> One such hassle I see with the current tests is that the dependencies on
> external modules such as xattr and acls. This could be overcome by
> writing C extensions that could use the C apis. Any other hassles you
> are noticing?
> 
> > You also want to make sure that you have the ability to drill down into
> > a single test failure without needing to run a bunch of goop around it.
> > The cthon suite is good for this since most of the tests are written in
> > C. That makes it easy to strace them to track down problems. I assume
> 
> It is possible to run strace the same way with these tests (like for
> e.g. the above direct IO strace output). The output does seem a little
> more verbose. But, any other problems you are seeing with these tests
> written in python?
> 
> > we'll be able to write tests in C and just have the python framework
> > call them?
> > 
> 
> I initially thought of writing these tests in C. But, later decided in
> favor of Python because of ease of use, smaller code size and
> availability of plenty of modules. Making it simpler (simpler here is
> subjective) means that should be little or no excuse for not writing an
> regression test when submitting a bug fix for a regression.
> 
> But, if the consensus is that we would prefer tests in C, I can redo the
> tests in C. If we do it in C, I think we could use CUnit framework for
> writing tests as it would be more natural.
> 

I'm fine with them in python, but eventually there may be need to write
certain tests in C. As long as the python framework can execute those
(and I'm fairly sure it can), I don't think it'll be a problem...


-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: Regression tests for CIFS client

[Suresh Jayaraman]

On 08/09/2012 08:33 PM, Jeff Layton wrote:
> On Thu, 09 Aug 2012 20:07:56 +0530
> Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:
>>>
>>> With all of the recent changes to the read/write code, I think we can
>>> reasonably do O_DIRECT now. Just make sure that you don't request an
>>> oplock on open and ensure that you're not using cache=loose codepaths.
>>>
>> Hmm, I'm not sure yet what is going on wrong.. I disabled oplocks by doing
>>
>>    echo N > /sys/module/cifs/parameters/enable_oplocks
>>
>> (Is there a way for an application to not request oplocks?)
>> and mounted with cache=strict and ran the directIO test alone.
>>
>> strace output (snip)
>> --------------
>> stat("/mnt/cifstests", 0x7fff317e68f0)  = -1 ENOENT (No such file or
>> directory)
>> mkdir("cifstests", 0755)                = 0
>> chdir("cifstests")                      = 0
>> open("testfile", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
>> fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
>> close(3)                                = 0
>> open("testfile", O_RDONLY|O_DIRECT)     = -1 EINVAL (Invalid argument)
>> write(1, "[Errno 22] Invalid argument: 'te"..., 40) = 40
>>
>> But I see from dmesg oplocks are being granted.
>>
> Sorry, I might not have been clear. I meant that in principle, we could
> probably fix the cifs client to do O_DIRECT properly now that we have
> the strictcache code. It doesn't do that currently, of course...

Ah, ok. What does it take to fix the cifs client? I could take a look.
Are there pointers to discussion threads on this topic?

>>
>> I was trying to set something like:
>>
>> 	user.mime_type = 'text/plain'
>>
>> (the same worked on ext4)
>>
> Are you testing against samba? You might need to set "ea support = yes".

You were spot on. I had assumed that EA would be supported by default.
Now, the extended attribute tests are passing.

>>>>
>>>> What do you think? Is it be a good idea?
>>>>
>>>> I know I have barely scratched the surface, but any suggestion on having
>>>> a working regression test is welcome. And of course, regression tests
>>>> are supposed to evolve over time. Would this be a convenient way to add
>>>> more tests?
>>>>
>>> Sounds like a very worthwhile endeavor. The key to any testing
>>> infrastructure is to make it very easy to run the tests. Any hassle in
>>> setting it up is a reason not to do so, so you want to make sure there
>>> are no such barriers.
>>
>> One such hassle I see with the current tests is that the dependencies on
>> external modules such as xattr and acls. This could be overcome by
>> writing C extensions that could use the C apis. Any other hassles you
>> are noticing?
>>
>>> You also want to make sure that you have the ability to drill down into
>>> a single test failure without needing to run a bunch of goop around it.
>>> The cthon suite is good for this since most of the tests are written in
>>> C. That makes it easy to strace them to track down problems. I assume
>>
>> It is possible to run strace the same way with these tests (like for
>> e.g. the above direct IO strace output). The output does seem a little
>> more verbose. But, any other problems you are seeing with these tests
>> written in python?
>>
>>> we'll be able to write tests in C and just have the python framework
>>> call them?
>>>
>>
>> I initially thought of writing these tests in C. But, later decided in
>> favor of Python because of ease of use, smaller code size and
>> availability of plenty of modules. Making it simpler (simpler here is
>> subjective) means that should be little or no excuse for not writing an
>> regression test when submitting a bug fix for a regression.
>>
>> But, if the consensus is that we would prefer tests in C, I can redo the
>> tests in C. If we do it in C, I think we could use CUnit framework for
>> writing tests as it would be more natural.
>>
> I'm fine with them in python, but eventually there may be need to write
> certain tests in C. As long as the python framework can execute those
> (and I'm fairly sure it can), I don't think it'll be a problem...
> 

Yeah, I agree that we will have a need to write some tests in C and it
is possible to invoke tests written in C with this framework. Perhaps,
I'll see whether I can have some examples so that it will be easy for
others to follow.


Thanks
Suresh

Re: Regression tests for CIFS client

[Jeff Layton]

On Fri, 10 Aug 2012 10:57:35 +0530
Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:

> On 08/09/2012 08:33 PM, Jeff Layton wrote:
> > On Thu, 09 Aug 2012 20:07:56 +0530
> > Suresh Jayaraman <sjayaraman-IBi9RG/b67k@public.gmane.org> wrote:
> >>>
> >>> With all of the recent changes to the read/write code, I think we can
> >>> reasonably do O_DIRECT now. Just make sure that you don't request an
> >>> oplock on open and ensure that you're not using cache=loose codepaths.
> >>>
> >> Hmm, I'm not sure yet what is going on wrong.. I disabled oplocks by doing
> >>
> >>    echo N > /sys/module/cifs/parameters/enable_oplocks
> >>
> >> (Is there a way for an application to not request oplocks?)
> >> and mounted with cache=strict and ran the directIO test alone.
> >>
> >> strace output (snip)
> >> --------------
> >> stat("/mnt/cifstests", 0x7fff317e68f0)  = -1 ENOENT (No such file or
> >> directory)
> >> mkdir("cifstests", 0755)                = 0
> >> chdir("cifstests")                      = 0
> >> open("testfile", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
> >> fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> >> close(3)                                = 0
> >> open("testfile", O_RDONLY|O_DIRECT)     = -1 EINVAL (Invalid argument)
> >> write(1, "[Errno 22] Invalid argument: 'te"..., 40) = 40
> >>
> >> But I see from dmesg oplocks are being granted.
> >>
> > Sorry, I might not have been clear. I meant that in principle, we could
> > probably fix the cifs client to do O_DIRECT properly now that we have
> > the strictcache code. It doesn't do that currently, of course...
> 
> Ah, ok. What does it take to fix the cifs client? I could take a look.
> Are there pointers to discussion threads on this topic?
> 

No real discussion on this so far, just something I had considered when
Pavel did the strictcache code. cache=strict (and cache=none for that
matter) basically does dio already when it doesn't have an oplock. When
cache=strict is in force you can just make an O_DIRECT open not request
one at all and you're basically done.

There are some things to be worked out:

1) what to do about cache=loose? Should an O_DIRECT open make it use a
different codepath?

2) what to do about mmap? Currently we fall back to buffered I/O
codepaths for mmap even when we don't have an oplock. Should we do
anything different if someone tries to mmap an O_DIRECT file?

One of the complicating factors is that we have different sets of
file_operations for different cache= values. If you unify those, it
might make it easier to do all of this.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>