[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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
                       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...


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]