Subject: Re: Possible VOP_LOOKUP() interface change
To: Charles M. Hannum <root@ihack.net>
From: Bill Studenmund <wrstuden@nas.nasa.gov>
List: tech-kern
Date: 09/04/1999 15:14:15
On Sat, 4 Sep 1999, Charles M. Hannum wrote:

> > But instead of hiding *PARENT, we change how lockparent and wantparent are
> > initialized. From
> > [...]
> > Then we'd only test ISLASTCN once with respect to *PARENT, and all the
> > ISLASTCN tests could go. But portal would keep working fine.
> 
> While it might *happen* to work by luck, the backdoor interface it
> uses would definitely *not* be correct, as it would still see the
> wrong cn_nameiop.  To make a file system like this work correctly, you
> would still need another interface to get additional components.

If you're concerned about portal_lookup() seeing the wrong cn_nameiop, why
propose changing our lookup(), given that the former is a consequence of
the latter? :-)

Given that portal_create, portal_mknod, portal_remove, portal_rename,
portal_mkdir, and portal_rmdir are all macros to either genfs_eopnotsupp
or genfs_eopnotsup_rele (which does all of the WILLRELE unlocking on
nodes), the cases which would have been caught before (DELETE and RENAME)
will error out. Not as soon as before, but the right thing will happen.
:-)

Also, why do we need another interface to get additional components? We
have cn_consume which lets the VOP_LOOKUP() decide how much to eat.
Granted we are letting the _LOOKUP routine decide how much to eat rather
than having something else feed down components, but is that really a big
deal? Especially as the one customer of this ability (portal) is going to
consume the whole remaining path?

> I'm definitely not interested in making portal even *more* klugy.

Sounds fair. But if we change lookup() (and how it calls the
VOP_LOOKUP()'s ), we'll need to do something to portal_lookup().

The real problem is that your suggestion doesn't leave room for the
VOP_LOOKUP routine to cleanly be able to decide that this was infact the
last component of the pathname. So we either: break portal, live with
portal_lookup() being "unclean", or modify your suggestion so that the
_LOOKUP routine can decide this was infact the last component.

While improved readability is good, I don't think it's worth killing
portal. Especially when there are other ways of getting this readability
which won't necessarily break portal. Some people use portal, and it's a
4.4BSD feature which makes its way into textbooks. :-) 

One additional thing we could do is change all the lookup routines to do

int	nameiop = (flags & ISLASTCN) ? cnp->cn_nameiop : LOOKUP;

Combined with the lockparent and wantparent initialization from the other
note, we'd put all of the test simplification regarding ISLASTCN into the
_lookup() routines, rather than in lookup(). I think we'd get the same
readability improvement you wanted (please show me how if we won't!), and
portal would keep working as-is (it'd still be able to decide that this
was indeed the last component even though lookup() didn't think so).

One other thing we'd do by getting the readability w/o changing lookup()
is show all the other *BSD's, hey, you can make your code much more
readable without an interface change. :-)

Thoughts?

Take care,

Bill