[libvirt] [TCK] [PATCH] Add --physdev-is-bridged to test cases

[Stefan Berger]

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