[Port-solaris] Re: vn_setpath

[Andrew Deason]

On Tue, 08 Jan 2013 19:01:51 +0100
"Frank Batschulat (private)" <frank.batschulat@drusus.de> wrote:

> We've discussed your issue, and I must admit we've not anticipated
> some file system implementations making use of vn_setpath(). looking
> at the one occurrence of vn_setpath() usage in OpenAFS its not much
> surprising to see it being placed in gafs_rename(), the VOP_RENAME()
> implementation. Usually vn_setpath() is only expected to be called by
> the pathname resolution and lookup code in the kernel. I suppose your
> code pre-dates the introduction of vn_renamepath() which is what our
> file system implementations actually use to deal with the update of
> the vnodes path after a successful rename.

If it only existed as of Solaris 10u8... yes, we predate it by quite a
lot!

> However, the way the above code is in gasf_rename() today, ie, freeing
> the existing vnode path upfront before calling vn_setpath(),
> vn_setpath() will always set v_path anyways regardless of whether the
> force flag is true or false.  So no harm will happen with the current
> compiled modules.

Okay, thanks, that is good to hear.

> +#if defined(AFS_SUN511_ENV)
> +	    vn_renamepath(pvp, vp, aname2, strlen(aname2));
> +#else

We usually do this kind of thing by testing directly for the existence
of the desired function. There is no problem with using this as soon as
it appears, correct?

> Btw, we've noticed something else you want to fix in afs_inactive():
> 
> 1674     /*
> 1675      * Solaris calls VOP_OPEN on exec, but doesn't call VOP_CLOSE when
> 1676      * the executable exits.  So we clean up the open count here.
> 1677      *
> 1678      * Only do this for mvstat 0 vnodes: when using fakestat, we can't
> 1679      * lose the open count for volume roots (mvstat 2), even though they
> 1680      * will get VOP_INACTIVE'd when released by afs_PutFakeStat().
> 1681      */
> 
> We have fixed the overall VOP_CLOSE() is missing in the exec path
> problem since Solaris 11 build 135.  This code above should be
> disabled in Solaris 11 and later. VOP_CLOSE() is called on exec (on
> the old executable), and exit and we also call VOP_OPEN on fork().
> However it is not fixed in Solaris 10.

That build number does not mean a lot to me; does that mean it's in
every public Solaris 11 release? I would be careful with disabling it,
since that's not really something we can easily write a direct test for.
And yes, thank you!

By the way, are you okay with being credited in patches for these
things? I would like to note where I got this information from when I
say we should do X, Y, or Z in e.g. commit messages.

-- 
Andrew Deason
adeason@sinenomine.net