Bruce,
Please pull the following branch that contains the pynfs updates I made at
BAT:
https://github.com/ffilz/pynfs/commits/master
Thanks
Frank
> 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
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.
> > > - 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
> > 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
> 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
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.
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.
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.
> 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
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.