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 ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list