Re: [libvirt] [TCK] [PATCH] Add --physdev-is-bridged to test cases
On 02/14/2013 05:25 PM, Stefan Berger wrote:
On 01/24/2013 12:48 PM, Laine Stump wrote:
> On 01/24/2013 11:34 AM, Stefan Berger wrote:
>> Follow recent changes in libvirt and add --physdev-is-bridged to
>> test cases where needed.
> ACK. (Does this mean that new libvirt-tck will fail when run against an
> older libvirt, though?)
I hadn't seen this message.
Yes, due to other rules now being generated the effect will be that
previous version of libvirt will fail that test.
>
> By the way, when the patch went into libvirt, the person who posted it
> mentioned that when restarting libvirtd after the first upgrade with
> that patch, the existing rules wouldn't get removed because they
> wouldn't be an exact match to what libvirt was trying to remove:
>
> On 01/18/2013 02:44 AM, Reinier Schoof wrote:
>> On a side note, please be aware that when upgrading to a libvirt
>> version with this patch included, libvirt will not be able to remove
>> the earlier ip(6)tables rules without the '--physdev-is-bridged'
>> addition. When restarting libvirt, it will look for rules that match
>> with '--physdev-is-bridged' and since that wasn't there before,
you'll
>> end up with a duplicate/malfunctioning ruleset. You'll have to remove
>> these rules/chains manually.
> Is this actually a problem? I had thought that nwfilter always removed
> entire chains instead of individual rules.
It will leave a stray rule and a user-defined table behind. I hadn't
tested an update and didn't think of this problem. Let me see how I
can solve this...
Ok, so here is a solution:
---
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 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;
}
I am guilty of breaking the upgrade path. I'll try this patch some more
and then post it separately...
Stefan