Skip to contentSkip to conversation

4.1 create_session: Skip test of CB_NULL for nfs4.1

PATCH Superseded Kinglong Mee 3 participants 7 messages Jul 28, 2015 – Jul 29, 2015

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 4/4 4.1 create_session: Skip test of CB_NULL for nfs4.1

---
 nfs4.1/server41tests/st_create_session.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
nfs4.1/server41tests/st_create_session.py +2 -2
index b42e0ab..4c56bb4 100644
@@ -333,7 +333,7 @@ def testManyClients(t, env):
 def testCallbackProgram(t, env):
     """Check server can handle random transient program number
 
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS20
     """
     cb_occurred = threading.Event()
@@ -360,7 +360,7 @@ def testCallbackProgram(t, env):
 def testCallbackVersion(t, env):
     """Check server sends callback program with a version listed in nfs4client.py
 
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS21
     """
     cb_occurred = threading.Event()
Signature · 1 line
2.4.3
#1 lore raw

Why are we removing these two test cases? The Ganesha NFS server at least passes them.

Frank

Quoted 44 lines

-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of Kinglong Mee
Sent: Tuesday, July 28, 2015 4:47 AM
To: J. Bruce Fields; [email protected]
Cc: [email protected]; [email protected]
Subject: [PATCH 4/4] 4.1 create_session: Skip test of CB_NULL for nfs4.1

---
 nfs4.1/server41tests/st_create_session.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
nfs4.1/server41tests/st_create_session.py +1 -1
index b42e0ab..4c56bb4 100644
@@ -333,7 +333,7 @@ def testManyClients(t, env):
 def testCallbackProgram(t, env):
     """Check server can handle random transient program number
 
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS20
     """
     cb_occurred = threading.Event()
@@ -360,7 +360,7 @@ def testCallbackProgram(t, env):
 def testCallbackVersion(t, env):
     """Check server sends callback program with a version listed in

nfs4client.py

nfs4.1/server41tests/st_create_session.py +1 -2
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS21
     """
     cb_occurred = threading.Event()
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

#2 lore raw
Quoted 1 line · Frank Filz

Why are we removing these two test cases? The Ganesha NFS server at least passes them.

And there's nothing wrong with passing the tests, but unfortunately there's not necessarily anything wrong with failing either: a 4.1 server isn't required to do a NULL callback to probe the backchannel.

There might be a better test (depend on delegation recalls instead of CB_NULL's?), till then I'm OK with turning them off by default.

--b.

Quoted 47 lines

Frank

Quoted 44 lines

-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of Kinglong Mee
Sent: Tuesday, July 28, 2015 4:47 AM
To: J. Bruce Fields; [email protected]
Cc: [email protected]; [email protected]
Subject: [PATCH 4/4] 4.1 create_session: Skip test of CB_NULL for nfs4.1

---
 nfs4.1/server41tests/st_create_session.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
nfs4.1/server41tests/st_create_session.py +1 -1
index b42e0ab..4c56bb4 100644
@@ -333,7 +333,7 @@ def testManyClients(t, env):
 def testCallbackProgram(t, env):
     """Check server can handle random transient program number
 
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS20
     """
     cb_occurred = threading.Event()
@@ -360,7 +360,7 @@ def testCallbackProgram(t, env):
 def testCallbackVersion(t, env):
     """Check server sends callback program with a version listed in

nfs4client.py

nfs4.1/server41tests/st_create_session.py +1 -2
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS21
     """
     cb_occurred = threading.Event()
--

2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 5 lines · Frank Filz
Quoted 1 line

Why are we removing these two test cases? The Ganesha NFS server at

least passes them.

And there's nothing wrong with passing the tests, but unfortunately

there's

Quoted 1 line

not necessarily anything wrong with failing either: a 4.1 server isn't

required

Quoted 4 lines

to do a NULL callback to probe the backchannel.

There might be a better test (depend on delegation recalls instead of CB_NULL's?), till then I'm OK with turning them off by default.

I guess I'd at least like to see some explanation of why we're turning the tests off by default. I'm just resistant to removing testing for no reason.

If the test doesn't actually test anything useful, that would be one explanation.

If it's something that's not required and not all servers do it, then that's another explanation.

There might be other explanations, just please give some insight.

Thanks

Frank

Quoted 41 lines
Quoted 41 lines
Quoted 41 lines

-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of Kinglong Mee
Sent: Tuesday, July 28, 2015 4:47 AM
To: J. Bruce Fields; [email protected]
Cc: [email protected]; [email protected]
Subject: [PATCH 4/4] 4.1 create_session: Skip test of CB_NULL for nfs4.1

---
 nfs4.1/server41tests/st_create_session.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
nfs4.1/server41tests/st_create_session.py +1 -1
index b42e0ab..4c56bb4 100644
@@ -333,7 +333,7 @@ def testManyClients(t, env):
 def testCallbackProgram(t, env):
     """Check server can handle random transient program number
 
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS20
     """
     cb_occurred = threading.Event() @@ -360,7 +360,7 @@ def

testCallbackProgram(t, env):
def testCallbackVersion(t, env):
"""Check server sends callback program with a version listed in
nfs4client.py

nfs4.1/server41tests/st_create_session.py +1 -2
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS21
     """
     cb_occurred = threading.Event()
--

2.4.3

Quoted 7 lines
Quoted 3 lines
Quoted 3 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More

majordomo

Quoted 1 line
Quoted 1 line

info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in

the

Quoted 2 lines

body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

#4 lore raw
Quoted 7 lines · Frank Filz
Quoted 5 lines · Frank Filz
Quoted 1 line

Why are we removing these two test cases? The Ganesha NFS server at

least passes them.

And there's nothing wrong with passing the tests, but unfortunately

there's

Quoted 1 line

not necessarily anything wrong with failing either: a 4.1 server isn't

Quoted 8 lines

required

Quoted 4 lines

to do a NULL callback to probe the backchannel.

There might be a better test (depend on delegation recalls instead of CB_NULL's?), till then I'm OK with turning them off by default.

I guess I'd at least like to see some explanation of why we're turning the tests off by default. I'm just resistant to removing testing for no reason.

The story is: we turned off the CB_NULL probe in knfsd in the 4.1 case; it's unnecessary, and it was causing some problems (due partly to other bugs, but why ask for trouble?). So these tests started failing. I'd fix the server if there was an actual bug, but it's the tests that are wrong in this case, so oh well.

Quoted 6 lines

If the test doesn't actually test anything useful, that would be one explanation.

If it's something that's not required and not all servers do it, then that's another explanation.

Right, this is it, CB_NULL probes aren't required.

We're testing something useful, but something you need a callback to test, so trying to get some other kind of callback (like a deleg recall) would be another option.

--b.

Quoted 58 lines

There might be other explanations, just please give some insight.

Thanks

Frank

Quoted 41 lines
Quoted 41 lines
Quoted 41 lines

-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of Kinglong Mee
Sent: Tuesday, July 28, 2015 4:47 AM
To: J. Bruce Fields; [email protected]
Cc: [email protected]; [email protected]
Subject: [PATCH 4/4] 4.1 create_session: Skip test of CB_NULL for nfs4.1

---
 nfs4.1/server41tests/st_create_session.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
nfs4.1/server41tests/st_create_session.py +1 -1
index b42e0ab..4c56bb4 100644
@@ -333,7 +333,7 @@ def testManyClients(t, env):
 def testCallbackProgram(t, env):
     """Check server can handle random transient program number
 
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS20
     """
     cb_occurred = threading.Event() @@ -360,7 +360,7 @@ def

testCallbackProgram(t, env):
def testCallbackVersion(t, env):
"""Check server sends callback program with a version listed in
nfs4client.py

nfs4.1/server41tests/st_create_session.py +1 -2
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS21
     """
     cb_occurred = threading.Event()
--

2.4.3

Quoted 7 lines
Quoted 3 lines
Quoted 3 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected] More

majordomo

Quoted 1 line
Quoted 1 line

info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in

the

Quoted 2 lines

body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

#5 lore raw
Quoted 10 lines · Frank Filz
Quoted 9 lines
Quoted 6 lines · Frank Filz
Quoted 2 lines

Why are we removing these two test cases? The Ganesha NFS server at

least passes them.

And there's nothing wrong with passing the tests, but unfortunately

there's

Quoted 2 lines

not necessarily anything wrong with failing either: a 4.1 server isn't

Quoted 8 lines
Quoted 8 lines

required

Quoted 4 lines

to do a NULL callback to probe the backchannel.

There might be a better test (depend on delegation recalls instead of CB_NULL's?), till then I'm OK with turning them off by default.

I guess I'd at least like to see some explanation of why we're turning the tests off by default. I'm just resistant to removing testing for no

reason.

Quoted 2 lines

The story is: we turned off the CB_NULL probe in knfsd in the 4.1 case;

it's

Quoted 2 lines

unnecessary, and it was causing some problems (due partly to other bugs, but why ask for trouble?). So these tests started failing. I'd fix the

server if

Quoted 1 line

there was an actual bug, but it's the tests that are wrong in this case,

so oh

Quoted 1 line

well.

Ah, ok. I'd love to understand the issues there better, maybe Ganesha shouldn't be doing the CB_NULL either...

Quoted 7 lines
Quoted 5 lines

If the test doesn't actually test anything useful, that would be one explanation.

If it's something that's not required and not all servers do it, then that's another explanation.

Right, this is it, CB_NULL probes aren't required.

Ok, with this perspective, I'm cool with the test being removed from default.

Quoted 2 lines

We're testing something useful, but something you need a callback to test, so trying to get some other kind of callback (like a deleg recall) would

be

Quoted 1 line

another option.

Hmm, more to think about from testing Ganesha perspective. The "simple" thing (i.e. using FSAL_VFS) to test in Ganesha doesn't do delegations, so the CB_NULL might be the only callback it makes, which might make it somewhat useful to prove Ganesha's callback code isn't totally broken...

Thanks for the additional explanation.

Frank

Quoted 48 lines
Quoted 48 lines

There might be other explanations, just please give some insight.

Thanks

Frank

Quoted 42 lines
Quoted 42 lines
Quoted 42 lines

-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of Kinglong Mee
Sent: Tuesday, July 28, 2015 4:47 AM
To: J. Bruce Fields; [email protected]
Cc: [email protected]; [email protected]
Subject: [PATCH 4/4] 4.1 create_session: Skip test of CB_NULL for
nfs4.1

---
 nfs4.1/server41tests/st_create_session.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
nfs4.1/server41tests/st_create_session.py +1 -1
index b42e0ab..4c56bb4 100644
@@ -333,7 +333,7 @@ def testManyClients(t, env):
 def testCallbackProgram(t, env):
     """Check server can handle random transient program number
 
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS20
     """
     cb_occurred = threading.Event() @@ -360,7 +360,7 @@ def

testCallbackProgram(t, env):
def testCallbackVersion(t, env):
"""Check server sends callback program with a version
listed in nfs4client.py

nfs4.1/server41tests/st_create_session.py +1 -2
-    FLAGS: create_session all
+    FLAGS:
     CODE: CSESS21
     """
     cb_occurred = threading.Event()
--

2.4.3

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

--
To unsubscribe from this list: send the line "unsubscribe

linux-nfs"

Quoted 10 lines
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Quoted 1 line

in the body of a message to [email protected] More

majordomo

Quoted 1 line
Quoted 1 line

info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in

the

Quoted 1 line

body of a message to [email protected] More majordomo

info

Quoted 1 line
Quoted 1 line

at http://vger.kernel.org/majordomo-info.html

#6 lore raw
Quoted 24 lines · Frank Filz
Quoted 10 lines · Frank Filz
Quoted 9 lines
Quoted 6 lines · Frank Filz
Quoted 2 lines

Why are we removing these two test cases? The Ganesha NFS server at

least passes them.

And there's nothing wrong with passing the tests, but unfortunately

there's

Quoted 2 lines

not necessarily anything wrong with failing either: a 4.1 server isn't

Quoted 8 lines
Quoted 8 lines

required

Quoted 4 lines

to do a NULL callback to probe the backchannel.

There might be a better test (depend on delegation recalls instead of CB_NULL's?), till then I'm OK with turning them off by default.

I guess I'd at least like to see some explanation of why we're turning the tests off by default. I'm just resistant to removing testing for no

reason.

Quoted 2 lines

The story is: we turned off the CB_NULL probe in knfsd in the 4.1 case;

it's

Quoted 2 lines

unnecessary, and it was causing some problems (due partly to other bugs, but why ask for trouble?). So these tests started failing. I'd fix the

Quoted 7 lines

server if

Quoted 1 line

there was an actual bug, but it's the tests that are wrong in this case,

so oh

Quoted 1 line

well.

Ah, ok. I'd love to understand the issues there better, maybe Ganesha shouldn't be doing the CB_NULL either...

I've actually forgotten; the patches are from April so linux-nfs archives from around there might have some discussion. I think there was actually a bug in client callback handling that frequent unnecessary CB_NULL's were triggering.

Quoted 13 lines
Quoted 7 lines
Quoted 5 lines

If the test doesn't actually test anything useful, that would be one explanation.

If it's something that's not required and not all servers do it, then that's another explanation.

Right, this is it, CB_NULL probes aren't required.

Ok, with this perspective, I'm cool with the test being removed from default.

Quoted 2 lines

We're testing something useful, but something you need a callback to test, so trying to get some other kind of callback (like a deleg recall) would

Quoted 7 lines

be

Quoted 1 line

another option.

Hmm, more to think about from testing Ganesha perspective. The "simple" thing (i.e. using FSAL_VFS) to test in Ganesha doesn't do delegations, so the CB_NULL might be the only callback it makes, which might make it somewhat useful to prove Ganesha's callback code isn't totally broken...

Yeah, for testing it's annoying that there's no reliable to provoke a callback. pynfs could probably be smarter about trying various methods, and also about reporting the difference between "X failed" and "I couldn't test X because I couldn't get server to optional thing Y".

--b.