On 5/1/23 05:19, Laine Stump wrote:
Instead of calling iptableSetupPrivateChains(), the network driver now calls virNetfilterSetupPrivateChains() (which right now always calls the iptables version of the function, but in the future might instead call the nftables version).
virNetFilterSetupPrivateChains() needs an argument to know which backend to call, and that means that networkSetupPrivateChains() has to take an argument (we can't rely on getting the setting from the driver config, because the unit tests don't initialize the network driver).
But networkSetupPrivateChains() was being called with virOnce(), and virOnce() doesn't support calling functions that require an argument (it's based on pthread_once(), which accepts no arguments, so it's not something we can easily fix in our implementation of virOnce()). So instead this patch changes things to handle the "do it once" functionality by adding a static lock, and putting all of networkSetupPrivateChains() (including the setting of "chainInitDone") inside a lock guard - now the places that used to call it via virOnce(), just call it directly instead.
(If it turns out to be significant, we could optimize this by checking for chainInitDone outside the lock guard, returning immediately if it's already set, and then moving the setting of chainInitDone up to the top of the guarded section.)
Signed-off-by: Laine Stump <laine@redhat.com> --- src/libvirt_private.syms | 1 + src/network/bridge_driver_linux.c | 30 +++++++++++++++--------------- src/util/viriptables.h | 7 ++++--- src/util/virnetfilter.c | 16 ++++++++++++++++ src/util/virnetfilter.h | 3 +++ 5 files changed, 39 insertions(+), 18 deletions(-)
This is where I stop my review for today as I have some errands to run. I'll resume tomorrow. So far, this looks good. Michal