ltp icon indicating copy to clipboard operation
ltp copied to clipboard

Refactoring locktests.py

Open michaelact opened this issue 4 years ago • 2 comments

Premise

As I see, this python script hasn't updated for 3 years, and maybe some newest python version, have bug while running it. I had this too, somehow an infinite loop happened. So, here I am trying to make a contribution by cleaning the code and fix bug that happened with me.

Changes

  • Cleaning code (following best-practice).
  • Added dependency argparse for parsing arguments.
  • Introduced more argument -u and --user for remote SSH.
  • Remove non-useful module & line.

Concerns

This script is not completely perfect. There is a bug I found while running it, it occurs in a C program on client:

locktests.c: In function ‘rapport’:
locktests.c:393:6: warning: type of ‘clnt’ defaults to ‘int’ [-Wimplicit-int]
 void rapport(clnt){
      ^~~~~~~

If I forgetting some step, please remind me by answering this discussion.

Hopefully it can be useful and thank you :).

michaelact avatar Jul 14 '21 14:07 michaelact

Thanks for your effort. But we want to get rid of any python (rewrite it into C - preferably or shell). See https://github.com/linux-test-project/ltp/issues/548 https://github.com/linux-test-project/ltp/issues/547. They're still relevant, but need cleanup.

The only excuse would be when adopted by kernel NFS maintainers (moving from LTP to their git) as they have few testsuites in python. I'd actually prefer if it was cleaned that way to be adopted by them. Maybe could you send it to linux-nfs mailing list? Or ask them?

See their reaction when I some time ago wanted to remove whole testcases/network/nfsv4/: https://lore.kernel.org/linux-nfs/[email protected]/

Reasons to drop:

  • outdated tests (from 2005)
  • not used (NFS kernel maintainers use pynfs [1])
  • written in Python (we support C and shell, see [2])

[1] http://git.linux-nfs.org/?p=bfields/pynfs.git;a=summary [2] https://github.com/linux-test-project/ltp/issues/5470

Unlike pynfs, these tests run on a real NFS client, and were designed to test client implementations, as well as the servers.

So if they get dropped from ltp, then we will have to figure out some other way of continuing to maintain them.

https://lore.kernel.org/linux-nfs/[email protected]/

Just for fun, I grepped through old mail to see if I could find any cases of these tests being used. I found one, in which Chuck reports an nfslock01 failure. Looks like it did find a real bug, which we fixed:

https://lore.kernel.org/r/[email protected] https://lore.kernel.org/r/[email protected]

https://lore.kernel.org/linux-nfs/[email protected]/

Looks like they may test some things (ACL enforcement, multi-client locking), that our other test suites don't.

On the other hand, if nobody's actually running them then maybe it's on us to adopt them if we want them. (Not volunteering for now.)

pevik avatar Jul 14 '21 16:07 pevik

Ohh I see. Thanks for the information, maybe I'll send it to them later.

michaelact avatar Jul 15 '21 04:07 michaelact