On Sun, Apr 21, 2024 at 10:53:21PM -0400, Laine Stump wrote:
> Modify networkSetupPrivateChains() in the network driver to accept a
> firewallBackend argument so it will know which backend to call. (right
> now it always calls the iptables version of the lower level function,
> but in the future it could instead call the nftables version based on
> configuration).
>
> 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()). To
> solve this dilemma, this patch eliminates use of virOnce() 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() can safely call it directly
> instead (adding in the necessary argument to specify backend).
>
> (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/network/bridge_driver_linux.c | 65 ++++++++++++++++++++++---------
> 1 file changed, 47 insertions(+), 18 deletions(-)
>
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index c2ef27f251..20671e3ec5 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -34,25 +34,53 @@ VIR_LOG_INIT("network.bridge_driver_linux");
>
> #define PROC_NET_ROUTE "/proc/net/route"
>
> -static virOnceControl createdOnce;
> +static virMutex chainInitLock = VIR_MUTEX_INITIALIZER;
> static bool chainInitDone; /* true iff networkSetupPrivateChains was ever called */
>
> static virErrorPtr errInitV4;
> static virErrorPtr errInitV6;
>
> -/* Usually only called via virOnce, but can also be called directly in
> - * response to firewalld reload (if chainInitDone == true)
> - */
> -static void networkSetupPrivateChains(void)
> +static void
> +networkFirewallBackendUnsetError(void)
> {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("firewall_backend wasn't set, and no usable setting could be auto-detected"));
> +}
> +
> +
> +static int
> +networkFirewallSetupPrivateChains(virFirewallBackend backend,
> + virFirewallLayer layer)
> +{
> + switch (backend) {
> + case VIR_FIREWALL_BACKEND_IPTABLES:
> + return iptablesSetupPrivateChains(layer);
> +
> + case VIR_FIREWALL_BACKEND_UNSET:
> + case VIR_FIREWALL_BACKEND_LAST:
> + networkFirewallBackendUnsetError();
for _LAST and default: cases, you should use
virReportEnumRangeError(virFirewallBackend, backend)
> + return -1;
> + }
> + return 0;
> +}
> +
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
