On 22.06.2015 12:23, kay wrote: > Hey guys, > > This is duplicate from Bugzilla > (https://bugzilla.redhat.com/show_bug.cgi?id=1181539), but I would > like to ask you if my patch looks good. > > "virsh net-update" starts iptables rules reload for NAT network > system. This event doesn't start network hooks, so all previous custom > iptables rules become unavailable. > > How reproducible: > Add "/etc/libvirt/hooks/network" with: > ------------------------------------------- > #!/bin/bash > echo "`date` $0 $@" >> /var/log/libvirt.log > ------------------------------------------- > Restart libvirt and run virsh net-update command: > > virsh net-update default modify ip-dhcp-host --live --config "<host > mac='52:54:00:97:eb:95' name='test' ip='192.168.122.253'/>" > > Actual results: > /var/log/libvirt.log log file doesn't contain events from virsh > net-update command. > > Expected results: > /var/log/libvirt.log log file should contain event. > > Additional info: > This bug brakes iptables hooks hack for FORWARD chain with NAT network. > > Patch file is attached. It was tested with libvirt 1.2.15 > > > libvirt_bridge_hook.patch > > > --- bridge_driver.c 2015-05-27 03:26:03.000000000 +0200 > +++ libvirt-1.2.16/src/network/bridge_driver.c 2015-06-22 09:18:28.829897135 +0200 > @@ -3301,6 +3301,10 @@ > if (needFirewallRefresh && networkAddFirewallRules(network->def) < 0) > goto cleanup; > > + if (needFirewallRefresh && networkRunHook(network, NULL, NULL, VIR_HOOK_NETWORK_OP_STARTED, VIR_HOOK_SUBOP_BEGIN) < 0) > + goto cleanup; > + This is not quite right. The network is not starting. You need to introduce new operation type to virHookNetworkOpType, e.g. VIR_HOOK_NETWORK_OP_UPDATED. Also, we don't like long lines ('make all syntax-check check' is your friend). Otherwise looking good. Also, do you mind proposing this as a regular patch to the list? > + > if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { > /* save updated persistent config to disk */ > if (virNetworkSaveConfig(driver->networkConfigDir, Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list