On 09/21/2012 01:46 PM, Laine Stump wrote:
> The bridge driver implementation of virNetworkUpdate() removes and
> re-adds iptables rules any time a network has an <ip>, <forward>, or
> <forward>/<interface> elements updated. There are some types of
> networks that have those elements and yet have no iptables rules
> associated with them, and unfortunately the functions that remove/add
> iptables rules don't check the type of network before attempting to
> remove/add the rules.
>
> Under normal circumstances I would refactor the lower level functions
> to be more robust, but to avoid code churn as much as possible, I've
> just added extra checks directly to networkUpdate().
> ---
> src/network/bridge_driver.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index fce1739..6e260f7 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2945,9 +2945,12 @@ networkUpdate(virNetworkPtr net,
> goto cleanup;
> }
>
> - if (section == VIR_NETWORK_SECTION_IP ||
> - section == VIR_NETWORK_SECTION_FORWARD ||
> - section == VIR_NETWORK_SECTION_FORWARD_INTERFACE) {
> + if ((section == VIR_NETWORK_SECTION_IP ||
> + section == VIR_NETWORK_SECTION_FORWARD ||
> + section == VIR_NETWORK_SECTION_FORWARD_INTERFACE) &&
> + (network->def->forwardType == VIR_NETWORK_FORWARD_NONE ||
Is network->def->forwardType something that can be changed by
networkUpdate()? If so,
> + network->def->forwardType == VIR_NETWORK_FORWARD_NAT ||
> + network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE)) {
> /* these could affect the iptables rules */
> networkRemoveIptablesRules(driver, network);
> if (networkAddIptablesRules(driver, network) < 0)
then it seems like you'd have to check the old type before
networkRemoveIptablesRules, and the new type before
networkAddIptablesRules. But if the forward type is always locked down
once the network is started (where changing types requires destroying
and restarting the network, rather than an on-the-fly update), then this
makes sense. Conditional ACK.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list