In D56560#1296776, @bz wrote:

Sorry but I don't know this code specifically and what these macros are doing or not in the end; unless I am going to read it all to understand I cannot help given there is no high level description of what you are trying to (de-)virtualize here.

I had initially asked if it was possible to run NFS exports from within jails. Rick made that happen in 2003(?). And that's kind-of as much as I know. I believe I had not reviewed any of that code back then; you may want to talk to people who did -- I think @markj , @jamie and @mjg did in the end when it came to jails and mounted and some other bits got refined. But that's just from vague memory.

In D56562#1295197, @glebius wrote:

In D56562#1294701, @rmacklem wrote:

For the old way done in FreeBSD-14, the kgss_gssd_handle
would be set NULL when the gssd daemon terminated, at least
when it happened gracefully.

I assume that, for netlink, this never happens?
(ie. Once clnt_nl_create() creates it, it lives forever.)
--> Which means checking for it being non-NULL is a waste

of time?

We have code under #if 0 that implements module unload. It actually should set client to NULL after clnt_destroy(). That's why I left these checks as is, keeping it one change at a time.

However, I agree that we should most likely remove them. As we have rpc_gss module as a proper depend on kggsapi module, and the rpc_gss module always grabs the pointer with kgss_gssd_client() that does CLNT_ACQUIRE()we can actually implement the unload in a manner that through the module lifetime the pointer is never NULL. And module allows unload only when there are no references and no depending modules. But that's definitely a separate patch not to be mixed with this one.

You might want to ask bz@ before getting rid of the
macros, since my recollection was that they were his
idea when I did the code?

I'm fine with getting rid of the macros.

Looks ok to me. If kgss_gssd_handle can never
be NULL, removing the checks for non-NULL
might simplify the code a bit?

This looks fine to me.

Actually that function should go away completely.
It is only used for NFSv4.0, which is now considered
deprecated in favour of NFSv4.1/4.2 and it only
deals with a callback address. Callback addresses
are used for NFSv4.0 only when delegations are
enabled and NFSv4.0 delegations are basically
useless.

Added a sentence w.r.t. when vfs.nfs.enable_uidtostring
needs to be set, as suggested by kib@.

I think I covered kib@'s inline comments.

Looks ok to me. Of course, since we no longer
use Heimdal, I'm not sure it matters?
(Maybe to MFC to FreeBSD-14?)

Thanks for the atomic_load_short explanation.

It all looks good to me, although I won't claim to
understand all the required semantics in
vn_lock_delayed_setsize().

Changed the variable name has_export to only_export,
as suggested by kib@. I also changed the comment
related to this vaiable.

Thanks for doing this. I think it does clarify
the relationship between EXTATTR_NAMESPACE_EMPTY
and EXTATTR_NAMESPACE_EMPTY_STRING for your
other patch.

It looks ok to me, although I do not know
anything about the atf testing part.

I probably should resign, since I don't understand
when/if ACCEPT4_INHERIT might be specified
as an argument for solisten_dequeue()?

This looks fine to me.
I will note that the NFS server never uses svc_vc_accept().
(The accept() is done in userspace.)

Added NET_EPOCH_ENTER()/NET_EPOCH_EXIT() and
cleaned up the code a bit.

As suggested by jilles@ and to be more consistent
with rmextattr(8), change "-S/--symlink" to "-h/--nofollow".

In D55023#1257720, @rmacklem wrote:

In D55023#1257715, @kib wrote:

BTW, chmod(1), chflags(1), ls(1) and find(1) consistently use -P for the same functionality. ls(1) does not fail if the argument is not a symbolic link.

I can switch it to "-P". It was Lionel Cons that suggested "-S".

In D55023#1257715, @kib wrote:

BTW, chmod(1), chflags(1), ls(1) and find(1) consistently use -P for the same functionality. ls(1) does not fail if the argument is not a symbolic link.

I'm going to redo this using getopt().
I didn't bother for one option, but I
now think that's silly.

Looks fine to me. I'll admit I didn't bother to look
to see if/where they get called.

I know nothing about gcc flags, but if it gets
rid of the error, that seems fine to me.

Do roundup2() after the min(args->count, MAXBSIZE)
as suggsted by markj@.

Did formatting changes suggested by Ziaee.

Made formatting changes suggested by kib@.

Updated as suggested by kib@.