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