linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* pynfs updates
@ 2013-09-30 18:17 Frank Filz
  2013-09-30 22:11 ` Bruce Fields
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Filz @ 2013-09-30 18:17 UTC (permalink / raw)
  To: Kernel NFS List, Bruce Fields, 'Ganesha NFS List'

Bruce,

Please pull the following branch that contains the pynfs updates I made at
BAT:

https://github.com/ffilz/pynfs/commits/master

Thanks

Frank



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: pynfs updates
  2013-09-30 18:17 pynfs updates Frank Filz
@ 2013-09-30 22:11 ` Bruce Fields
  2013-09-30 23:54   ` Frank Filz
  0 siblings, 1 reply; 11+ messages in thread
From: Bruce Fields @ 2013-09-30 22:11 UTC (permalink / raw)
  To: Frank Filz; +Cc: Kernel NFS List, 'Ganesha NFS List'

On Mon, Sep 30, 2013 at 02:17:43PM -0400, Frank Filz wrote:
> Bruce,
> 
> Please pull the following branch that contains the pynfs updates I made at
> BAT:
> 
> https://github.com/ffilz/pynfs/commits/master

Thanks!  A few questions:

	- "4.1 server tests: Fix some exception handling": could you
	  include in the changelog and explanation of what problem this
	  fixes?  (And ditto, maybe, for the following commit?)

	- "Fix SEQ9d to work in home directory instead of root": did you
	  intend to include the chunk in nfs4.1/nfs4lib.py?  It looks
	  irrelevant.

	- "Add two SECINFO_NO_NAME tests for SECINFO_STYLE4_PARENT":
		- SECNN3: is / required to have no parent?  (I'd assumed
		  here that it would also be OK to follow the convention
		  that / is its own parent, but I'll admit to not having
		  thought about this much.)
		- SECNN4: is env.home necessarily unequal to "/"?  Would
		  seem better to do the lookup in a subdirectory just to
		  be certain.

--b.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: pynfs updates
  2013-09-30 22:11 ` Bruce Fields
@ 2013-09-30 23:54   ` Frank Filz
  2013-10-01 14:26     ` 'Bruce Fields'
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Filz @ 2013-09-30 23:54 UTC (permalink / raw)
  To: 'Bruce Fields'
  Cc: 'Kernel NFS List', 'Ganesha NFS List'

> Thanks!  A few questions:
> 
> 	- "4.1 server tests: Fix some exception handling": could you
> 	  include in the changelog and explanation of what problem this
> 	  fixes?  (And ditto, maybe, for the following commit?)

Ok, I'll re-examine those. They were some things I stumbled on as I tried to
resolve some other issues with pynfs.

> 	- "Fix SEQ9d to work in home directory instead of root": did you
> 	  intend to include the chunk in nfs4.1/nfs4lib.py?  It looks
> 	  irrelevant.

Hmm, will fix that.

> 	- "Add two SECINFO_NO_NAME tests for
> SECINFO_STYLE4_PARENT":
> 		- SECNN3: is / required to have no parent?  (I'd assumed
> 		  here that it would also be OK to follow the convention
> 		  that / is its own parent, but I'll admit to not having
> 		  thought about this much.)

>From LOOKUPP:

18.14.3. DESCRIPTION
The current filehandle is assumed to refer to a regular directory or
a named attribute directory. LOOKUPP assigns the filehandle for its
parent directory to be the current filehandle. If there is no parent
directory, an NFS4ERR_NOENT error must be returned. Therefore,
NFS4ERR_NOENT will be returned by the server when the current
filehandle is at the root or top of the server's file tree.

>From SECINFO_NO_NAME:

18.45.3. DESCRIPTION

...

If the style selected is SECINFO_STYLE4_PARENT,
then SECINFO should apply the same access methodology used for
LOOKUPP when evaluating the traversal to the parent directory.

...

If SECINFO_STYLE4_PARENT is specified and there is no parent
directory, SECINFO_NO_NAME MUST return NFS4ERR_NOENT.


> 		- SECNN4: is env.home necessarily unequal to "/"?  Would
> 		  seem better to do the lookup in a subdirectory just to
> 		  be certain.

Env.home is the directory you specify on the command line, I think the
presumption is that it is a writeable file system. Pynfs creates tmp and
tree directories in home (and maybe some files also?). Guess if / was
writeable, you could specify /, so yea, maybe it should go into tmp.

A better test might actually be to do LOOKUP down to home and even into tmp,
looking for a junction, and then do the SECINFO_NO_NAME(parent) on the
directory handle just across the junction if one was found.

Thanks for the review.

Frank


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: pynfs updates
  2013-09-30 23:54   ` Frank Filz
@ 2013-10-01 14:26     ` 'Bruce Fields'
  2013-10-01 14:30       ` 'Bruce Fields'
  2013-10-01 18:21       ` Frank Filz
  0 siblings, 2 replies; 11+ messages in thread
From: 'Bruce Fields' @ 2013-10-01 14:26 UTC (permalink / raw)
  To: Frank Filz; +Cc: 'Kernel NFS List', 'Ganesha NFS List'

On Mon, Sep 30, 2013 at 07:54:52PM -0400, Frank Filz wrote:
> > 	- "Add two SECINFO_NO_NAME tests for
> > SECINFO_STYLE4_PARENT":
> > 		- SECNN3: is / required to have no parent?  (I'd assumed
> > 		  here that it would also be OK to follow the convention
> > 		  that / is its own parent, but I'll admit to not having
> > 		  thought about this much.)
> 
> >From LOOKUPP:
> 
> 18.14.3. DESCRIPTION
> The current filehandle is assumed to refer to a regular directory or
> a named attribute directory. LOOKUPP assigns the filehandle for its
> parent directory to be the current filehandle. If there is no parent
> directory, an NFS4ERR_NOENT error must be returned. Therefore,
> NFS4ERR_NOENT will be returned by the server when the current
> filehandle is at the root or top of the server's file tree.

OK, fine.

> > 		- SECNN4: is env.home necessarily unequal to "/"?  Would
> > 		  seem better to do the lookup in a subdirectory just to
> > 		  be certain.
> 
> Env.home is the directory you specify on the command line, I think the
> presumption is that it is a writeable file system. Pynfs creates tmp and
> tree directories in home (and maybe some files also?). Guess if / was
> writeable, you could specify /, so yea, maybe it should go into tmp.

Sounds good.

> 
> A better test might actually be to do LOOKUP down to home and even into tmp,
> looking for a junction, and then do the SECINFO_NO_NAME(parent) on the
> directory handle just across the junction if one was found.

Yeah it'd be nice to check that cross-filesystem case but I don't think
it's necessary (and you still have to deal with the case where a
mountpoint's not found).

If tests at mountpoints were useful perhaps we could pass in a
mountpoint on the commandline.  Or add some sort of export-configuration
interface to the serverhelper script and let pynfs setup exports itself.

--b.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: pynfs updates
  2013-10-01 14:26     ` 'Bruce Fields'
@ 2013-10-01 14:30       ` 'Bruce Fields'
  2013-10-01 15:42         ` Frank Filz
  2013-10-01 19:05         ` Frank Filz
  2013-10-01 18:21       ` Frank Filz
  1 sibling, 2 replies; 11+ messages in thread
From: 'Bruce Fields' @ 2013-10-01 14:30 UTC (permalink / raw)
  To: Frank Filz; +Cc: 'Kernel NFS List', 'Ganesha NFS List'

One more problem: CSID10 is failing against the Linux server with
NFS4ERR_TOO_MANY_OPS, because each of those lookups is actually a full
lookup from PUTROOTFH to /, resulting in 17 ops on my setup.  Could we
maybe work relative to the parent directory instead?

--b.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: pynfs updates
  2013-10-01 14:30       ` 'Bruce Fields'
@ 2013-10-01 15:42         ` Frank Filz
  2013-10-01 19:05         ` Frank Filz
  1 sibling, 0 replies; 11+ messages in thread
From: Frank Filz @ 2013-10-01 15:42 UTC (permalink / raw)
  To: 'Bruce Fields'
  Cc: 'Kernel NFS List', 'Ganesha NFS List'

> One more problem: CSID10 is failing against the Linux server with
> NFS4ERR_TOO_MANY_OPS, because each of those lookups is actually a full
> lookup from PUTROOTFH to /, resulting in 17 ops on my setup.  Could we
> maybe work relative to the parent directory instead?

Sure, I'll rework that one.

> > A better test might actually be to do LOOKUP down to home and even
> > into tmp, looking for a junction, and then do the
> > SECINFO_NO_NAME(parent) on the directory handle just across the
> junction if one was found.
> 
> Yeah it'd be nice to check that cross-filesystem case but I don't think
it's
> necessary (and you still have to deal with the case where a mountpoint's
not
> found).

Well, the cross file system case is actually where you would need to use
SECINFO_NO_NAME. For some reason, you just have a handle to a directory
inside the export and want to navigate back up the tree. In doing so, you
cross back over a junction to a file system that is exported with a
different security flavor.

On the other hand, generally that higher level file system should include
all the security flavors used by the lower level file systems. Unless
SECINFO_NO_NAME lets you cross a junction where the new file system doesn't
have security flavors in common with the upper level file system, but I
don't think it does. Does anyone know the rationale of SECINFO_NO_NAME
(parent)? In fact is there really any use of SECINFO_NO_NAME other than to
get the secinfo for the root or public file handle? I guess it does also
allow a client to recover from the security flavors for a given file system
being changed on the fly (or perhaps after a migration event).

> If tests at mountpoints were useful perhaps we could pass in a mountpoint
> on the commandline.  Or add some sort of export-configuration interface to
> the serverhelper script and let pynfs setup exports itself.

Frank



^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: pynfs updates
  2013-10-01 14:26     ` 'Bruce Fields'
  2013-10-01 14:30       ` 'Bruce Fields'
@ 2013-10-01 18:21       ` Frank Filz
  2013-10-01 18:45         ` 'Bruce Fields'
  1 sibling, 1 reply; 11+ messages in thread
From: Frank Filz @ 2013-10-01 18:21 UTC (permalink / raw)
  To: 'Bruce Fields'
  Cc: 'Kernel NFS List', 'Ganesha NFS List'

> > > 		- SECNN4: is env.home necessarily unequal to "/"?  Would
> > > 		  seem better to do the lookup in a subdirectory just to
> > > 		  be certain.
> >
> > Env.home is the directory you specify on the command line, I think the
> > presumption is that it is a writeable file system. Pynfs creates tmp
> > and tree directories in home (and maybe some files also?). Guess if /
> > was writeable, you could specify /, so yea, maybe it should go into tmp.
> 
> Sounds good.

env.home does actually include traversing into tmp, so I will leave this
test alone.

> > A better test might actually be to do LOOKUP down to home and even
> > into tmp, looking for a junction, and then do the
> > SECINFO_NO_NAME(parent) on the directory handle just across the
> junction if one was found.
> 
> Yeah it'd be nice to check that cross-filesystem case but I don't think
it's
> necessary (and you still have to deal with the case where a mountpoint's
not
> found).
> 
> If tests at mountpoints were useful perhaps we could pass in a mountpoint
> on the commandline.  Or add some sort of export-configuration interface to
> the serverhelper script and let pynfs setup exports itself.

Yea, that might be something interesting to explore at some point.

Frank


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: pynfs updates
  2013-10-01 18:21       ` Frank Filz
@ 2013-10-01 18:45         ` 'Bruce Fields'
  0 siblings, 0 replies; 11+ messages in thread
From: 'Bruce Fields' @ 2013-10-01 18:45 UTC (permalink / raw)
  To: Frank Filz; +Cc: 'Kernel NFS List', 'Ganesha NFS List'

On Tue, Oct 01, 2013 at 02:21:01PM -0400, Frank Filz wrote:
> > > > 		- SECNN4: is env.home necessarily unequal to "/"?  Would
> > > > 		  seem better to do the lookup in a subdirectory just to
> > > > 		  be certain.
> > >
> > > Env.home is the directory you specify on the command line, I think the
> > > presumption is that it is a writeable file system. Pynfs creates tmp
> > > and tree directories in home (and maybe some files also?). Guess if /
> > > was writeable, you could specify /, so yea, maybe it should go into tmp.
> > 
> > Sounds good.
> 
> env.home does actually include traversing into tmp, so I will leave this
> test alone.

OK!

--b.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: pynfs updates
  2013-10-01 14:30       ` 'Bruce Fields'
  2013-10-01 15:42         ` Frank Filz
@ 2013-10-01 19:05         ` Frank Filz
  2013-10-02 11:36           ` 'Bruce Fields'
  1 sibling, 1 reply; 11+ messages in thread
From: Frank Filz @ 2013-10-01 19:05 UTC (permalink / raw)
  To: 'Bruce Fields'
  Cc: 'Kernel NFS List', 'Ganesha NFS List'

> One more problem: CSID10 is failing against the Linux server with
> NFS4ERR_TOO_MANY_OPS, because each of those lookups is actually a full
> lookup from PUTROOTFH to /, resulting in 17 ops on my setup.  Could we
> maybe work relative to the parent directory instead?

Ok, was able to fix this by doing a LOOKUP sequence in a separate compound
followed by GETFH then in the compound that tests SAVEFH/RESTOREFH, just do
PUTFH that saved FH.

Things could be a lot smoother as discussed on IRC if the initialization
stored away env.home_fh.

Then with some work, this test could be simplified to:

SEQUENCE, PUTFH(env.home_fh), OPEN, GETFH, SAVEFH, PUTFH(env.home_fh),
RESTOREFH, CLOSE

Note that that GETFH is not actually used by this test, but presumably
open_create_op() would produce:

PUTFH(env.home_fh), OPEN, GETFH

Instead of what it currently does:

PUTROOTFH, LOOKUP..., OPEN, GETFH

The new branch is here:

https://github.com/ffilz/pynfs/commits/master

Frank



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: pynfs updates
  2013-10-01 19:05         ` Frank Filz
@ 2013-10-02 11:36           ` 'Bruce Fields'
  2013-10-02 15:58             ` Frank Filz
  0 siblings, 1 reply; 11+ messages in thread
From: 'Bruce Fields' @ 2013-10-02 11:36 UTC (permalink / raw)
  To: Frank Filz; +Cc: 'Kernel NFS List', 'Ganesha NFS List'

On Tue, Oct 01, 2013 at 03:05:23PM -0400, Frank Filz wrote:
> > One more problem: CSID10 is failing against the Linux server with
> > NFS4ERR_TOO_MANY_OPS, because each of those lookups is actually a full
> > lookup from PUTROOTFH to /, resulting in 17 ops on my setup.  Could we
> > maybe work relative to the parent directory instead?
> 
> Ok, was able to fix this by doing a LOOKUP sequence in a separate compound
> followed by GETFH then in the compound that tests SAVEFH/RESTOREFH, just do
> PUTFH that saved FH.
> 
> Things could be a lot smoother as discussed on IRC if the initialization
> stored away env.home_fh.
> 
> Then with some work, this test could be simplified to:
> 
> SEQUENCE, PUTFH(env.home_fh), OPEN, GETFH, SAVEFH, PUTFH(env.home_fh),
> RESTOREFH, CLOSE
> 
> Note that that GETFH is not actually used by this test, but presumably
> open_create_op() would produce:
> 
> PUTFH(env.home_fh), OPEN, GETFH
> 
> Instead of what it currently does:
> 
> PUTROOTFH, LOOKUP..., OPEN, GETFH
> 
> The new branch is here:
> 
> https://github.com/ffilz/pynfs/commits/master

Pulled, thanks!

--b.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: pynfs updates
  2013-10-02 11:36           ` 'Bruce Fields'
@ 2013-10-02 15:58             ` Frank Filz
  0 siblings, 0 replies; 11+ messages in thread
From: Frank Filz @ 2013-10-02 15:58 UTC (permalink / raw)
  To: 'Bruce Fields'
  Cc: 'Kernel NFS List', 'Ganesha NFS List'

> > The new branch is here:
> >
> > https://github.com/ffilz/pynfs/commits/master
> 
> Pulled, thanks!

And thank you. Bakeathon was very productive for Ganesha in this department.
We went from 34 failures to 10 failures.

Thanks

Frank



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-10-02 15:59 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-30 18:17 pynfs updates Frank Filz
2013-09-30 22:11 ` Bruce Fields
2013-09-30 23:54   ` Frank Filz
2013-10-01 14:26     ` 'Bruce Fields'
2013-10-01 14:30       ` 'Bruce Fields'
2013-10-01 15:42         ` Frank Filz
2013-10-01 19:05         ` Frank Filz
2013-10-02 11:36           ` 'Bruce Fields'
2013-10-02 15:58             ` Frank Filz
2013-10-01 18:21       ` Frank Filz
2013-10-01 18:45         ` 'Bruce Fields'

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).