On Wed 2018-11-14 21:14:08 +0100, Fabian Grünbichler wrote: > Maybe going with the meta package is a good idea then - especially since > it allows us a cheap 'opt-in' for the reloading behaviour.
yep, i'm leaning in this direction too. > I was thinking about changes to the wg-quick template unit (e.g., adding > more isolation features). But those are likely not as important as > changes to the module, so if we simplify to two (or no ;)) options, I'd > also go with the 'module' semantics. cool, sounds like we're converging. > Sounds good to me. And was so straightforward I just went ahead and > pushed that (the previous iteration is available as > mr/reload-on-upgrade_v1 tag). You probably still want to adapt the echo > statements and package description ;) This looks great! a few more nit-picking questions: a) is there a risk that we could somehow have the wireguard kernel module loaded, but *not* have wireguard.ko available for finding via modinfo -F ? If that's the case, then it looks like the postinst script will exit with an error, rather than exiting cleanly. b) is there a reason why we walk through $units in a for loop, when systemctl is supposed to be able to take an arbitrary number of units as arguments? that is, why not just do: systemctl stop $units || true instead of this whole rigamarole: for unit in $units; do echo "$unit" >&2 systemctl stop "$unit" >/dev/null || true done c) where does "sleep 3" come from? why 3 seconds instead of 1 or 5? i'm always a bit leery of magic numbers like this. d) is it possible to avoid stopping and restarting wg-quick@ units by comparing it to "wg show" *before* stopping and starting wg-quick@ units? If it's not too clever to do, it might at least avoid some unnecessary network disruption on some systems. let me know what you think! thanks for being so gracious about the back-and-forth here, and for your work on getting this into shape! --dkg
signature.asc
Description: PGP signature