I'll admit I cannot remember what olce@ decided.
Note that Sun RPC will always fill in at least one gid
in the RPC request. (The RPC request has a gid and
a gid list in it. The list can be 0 length, but there will
always be 1 gid.)

Thanks everyone for your comments. I plan on
committing this later to-day, so if you have any
more suggestions, please make them soon.

In D49851#1169308, @ziaee wrote:

In D49851#1169305, @rmacklem wrote:

In D49851#1169304, @ziaee wrote:

git-arc -c D49851 errors on the *.xattr files

No idea what you are talking about. Why would
git have anything to do with a man page?

Oops, I meant git-arc patch -c D49851, which applies the patch to the tree.

Ok. I don't use git arc or anything like that to apply the
patch to the tree. I set it up manually in a working branch.

Marked inline comments as done.

In D49851#1169304, @ziaee wrote:

git-arc -c D49851 errors on the *.xattr files

Made changes that kib@ and markj@ suggested.

Answered the inline questions. I will update the diff soon.

Looks ok to me, but I don't know much about renamelock.

Put .Dv before UF_HIDDEN and UF_SYSTEM as suggested
by ziaee@ and replaced va_flags with flags as suggested
by kib@. (I did not put .Ar in front of flags, since it is not
an argument to pathconf. Should I?)

Reword the description to reference chflags(2)
as suggested by Pau Amma.

Deleted a few sentences that seemed redundant.
I think what now remains in the man page is useful
information.

This patch is bogus and will not fix the crash.
It is rpc_gss_init() that needs to set CURVNET.

In D50962#1163352, @rmacklem wrote:

In D50962#1163337, @glebius wrote:

What is the problem we are trying to fix here? What is the panic trace?

The function rpctls_connect() is always called with socket's vnet set by clnt_reconnect_connect(). In general setting vnet0 is never correct. I understand that right now NFS may not fully support the VIMAGE, but we should slowly aim towards correct support, rather than just blindly plugging the panics.

The panic occurred because the CLNT_CALL_MBUF() called clnt_nl_call()
which asserts CURVNET_ASSERT_SET(). I don't have the stack trace. It
happened during Bakeathon testing over a month ago.
I fixed it by putting CURVNET_SET(TD_TO_VNET(curthread), but that
won't work when in a jail.

The CURVNET_SET(so->so_vnet) just before rpctls_connect()
doesn't always work. My guess would be something like...

• readahead/writebehind thread (from taskqueue, so it is only a kernel thread) does a clnt_reconnect_connect(). clnt_reconnect_connect()->__rpc_nconf2socket()->socreate()->soalloc(CRED_TO_VNET(cred)) I have no idea what curthread->td_ucred is going to be for taskqueue threads.

Anyhow, I cannot recreate this on local testing (during the Bakeathon I am testing against
non-FreeBSD servers over a very slow vpn) and don't have the entire panic/crash.

All I know is that the NFS client mounts are always vnet0, since they cannot be
done in a jail, even though processes/threads running in a jail can use the mount.
(I suspect you are correct, in that the CURVNET_SET()/CURVNET_RESTORE() around
are broken and don't always work. If they can't use so->so_vnet and they can't use
CRED_TO_VNET(), setting vnet0 seems like the only option?)

Ignore the above paragraph. I found where I scribbled down the crash. The calls were:

In D50962#1163337, @glebius wrote:

What is the problem we are trying to fix here? What is the panic trace?

The function rpctls_connect() is always called with socket's vnet set by clnt_reconnect_connect(). In general setting vnet0 is never correct. I understand that right now NFS may not fully support the VIMAGE, but we should slowly aim towards correct support, rather than just blindly plugging the panics.

I realized that this is a NFS client side call,
so it needs to set vnet0.
I'll do that in the specific RPC-over-TLS code.

In D50624#1158192, @khng wrote:

By the way, given that rpcbind and nfsd will install a default pidfile to /var/run even without specifying -P: shall we document that somewhere in RELNOTES and the commit message, or do we make the errno == EEXIST case a warn() call instead of errx()? I am thinking of that because some people might spin up multiple nfsd in their setup without using the rc.d/nfsd script, and with different bound IP for each nfsd spun.

I'll leave the minor nits up to you.
Are you a committer or do you need me
to do it?

In D50627#1156884, @des wrote:

then either fix rpcbind so it doesn't print an error when started before krpc is loaded, or approve this review... your choice

In D50627#1156452, @des wrote:

In D50627#1156360, @rmacklem wrote:

Are you sure? I didn't think the krpc used rpcbind.

It's the other way around; rpcbind isn't useful without krpc, and while krpc usually ends up getting loaded sooner or later, if it's not present when rpcbind starts, you get error messages on the console.

Are you sure? I didn't think the krpc used rpcbind.

The main change is to zfs_readdir(), so that it skips over
forbidden names for a named attribute directory.

Good questions! Maybe they need to be asked on a mailing
list for FreeBSD, but at least I'd like to hear your thoughts.

Added support for _PC_HAS_NAMEDATTR, which is
mainly a new function called zfs_has_namedattr().

In D50140#1143762, @kib wrote:

Does Solaris use the pathconf() to report xattr existence for the specific node?

It seems to do so. Solaris calls it _PC_XATTR_EXISTS.
It seems to work for local files on Solaris, but returns random junk for an NFSv4 mount.
(Since things look correct on the wire, I think this is just a Solaris bug?)

In D50141#1143706, @ziaee wrote:

Are these Function Arguments? Can I alphabetize these?

Clarified when named attributes and named attribute
directories are created.