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@xxxxxxxxxx> > --- > 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