[jail] removal by jid doesn't trigger pre/post stop scripts

[mizhka]

Summary

Hi,

The "jail -r" doesn't trigger pre/post stop scripts (and more) described in config file if jid is specified instead of name. Debugging showed that in case of "removal by jid", jail doesn't fetch "cfjail" for jail list. Instead, it's create fake structure without scripts from config file.

The patch changes this code path to lookup jail from list of jails loaded from config file.

Test Plan

This patch works at least for me.
Few basic tests are written for regression testing.

[kevans]

usr.sbin/jail/state.c

75

I don't think this part can work. dep_setup is called exactly once- if !docf, then we never setup jails_byname just below and this condition is trivially true.

I would consider, perhaps, reworking running_jail ever-so-slightly to take a struct cfjail **jail and populate *jail if jail != NULL -- and always returning the jid if it's running.

[mizhka]

usr.sbin/jail/state.c

75

1. running_jail returns running cfjail* even if there is no config file. So this part is supposed to work as before.
2. now we return cfjail with name and jid. So it's easy to fetch jid from cfjail. in case of extra argument (I supposed) it will be strange to return same info twice: as jid and as cfjail. If such API looks good, I will add extra argument. I preferred to keep API as simple as possible.

[kevans]

usr.sbin/jail/state.c

75

I suspect this leads to a segfault before we get to fake the cfjail *, though, with an uninitialized jails_byname

[mizhka]

Fix SEGFAULT on uninitialized jails_byname.

[mizhka]

Add basic jail tests:

• create/stop/list/restart jails
• create/list nested jail
• execute commands prestart and prestop

Few tests are not passed on my machine (13-CURRENT r349278):

• prestop on removal by jid (origin issue)
• "jls -n" doesn't work inside jail if there is nested jail (potential new issue)

Feel free to comment.

[mizhka]

Fix regression found thanks to tests:

• "jail -r jailname" returned error: jail not found.

Now tests are passed

[kevans]

etc/mtree/BSD.tests.dist

1071

Sorry- this should sort after fstyp

usr.sbin/jail/state.c

49

Explicit initialization to NULL shouldn't be required, since this is a global

usr.sbin/jail/tests/jail_basic_test.sh

1

We generally omit the shebang for atf-sh tests -- I don't recall the exact reasoning, though the framework (kyua, perhaps? or perhaps the build framework) will DTRT.

[mizhka]

Thank you, @kevans! fixed.

[kevans]

I think this looks good to me now

[ray]

Like it. Thanks!

[jamie]

Aside from a few trailing whitespaces, this all looks good to me.