On 02/14/2013 06:26 PM, Stefan Berger wrote:

Between revision 65fb9d49 and before this patch, an upgrade of libvirt while VMs are running and instantiating iptables filtering rules due to nwfilter rules, may leave stray iptables rules behind when shutting VMs down. Left-over iptables rules may look like this:

Chain FP-vnet0 (1 references) target prot opt source destination DROP tcp -- 0.0.0.0/0 0.0.0.0/0 tcp spt:122 ACCEPT all -- 0.0.0.0/0 0.0.0.0/0

[...]

Chain libvirt-out (1 references) target prot opt source destination FO-vnet0 all -- 0.0.0.0/0 0.0.0.0/0 [goto] PHYSDEV match --physdev-out vnet0

The reason is that the recent nwfilter code only removed filtering rules in the libvirt-out chain that contain the --physdev-is-bridged parameter. Older rules didn't match and were not removed.

Note that the user-defined chain FO-vnet0 could not be removed due to the reference from the rule in libvirt-out.

Often the work around may be done through

service iptables restart kill -SIGHUP $(pidof libvirtd)

This patch now also removes older libvirt versions' iptables rules.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

Assuming that the scripts you create ignore non-zero exit codes and extra output to stdout (which would both bethe result of running theses new commands on systems that don't need them), ACK. (Actually that is a problem for the network driver's iptables usage - if it tries to remove a rule that isn't there, a warning will be put in the system log.)

--- src/nwfilter/nwfilter_ebiptables_driver.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)

Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -167,16 +167,24 @@ static const char ebiptables_script_set_

#define PHYSDEV_IN "--physdev-in" #define PHYSDEV_OUT "--physdev-is-bridged --physdev-out" +/* + * Previous versions of libvirt only used --physdev-out. + * To be able to upgrade with running VMs we need to be able to + * remove rules generated by those older versions of libvirt. + */ +#define PHYSDEV_OUT_OLD "--physdev-out"

static const char *m_state_out_str = "-m state --state NEW,ESTABLISHED"; static const char *m_state_in_str = "-m state --state ESTABLISHED"; static const char *m_physdev_in_str = "-m physdev " PHYSDEV_IN; static const char *m_physdev_out_str = "-m physdev " PHYSDEV_OUT; +static const char *m_physdev_out_old_str = "-m physdev " PHYSDEV_OUT_OLD;

#define MATCH_STATE_OUT m_state_out_str #define MATCH_STATE_IN m_state_in_str #define MATCH_PHYSDEV_IN m_physdev_in_str #define MATCH_PHYSDEV_OUT m_physdev_out_str +#define MATCH_PHYSDEV_OUT_OLD m_physdev_out_old_str

#define COMMENT_VARNAME "comment"

@@ -821,6 +829,8 @@ _iptablesUnlinkRootChain(virBufferPtr bu : CHAINPREFIX_HOST_OUT; const char *match = (incoming) ? MATCH_PHYSDEV_IN : MATCH_PHYSDEV_OUT; + const char *old_match = (incoming) ? NULL + : MATCH_PHYSDEV_OUT_OLD;

PRINT_IPT_ROOT_CHAIN(chain, chainPrefix, ifname);

@@ -830,6 +840,18 @@ _iptablesUnlinkRootChain(virBufferPtr bu basechain, match, ifname, chain);

+ /* + * Previous versions of libvirt may have created a rule + * with the --physdev-is-bridged missing. Remove this one + * as well. + */ + if (old_match) + virBufferAsprintf(buf, + "$IPT -D %s " + "%s %s -g %s" CMD_SEPARATOR, + basechain, + old_match, ifname, chain); + return 0; }