jail: consistently populate the KP_JID and KP_NAME parameters

[kevans]

Summary

The gaps here, specifically, were:

• When we have to discover a running jail's jid from name, we should populate the missing jid param
• When we populate jid/name from the config, if the name is a jid we wouldn't populate the name; now we do both.
• When we create a jail, we should populate jid and name with whatever details we have now that we didn't both.

As a consequence, we can cleanup a few things:

• vnet.interface and zfs.dataset can just always the jid
• Trying to populate JNAME should always work now, where it would be a little crashy before if you create a jail that didn't have a name or jid on the command line
• We can simplify the just-prior JID population now that we'll keep a stringified jid in our intparams.

This primarily fixes the below, but the issues with vnet.interface and
zfs.dataset were pre-existing.

Fixes: d8f021add40c3 ("jail: add JID, JNAME and JPATH to env [...]")

[jamie]

usr.sbin/jail/config.c

165–166

This breaks "they may explicitly be set later on." This little config file will yield 'cannot redefine parameter "name".'

47 { name=foo; persist; }

This could be moved after the "collect parameters" section. Then you can test and silently ignore if the name was already set.

[kevans]

usr.sbin/jail/config.c

165–166

Oh, interesting; I guess I assumed from the comment that was already there that this one would simply be overwritten if it was specified later. I'll add some more tests and fix it.

[kevans]

Fix name-override bit

This uses my proposed jls -c from https://reviews.freebsd.org/D51541, but I'll
happily unwind that if we don't like the idea of -c.

[jamie]

usr.sbin/jail/config.c

190

I think namep is redundant, since the only reason for intparams[KP_NAME] to be unset is the same condition that sets namep=KP_JID (that j->name was numeric).

[kevans]

usr.sbin/jail/config.c

190

Oh, good point.

[kevans]

Revert more of the config.c diff; amending the comment and checking for KP_NAME
later is sufficient.

[zlei]

I was writing tests and found

# jail -vc vnet vnet.interface=epair0a persist
jail_set(JAIL_CREATE) vnet=new persist
created
run command: /sbin/ifconfig epair0a vnet
ifconfig: 'vnet' requires argument
jail: /sbin/ifconfig epair0a vnet: failed
removed
jail -c vnet vnet.interface=epair0a persist

does not work on stable/14.

Nice fix !

[kevans]

In D51502#1295042, @zlei wrote:

I was writing tests and found

# jail -vc vnet vnet.interface=epair0a persist
jail_set(JAIL_CREATE) vnet=new persist
created
run command: /sbin/ifconfig epair0a vnet
ifconfig: 'vnet' requires argument
jail: /sbin/ifconfig epair0a vnet: failed
removed
jail -c vnet vnet.interface=epair0a persist

does not work on stable/14.

Nice fix !

I had considered MFC but decided not to only because it was a reaction to the Fixes commit. There's clear value outside of that, but I didn't think it'd be 'enough' since I hadn't seen any complaints about it.

That said, I don't really object and can merge it today if that would be useful. It just needs b81fd3fc8b20eaad64b5c41826432124fd92d6a7 to go along with it.

[ngie]

LGTM. Just some drive-by comments after the fact.

usr.sbin/jail/tests/jail_basic_test.sh

189

This is the same thing expressed differently (less chance for typos).

193

Missing newline between } and clean_jails().