From 0e3c86dcdc49890eecaa2d1ea64c0012e569682f Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Thu, 17 Feb 2011 22:07:00 +0100
Subject: Use a semaphore to ensure we don't run more than one iptables-restore
 at a time.

---
 nova/virt/libvirt_conn.py | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 4e0fd106f..7548fff63 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -46,6 +46,7 @@ from xml.dom import minidom
 
 from eventlet import greenthread
 from eventlet import event
+from eventlet import semaphore
 from eventlet import tpool
 
 import IPy
@@ -63,6 +64,7 @@ from nova.compute import power_state
 from nova.virt import disk
 from nova.virt import images
 
+libvirt_semaphore = semaphore.Semaphore()
 libvirt = None
 libxml2 = None
 Template = None
@@ -1237,17 +1239,19 @@ class IptablesFirewallDriver(FirewallDriver):
         self.apply_ruleset()
 
     def apply_ruleset(self):
-        current_filter, _ = self.execute('sudo iptables-save -t filter')
-        current_lines = current_filter.split('\n')
-        new_filter = self.modify_rules(current_lines, 4)
-        self.execute('sudo iptables-restore',
-                     process_input='\n'.join(new_filter))
-        if(FLAGS.use_ipv6):
-            current_filter, _ = self.execute('sudo ip6tables-save -t filter')
+        with libvirt_semaphore:
+            current_filter, _ = self.execute('sudo iptables-save -t filter')
             current_lines = current_filter.split('\n')
-            new_filter = self.modify_rules(current_lines, 6)
-            self.execute('sudo ip6tables-restore',
+            new_filter = self.modify_rules(current_lines, 4)
+            self.execute('sudo iptables-restore',
                          process_input='\n'.join(new_filter))
+            if(FLAGS.use_ipv6):
+                current_filter, _ = self.execute('sudo ip6tables-save '
+                                                 '-t filter')
+                current_lines = current_filter.split('\n')
+                new_filter = self.modify_rules(current_lines, 6)
+                self.execute('sudo ip6tables-restore',
+                             process_input='\n'.join(new_filter))
 
     def modify_rules(self, current_lines, ip_version=4):
         ctxt = context.get_admin_context()
-- 
cgit 


From 5812a95736b9a16733b99700e8664dd29ae34def Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Fri, 18 Feb 2011 22:10:06 +0100
Subject: Introduce IptablesManager in linux_net. Port every use of iptables in
 linux_net to it.

---
 nova/network/linux_net.py  | 287 ++++++++++++++++++++++++++++-----------------
 nova/tests/test_network.py |  59 ++++++++++
 nova/utils.py              |  61 ++++++----
 nova/virt/libvirt_conn.py  |  28 +++--
 4 files changed, 287 insertions(+), 148 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index c1cbff7d8..3d267d941 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -17,6 +17,7 @@
 Implements vlans, bridges, and iptables rules using linux utilities.
 """
 
+import inspect
 import os
 
 from nova import db
@@ -25,7 +26,6 @@ from nova import flags
 from nova import log as logging
 from nova import utils
 
-
 LOG = logging.getLogger("nova.linux_net")
 
 
@@ -63,73 +63,168 @@ flags.DEFINE_string('dmz_cidr', '10.128.0.0/24',
                     'dmz range that should be accepted')
 
 
+binary_name = os.path.basename(inspect.stack()[-1][1])
+
+
+class IptablesRule(object):
+    def __init__(self, chain, rule, wrap=True):
+        self.chain = chain
+        self.rule = rule
+        self.wrap = wrap
+
+    def __eq__(self, other):
+        return ((self.chain == other.chain) and
+                (self.rule == other.rule) and
+                (self.wrap == other.wrap))
+
+    def __ne__(self, other):
+        return ((self.chain != other.chain) or
+                (self.rule != other.rule) or
+                (self.wrap != other.wrap))
+
+    def __str__(self):
+        if self.wrap:
+            chain = '%s-%s' % (binary_name, self.chain)
+        else:
+            chain = self.chain
+        return '-A %s %s' % (chain, self.rule)
+
+
+class IptablesTable(object):
+    def __init__(self):
+        self.rules = []
+        self.chains = set()
+
+    def add_chain(self, name):
+        self.chains.add(name)
+
+    def remove_chain(self, name):
+        self.chains.remove(name)
+
+    def add_rule(self, chain, rule, wrap=True):
+        if wrap and chain not in self.chains:
+            raise ValueError(_("Unknown chain: %r") % chain)
+
+        self.rules.append(IptablesRule(chain, rule, wrap))
+
+    def remove_rule(self, chain, rule):
+        self.rules.remove(IptablesRule(chain, rule))
+
+class IptablesManager(object):
+    def __init__(self, execute=None):
+        if not execute:
+            if FLAGS.fake_network:
+                self.execute = lambda *args, **kwargs: ('', '')
+            else:
+                self.execute = utils.execute
+        else:
+            self.execute = execute
+
+        self.ipv4 = { 'filter': IptablesTable(),
+                      'nat': IptablesTable() }
+        self.ipv6 = { 'filter': IptablesTable(),
+                      'nat': IptablesTable() }
+
+        self.ipv4['nat'].add_chain('SNATTING')
+        self.ipv4['nat'].add_rule('POSTROUTING',
+                                  '-j %s-SNATTING' % (binary_name,),
+                                  wrap=False)
+
+        self.ipv4['filter'].add_chain('local')
+        self.ipv4['filter'].add_rule('FORWARD',
+                                    '-j %s-local' % (binary_name,),
+                                    wrap=False)
+
+        # Wrap the builtin chains
+        builtin_chains = {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
+                          'nat': ['PREROUTING', 'INPUT',
+                                  'OUTPUT', 'POSTROUTING']}
+
+        for table, chains in builtin_chains.iteritems():
+            for chain in chains:
+                self.ipv4[table].add_chain(chain)
+                self.ipv4[table].add_rule(chain,
+                                          '-j %s-%s' % (binary_name, chain),
+                                          wrap=False)
+
+
+    def apply(self):
+        s = [('iptables', self.ipv4)]
+        if FLAGS.use_ipv6:
+            s += [('ip6tables', self.ipv6)]
+
+        for cmd, tables in s:
+            for table in tables:
+                current_filter, _ = self.execute('sudo %s-save -t %s' %
+                                                 (cmd, table), attempts=5)
+                current_lines = current_filter.split('\n')
+                new_filter = self.modify_rules(current_lines, tables[table])
+                self.execute('sudo %s-restore' % (cmd,),
+                             process_input='\n'.join(new_filter),
+                             attempts=5)
+
+    def modify_rules(self, current_lines, table, binary=None):
+
+        chains = table.chains
+        rules = table.rules
+
+        # Remove any trace of our rules
+        new_filter = filter(lambda l: '%s' % binary not in l, current_lines)
+
+        seen_chains = False
+        for rules_index in range(len(new_filter)):
+            if not seen_chains:
+                if new_filter[rules_index].startswith(':'):
+                    seen_chains = True
+            elif seen_chains == 1:
+                if not new_filter[rules_index].startswith(':'):
+                    break
+
+        new_filter[rules_index:rules_index] = [str(rule) for rule in rules]
+        new_filter[rules_index:rules_index] = [':%s-%s - [0:0]' % \
+                                               (binary_name, name,) \
+                                               for name in chains]
+
+        return new_filter
+
+
+iptables_manager = IptablesManager()
+
+
 def metadata_forward():
     """Create forwarding rule for metadata"""
-    _confirm_rule("PREROUTING", "-t nat -s 0.0.0.0/0 "
-             "-d 169.254.169.254/32 -p tcp -m tcp --dport 80 -j DNAT "
-             "--to-destination %s:%s" % (FLAGS.ec2_dmz_host, FLAGS.ec2_port))
+    iptables_manager.ipv4['nat'].add_rule("PREROUTING",
+                                          "-s 0.0.0.0/0 -d 169.254.169.254/32 "
+                                          "-p tcp -m tcp --dport 80 -j DNAT "
+                                          "--to-destination %s:%s" % \
+                                          (FLAGS.ec2_dmz_host, FLAGS.ec2_port))
+    iptables_manager.apply()
 
 
 def init_host():
     """Basic networking setup goes here"""
 
-    if FLAGS.use_nova_chains:
-        _execute("sudo iptables -N nova_input", check_exit_code=False)
-        _execute("sudo iptables -D %s -j nova_input" % FLAGS.input_chain,
-                 check_exit_code=False)
-        _execute("sudo iptables -A %s -j nova_input" % FLAGS.input_chain)
-
-        _execute("sudo iptables -N nova_forward", check_exit_code=False)
-        _execute("sudo iptables -D FORWARD -j nova_forward",
-                 check_exit_code=False)
-        _execute("sudo iptables -A FORWARD -j nova_forward")
-
-        _execute("sudo iptables -N nova_output", check_exit_code=False)
-        _execute("sudo iptables -D OUTPUT -j nova_output",
-                 check_exit_code=False)
-        _execute("sudo iptables -A OUTPUT -j nova_output")
-
-        _execute("sudo iptables -t nat -N nova_prerouting",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -D PREROUTING -j nova_prerouting",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -A PREROUTING -j nova_prerouting")
-
-        _execute("sudo iptables -t nat -N nova_postrouting",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -D POSTROUTING -j nova_postrouting",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -A POSTROUTING -j nova_postrouting")
-
-        _execute("sudo iptables -t nat -N nova_snatting",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -D POSTROUTING -j nova_snatting",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -A POSTROUTING -j nova_snatting")
-
-        _execute("sudo iptables -t nat -N nova_output", check_exit_code=False)
-        _execute("sudo iptables -t nat -D OUTPUT -j nova_output",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -A OUTPUT -j nova_output")
-    else:
-        # NOTE(vish): This makes it easy to ensure snatting rules always
-        #             come after the accept rules in the postrouting chain
-        _execute("sudo iptables -t nat -N SNATTING",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -D POSTROUTING -j SNATTING",
-                 check_exit_code=False)
-        _execute("sudo iptables -t nat -A POSTROUTING -j SNATTING")
-
     # NOTE(devcamcar): Cloud public SNAT entries and the default
     # SNAT rule for outbound traffic.
-    _confirm_rule("SNATTING", "-t nat -s %s "
-             "-j SNAT --to-source %s"
-             % (FLAGS.fixed_range, FLAGS.routing_source_ip), append=True)
+    iptables_manager.ipv4['nat'].add_rule("SNATTING",
+                                          "-s %s -j SNAT --to-source %s" % \
+                                           (FLAGS.fixed_range,
+                                            FLAGS.routing_source_ip))
+
+    iptables_manager.ipv4['nat'].add_rule("POSTROUTING",
+                                          "-s %s -j SNAT --to-source %s" % \
+                                           (FLAGS.fixed_range,
+                                            FLAGS.routing_source_ip))
 
-    _confirm_rule("POSTROUTING", "-t nat -s %s -d %s -j ACCEPT" %
-                  (FLAGS.fixed_range, FLAGS.dmz_cidr))
-    _confirm_rule("POSTROUTING", "-t nat -s %(range)s -d %(range)s -j ACCEPT" %
-                  {'range': FLAGS.fixed_range})
+    iptables_manager.ipv4['nat'].add_rule("POSTROUTING",
+                                          "-s %s -d %s -j ACCEPT" % \
+                                          (FLAGS.fixed_range, FLAGS.dmz_cidr))
+
+    iptables_manager.ipv4['nat'].add_rule("POSTROUTING",
+                                          "-s %(range)s -d %(range)s "
+                                          "-j ACCEPT" % \
+                                          {'range': FLAGS.fixed_range})
+    iptables_manager.apply()
 
 
 def bind_floating_ip(floating_ip, check_exit_code=True):
@@ -147,32 +242,33 @@ def unbind_floating_ip(floating_ip):
 
 def ensure_vlan_forward(public_ip, port, private_ip):
     """Sets up forwarding rules for vlan"""
-    _confirm_rule("FORWARD", "-d %s -p udp --dport 1194 -j ACCEPT" %
-                  private_ip)
-    _confirm_rule("PREROUTING",
-                  "-t nat -d %s -p udp --dport %s -j DNAT --to %s:1194"
-            % (public_ip, port, private_ip))
+    iptables_manager.ipv4['filter'].add_rule("FORWARD",
+                                             "-d %s -p udp "
+                                             "--dport 1194 "
+                                             "-j ACCEPT" % private_ip)
+    iptables_manager.ipv4['nat'].add_rule("PREROUTING",
+                                          "-d %s -p udp "
+                                          "--dport %s -j DNAT --to %s:1194" %
+                                          (public_ip, port, private_ip))
+    iptables_manager.apply()
 
 
 def ensure_floating_forward(floating_ip, fixed_ip):
     """Ensure floating ip forwarding rule"""
-    _confirm_rule("PREROUTING", "-t nat -d %s -j DNAT --to %s"
-                           % (floating_ip, fixed_ip))
-    _confirm_rule("OUTPUT", "-t nat -d %s -j DNAT --to %s"
-                           % (floating_ip, fixed_ip))
-    _confirm_rule("SNATTING", "-t nat -s %s -j SNAT --to %s"
-                           % (fixed_ip, floating_ip))
-
+    for chain, rule in floating_forward_rules(floating_ip, fixed_ip):
+        iptables_manager.ipv4['nat'].add_rule(chain, rule)
+    iptables_manager.apply()
 
 def remove_floating_forward(floating_ip, fixed_ip):
     """Remove forwarding for floating ip"""
-    _remove_rule("PREROUTING", "-t nat -d %s -j DNAT --to %s"
-                          % (floating_ip, fixed_ip))
-    _remove_rule("OUTPUT", "-t nat -d %s -j DNAT --to %s"
-                          % (floating_ip, fixed_ip))
-    _remove_rule("SNATTING", "-t nat -s %s -j SNAT --to %s"
-                          % (fixed_ip, floating_ip))
+    for chain, rule in floating_forward_rules(floating_ip, fixed_ip):
+        iptables_manager.ipv4['nat'].remove_rule(chain, rule)
+    iptables_manager.apply()
 
+def floating_forward_rules(floating_ip, fixed_ip):
+    return [("PREROUTING", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
+            ("OUTPUT", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
+            ("SNATTING", "-d %s -j DNAT --to %s" % (fixed_ip, floating_ip))]
 
 def ensure_vlan_bridge(vlan_num, bridge, net_attrs=None):
     """Create a vlan and bridge unless they already exist"""
@@ -258,19 +354,12 @@ def ensure_bridge(bridge, interface, net_attrs=None):
                            "enslave it to bridge %s.\n" % (interface, bridge)):
             raise exception.Error("Failed to add interface: %s" % err)
 
-    if FLAGS.use_nova_chains:
-        (out, err) = _execute("sudo iptables -N nova_forward",
-                              check_exit_code=False)
-        if err != 'iptables: Chain already exists.\n':
-            # NOTE(vish): chain didn't exist link chain
-            _execute("sudo iptables -D FORWARD -j nova_forward",
-                     check_exit_code=False)
-            _execute("sudo iptables -A FORWARD -j nova_forward")
-
-    _confirm_rule("FORWARD", "--in-interface %s -j ACCEPT" % bridge)
-    _confirm_rule("FORWARD", "--out-interface %s -j ACCEPT" % bridge)
-    _execute("sudo iptables -N nova-local", check_exit_code=False)
-    _confirm_rule("FORWARD", "-j nova-local")
+    iptables_manager.ipv4['filter'].add_rule("FORWARD",
+                                             "--in-interface %s -j ACCEPT" % \
+                                             bridge)
+    iptables_manager.ipv4['filter'].add_rule("FORWARD",
+                                             "--out-interface %s -j ACCEPT" % \
+                                             bridge)
 
 
 def get_dhcp_hosts(context, network_id):
@@ -390,26 +479,6 @@ def _device_exists(device):
     return not err
 
 
-def _confirm_rule(chain, cmd, append=False):
-    """Delete and re-add iptables rule"""
-    if FLAGS.use_nova_chains:
-        chain = "nova_%s" % chain.lower()
-    if append:
-        loc = "-A"
-    else:
-        loc = "-I"
-    _execute("sudo iptables --delete %s %s" % (chain, cmd),
-             check_exit_code=False)
-    _execute("sudo iptables %s %s %s" % (loc, chain, cmd))
-
-
-def _remove_rule(chain, cmd):
-    """Remove iptables rule"""
-    if FLAGS.use_nova_chains:
-        chain = "%s" % chain.lower()
-    _execute("sudo iptables --delete %s %s" % (chain, cmd))
-
-
 def _dnsmasq_cmd(net):
     """Builds dnsmasq command"""
     cmd = ['sudo -E dnsmasq',
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index 00f9323f3..b28d64245 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -29,11 +29,70 @@ from nova import log as logging
 from nova import test
 from nova import utils
 from nova.auth import manager
+from nova.network import linux_net
 
 FLAGS = flags.FLAGS
 LOG = logging.getLogger('nova.tests.network')
 
 
+class IptablesManagerTestCase(test.TestCase):
+    sample_filter = """# Completed on Fri Feb 18 15:17:05 2011
+# Generated by iptables-save v1.4.10 on Fri Feb 18 15:17:05 2011
+*filter
+:INPUT ACCEPT [2223527:305688874]
+:FORWARD ACCEPT [0:0]
+:OUTPUT ACCEPT [2172501:140856656]
+-A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT 
+-A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT 
+-A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT 
+-A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT 
+-A FORWARD -d 192.168.122.0/24 -o virbr0 -m state --state RELATED,ESTABLISHED -j ACCEPT 
+-A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT 
+-A FORWARD -i virbr0 -o virbr0 -j ACCEPT 
+-A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable 
+-A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable 
+COMMIT
+# Completed on Fri Feb 18 15:17:05 2011"""
+
+    def setUp(self):
+        super(IptablesManagerTestCase, self).setUp()
+        self.manager = linux_net.IptablesManager()
+
+    def test_rules_are_wrapped(self):
+        current_lines = self.sample_filter.split('\n')
+
+        table = self.manager.ipv4['filter']
+        table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
+        new_lines = self.manager.modify_rules(current_lines, table)
+        self.assertTrue('-A run_tests.py-FORWARD '
+                        '-s 1.2.3.4/5 -j DROP' in new_lines)
+
+        table.remove_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
+        new_lines = self.manager.modify_rules(current_lines, table)
+        self.assertTrue('-A run_tests.py-FORWARD '
+                        '-s 1.2.3.4/5 -j DROP' not in new_lines)
+
+    def test_wrapper_rules_in_place(self):
+        current_lines = self.sample_filter.split('\n')
+
+        # TODO(soren): Add stuff for ipv6
+        check_matrix = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
+                            'nat': ['PREROUTING', 'INPUT',
+                                    'OUTPUT', 'POSTROUTING']} }
+
+        for ip_version in check_matrix:
+            ip = getattr(self.manager, 'ipv%d' % ip_version)
+            for table_name in ip:
+                table = ip[table_name]
+                new_lines = self.manager.modify_rules(current_lines, table)
+                for chain in check_matrix[ip_version][table_name]:
+                    self.assertTrue(':run_tests.py-%s - [0:0]' % \
+                                    (chain,) in new_lines)
+                    self.assertTrue('-A %s -j run_tests.py-%s' % \
+                                    (chain, chain) in new_lines)
+                print '\n'.join(new_lines)
+
+
 class NetworkTestCase(test.TestCase):
     """Test cases for network code"""
     def setUp(self):
diff --git a/nova/utils.py b/nova/utils.py
index ba71ebf39..bf3a4b098 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -124,32 +124,41 @@ def fetchfile(url, target):
     execute("curl --fail %s -o %s" % (url, target))
 
 
-def execute(cmd, process_input=None, addl_env=None, check_exit_code=True):
-    LOG.debug(_("Running cmd (subprocess): %s"), cmd)
-    env = os.environ.copy()
-    if addl_env:
-        env.update(addl_env)
-    obj = subprocess.Popen(cmd, shell=True, stdin=subprocess.PIPE,
-        stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
-    result = None
-    if process_input != None:
-        result = obj.communicate(process_input)
-    else:
-        result = obj.communicate()
-    obj.stdin.close()
-    if obj.returncode:
-        LOG.debug(_("Result was %s") % obj.returncode)
-        if check_exit_code and obj.returncode != 0:
-            (stdout, stderr) = result
-            raise ProcessExecutionError(exit_code=obj.returncode,
-                                        stdout=stdout,
-                                        stderr=stderr,
-                                        cmd=cmd)
-    # NOTE(termie): this appears to be necessary to let the subprocess call
-    #               clean something up in between calls, without it two
-    #               execute calls in a row hangs the second one
-    greenthread.sleep(0)
-    return result
+def execute(cmd, process_input=None, addl_env=None, check_exit_code=True, attempts=1):
+    while attempts > 0:
+        attempts -= 1
+        try:
+            LOG.debug(_("Running cmd (subprocess): %s"), cmd)
+            env = os.environ.copy()
+            if addl_env:
+                env.update(addl_env)
+            obj = subprocess.Popen(cmd, shell=True, stdin=subprocess.PIPE,
+                stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
+            result = None
+            if process_input != None:
+                result = obj.communicate(process_input)
+            else:
+                result = obj.communicate()
+            obj.stdin.close()
+            if obj.returncode:
+                LOG.debug(_("Result was %s") % obj.returncode)
+                if check_exit_code and obj.returncode != 0:
+                    (stdout, stderr) = result
+                    raise ProcessExecutionError(exit_code=obj.returncode,
+                                                stdout=stdout,
+                                                stderr=stderr,
+                                                cmd=cmd)
+            # NOTE(termie): this appears to be necessary to let the subprocess call
+            #               clean something up in between calls, without it two
+            #               execute calls in a row hangs the second one
+            greenthread.sleep(0)
+            return result
+        except ProcessExecutionError:
+            if not attempts:
+                raise
+            else:
+                greenthread.sleep(random.randint(50,300)/100)
+                pass
 
 
 def ssh_execute(ssh, cmd, process_input=None,
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 7548fff63..11b3acbf6 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -64,7 +64,6 @@ from nova.compute import power_state
 from nova.virt import disk
 from nova.virt import images
 
-libvirt_semaphore = semaphore.Semaphore()
 libvirt = None
 libxml2 = None
 Template = None
@@ -1239,19 +1238,22 @@ class IptablesFirewallDriver(FirewallDriver):
         self.apply_ruleset()
 
     def apply_ruleset(self):
-        with libvirt_semaphore:
-            current_filter, _ = self.execute('sudo iptables-save -t filter')
+        current_filter, _ = self.execute('sudo iptables-save -t filter',
+                                         attempts=5)
+        current_lines = current_filter.split('\n')
+        new_filter = self.modify_rules(current_lines, 4)
+        self.execute('sudo iptables-restore',
+                     process_input='\n'.join(new_filter),
+                     attempts=5)
+        if(FLAGS.use_ipv6):
+            current_filter, _ = self.execute('sudo ip6tables-save '
+                                             '-t filter',
+                                             attempts=5)
             current_lines = current_filter.split('\n')
-            new_filter = self.modify_rules(current_lines, 4)
-            self.execute('sudo iptables-restore',
-                         process_input='\n'.join(new_filter))
-            if(FLAGS.use_ipv6):
-                current_filter, _ = self.execute('sudo ip6tables-save '
-                                                 '-t filter')
-                current_lines = current_filter.split('\n')
-                new_filter = self.modify_rules(current_lines, 6)
-                self.execute('sudo ip6tables-restore',
-                             process_input='\n'.join(new_filter))
+            new_filter = self.modify_rules(current_lines, 6)
+            self.execute('sudo ip6tables-restore',
+                         process_input='\n'.join(new_filter),
+                         attempts=5)
 
     def modify_rules(self, current_lines, ip_version=4):
         ctxt = context.get_admin_context()
-- 
cgit 


From cfd6d4e403dcb2405cd7ff48bad3083a02159d2c Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Sat, 19 Feb 2011 00:14:08 +0100
Subject: Port libvirt_conn.IptablesDriver over to use
 linux_net.IptablesManager

---
 nova/network/linux_net.py |  17 +++-
 nova/tests/test_virt.py   |  55 ++++++++----
 nova/virt/libvirt_conn.py | 215 ++++++++++++++++++++--------------------------
 3 files changed, 145 insertions(+), 142 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 3d267d941..c11d34922 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -100,13 +100,23 @@ class IptablesTable(object):
 
     def remove_chain(self, name):
         self.chains.remove(name)
+        self.rules = filter(lambda r: r.chain != name, self.rules)
 
     def add_rule(self, chain, rule, wrap=True):
         if wrap and chain not in self.chains:
             raise ValueError(_("Unknown chain: %r") % chain)
 
+        if '$' in rule:
+            rule = ' '.join(map(self._wrap_target_chain, rule.split(' ')))
+
+        print 'Adding rule: %r' % rule
         self.rules.append(IptablesRule(chain, rule, wrap))
 
+    def _wrap_target_chain(self, s):
+        if s.startswith('$'):
+            return '%s-%s' % (binary_name, s[1:])
+        return s
+
     def remove_rule(self, chain, rule):
         self.rules.remove(IptablesRule(chain, rule))
 
@@ -122,8 +132,7 @@ class IptablesManager(object):
 
         self.ipv4 = { 'filter': IptablesTable(),
                       'nat': IptablesTable() }
-        self.ipv6 = { 'filter': IptablesTable(),
-                      'nat': IptablesTable() }
+        self.ipv6 = { 'filter': IptablesTable() }
 
         self.ipv4['nat'].add_chain('SNATTING')
         self.ipv4['nat'].add_rule('POSTROUTING',
@@ -135,6 +144,10 @@ class IptablesManager(object):
                                     '-j %s-local' % (binary_name,),
                                     wrap=False)
 
+        self.ipv4['filter'].add_rule('OUTPUT',
+                                    '-j %s-local' % (binary_name,),
+                                    wrap=False)
+
         # Wrap the builtin chains
         builtin_chains = {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
                           'nat': ['PREROUTING', 'INPUT',
diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index 6e5a0114b..a88e01818 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -14,6 +14,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import re
 from xml.etree.ElementTree import fromstring as xml_to_tree
 from xml.dom.minidom import parseString as xml_to_dom
 
@@ -233,16 +234,22 @@ class IptablesFirewallTestCase(test.TestCase):
         self.manager.delete_user(self.user)
         super(IptablesFirewallTestCase, self).tearDown()
 
-    in_rules = [
+    in_nat_rules = [
+      '# Generated by iptables-save v1.4.10 on Sat Feb 19 00:03:19 2011',
+      '*nat',
+      ':PREROUTING ACCEPT [1170:189210]',
+      ':INPUT ACCEPT [844:71028]',
+      ':OUTPUT ACCEPT [5149:405186]',
+      ':POSTROUTING ACCEPT [5063:386098]'
+    ]
+
+    in_filter_rules = [
       '# Generated by iptables-save v1.4.4 on Mon Dec  6 11:54:13 2010',
       '*filter',
       ':INPUT ACCEPT [969615:281627771]',
       ':FORWARD ACCEPT [0:0]',
       ':OUTPUT ACCEPT [915599:63811649]',
       ':nova-block-ipv4 - [0:0]',
-      '-A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT ',
-      '-A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT ',
-      '-A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT ',
       '-A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT ',
       '-A FORWARD -d 192.168.122.0/24 -o virbr0 -m state --state RELATED'
       ',ESTABLISHED -j ACCEPT ',
@@ -254,7 +261,7 @@ class IptablesFirewallTestCase(test.TestCase):
       '# Completed on Mon Dec  6 11:54:13 2010',
     ]
 
-    in6_rules = [
+    in6_filter_rules = [
       '# Generated by ip6tables-save v1.4.4 on Tue Jan 18 23:47:56 2011',
       '*filter',
       ':INPUT ACCEPT [349155:75810423]',
@@ -314,23 +321,31 @@ class IptablesFirewallTestCase(test.TestCase):
         instance_ref = db.instance_get(admin_ctxt, instance_ref['id'])
 
 #        self.fw.add_instance(instance_ref)
-        def fake_iptables_execute(cmd, process_input=None):
+        def fake_iptables_execute(cmd, process_input=None, attempts=5):
             if cmd == 'sudo ip6tables-save -t filter':
-                return '\n'.join(self.in6_rules), None
+                return '\n'.join(self.in6_filter_rules), None
             if cmd == 'sudo iptables-save -t filter':
-                return '\n'.join(self.in_rules), None
+                return '\n'.join(self.in_filter_rules), None
+            if cmd == 'sudo iptables-save -t nat':
+                return '\n'.join(self.in_nat_rules), None
             if cmd == 'sudo iptables-restore':
-                self.out_rules = process_input.split('\n')
+                lines = process_input.split('\n')
+                if '*filter' in lines:
+                    self.out_rules = lines
                 return '', ''
             if cmd == 'sudo ip6tables-restore':
-                self.out6_rules = process_input.split('\n')
+                lines = process_input.split('\n')
+                if '*filter' in lines:
+                    self.out6_rules = lines
                 return '', ''
-        self.fw.execute = fake_iptables_execute
+
+        from nova.network import linux_net
+        linux_net.iptables_manager.execute = fake_iptables_execute
 
         self.fw.prepare_instance_filter(instance_ref)
         self.fw.apply_instance_filter(instance_ref)
 
-        in_rules = filter(lambda l: not l.startswith('#'), self.in_rules)
+        in_rules = filter(lambda l: not l.startswith('#'), self.in_filter_rules)
         for rule in in_rules:
             if not 'nova' in rule:
                 self.assertTrue(rule in self.out_rules,
@@ -338,6 +353,7 @@ class IptablesFirewallTestCase(test.TestCase):
 
         instance_chain = None
         for rule in self.out_rules:
+            print rule
             # This is pretty crude, but it'll do for now
             if '-d 10.11.12.13 -j' in rule:
                 instance_chain = rule.split(' ')[-1]
@@ -353,17 +369,18 @@ class IptablesFirewallTestCase(test.TestCase):
         self.assertTrue(security_group_chain,
                         "The security group chain wasn't added")
 
-        self.assertTrue('-A %s -p icmp -s 192.168.11.0/24 -j ACCEPT' % \
-                               security_group_chain in self.out_rules,
+        regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -j ACCEPT')
+        self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "ICMP acceptance rule wasn't added")
 
-        self.assertTrue('-A %s -p icmp -s 192.168.11.0/24 -m icmp --icmp-type '
-                        '8 -j ACCEPT' % security_group_chain in self.out_rules,
+        regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -m icmp '
+                           '--icmp-type 8 -j ACCEPT')
+        self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "ICMP Echo Request acceptance rule wasn't added")
 
-        self.assertTrue('-A %s -p tcp -s 192.168.10.0/24 -m multiport '
-                        '--dports 80:81 -j ACCEPT' % security_group_chain \
-                            in self.out_rules,
+        regex = re.compile('-A .* -p tcp -s 192.168.10.0/24 -m multiport '
+                           '--dports 80:81 -j ACCEPT')
+        self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "TCP port 80/81 acceptance rule wasn't added")
 
 
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 11b3acbf6..976ccaca5 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -57,7 +57,6 @@ from nova import exception
 from nova import flags
 from nova import log as logging
 from nova import utils
-#from nova.api import context
 from nova.auth import manager
 from nova.compute import instance_types
 from nova.compute import power_state
@@ -1207,10 +1206,14 @@ class NWFilterFirewall(FirewallDriver):
 
 class IptablesFirewallDriver(FirewallDriver):
     def __init__(self, execute=None, **kwargs):
-        self.execute = execute or utils.execute
+        from nova.network import linux_net
+        self.iptables = linux_net.iptables_manager
         self.instances = {}
         self.nwfilter = NWFilterFirewall(kwargs['get_connection'])
 
+        self.iptables.ipv4['filter'].add_chain('sg-fallback')
+        self.iptables.ipv4['filter'].add_rule('sg-fallback', '-j DROP')
+
     def setup_basic_filtering(self, instance):
         """Use NWFilter from libvirt for this."""
         return self.nwfilter.setup_basic_filtering(instance)
@@ -1226,124 +1229,92 @@ class IptablesFirewallDriver(FirewallDriver):
             LOG.info(_('Attempted to unfilter instance %s which is not '
                        'filtered'), instance['id'])
 
-    def add_instance(self, instance):
+    def prepare_instance_filter(self, instance):
         self.instances[instance['id']] = instance
 
+        chain_name = self._instance_chain_name(instance)
+
+        self.iptables.ipv4['filter'].add_chain(chain_name)
+        ipv4_address = self._ip_for_instance(instance)
+        self.iptables.ipv4['filter'].add_rule('local',
+                                              '-d %s -j $%s' %
+                                              (ipv4_address, chain_name))
+
+        if FLAGS.use_ipv6:
+            self.iptables.ipv6['filter'].add_chain(chain_name)
+            ipv6_address = self._ip_for_instance_v6(instance)
+            self.iptables.ipv4['filter'].add_rule('local',
+                                                  '-d %s -j $%s' %
+                                                  (ipv6_address,
+                                                   chain_name))
+
+        ipv4_rules, ipv6_rules = self.instance_rules(instance)
+
+        for rule in ipv4_rules:
+            self.iptables.ipv4['filter'].add_rule(chain_name, rule)
+
+        if FLAGS.use_ipv6:
+            for rule in ipv6_rules:
+                self.iptables.ipv6['filter'].add_rule(chain_name, rule)
+
+        self.iptables.apply()
+
     def unfilter_instance(self, instance):
-        self.remove_instance(instance)
-        self.apply_ruleset()
+        chain_name = self._instance_chain_name(instance)
 
-    def prepare_instance_filter(self, instance):
-        self.add_instance(instance)
-        self.apply_ruleset()
+        self.iptables.ipv4['filter'].remove_chain(chain_name)
+        if FLAGS.use_ipv6:
+            self.iptables.ipv6['filter'].remove_chain(chain_name)
 
-    def apply_ruleset(self):
-        current_filter, _ = self.execute('sudo iptables-save -t filter',
-                                         attempts=5)
-        current_lines = current_filter.split('\n')
-        new_filter = self.modify_rules(current_lines, 4)
-        self.execute('sudo iptables-restore',
-                     process_input='\n'.join(new_filter),
-                     attempts=5)
-        if(FLAGS.use_ipv6):
-            current_filter, _ = self.execute('sudo ip6tables-save '
-                                             '-t filter',
-                                             attempts=5)
-            current_lines = current_filter.split('\n')
-            new_filter = self.modify_rules(current_lines, 6)
-            self.execute('sudo ip6tables-restore',
-                         process_input='\n'.join(new_filter),
-                         attempts=5)
-
-    def modify_rules(self, current_lines, ip_version=4):
+        self.iptables.apply()
+
+
+    def instance_rules(self, instance):
         ctxt = context.get_admin_context()
-        # Remove any trace of nova rules.
-        new_filter = filter(lambda l: 'nova-' not in l, current_lines)
-
-        seen_chains = False
-        for rules_index in range(len(new_filter)):
-            if not seen_chains:
-                if new_filter[rules_index].startswith(':'):
-                    seen_chains = True
-            elif seen_chains == 1:
-                if not new_filter[rules_index].startswith(':'):
-                    break
 
-        our_chains = [':nova-fallback - [0:0]']
-        our_rules = ['-A nova-fallback -j DROP']
-
-        our_chains += [':nova-local - [0:0]']
-        our_rules += ['-A FORWARD -j nova-local']
-        our_rules += ['-A OUTPUT -j nova-local']
-
-        security_groups = {}
-        # Add our chains
-        # First, we add instance chains and rules
-        for instance_id in self.instances:
-            instance = self.instances[instance_id]
-            chain_name = self._instance_chain_name(instance)
-            if(ip_version == 4):
-                ip_address = self._ip_for_instance(instance)
-            elif(ip_version == 6):
-                ip_address = self._ip_for_instance_v6(instance)
-
-            our_chains += [':%s - [0:0]' % chain_name]
-
-            # Jump to the per-instance chain
-            our_rules += ['-A nova-local -d %s -j %s' % (ip_address,
-                                                         chain_name)]
-
-            # Always drop invalid packets
-            our_rules += ['-A %s -m state --state '
-                          'INVALID -j DROP' % (chain_name,)]
-
-            # Allow established connections
-            our_rules += ['-A %s -m state --state '
-                          'ESTABLISHED,RELATED -j ACCEPT' % (chain_name,)]
-
-            # Jump to each security group chain in turn
-            for security_group in \
-                            db.security_group_get_by_instance(ctxt,
-                                                              instance['id']):
-                security_groups[security_group['id']] = security_group
-
-                sg_chain_name = self._security_group_chain_name(
-                                                          security_group['id'])
+        ipv4_rules = []
+        ipv6_rules = []
 
-                our_rules += ['-A %s -j %s' % (chain_name, sg_chain_name)]
-
-            if(ip_version == 4):
-                # Allow DHCP responses
-                dhcp_server = self._dhcp_server_for_instance(instance)
-                our_rules += ['-A %s -s %s -p udp --sport 67 --dport 68 '
-                                    '-j ACCEPT ' % (chain_name, dhcp_server)]
-                #Allow project network traffic
-                if (FLAGS.allow_project_net_traffic):
-                    cidr = self._project_cidr_for_instance(instance)
-                    our_rules += ['-A %s -s %s -j ACCEPT' % (chain_name, cidr)]
-            elif(ip_version == 6):
-                # Allow RA responses
-                ra_server = self._ra_server_for_instance(instance)
-                if ra_server:
-                    our_rules += ['-A %s -s %s -p icmpv6 -j ACCEPT' %
-                                  (chain_name, ra_server + "/128")]
-                #Allow project network traffic
-                if (FLAGS.allow_project_net_traffic):
-                    cidrv6 = self._project_cidrv6_for_instance(instance)
-                    our_rules += ['-A %s -s %s -j ACCEPT' %
-                                        (chain_name, cidrv6)]
-
-            # If nothing matches, jump to the fallback chain
-            our_rules += ['-A %s -j nova-fallback' % (chain_name,)]
+        # Always drop invalid packets
+        ipv4_rules += ['-m state --state ' 'INVALID -j DROP']
+        ipv6_rules += ['-m state --state ' 'INVALID -j DROP']
 
-        # then, security group chains and rules
-        for security_group_id in security_groups:
-            chain_name = self._security_group_chain_name(security_group_id)
-            our_chains += [':%s - [0:0]' % chain_name]
+        # Allow established connections
+        ipv4_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
+        ipv6_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
+
+        dhcp_server = self._dhcp_server_for_instance(instance)
+        ipv4_rules += ['-s %s -p udp --sport 67 --dport 68 '
+                       '-j ACCEPT' % (dhcp_server,)]
+
+        #Allow project network traffic
+        if FLAGS.allow_project_net_traffic:
+            cidr = self._project_cidr_for_instance(instance)
+            ipv4_rules += ['-s %s -j ACCEPT' % (cidr,)]
+
+        # We wrap these in FLAGS.use_ipv6 because they might cause
+        # a DB lookup. The other ones are just list operations, so
+        # they're not worth the clutter.
+        if FLAGS.use_ipv6:
+            # Allow RA responses
+            ra_server = self._ra_server_for_instance(instance)
+            if ra_server:
+                ipv6_rules += ['-s %s/128 -p icmpv6 -j ACCEPT' % (ra_server,)]
 
-            rules = \
-              db.security_group_rule_get_by_security_group(ctxt,
-                                                          security_group_id)
+            #Allow project network traffic
+            if FLAGS.allow_project_net_traffic:
+                cidrv6 = self._project_cidrv6_for_instance(instance)
+                ipv6_rules += ['-s %s -j ACCEPT' % (cidrv6,)]
+
+
+        security_groups = db.security_group_get_by_instance(ctxt,
+                                                            instance['id'])
+
+
+        # then, security group chains and rules
+        for security_group in security_groups:
+            rules = db.security_group_rule_get_by_security_group(ctxt,
+                                                          security_group['id'])
 
             for rule in rules:
                 logging.info('%r', rule)
@@ -1354,14 +1325,16 @@ class IptablesFirewallDriver(FirewallDriver):
                     continue
 
                 version = _get_ip_version(rule.cidr)
-                if version != ip_version:
-                    continue
+                if version == 4:
+                    rules = ipv4_rules
+                else:
+                    rules = ipv6_rules
 
                 protocol = rule.protocol
                 if version == 6 and rule.protocol == 'icmp':
                     protocol = 'icmpv6'
 
-                args = ['-A', chain_name, '-p', protocol, '-s', rule.cidr]
+                args = ['-p', protocol, '-s', rule.cidr]
 
                 if rule.protocol in ['udp', 'tcp']:
                     if rule.from_port == rule.to_port:
@@ -1382,20 +1355,20 @@ class IptablesFirewallDriver(FirewallDriver):
                             icmp_type_arg += '/%s' % icmp_code
 
                     if icmp_type_arg:
-                        if(ip_version == 4):
+                        if version == 4:
                             args += ['-m', 'icmp', '--icmp-type',
                                      icmp_type_arg]
-                        elif(ip_version == 6):
+                        elif version == 6:
                             args += ['-m', 'icmp6', '--icmpv6-type',
                                      icmp_type_arg]
 
                 args += ['-j ACCEPT']
-                our_rules += [' '.join(args)]
+                rules += [' '.join(args)]
+
+        ipv4_rules += ['-j $fallback']
+        ipv6_rules += ['-j $fallback']
 
-        new_filter[rules_index:rules_index] = our_rules
-        new_filter[rules_index:rules_index] = our_chains
-        logging.info('new_filter: %s', '\n'.join(new_filter))
-        return new_filter
+        return ipv4_rules, ipv6_rules
 
     def refresh_security_group_members(self, security_group):
         pass
@@ -1407,7 +1380,7 @@ class IptablesFirewallDriver(FirewallDriver):
         return 'nova-sg-%s' % (security_group_id,)
 
     def _instance_chain_name(self, instance):
-        return 'nova-inst-%s' % (instance['id'],)
+        return 'inst-%s' % (instance['id'],)
 
     def _ip_for_instance(self, instance):
         return db.instance_get_fixed_address(context.get_admin_context(),
-- 
cgit 


From 99760bd7a51371b29cf0f76134187dc81e7545d0 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Sat, 19 Feb 2011 00:30:44 +0100
Subject: Rename a few things for more clarity.

---
 nova/network/linux_net.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index c11d34922..b657ab4bc 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -109,7 +109,6 @@ class IptablesTable(object):
         if '$' in rule:
             rule = ' '.join(map(self._wrap_target_chain, rule.split(' ')))
 
-        print 'Adding rule: %r' % rule
         self.rules.append(IptablesRule(chain, rule, wrap))
 
     def _wrap_target_chain(self, s):
@@ -168,9 +167,9 @@ class IptablesManager(object):
 
         for cmd, tables in s:
             for table in tables:
-                current_filter, _ = self.execute('sudo %s-save -t %s' %
-                                                 (cmd, table), attempts=5)
-                current_lines = current_filter.split('\n')
+                current_table, _ = self.execute('sudo %s-save -t %s' %
+                                                (cmd, table), attempts=5)
+                current_lines = current_table.split('\n')
                 new_filter = self.modify_rules(current_lines, tables[table])
                 self.execute('sudo %s-restore' % (cmd,),
                              process_input='\n'.join(new_filter),
@@ -182,7 +181,7 @@ class IptablesManager(object):
         rules = table.rules
 
         # Remove any trace of our rules
-        new_filter = filter(lambda l: '%s' % binary not in l, current_lines)
+        new_filter = filter(lambda l: binary_name not in l, current_lines)
 
         seen_chains = False
         for rules_index in range(len(new_filter)):
-- 
cgit 


From 23729c543350ce4ce563077522f18d0bedd1e61b Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Sat, 19 Feb 2011 00:36:34 +0100
Subject: Security group fallback is named sg-fallback.

---
 nova/virt/libvirt_conn.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 976ccaca5..0ddf889a1 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1365,8 +1365,8 @@ class IptablesFirewallDriver(FirewallDriver):
                 args += ['-j ACCEPT']
                 rules += [' '.join(args)]
 
-        ipv4_rules += ['-j $fallback']
-        ipv6_rules += ['-j $fallback']
+        ipv4_rules += ['-j $sg-fallback']
+        ipv6_rules += ['-j $sg-fallback']
 
         return ipv4_rules, ipv6_rules
 
-- 
cgit 


From d0733621758985bdd621a05c7c8a53fe27aa62f2 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Sat, 19 Feb 2011 01:28:26 +0100
Subject: Wrap iptables calls in a semaphore.

---
 nova/network/linux_net.py | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index b81a704bf..ecda450bf 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -20,6 +20,8 @@ Implements vlans, bridges, and iptables rules using linux utilities.
 import inspect
 import os
 
+from eventlet import semaphore
+
 from nova import db
 from nova import exception
 from nova import flags
@@ -149,8 +151,7 @@ class IptablesManager(object):
 
         # Wrap the builtin chains
         builtin_chains = {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
-                          'nat': ['PREROUTING', 'INPUT',
-                                  'OUTPUT', 'POSTROUTING']}
+                          'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']}
 
         for table, chains in builtin_chains.iteritems():
             for chain in chains:
@@ -158,22 +159,24 @@ class IptablesManager(object):
                 self.ipv4[table].add_rule(chain,
                                           '-j %s-%s' % (binary_name, chain),
                                           wrap=False)
+        self.semaphore = semaphore.Semaphore()
 
 
     def apply(self):
-        s = [('iptables', self.ipv4)]
-        if FLAGS.use_ipv6:
-            s += [('ip6tables', self.ipv6)]
-
-        for cmd, tables in s:
-            for table in tables:
-                current_table, _ = self.execute('sudo %s-save -t %s' %
-                                                (cmd, table), attempts=5)
-                current_lines = current_table.split('\n')
-                new_filter = self.modify_rules(current_lines, tables[table])
-                self.execute('sudo %s-restore' % (cmd,),
-                             process_input='\n'.join(new_filter),
-                             attempts=5)
+        with self.semaphore:
+            s = [('iptables', self.ipv4)]
+            if FLAGS.use_ipv6:
+                s += [('ip6tables', self.ipv6)]
+
+            for cmd, tables in s:
+                for table in tables:
+                    current_table, _ = self.execute('sudo %s-save -t %s' %
+                                                    (cmd, table), attempts=5)
+                    current_lines = current_table.split('\n')
+                    new_filter = self.modify_rules(current_lines, tables[table])
+                    self.execute('sudo %s-restore' % (cmd,),
+                                 process_input='\n'.join(new_filter),
+                                 attempts=5)
 
     def modify_rules(self, current_lines, table, binary=None):
 
-- 
cgit 


From e729c49543c5acf354b154a3e2d9fd76a2f7da35 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 09:17:33 +0100
Subject: Fix refresh sec groups.

---
 nova/virt/libvirt_conn.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 0ddf889a1..3faf01f4b 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1231,7 +1231,10 @@ class IptablesFirewallDriver(FirewallDriver):
 
     def prepare_instance_filter(self, instance):
         self.instances[instance['id']] = instance
+        self.add_filters_for_instance(instance)
+        self.iptables.apply()
 
+    def add_filters_for_instance(self, instance):
         chain_name = self._instance_chain_name(instance)
 
         self.iptables.ipv4['filter'].add_chain(chain_name)
@@ -1257,18 +1260,17 @@ class IptablesFirewallDriver(FirewallDriver):
             for rule in ipv6_rules:
                 self.iptables.ipv6['filter'].add_rule(chain_name, rule)
 
+    def unfilter_instance(self, instance):
+        self.remove_filters_for_instance(instance)
         self.iptables.apply()
 
-    def unfilter_instance(self, instance):
+    def remove_filters_for_instance(self, instance):
         chain_name = self._instance_chain_name(instance)
 
         self.iptables.ipv4['filter'].remove_chain(chain_name)
         if FLAGS.use_ipv6:
             self.iptables.ipv6['filter'].remove_chain(chain_name)
 
-        self.iptables.apply()
-
-
     def instance_rules(self, instance):
         ctxt = context.get_admin_context()
 
@@ -1374,7 +1376,10 @@ class IptablesFirewallDriver(FirewallDriver):
         pass
 
     def refresh_security_group_rules(self, security_group):
-        self.apply_ruleset()
+        for instance in self.instances:
+            self.remove_filters_for_instance(instance)
+            self.add_filters_for_instance(instance)
+        self.iptables.apply()
 
     def _security_group_chain_name(self, security_group_id):
         return 'nova-sg-%s' % (security_group_id,)
-- 
cgit 


From cbb0402efac4ededdda0ac2097ec087216e23931 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 10:18:43 +0100
Subject: Also remove rules that jump to deleted chains.

---
 nova/network/linux_net.py | 5 ++++-
 nova/virt/libvirt_conn.py | 7 ++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index ecda450bf..1f96a4d55 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -104,6 +104,9 @@ class IptablesTable(object):
         self.chains.remove(name)
         self.rules = filter(lambda r: r.chain != name, self.rules)
 
+        jump_snippet = '-j %s-%s' % (binary_name, name)
+        self.rules = filter(lambda r: jump_snippet not in r.rule, self.rules)
+
     def add_rule(self, chain, rule, wrap=True):
         if wrap and chain not in self.chains:
             raise ValueError(_("Unknown chain: %r") % chain)
@@ -283,7 +286,7 @@ def remove_floating_forward(floating_ip, fixed_ip):
 def floating_forward_rules(floating_ip, fixed_ip):
     return [("PREROUTING", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
             ("OUTPUT", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
-            ("SNATTING", "-d %s -j DNAT --to %s" % (fixed_ip, floating_ip))]
+            ("SNATTING", "-d %s -j SNAT --to %s" % (fixed_ip, floating_ip))]
 
 def ensure_vlan_bridge(vlan_num, bridge, net_attrs=None):
     """Create a vlan and bridge unless they already exist"""
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 3faf01f4b..daf8f0ed7 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -44,9 +44,6 @@ import uuid
 from xml.dom import minidom
 
 
-from eventlet import greenthread
-from eventlet import event
-from eventlet import semaphore
 from eventlet import tpool
 
 import IPy
@@ -1246,7 +1243,7 @@ class IptablesFirewallDriver(FirewallDriver):
         if FLAGS.use_ipv6:
             self.iptables.ipv6['filter'].add_chain(chain_name)
             ipv6_address = self._ip_for_instance_v6(instance)
-            self.iptables.ipv4['filter'].add_rule('local',
+            self.iptables.ipv6['filter'].add_rule('local',
                                                   '-d %s -j $%s' %
                                                   (ipv6_address,
                                                    chain_name))
@@ -1376,7 +1373,7 @@ class IptablesFirewallDriver(FirewallDriver):
         pass
 
     def refresh_security_group_rules(self, security_group):
-        for instance in self.instances:
+        for instance in self.instances.values():
             self.remove_filters_for_instance(instance)
             self.add_filters_for_instance(instance)
         self.iptables.apply()
-- 
cgit 


From 9eebe4317f86ae13ffeaca1622e9fc555bc28ebc Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 10:42:59 +0100
Subject: Unfilter instance correctly on termination.

---
 nova/network/linux_net.py | 4 ++++
 nova/virt/libvirt_conn.py | 8 +++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 1f96a4d55..1145bfa7a 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -101,6 +101,10 @@ class IptablesTable(object):
         self.chains.add(name)
 
     def remove_chain(self, name):
+        if name not in self.chain:
+            LOG.debug(_("Attempted to remove chain %s which doesn't exist"),
+                      name)
+            return
         self.chains.remove(name)
         self.rules = filter(lambda r: r.chain != name, self.rules)
 
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index daf8f0ed7..0c355e48e 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1219,9 +1219,11 @@ class IptablesFirewallDriver(FirewallDriver):
         """No-op. Everything is done in prepare_instance_filter"""
         pass
 
-    def remove_instance(self, instance):
+    def unfilter_instance(self, instance):
         if instance['id'] in self.instances:
             del self.instances[instance['id']]
+            self.remove_filters_for_instance(instance)
+            self.iptables.apply()
         else:
             LOG.info(_('Attempted to unfilter instance %s which is not '
                        'filtered'), instance['id'])
@@ -1257,10 +1259,6 @@ class IptablesFirewallDriver(FirewallDriver):
             for rule in ipv6_rules:
                 self.iptables.ipv6['filter'].add_rule(chain_name, rule)
 
-    def unfilter_instance(self, instance):
-        self.remove_filters_for_instance(instance)
-        self.iptables.apply()
-
     def remove_filters_for_instance(self, instance):
         chain_name = self._instance_chain_name(instance)
 
-- 
cgit 


From 384a4525e9d6de54158cd170487fce95df814b52 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 11:01:27 +0100
Subject: Fix typo

---
 nova/network/linux_net.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 1145bfa7a..c5779f85f 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -101,7 +101,7 @@ class IptablesTable(object):
         self.chains.add(name)
 
     def remove_chain(self, name):
-        if name not in self.chain:
+        if name not in self.chains:
             LOG.debug(_("Attempted to remove chain %s which doesn't exist"),
                       name)
             return
-- 
cgit 


From 15203c9ceaa94f0cd5bad96622ee93af7662bcce Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 12:22:29 +0100
Subject: Allow non-existing rules to be removed.

---
 nova/network/linux_net.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index c5779f85f..d4cfbbde9 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -125,8 +125,12 @@ class IptablesTable(object):
             return '%s-%s' % (binary_name, s[1:])
         return s
 
-    def remove_rule(self, chain, rule):
-        self.rules.remove(IptablesRule(chain, rule))
+    def remove_rule(self, *args, **kwargs):
+        try:
+            self.rules.remove(IptablesRule(*args, **kwargs))
+        except ValueError:
+            LOG.debug(_("Tried to remove rule that wasn't there: %r %r"),
+                      args, kwargs)
 
 class IptablesManager(object):
     def __init__(self, execute=None):
-- 
cgit 


From a57dffb5fdfbfac59b9ddbe7b33d6f03b7b748ba Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 14:16:42 +0100
Subject: PEP-8 fixes

---
 nova/network/linux_net.py  | 28 ++++++++++++++++++++--------
 nova/tests/test_network.py | 21 +++++++++------------
 nova/tests/test_virt.py    |  3 ++-
 nova/utils.py              | 12 ++++++------
 nova/virt/libvirt_conn.py  |  2 --
 5 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index d4cfbbde9..b5d1323a1 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -129,8 +129,10 @@ class IptablesTable(object):
         try:
             self.rules.remove(IptablesRule(*args, **kwargs))
         except ValueError:
-            LOG.debug(_("Tried to remove rule that wasn't there: %r %r"),
-                      args, kwargs)
+            LOG.debug(_("Tried to remove rule that wasn't there:"
+                        " %(args)r %(kwargs)r"), {'args': args,
+                                                  'kwargs': kwargs})
+
 
 class IptablesManager(object):
     def __init__(self, execute=None):
@@ -142,9 +144,9 @@ class IptablesManager(object):
         else:
             self.execute = execute
 
-        self.ipv4 = { 'filter': IptablesTable(),
-                      'nat': IptablesTable() }
-        self.ipv6 = { 'filter': IptablesTable() }
+        self.ipv4 = {'filter': IptablesTable(),
+                     'nat': IptablesTable()}
+        self.ipv6 = {'filter': IptablesTable()}
 
         self.ipv4['nat'].add_chain('SNATTING')
         self.ipv4['nat'].add_rule('POSTROUTING',
@@ -155,11 +157,18 @@ class IptablesManager(object):
         self.ipv4['filter'].add_rule('FORWARD',
                                     '-j %s-local' % (binary_name,),
                                     wrap=False)
-
         self.ipv4['filter'].add_rule('OUTPUT',
                                     '-j %s-local' % (binary_name,),
                                     wrap=False)
 
+        self.ipv6['filter'].add_chain('local')
+        self.ipv6['filter'].add_rule('FORWARD',
+                                    '-j %s-local' % (binary_name,),
+                                    wrap=False)
+        self.ipv6['filter'].add_rule('OUTPUT',
+                                    '-j %s-local' % (binary_name,),
+                                    wrap=False)
+
         # Wrap the builtin chains
         builtin_chains = {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
                           'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']}
@@ -172,7 +181,6 @@ class IptablesManager(object):
                                           wrap=False)
         self.semaphore = semaphore.Semaphore()
 
-
     def apply(self):
         with self.semaphore:
             s = [('iptables', self.ipv4)]
@@ -184,7 +192,8 @@ class IptablesManager(object):
                     current_table, _ = self.execute('sudo %s-save -t %s' %
                                                     (cmd, table), attempts=5)
                     current_lines = current_table.split('\n')
-                    new_filter = self.modify_rules(current_lines, tables[table])
+                    new_filter = self.modify_rules(current_lines,
+                                                   tables[table])
                     self.execute('sudo %s-restore' % (cmd,),
                                  process_input='\n'.join(new_filter),
                                  attempts=5)
@@ -285,17 +294,20 @@ def ensure_floating_forward(floating_ip, fixed_ip):
         iptables_manager.ipv4['nat'].add_rule(chain, rule)
     iptables_manager.apply()
 
+
 def remove_floating_forward(floating_ip, fixed_ip):
     """Remove forwarding for floating ip"""
     for chain, rule in floating_forward_rules(floating_ip, fixed_ip):
         iptables_manager.ipv4['nat'].remove_rule(chain, rule)
     iptables_manager.apply()
 
+
 def floating_forward_rules(floating_ip, fixed_ip):
     return [("PREROUTING", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
             ("OUTPUT", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
             ("SNATTING", "-d %s -j SNAT --to %s" % (fixed_ip, floating_ip))]
 
+
 def ensure_vlan_bridge(vlan_num, bridge, net_attrs=None):
     """Create a vlan and bridge unless they already exist"""
     interface = ensure_vlan(vlan_num)
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index b28d64245..c9a62a391 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -42,15 +42,14 @@ class IptablesManagerTestCase(test.TestCase):
 :INPUT ACCEPT [2223527:305688874]
 :FORWARD ACCEPT [0:0]
 :OUTPUT ACCEPT [2172501:140856656]
--A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT 
--A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT 
--A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT 
--A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT 
--A FORWARD -d 192.168.122.0/24 -o virbr0 -m state --state RELATED,ESTABLISHED -j ACCEPT 
--A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT 
--A FORWARD -i virbr0 -o virbr0 -j ACCEPT 
--A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable 
--A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable 
+-A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT
+-A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT
+-A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT
+-A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT
+-A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT
+-A FORWARD -i virbr0 -o virbr0 -j ACCEPT
+-A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable
+-A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable
 COMMIT
 # Completed on Fri Feb 18 15:17:05 2011"""
 
@@ -77,8 +76,7 @@ COMMIT
 
         # TODO(soren): Add stuff for ipv6
         check_matrix = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
-                            'nat': ['PREROUTING', 'INPUT',
-                                    'OUTPUT', 'POSTROUTING']} }
+                            'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']}}
 
         for ip_version in check_matrix:
             ip = getattr(self.manager, 'ipv%d' % ip_version)
@@ -90,7 +88,6 @@ COMMIT
                                     (chain,) in new_lines)
                     self.assertTrue('-A %s -j run_tests.py-%s' % \
                                     (chain, chain) in new_lines)
-                print '\n'.join(new_lines)
 
 
 class NetworkTestCase(test.TestCase):
diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index a88e01818..11201788c 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -345,7 +345,8 @@ class IptablesFirewallTestCase(test.TestCase):
         self.fw.prepare_instance_filter(instance_ref)
         self.fw.apply_instance_filter(instance_ref)
 
-        in_rules = filter(lambda l: not l.startswith('#'), self.in_filter_rules)
+        in_rules = filter(lambda l: not l.startswith('#'),
+                          self.in_filter_rules)
         for rule in in_rules:
             if not 'nova' in rule:
                 self.assertTrue(rule in self.out_rules,
diff --git a/nova/utils.py b/nova/utils.py
index 644bf18fd..5b44bccb5 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -126,7 +126,8 @@ def fetchfile(url, target):
     execute("curl --fail %s -o %s" % (url, target))
 
 
-def execute(cmd, process_input=None, addl_env=None, check_exit_code=True, attempts=1):
+def execute(cmd, process_input=None, addl_env=None, check_exit_code=True,
+            attempts=1):
     while attempts > 0:
         attempts -= 1
         try:
@@ -150,17 +151,16 @@ def execute(cmd, process_input=None, addl_env=None, check_exit_code=True, attemp
                                                 stdout=stdout,
                                                 stderr=stderr,
                                                 cmd=cmd)
-            # NOTE(termie): this appears to be necessary to let the subprocess call
-            #               clean something up in between calls, without it two
-            #               execute calls in a row hangs the second one
+            # NOTE(termie): this appears to be necessary to let the subprocess
+            #               call clean something up in between calls, without
+            #               it two execute calls in a row hangs the second one
             greenthread.sleep(0)
             return result
         except ProcessExecutionError:
             if not attempts:
                 raise
             else:
-                greenthread.sleep(random.randint(50,300)/100)
-                pass
+                greenthread.sleep(random.randint(20, 200) / 100.0)
 
 
 def ssh_execute(ssh, cmd, process_input=None,
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 0c355e48e..7f74e3505 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1303,11 +1303,9 @@ class IptablesFirewallDriver(FirewallDriver):
                 cidrv6 = self._project_cidrv6_for_instance(instance)
                 ipv6_rules += ['-s %s -j ACCEPT' % (cidrv6,)]
 
-
         security_groups = db.security_group_get_by_instance(ctxt,
                                                             instance['id'])
 
-
         # then, security group chains and rules
         for security_group in security_groups:
             rules = db.security_group_rule_get_by_security_group(ctxt,
-- 
cgit 


From 3d2ec0f594e02018a32c8d0d7a8cc46f7ab4c849 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 14:39:02 +0100
Subject: Wrap ipv6 rules, too

---
 nova/network/linux_net.py  | 26 +++++++++++++++++---------
 nova/tests/test_network.py |  3 ++-
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index b5d1323a1..f47219b2e 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -170,15 +170,23 @@ class IptablesManager(object):
                                     wrap=False)
 
         # Wrap the builtin chains
-        builtin_chains = {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
-                          'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']}
-
-        for table, chains in builtin_chains.iteritems():
-            for chain in chains:
-                self.ipv4[table].add_chain(chain)
-                self.ipv4[table].add_rule(chain,
-                                          '-j %s-%s' % (binary_name, chain),
-                                          wrap=False)
+        builtin_chains = { 4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
+                               'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']},
+                           6: {'filter': ['INPUT', 'OUTPUT', 'FORWARD']}}
+
+        for ip_version in builtin_chains:
+            if ip_version == 4:
+                tables = self.ipv4
+            elif ip_version == 6:
+                tables = self.ipv6
+
+            for table, chains in builtin_chains[ip_version].iteritems():
+                for chain in chains:
+                    tables[table].add_chain(chain)
+                    tables[table].add_rule(chain,
+                                           '-j %s-%s' % (binary_name, chain),
+                                           wrap=False)
+
         self.semaphore = semaphore.Semaphore()
 
     def apply(self):
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index c9a62a391..f1d4fe133 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -76,7 +76,8 @@ COMMIT
 
         # TODO(soren): Add stuff for ipv6
         check_matrix = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
-                            'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']}}
+                            'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']},
+                        6: {'filter': ['INPUT', 'OUTPUT', 'FORWARD']}}
 
         for ip_version in check_matrix:
             ip = getattr(self.manager, 'ipv%d' % ip_version)
-- 
cgit 


From 1ed4df93a246a06518f2c216cd0fc60ca67eb5c4 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 14:39:38 +0100
Subject: More PEP-8

---
 nova/network/linux_net.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index f47219b2e..d7a3075cb 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -170,9 +170,9 @@ class IptablesManager(object):
                                     wrap=False)
 
         # Wrap the builtin chains
-        builtin_chains = { 4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
-                               'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']},
-                           6: {'filter': ['INPUT', 'OUTPUT', 'FORWARD']}}
+        builtin_chains = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
+                              'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']},
+                          6: {'filter': ['INPUT', 'OUTPUT', 'FORWARD']}}
 
         for ip_version in builtin_chains:
             if ip_version == 4:
-- 
cgit 


From fc0ea52d9379649d28de88d4fa1628e455533842 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 21 Feb 2011 23:08:26 +0100
Subject: Add a bunch of docs for the new iptables hotness.

---
 nova/network/linux_net.py | 72 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 68 insertions(+), 4 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index d7a3075cb..3b6ec9338 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -69,6 +69,11 @@ binary_name = os.path.basename(inspect.stack()[-1][1])
 
 
 class IptablesRule(object):
+    """An iptables rule
+
+    You shouldn't need to use this class directly, it's only used by
+    IptablesManager
+    """
     def __init__(self, chain, rule, wrap=True):
         self.chain = chain
         self.rule = rule
@@ -93,14 +98,33 @@ class IptablesRule(object):
 
 
 class IptablesTable(object):
+    """An iptables table"""
+
     def __init__(self):
         self.rules = []
         self.chains = set()
 
     def add_chain(self, name):
+        """Adds a named chain to the table
+
+        The chain name is wrapped to be unique for the component creating
+        it, so different components of Nova can safely create identically
+        named chains without interfering with one another.
+
+        At the moment, its wrapped name is <binary name>-<chain name>,
+        so if nova-compute creates a chain named "OUTPUT", it'll actually
+        end up named "nova-compute-OUTPUT".
+        """
         self.chains.add(name)
 
     def remove_chain(self, name):
+        """Remove named chain
+
+        This removal "cascades". All rule in the chain are removed, as are
+        all rules in other chains that jump to it.
+
+        If the chain is not found, this is merely logged.
+        """
         if name not in self.chains:
             LOG.debug(_("Attempted to remove chain %s which doesn't exist"),
                       name)
@@ -112,6 +136,15 @@ class IptablesTable(object):
         self.rules = filter(lambda r: jump_snippet not in r.rule, self.rules)
 
     def add_rule(self, chain, rule, wrap=True):
+        """Add a rule to the table
+
+        This is just like what you'd feed to iptables, just without
+        the "-A <chain name>" bit at the start.
+
+        However, if you need to jump to one of your wrapped chains,
+        prepend its name with a '$' which will ensure the wrapping
+        is applied correctly.
+        """
         if wrap and chain not in self.chains:
             raise ValueError(_("Unknown chain: %r") % chain)
 
@@ -125,7 +158,13 @@ class IptablesTable(object):
             return '%s-%s' % (binary_name, s[1:])
         return s
 
-    def remove_rule(self, *args, **kwargs):
+    def remove_rule(self, chain, rule, wrap=True):
+        """Remove a rule from a chain
+
+        Note: The rule must be exactly identical to the one that was added.
+        You cannot switch arguments around like you can with the iptables
+        CLI tool.
+        """
         try:
             self.rules.remove(IptablesRule(*args, **kwargs))
         except ValueError:
@@ -135,6 +174,22 @@ class IptablesTable(object):
 
 
 class IptablesManager(object):
+    """Wrapper for iptables
+
+    See IptablesTable for some usage docs
+
+    A number of chains are set up to begin with.
+
+    For ipv4, the filter table has a INPUT, OUTPUT, FORWARD, and local chains
+    already set up, while the NAT chain has PREROUTING, OUTPUT, POSTROUTING,
+    and SNATTING. Except for "local" and "SNATTING" these are all set up so
+    that they are applied at the beginning of their non-wrapped counterparts.
+    "SNATTING" is jumped to from nat/POSTROUTING and "local" is jumped to from
+    filter/OUTPUT and filter/FORWARD.
+
+    For ipv6, the filter table has INPUT, OUTPUT, FORWARD, and local. "local"
+    has the same semantics as for ipv4.
+    """
     def __init__(self, execute=None):
         if not execute:
             if FLAGS.fake_network:
@@ -190,6 +245,16 @@ class IptablesManager(object):
         self.semaphore = semaphore.Semaphore()
 
     def apply(self):
+        """Apply the current in-memory set of iptables rules
+
+        This will blow away any rules left over from previous runs of the
+        same component of Nova, and replace them with our current set of
+        rules. This happens atomically, thanks to iptables-restore.
+        
+        We wrap the call in a semaphore lock, so that we don't race with
+        ourselves. In the event of a race with another component running
+        an iptables-* command at the same time, we retry up to 5 times.
+        """
         with self.semaphore:
             s = [('iptables', self.ipv4)]
             if FLAGS.use_ipv6:
@@ -200,14 +265,13 @@ class IptablesManager(object):
                     current_table, _ = self.execute('sudo %s-save -t %s' %
                                                     (cmd, table), attempts=5)
                     current_lines = current_table.split('\n')
-                    new_filter = self.modify_rules(current_lines,
+                    new_filter = self._modify_rules(current_lines,
                                                    tables[table])
                     self.execute('sudo %s-restore' % (cmd,),
                                  process_input='\n'.join(new_filter),
                                  attempts=5)
 
-    def modify_rules(self, current_lines, table, binary=None):
-
+    def _modify_rules(self, current_lines, table, binary=None):
         chains = table.chains
         rules = table.rules
 
-- 
cgit 


From c53bb1718a9b5900d09637d0ee966dadbf073900 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 09:00:29 +0100
Subject: Address some review comments.

---
 nova/network/linux_net.py | 16 ++++++++--------
 nova/tests/test_virt.py   |  1 -
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 3b6ec9338..42af73e74 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -54,8 +54,6 @@ flags.DEFINE_string('dhcpbridge', _bin_file('nova-dhcpbridge'),
                         'location of nova-dhcpbridge')
 flags.DEFINE_string('routing_source_ip', '$my_ip',
                     'Public IP of network host')
-flags.DEFINE_bool('use_nova_chains', False,
-                  'use the nova_ routing chains instead of default')
 flags.DEFINE_string('input_chain', 'INPUT',
                     'chain to add nova_input to')
 
@@ -266,7 +264,7 @@ class IptablesManager(object):
                                                     (cmd, table), attempts=5)
                     current_lines = current_table.split('\n')
                     new_filter = self._modify_rules(current_lines,
-                                                   tables[table])
+                                                    tables[table])
                     self.execute('sudo %s-restore' % (cmd,),
                                  process_input='\n'.join(new_filter),
                                  attempts=5)
@@ -276,15 +274,17 @@ class IptablesManager(object):
         rules = table.rules
 
         # Remove any trace of our rules
-        new_filter = filter(lambda l: binary_name not in l, current_lines)
+        new_filter = filter(lambda line: binary_name not in line,
+                            current_lines)
 
         seen_chains = False
-        for rules_index in range(len(new_filter)):
+        rules_index = 0
+        for rules_index, rule in enumerate(new_filter):
             if not seen_chains:
-                if new_filter[rules_index].startswith(':'):
+                if rule.startswith(':'):
                     seen_chains = True
-            elif seen_chains == 1:
-                if not new_filter[rules_index].startswith(':'):
+            else:
+                if not rule.startswith(':'):
                     break
 
         new_filter[rules_index:rules_index] = [str(rule) for rule in rules]
diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index 11201788c..c2c7c8337 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -354,7 +354,6 @@ class IptablesFirewallTestCase(test.TestCase):
 
         instance_chain = None
         for rule in self.out_rules:
-            print rule
             # This is pretty crude, but it'll do for now
             if '-d 10.11.12.13 -j' in rule:
                 instance_chain = rule.split(' ')[-1]
-- 
cgit 


From 70d526dc44299b6bd54a5757d013ca3109887747 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 09:28:02 +0100
Subject: Add a new chain, floating-ip-snat, at the top of SNATTING, so that
 SNATting for floating ips gets applied before the default SNAT rule.

---
 nova/network/linux_net.py | 43 ++++++++++++++++++-------------------------
 1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 42af73e74..6b0735b5c 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -201,26 +201,13 @@ class IptablesManager(object):
                      'nat': IptablesTable()}
         self.ipv6 = {'filter': IptablesTable()}
 
-        self.ipv4['nat'].add_chain('SNATTING')
-        self.ipv4['nat'].add_rule('POSTROUTING',
-                                  '-j %s-SNATTING' % (binary_name,),
-                                  wrap=False)
-
         self.ipv4['filter'].add_chain('local')
-        self.ipv4['filter'].add_rule('FORWARD',
-                                    '-j %s-local' % (binary_name,),
-                                    wrap=False)
-        self.ipv4['filter'].add_rule('OUTPUT',
-                                    '-j %s-local' % (binary_name,),
-                                    wrap=False)
+        self.ipv4['filter'].add_rule('FORWARD', '-j $local', wrap=False)
+        self.ipv4['filter'].add_rule('OUTPUT', '-j $local', wrap=False)
 
         self.ipv6['filter'].add_chain('local')
-        self.ipv6['filter'].add_rule('FORWARD',
-                                    '-j %s-local' % (binary_name,),
-                                    wrap=False)
-        self.ipv6['filter'].add_rule('OUTPUT',
-                                    '-j %s-local' % (binary_name,),
-                                    wrap=False)
+        self.ipv6['filter'].add_rule('FORWARD', '-j $local', wrap=False)
+        self.ipv6['filter'].add_rule('OUTPUT', '-j $local', wrap=False)
 
         # Wrap the builtin chains
         builtin_chains = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
@@ -236,12 +223,22 @@ class IptablesManager(object):
             for table, chains in builtin_chains[ip_version].iteritems():
                 for chain in chains:
                     tables[table].add_chain(chain)
-                    tables[table].add_rule(chain,
-                                           '-j %s-%s' % (binary_name, chain),
+                    tables[table].add_rule(chain, '-j $%s' % (chain,),
                                            wrap=False)
 
+        # We add a SNATTING chain after our (wrapped) POSTROUTING chain
+        # so that rules added there will be applied after whatever we have in
+        # (the wrapped) POSTROUTING.
+        self.ipv4['nat'].add_chain('SNATTING')
+        self.ipv4['nat'].add_rule('POSTROUTING', '-j $SNATTING', wrap=False)
+
+        self.ipv4['nat'].add_chain('floating-ip-snat')
+        self.ipv4['nat'].add_rule('SNATTING', '-j $floating-ip-snat')
+
         self.semaphore = semaphore.Semaphore()
 
+        iptables_manager.apply()
+
     def apply(self):
         """Apply the current in-memory set of iptables rules
 
@@ -318,11 +315,6 @@ def init_host():
                                            (FLAGS.fixed_range,
                                             FLAGS.routing_source_ip))
 
-    iptables_manager.ipv4['nat'].add_rule("POSTROUTING",
-                                          "-s %s -j SNAT --to-source %s" % \
-                                           (FLAGS.fixed_range,
-                                            FLAGS.routing_source_ip))
-
     iptables_manager.ipv4['nat'].add_rule("POSTROUTING",
                                           "-s %s -d %s -j ACCEPT" % \
                                           (FLAGS.fixed_range, FLAGS.dmz_cidr))
@@ -377,7 +369,8 @@ def remove_floating_forward(floating_ip, fixed_ip):
 def floating_forward_rules(floating_ip, fixed_ip):
     return [("PREROUTING", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
             ("OUTPUT", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
-            ("SNATTING", "-d %s -j SNAT --to %s" % (fixed_ip, floating_ip))]
+            ("floating-ip-snat",
+             "-s %s -j SNAT --to %s" % (fixed_ip, floating_ip))]
 
 
 def ensure_vlan_bridge(vlan_num, bridge, net_attrs=None):
-- 
cgit 


From bf37fb0ab5503a077a3d9e4109990d252e27cb15 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 11:29:58 +0100
Subject: Add a bunch of tests for everything.

Add a 'head' kwarg to add_rule that lets the rule bubble to the top. This is needed for nova-filter-top to end up at the top.
---
 nova/network/linux_net.py  | 120 ++++++++++++++++++++++++++++++------------
 nova/tests/test_network.py | 128 +++++++++++++++++++++++++++++++++++++--------
 2 files changed, 192 insertions(+), 56 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 6b0735b5c..2ccb5969e 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -72,19 +72,22 @@ class IptablesRule(object):
     You shouldn't need to use this class directly, it's only used by
     IptablesManager
     """
-    def __init__(self, chain, rule, wrap=True):
+    def __init__(self, chain, rule, wrap=True, head=False):
         self.chain = chain
         self.rule = rule
         self.wrap = wrap
+        self.head = head
 
     def __eq__(self, other):
         return ((self.chain == other.chain) and
                 (self.rule == other.rule) and
+                (self.head == other.head) and
                 (self.wrap == other.wrap))
 
     def __ne__(self, other):
         return ((self.chain != other.chain) or
                 (self.rule != other.rule) or
+                (self.head != other.head) or
                 (self.wrap != other.wrap))
 
     def __str__(self):
@@ -101,8 +104,9 @@ class IptablesTable(object):
     def __init__(self):
         self.rules = []
         self.chains = set()
+        self.unwrapped_chains = set()
 
-    def add_chain(self, name):
+    def add_chain(self, name, wrap=True):
         """Adds a named chain to the table
 
         The chain name is wrapped to be unique for the component creating
@@ -113,9 +117,12 @@ class IptablesTable(object):
         so if nova-compute creates a chain named "OUTPUT", it'll actually
         end up named "nova-compute-OUTPUT".
         """
-        self.chains.add(name)
+        if wrap:
+            self.chains.add(name)
+        else:
+            self.unwrapped_chains.add(name)
 
-    def remove_chain(self, name):
+    def remove_chain(self, name, wrap=True):
         """Remove named chain
 
         This removal "cascades". All rule in the chain are removed, as are
@@ -123,17 +130,27 @@ class IptablesTable(object):
 
         If the chain is not found, this is merely logged.
         """
-        if name not in self.chains:
+        if wrap:
+            chain_set = self.chains
+        else:
+            chain_set = self.unwrapped_chains
+
+        if name not in chain_set:
             LOG.debug(_("Attempted to remove chain %s which doesn't exist"),
                       name)
             return
-        self.chains.remove(name)
+
+        chain_set.remove(name)
         self.rules = filter(lambda r: r.chain != name, self.rules)
 
-        jump_snippet = '-j %s-%s' % (binary_name, name)
+        if wrap:
+            jump_snippet = '-j %s-%s' % (binary_name, name)
+        else:
+            jump_snippet = '-j %s' % (name,)
+
         self.rules = filter(lambda r: jump_snippet not in r.rule, self.rules)
 
-    def add_rule(self, chain, rule, wrap=True):
+    def add_rule(self, chain, rule, wrap=True, head=False):
         """Add a rule to the table
 
         This is just like what you'd feed to iptables, just without
@@ -149,14 +166,14 @@ class IptablesTable(object):
         if '$' in rule:
             rule = ' '.join(map(self._wrap_target_chain, rule.split(' ')))
 
-        self.rules.append(IptablesRule(chain, rule, wrap))
+        self.rules.append(IptablesRule(chain, rule, wrap, head))
 
     def _wrap_target_chain(self, s):
         if s.startswith('$'):
             return '%s-%s' % (binary_name, s[1:])
         return s
 
-    def remove_rule(self, chain, rule, wrap=True):
+    def remove_rule(self, chain, rule, wrap=True, head=False):
         """Remove a rule from a chain
 
         Note: The rule must be exactly identical to the one that was added.
@@ -164,11 +181,12 @@ class IptablesTable(object):
         CLI tool.
         """
         try:
-            self.rules.remove(IptablesRule(*args, **kwargs))
+            self.rules.remove(IptablesRule(chain, rule, wrap, head))
         except ValueError:
             LOG.debug(_("Tried to remove rule that wasn't there:"
-                        " %(args)r %(kwargs)r"), {'args': args,
-                                                  'kwargs': kwargs})
+                        " %(chain)r %(rule)r %(wrap)r %(head)r"),
+                      {'chain': chain, 'rule': rule,
+                       'head': head, 'wrap': wrap})
 
 
 class IptablesManager(object):
@@ -178,15 +196,19 @@ class IptablesManager(object):
 
     A number of chains are set up to begin with.
 
-    For ipv4, the filter table has a INPUT, OUTPUT, FORWARD, and local chains
-    already set up, while the NAT chain has PREROUTING, OUTPUT, POSTROUTING,
-    and SNATTING. Except for "local" and "SNATTING" these are all set up so
-    that they are applied at the beginning of their non-wrapped counterparts.
-    "SNATTING" is jumped to from nat/POSTROUTING and "local" is jumped to from
-    filter/OUTPUT and filter/FORWARD.
+    First, nova-filter-top. It's added at the top of FORWARD and OUTPUT. Its
+    name is not wrapped, so it's shared between the various nova workers. It's
+    intended for rules that need to live at the top of the FORWARD and OUTPUT
+    chains. It's in both the ipv4 and ipv6 set of tables.
 
-    For ipv6, the filter table has INPUT, OUTPUT, FORWARD, and local. "local"
-    has the same semantics as for ipv4.
+    For ipv4 and ipv6, the builtin INPUT, OUTPUT, and FORWARD filter chains are
+    wrapped, meaning that the "real" INPUT chain has a rule that jumps to the
+    wrapped INPUT chain, etc. Additionally, there's a wrapped chain named
+    "local" which is jumped to from nova-filter-top.
+
+    For ipv4, the builtin PREROUTING, OUTPUT, and POSTROUTING nat chains are
+    wrapped in the same was as the builtin filter chains. Additionally, there's
+    a SNATTING chain that is applied after the POSTROUTING chain.
     """
     def __init__(self, execute=None):
         if not execute:
@@ -201,13 +223,19 @@ class IptablesManager(object):
                      'nat': IptablesTable()}
         self.ipv6 = {'filter': IptablesTable()}
 
-        self.ipv4['filter'].add_chain('local')
-        self.ipv4['filter'].add_rule('FORWARD', '-j $local', wrap=False)
-        self.ipv4['filter'].add_rule('OUTPUT', '-j $local', wrap=False)
+        # Add a nova-filter-top chain. It's intended to be shared
+        # among the various nova components. It sits at the very top
+        # of FORWARD and OUTPUT.
+        for tables in [self.ipv4, self.ipv6]:
+            tables['filter'].add_chain('nova-filter-top', wrap=False)
+            tables['filter'].add_rule('FORWARD', '-j nova-filter-top',
+                                      wrap=False, head=True)
+            tables['filter'].add_rule('OUTPUT', '-j nova-filter-top',
+                                      wrap=False, head=True)
 
-        self.ipv6['filter'].add_chain('local')
-        self.ipv6['filter'].add_rule('FORWARD', '-j $local', wrap=False)
-        self.ipv6['filter'].add_rule('OUTPUT', '-j $local', wrap=False)
+            tables['filter'].add_chain('local')
+            tables['filter'].add_rule('nova-filter-top', '-j $local',
+                                      wrap=False)
 
         # Wrap the builtin chains
         builtin_chains = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
@@ -226,18 +254,27 @@ class IptablesManager(object):
                     tables[table].add_rule(chain, '-j $%s' % (chain,),
                                            wrap=False)
 
-        # We add a SNATTING chain after our (wrapped) POSTROUTING chain
-        # so that rules added there will be applied after whatever we have in
-        # (the wrapped) POSTROUTING.
+        # Add a nova-postrouting-bottom chain. It's intended to be shared
+        # among the various nova components. We set it as the last chain
+        # of POSTROUTING chain.
+        self.ipv4['nat'].add_chain('nova-postrouting-bottom', wrap=False)
+        self.ipv4['nat'].add_rule('POSTROUTING', '-j nova-postrouting-bottom',
+                                  wrap=False)
+
+
+        # We add a SNATTING chain to the shared nova-postrouting-bottom chain
+        # so that it's applied last.
         self.ipv4['nat'].add_chain('SNATTING')
-        self.ipv4['nat'].add_rule('POSTROUTING', '-j $SNATTING', wrap=False)
+        self.ipv4['nat'].add_rule('nova-postrouting-bottom', '-j $SNATTING', wrap=False)
 
+        # And then we add a floating-ip-snat chain and jump to first thing in the SNATTING
+        # chain.
         self.ipv4['nat'].add_chain('floating-ip-snat')
         self.ipv4['nat'].add_rule('SNATTING', '-j $floating-ip-snat')
 
         self.semaphore = semaphore.Semaphore()
 
-        iptables_manager.apply()
+        self.apply()
 
     def apply(self):
         """Apply the current in-memory set of iptables rules
@@ -245,7 +282,7 @@ class IptablesManager(object):
         This will blow away any rules left over from previous runs of the
         same component of Nova, and replace them with our current set of
         rules. This happens atomically, thanks to iptables-restore.
-        
+
         We wrap the call in a semaphore lock, so that we don't race with
         ourselves. In the event of a race with another component running
         an iptables-* command at the same time, we retry up to 5 times.
@@ -267,6 +304,7 @@ class IptablesManager(object):
                                  attempts=5)
 
     def _modify_rules(self, current_lines, table, binary=None):
+        unwrapped_chains = table.unwrapped_chains
         chains = table.chains
         rules = table.rules
 
@@ -284,11 +322,25 @@ class IptablesManager(object):
                 if not rule.startswith(':'):
                     break
 
-        new_filter[rules_index:rules_index] = [str(rule) for rule in rules]
+        new_filter[rules_index:rules_index] = [str(rule) for rule in rules
+                                               if rule.head or
+                                                  str(rule) not in new_filter]
+        new_filter[rules_index:rules_index] = [':%s - [0:0]' % \
+                                               (name,) \
+                                               for name in unwrapped_chains]
         new_filter[rules_index:rules_index] = [':%s-%s - [0:0]' % \
                                                (binary_name, name,) \
                                                for name in chains]
 
+        seen_lines = set()
+        def _weed_out_duplicates(line):
+            if line in seen_lines:
+                return False
+            else:
+                seen_lines.add(line)
+                return True
+
+        new_filter = filter(_weed_out_duplicates, new_filter)
         return new_filter
 
 
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index f1d4fe133..afd38272d 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -36,12 +36,22 @@ LOG = logging.getLogger('nova.tests.network')
 
 
 class IptablesManagerTestCase(test.TestCase):
-    sample_filter = """# Completed on Fri Feb 18 15:17:05 2011
-# Generated by iptables-save v1.4.10 on Fri Feb 18 15:17:05 2011
+    sample_filter = """# Generated by iptables-save on Fri Feb 18 15:17:05 2011
 *filter
 :INPUT ACCEPT [2223527:305688874]
 :FORWARD ACCEPT [0:0]
 :OUTPUT ACCEPT [2172501:140856656]
+:nova-compute-FORWARD - [0:0]
+:nova-compute-INPUT - [0:0]
+:nova-compute-local - [0:0]
+:nova-compute-OUTPUT - [0:0]
+:nova-filter-top - [0:0]
+-A FORWARD -j nova-filter-top
+-A OUTPUT -j nova-filter-top
+-A nova-filter-top -j nova-compute-local
+-A INPUT -j nova-compute-INPUT
+-A OUTPUT -j nova-compute-OUTPUT
+-A FORWARD -j nova-compute-FORWARD
 -A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT
 -A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT
 -A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT
@@ -53,42 +63,116 @@ class IptablesManagerTestCase(test.TestCase):
 COMMIT
 # Completed on Fri Feb 18 15:17:05 2011"""
 
+    sample_nat = """# Generated by iptables-save on Fri Feb 18 15:17:05 2011
+*nat
+:PREROUTING ACCEPT [3936:762355]
+:INPUT ACCEPT [2447:225266]
+:OUTPUT ACCEPT [63491:4191863]
+:POSTROUTING ACCEPT [63112:4108641]
+:nova-compute-OUTPUT - [0:0]
+:nova-compute-floating-ip-snat - [0:0]
+:nova-compute-SNATTING - [0:0]
+:nova-compute-PREROUTING - [0:0]
+:nova-compute-POSTROUTING - [0:0]
+:nova-postrouting-bottom - [0:0]
+-A PREROUTING -j nova-compute-PREROUTING
+-A OUTPUT -j nova-compute-OUTPUT
+-A POSTROUTING -j nova-compute-POSTROUTING
+-A POSTROUTING -j nova-postrouting-bottom
+-A nova-postrouting-bottom -j nova-compute-SNATTING
+-A nova-compute-SNATTING -j nova-compute-floating-ip-snat
+COMMIT
+# Completed on Fri Feb 18 15:17:05 2011
+"""
+
     def setUp(self):
         super(IptablesManagerTestCase, self).setUp()
         self.manager = linux_net.IptablesManager()
 
-    def test_rules_are_wrapped(self):
+        
+    def test_filter_rules_are_wrapped(self):
         current_lines = self.sample_filter.split('\n')
 
         table = self.manager.ipv4['filter']
         table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
-        new_lines = self.manager.modify_rules(current_lines, table)
+        new_lines = self.manager._modify_rules(current_lines, table)
         self.assertTrue('-A run_tests.py-FORWARD '
                         '-s 1.2.3.4/5 -j DROP' in new_lines)
 
         table.remove_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
-        new_lines = self.manager.modify_rules(current_lines, table)
+        new_lines = self.manager._modify_rules(current_lines, table)
         self.assertTrue('-A run_tests.py-FORWARD '
                         '-s 1.2.3.4/5 -j DROP' not in new_lines)
 
-    def test_wrapper_rules_in_place(self):
-        current_lines = self.sample_filter.split('\n')
+    def test_nat_rules(self):
+        current_lines = self.sample_nat.split('\n')
+        new_lines = self.manager._modify_rules(current_lines,
+                                               self.manager.ipv4['nat'])
+
+        for line in [':nova-compute-OUTPUT - [0:0]',
+                     ':nova-compute-floating-ip-snat - [0:0]',
+                     ':nova-compute-SNATTING - [0:0]',
+                     ':nova-compute-PREROUTING - [0:0]',
+                     ':nova-compute-POSTROUTING - [0:0]']:
+             self.assertTrue(line in new_lines, "One of nova-compute's chains "
+                                                "went missing.")
+
+        seen_lines = set()
+        for line in new_lines:
+            self.assertTrue(line not in seen_lines,
+                            "Duplicate line: %s" % line)
+            seen_lines.add(line)
 
-        # TODO(soren): Add stuff for ipv6
-        check_matrix = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
-                            'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']},
-                        6: {'filter': ['INPUT', 'OUTPUT', 'FORWARD']}}
-
-        for ip_version in check_matrix:
-            ip = getattr(self.manager, 'ipv%d' % ip_version)
-            for table_name in ip:
-                table = ip[table_name]
-                new_lines = self.manager.modify_rules(current_lines, table)
-                for chain in check_matrix[ip_version][table_name]:
-                    self.assertTrue(':run_tests.py-%s - [0:0]' % \
-                                    (chain,) in new_lines)
-                    self.assertTrue('-A %s -j run_tests.py-%s' % \
-                                    (chain, chain) in new_lines)
+        last_postrouting_line = ''
+
+        for line in new_lines:
+            if line.startswith('-A POSTROUTING'):
+                last_postrouting_line = line
+
+        self.assertTrue('-j nova-postrouting-bottom' in last_postrouting_line,
+                        "Last POSTROUTING rule does not jump to "
+                        "nova-postouting-bottom: %s" % last_postrouting_line)
+
+        for chain in ['POSTROUTING', 'PREROUTING', 'OUTPUT']:
+            self.assertTrue('-A %s -j run_tests.py-%s' \
+                            % (chain, chain) in new_lines,
+                            "Built-in chain %s not wrapped" % (chain,))
+
+
+    def test_filter_rules(self):
+        current_lines = self.sample_filter.split('\n')
+        new_lines = self.manager._modify_rules(current_lines,
+                                               self.manager.ipv4['filter'])
+
+        for line in [':nova-compute-FORWARD - [0:0]',
+                     ':nova-compute-INPUT - [0:0]',
+                     ':nova-compute-local - [0:0]',
+                     ':nova-compute-OUTPUT - [0:0]']:
+             self.assertTrue(line in new_lines, "One of nova-compute's chains"
+                                                " went missing.")
+
+        seen_lines = set()
+        for line in new_lines:
+            self.assertTrue(line not in seen_lines,
+                            "Duplicate line: %s" % line)
+            seen_lines.add(line)
+
+        for chain in ['FORWARD', 'OUTPUT']:
+            for line in new_lines:
+                if line.startswith('-A %s' % chain):
+                    self.assertTrue('-j nova-filter-top' in line,
+                                    "First %s rule does not "
+                                    "jump to nova-filter-top" % chain)
+                    break
+
+        self.assertTrue('-A nova-filter-top '
+                        '-j run_tests.py-local' in new_lines,
+                        "nova-filter-top does not jump to wrapped local chain")
+
+        for chain in ['INPUT', 'OUTPUT', 'FORWARD']:
+            self.assertTrue('-A %s -j run_tests.py-%s' \
+                            % (chain, chain) in new_lines,
+                            "Built-in chain %s not wrapped" % (chain,))
 
 
 class NetworkTestCase(test.TestCase):
-- 
cgit 


From 54f2362d09393ad6ccdfee5689d4f547c69b3f42 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 11:47:46 +0100
Subject: Remove leftover from debugging.

---
 nova/network/linux_net.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 2ccb5969e..5f480f633 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -274,8 +274,6 @@ class IptablesManager(object):
 
         self.semaphore = semaphore.Semaphore()
 
-        self.apply()
-
     def apply(self):
         """Apply the current in-memory set of iptables rules
 
-- 
cgit 


From b5e6601f76d64a96d6c7de5e9acdf5a8cf0fe8e9 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 12:21:29 +0100
Subject: PEP8 adjustments.

---
 nova/network/linux_net.py  |  9 +++++----
 nova/tests/test_network.py | 10 ++++------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 5f480f633..7c4c16810 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -261,14 +261,14 @@ class IptablesManager(object):
         self.ipv4['nat'].add_rule('POSTROUTING', '-j nova-postrouting-bottom',
                                   wrap=False)
 
-
         # We add a SNATTING chain to the shared nova-postrouting-bottom chain
         # so that it's applied last.
         self.ipv4['nat'].add_chain('SNATTING')
-        self.ipv4['nat'].add_rule('nova-postrouting-bottom', '-j $SNATTING', wrap=False)
+        self.ipv4['nat'].add_rule('nova-postrouting-bottom', '-j $SNATTING',
+                                  wrap=False)
 
-        # And then we add a floating-ip-snat chain and jump to first thing in the SNATTING
-        # chain.
+        # And then we add a floating-ip-snat chain and jump to first thing in
+        # the SNATTING chain.
         self.ipv4['nat'].add_chain('floating-ip-snat')
         self.ipv4['nat'].add_rule('SNATTING', '-j $floating-ip-snat')
 
@@ -331,6 +331,7 @@ class IptablesManager(object):
                                                for name in chains]
 
         seen_lines = set()
+
         def _weed_out_duplicates(line):
             if line in seen_lines:
                 return False
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index afd38272d..2bdf3709e 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -89,7 +89,6 @@ COMMIT
         super(IptablesManagerTestCase, self).setUp()
         self.manager = linux_net.IptablesManager()
 
-        
     def test_filter_rules_are_wrapped(self):
         current_lines = self.sample_filter.split('\n')
 
@@ -114,8 +113,8 @@ COMMIT
                      ':nova-compute-SNATTING - [0:0]',
                      ':nova-compute-PREROUTING - [0:0]',
                      ':nova-compute-POSTROUTING - [0:0]']:
-             self.assertTrue(line in new_lines, "One of nova-compute's chains "
-                                                "went missing.")
+            self.assertTrue(line in new_lines, "One of nova-compute's chains "
+                                               "went missing.")
 
         seen_lines = set()
         for line in new_lines:
@@ -138,7 +137,6 @@ COMMIT
                             % (chain, chain) in new_lines,
                             "Built-in chain %s not wrapped" % (chain,))
 
-
     def test_filter_rules(self):
         current_lines = self.sample_filter.split('\n')
         new_lines = self.manager._modify_rules(current_lines,
@@ -148,8 +146,8 @@ COMMIT
                      ':nova-compute-INPUT - [0:0]',
                      ':nova-compute-local - [0:0]',
                      ':nova-compute-OUTPUT - [0:0]']:
-             self.assertTrue(line in new_lines, "One of nova-compute's chains"
-                                                " went missing.")
+            self.assertTrue(line in new_lines, "One of nova-compute's chains"
+                                               " went missing.")
 
         seen_lines = set()
         for line in new_lines:
-- 
cgit 


From 92a693b04feddad2420a835a12f53520c5529d8f Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 12:42:38 +0100
Subject: floating-ip-snat was too long. Use floating-snat instead.

---
 nova/network/linux_net.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 7c4c16810..5e89a1df6 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -267,10 +267,10 @@ class IptablesManager(object):
         self.ipv4['nat'].add_rule('nova-postrouting-bottom', '-j $SNATTING',
                                   wrap=False)
 
-        # And then we add a floating-ip-snat chain and jump to first thing in
+        # And then we add a floating-snat chain and jump to first thing in
         # the SNATTING chain.
-        self.ipv4['nat'].add_chain('floating-ip-snat')
-        self.ipv4['nat'].add_rule('SNATTING', '-j $floating-ip-snat')
+        self.ipv4['nat'].add_chain('floating-snat')
+        self.ipv4['nat'].add_rule('SNATTING', '-j $floating-snat')
 
         self.semaphore = semaphore.Semaphore()
 
@@ -420,7 +420,7 @@ def remove_floating_forward(floating_ip, fixed_ip):
 def floating_forward_rules(floating_ip, fixed_ip):
     return [("PREROUTING", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
             ("OUTPUT", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
-            ("floating-ip-snat",
+            ("floating-snat",
              "-s %s -j SNAT --to %s" % (fixed_ip, floating_ip))]
 
 
-- 
cgit 


From 443f01ef7d977ba6ff86508b908f66733bdd989e Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 14:32:20 +0100
Subject: Account for the fact that iptables-save outputs rules with a space at
 the end. Reverse the rule deduplication so that the last one takes
 precedence.

---
 nova/network/linux_net.py  | 45 ++++++++++++++++++++++++++++++---------------
 nova/tests/test_network.py | 42 ++++++++++++++++++++++--------------------
 2 files changed, 52 insertions(+), 35 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 5e89a1df6..849181641 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -72,22 +72,22 @@ class IptablesRule(object):
     You shouldn't need to use this class directly, it's only used by
     IptablesManager
     """
-    def __init__(self, chain, rule, wrap=True, head=False):
+    def __init__(self, chain, rule, wrap=True, top=False):
         self.chain = chain
         self.rule = rule
         self.wrap = wrap
-        self.head = head
+        self.top = top
 
     def __eq__(self, other):
         return ((self.chain == other.chain) and
                 (self.rule == other.rule) and
-                (self.head == other.head) and
+                (self.top == other.top) and
                 (self.wrap == other.wrap))
 
     def __ne__(self, other):
         return ((self.chain != other.chain) or
                 (self.rule != other.rule) or
-                (self.head != other.head) or
+                (self.top != other.top) or
                 (self.wrap != other.wrap))
 
     def __str__(self):
@@ -150,7 +150,7 @@ class IptablesTable(object):
 
         self.rules = filter(lambda r: jump_snippet not in r.rule, self.rules)
 
-    def add_rule(self, chain, rule, wrap=True, head=False):
+    def add_rule(self, chain, rule, wrap=True, top=False):
         """Add a rule to the table
 
         This is just like what you'd feed to iptables, just without
@@ -166,14 +166,14 @@ class IptablesTable(object):
         if '$' in rule:
             rule = ' '.join(map(self._wrap_target_chain, rule.split(' ')))
 
-        self.rules.append(IptablesRule(chain, rule, wrap, head))
+        self.rules.append(IptablesRule(chain, rule, wrap, top))
 
     def _wrap_target_chain(self, s):
         if s.startswith('$'):
             return '%s-%s' % (binary_name, s[1:])
         return s
 
-    def remove_rule(self, chain, rule, wrap=True, head=False):
+    def remove_rule(self, chain, rule, wrap=True, top=False):
         """Remove a rule from a chain
 
         Note: The rule must be exactly identical to the one that was added.
@@ -181,12 +181,12 @@ class IptablesTable(object):
         CLI tool.
         """
         try:
-            self.rules.remove(IptablesRule(chain, rule, wrap, head))
+            self.rules.remove(IptablesRule(chain, rule, wrap, top))
         except ValueError:
             LOG.debug(_("Tried to remove rule that wasn't there:"
-                        " %(chain)r %(rule)r %(wrap)r %(head)r"),
+                        " %(chain)r %(rule)r %(wrap)r %(top)r"),
                       {'chain': chain, 'rule': rule,
-                       'head': head, 'wrap': wrap})
+                       'top': top, 'wrap': wrap})
 
 
 class IptablesManager(object):
@@ -229,9 +229,9 @@ class IptablesManager(object):
         for tables in [self.ipv4, self.ipv6]:
             tables['filter'].add_chain('nova-filter-top', wrap=False)
             tables['filter'].add_rule('FORWARD', '-j nova-filter-top',
-                                      wrap=False, head=True)
+                                      wrap=False, top=True)
             tables['filter'].add_rule('OUTPUT', '-j nova-filter-top',
-                                      wrap=False, head=True)
+                                      wrap=False, top=True)
 
             tables['filter'].add_chain('local')
             tables['filter'].add_rule('nova-filter-top', '-j $local',
@@ -320,9 +320,19 @@ class IptablesManager(object):
                 if not rule.startswith(':'):
                     break
 
-        new_filter[rules_index:rules_index] = [str(rule) for rule in rules
-                                               if rule.head or
-                                                  str(rule) not in new_filter]
+        our_rules = []
+        for rule in rules:
+            rule_str = str(rule)
+            if rule.top:
+                # rule.top == True means we want this rule to be at the top.
+                # Further down, we weed out duplicates from the bottom of the
+                # list, so here we remove the dupes ahead of time.
+                new_filter = filter(lambda s: s.strip() != rule_str.strip(),
+                                    new_filter)
+            our_rules += [rule_str]
+
+        new_filter[rules_index:rules_index] = our_rules
+
         new_filter[rules_index:rules_index] = [':%s - [0:0]' % \
                                                (name,) \
                                                for name in unwrapped_chains]
@@ -333,13 +343,18 @@ class IptablesManager(object):
         seen_lines = set()
 
         def _weed_out_duplicates(line):
+            line = line.strip()
             if line in seen_lines:
                 return False
             else:
                 seen_lines.add(line)
                 return True
 
+        # We filter duplicates, letting the *last* occurrence take
+        # precendence.
+        new_filter.reverse()
         new_filter = filter(_weed_out_duplicates, new_filter)
+        new_filter.reverse()
         return new_filter
 
 
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index 2bdf3709e..b1d70e323 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -46,20 +46,20 @@ class IptablesManagerTestCase(test.TestCase):
 :nova-compute-local - [0:0]
 :nova-compute-OUTPUT - [0:0]
 :nova-filter-top - [0:0]
--A FORWARD -j nova-filter-top
--A OUTPUT -j nova-filter-top
--A nova-filter-top -j nova-compute-local
--A INPUT -j nova-compute-INPUT
--A OUTPUT -j nova-compute-OUTPUT
--A FORWARD -j nova-compute-FORWARD
--A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT
--A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT
--A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT
--A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT
--A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT
--A FORWARD -i virbr0 -o virbr0 -j ACCEPT
--A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable
--A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable
+-A FORWARD -j nova-filter-top 
+-A OUTPUT -j nova-filter-top 
+-A nova-filter-top -j nova-compute-local 
+-A INPUT -j nova-compute-INPUT 
+-A OUTPUT -j nova-compute-OUTPUT 
+-A FORWARD -j nova-compute-FORWARD 
+-A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT 
+-A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT 
+-A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT 
+-A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT 
+-A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT 
+-A FORWARD -i virbr0 -o virbr0 -j ACCEPT 
+-A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable 
+-A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable 
 COMMIT
 # Completed on Fri Feb 18 15:17:05 2011"""
 
@@ -75,12 +75,12 @@ COMMIT
 :nova-compute-PREROUTING - [0:0]
 :nova-compute-POSTROUTING - [0:0]
 :nova-postrouting-bottom - [0:0]
--A PREROUTING -j nova-compute-PREROUTING
--A OUTPUT -j nova-compute-OUTPUT
--A POSTROUTING -j nova-compute-POSTROUTING
--A POSTROUTING -j nova-postrouting-bottom
--A nova-postrouting-bottom -j nova-compute-SNATTING
--A nova-compute-SNATTING -j nova-compute-floating-ip-snat
+-A PREROUTING -j nova-compute-PREROUTING 
+-A OUTPUT -j nova-compute-OUTPUT 
+-A POSTROUTING -j nova-compute-POSTROUTING 
+-A POSTROUTING -j nova-postrouting-bottom 
+-A nova-postrouting-bottom -j nova-compute-SNATTING 
+-A nova-compute-SNATTING -j nova-compute-floating-ip-snat 
 COMMIT
 # Completed on Fri Feb 18 15:17:05 2011
 """
@@ -118,6 +118,7 @@ COMMIT
 
         seen_lines = set()
         for line in new_lines:
+            line = line.strip()
             self.assertTrue(line not in seen_lines,
                             "Duplicate line: %s" % line)
             seen_lines.add(line)
@@ -151,6 +152,7 @@ COMMIT
 
         seen_lines = set()
         for line in new_lines:
+            line = line.strip()
             self.assertTrue(line not in seen_lines,
                             "Duplicate line: %s" % line)
             seen_lines.add(line)
-- 
cgit 


From 7eee81ee6480a36b179ae26be88ebad9415c4b62 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 14:40:00 +0100
Subject: PEP8 again

---
 nova/tests/test_network.py | 103 +++++++++++++++++++++++----------------------
 1 file changed, 52 insertions(+), 51 deletions(-)

diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index b1d70e323..d3a23abf4 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -36,61 +36,62 @@ LOG = logging.getLogger('nova.tests.network')
 
 
 class IptablesManagerTestCase(test.TestCase):
-    sample_filter = """# Generated by iptables-save on Fri Feb 18 15:17:05 2011
-*filter
-:INPUT ACCEPT [2223527:305688874]
-:FORWARD ACCEPT [0:0]
-:OUTPUT ACCEPT [2172501:140856656]
-:nova-compute-FORWARD - [0:0]
-:nova-compute-INPUT - [0:0]
-:nova-compute-local - [0:0]
-:nova-compute-OUTPUT - [0:0]
-:nova-filter-top - [0:0]
--A FORWARD -j nova-filter-top 
--A OUTPUT -j nova-filter-top 
--A nova-filter-top -j nova-compute-local 
--A INPUT -j nova-compute-INPUT 
--A OUTPUT -j nova-compute-OUTPUT 
--A FORWARD -j nova-compute-FORWARD 
--A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT 
--A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT 
--A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT 
--A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT 
--A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT 
--A FORWARD -i virbr0 -o virbr0 -j ACCEPT 
--A FORWARD -o virbr0 -j REJECT --reject-with icmp-port-unreachable 
--A FORWARD -i virbr0 -j REJECT --reject-with icmp-port-unreachable 
-COMMIT
-# Completed on Fri Feb 18 15:17:05 2011"""
-
-    sample_nat = """# Generated by iptables-save on Fri Feb 18 15:17:05 2011
-*nat
-:PREROUTING ACCEPT [3936:762355]
-:INPUT ACCEPT [2447:225266]
-:OUTPUT ACCEPT [63491:4191863]
-:POSTROUTING ACCEPT [63112:4108641]
-:nova-compute-OUTPUT - [0:0]
-:nova-compute-floating-ip-snat - [0:0]
-:nova-compute-SNATTING - [0:0]
-:nova-compute-PREROUTING - [0:0]
-:nova-compute-POSTROUTING - [0:0]
-:nova-postrouting-bottom - [0:0]
--A PREROUTING -j nova-compute-PREROUTING 
--A OUTPUT -j nova-compute-OUTPUT 
--A POSTROUTING -j nova-compute-POSTROUTING 
--A POSTROUTING -j nova-postrouting-bottom 
--A nova-postrouting-bottom -j nova-compute-SNATTING 
--A nova-compute-SNATTING -j nova-compute-floating-ip-snat 
-COMMIT
-# Completed on Fri Feb 18 15:17:05 2011
-"""
+    sample_filter = ['#Generated by iptables-save on Fri Feb 18 15:17:05 2011',
+                     '*filter',
+                     ':INPUT ACCEPT [2223527:305688874]',
+                     ':FORWARD ACCEPT [0:0]',
+                     ':OUTPUT ACCEPT [2172501:140856656]',
+                     ':nova-compute-FORWARD - [0:0]',
+                     ':nova-compute-INPUT - [0:0]',
+                     ':nova-compute-local - [0:0]',
+                     ':nova-compute-OUTPUT - [0:0]',
+                     ':nova-filter-top - [0:0]',
+                     '-A FORWARD -j nova-filter-top ',
+                     '-A OUTPUT -j nova-filter-top ',
+                     '-A nova-filter-top -j nova-compute-local ',
+                     '-A INPUT -j nova-compute-INPUT ',
+                     '-A OUTPUT -j nova-compute-OUTPUT ',
+                     '-A FORWARD -j nova-compute-FORWARD ',
+                     '-A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT ',
+                     '-A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT ',
+                     '-A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT ',
+                     '-A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT ',
+                     '-A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT ',
+                     '-A FORWARD -i virbr0 -o virbr0 -j ACCEPT ',
+                     '-A FORWARD -o virbr0 -j REJECT --reject-with '
+                     'icmp-port-unreachable ',
+                     '-A FORWARD -i virbr0 -j REJECT --reject-with '
+                     'icmp-port-unreachable ',
+                     'COMMIT',
+                     '# Completed on Fri Feb 18 15:17:05 2011']
+
+    sample_nat = ['# Generated by iptables-save on Fri Feb 18 15:17:05 2011',
+                  '*nat',
+                  ':PREROUTING ACCEPT [3936:762355]',
+                  ':INPUT ACCEPT [2447:225266]',
+                  ':OUTPUT ACCEPT [63491:4191863]',
+                  ':POSTROUTING ACCEPT [63112:4108641]',
+                  ':nova-compute-OUTPUT - [0:0]',
+                  ':nova-compute-floating-ip-snat - [0:0]',
+                  ':nova-compute-SNATTING - [0:0]',
+                  ':nova-compute-PREROUTING - [0:0]',
+                  ':nova-compute-POSTROUTING - [0:0]',
+                  ':nova-postrouting-bottom - [0:0]',
+                  '-A PREROUTING -j nova-compute-PREROUTING ',
+                  '-A OUTPUT -j nova-compute-OUTPUT ',
+                  '-A POSTROUTING -j nova-compute-POSTROUTING ',
+                  '-A POSTROUTING -j nova-postrouting-bottom ',
+                  '-A nova-postrouting-bottom -j nova-compute-SNATTING ',
+                  '-A nova-compute-SNATTING -j nova-compute-floating-ip-snat ',
+                  'COMMIT',
+                  '# Completed on Fri Feb 18 15:17:05 2011']
 
     def setUp(self):
         super(IptablesManagerTestCase, self).setUp()
         self.manager = linux_net.IptablesManager()
 
     def test_filter_rules_are_wrapped(self):
-        current_lines = self.sample_filter.split('\n')
+        current_lines = self.sample_filter
 
         table = self.manager.ipv4['filter']
         table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
@@ -104,7 +105,7 @@ COMMIT
                         '-s 1.2.3.4/5 -j DROP' not in new_lines)
 
     def test_nat_rules(self):
-        current_lines = self.sample_nat.split('\n')
+        current_lines = self.sample_nat
         new_lines = self.manager._modify_rules(current_lines,
                                                self.manager.ipv4['nat'])
 
@@ -139,7 +140,7 @@ COMMIT
                             "Built-in chain %s not wrapped" % (chain,))
 
     def test_filter_rules(self):
-        current_lines = self.sample_filter.split('\n')
+        current_lines = self.sample_filter
         new_lines = self.manager._modify_rules(current_lines,
                                                self.manager.ipv4['filter'])
 
-- 
cgit 


From 8b0e8b155eab313e0caece48eee609d12df5e5d4 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Tue, 22 Feb 2011 20:45:36 +0100
Subject: Rename "SNATTING" chain to "snat".

---
 nova/network/linux_net.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 849181641..34337901a 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -208,7 +208,7 @@ class IptablesManager(object):
 
     For ipv4, the builtin PREROUTING, OUTPUT, and POSTROUTING nat chains are
     wrapped in the same was as the builtin filter chains. Additionally, there's
-    a SNATTING chain that is applied after the POSTROUTING chain.
+    a snat chain that is applied after the POSTROUTING chain.
     """
     def __init__(self, execute=None):
         if not execute:
@@ -261,16 +261,16 @@ class IptablesManager(object):
         self.ipv4['nat'].add_rule('POSTROUTING', '-j nova-postrouting-bottom',
                                   wrap=False)
 
-        # We add a SNATTING chain to the shared nova-postrouting-bottom chain
+        # We add a snat chain to the shared nova-postrouting-bottom chain
         # so that it's applied last.
-        self.ipv4['nat'].add_chain('SNATTING')
-        self.ipv4['nat'].add_rule('nova-postrouting-bottom', '-j $SNATTING',
+        self.ipv4['nat'].add_chain('snat')
+        self.ipv4['nat'].add_rule('nova-postrouting-bottom', '-j $snat',
                                   wrap=False)
 
         # And then we add a floating-snat chain and jump to first thing in
-        # the SNATTING chain.
+        # the snat chain.
         self.ipv4['nat'].add_chain('floating-snat')
-        self.ipv4['nat'].add_rule('SNATTING', '-j $floating-snat')
+        self.ipv4['nat'].add_rule('snat', '-j $floating-snat')
 
         self.semaphore = semaphore.Semaphore()
 
@@ -376,7 +376,7 @@ def init_host():
 
     # NOTE(devcamcar): Cloud public SNAT entries and the default
     # SNAT rule for outbound traffic.
-    iptables_manager.ipv4['nat'].add_rule("SNATTING",
+    iptables_manager.ipv4['nat'].add_rule("snat",
                                           "-s %s -j SNAT --to-source %s" % \
                                            (FLAGS.fixed_range,
                                             FLAGS.routing_source_ip))
-- 
cgit 


From 0d3a1f9c7478ae3c4f042e1876d47359804e1973 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 28 Feb 2011 15:29:42 +0100
Subject: Wrap IptablesManager.apply() calls in utils.synchronized to avoid
 having different workers step on each other's toes.

---
 nova/network/linux_net.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 34337901a..8832f7c68 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -274,6 +274,7 @@ class IptablesManager(object):
 
         self.semaphore = semaphore.Semaphore()
 
+    @utils.synchronized('iptables')
     def apply(self):
         """Apply the current in-memory set of iptables rules
 
-- 
cgit 


From 8c3bc15c96c6a6f1c99d829337921f2645608410 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 7 Mar 2011 21:43:31 +0100
Subject: Make iptables rules class __ne__ just be inverted __eq__.

---
 nova/network/linux_net.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 8832f7c68..57e77f8d5 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -85,10 +85,7 @@ class IptablesRule(object):
                 (self.wrap == other.wrap))
 
     def __ne__(self, other):
-        return ((self.chain != other.chain) or
-                (self.rule != other.rule) or
-                (self.top != other.top) or
-                (self.wrap != other.wrap))
+        return not self == other
 
     def __str__(self):
         if self.wrap:
-- 
cgit 


From 7b7abe7e7a25c0cd07c64c34f69ce050c669cfc3 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 7 Mar 2011 21:54:25 +0100
Subject: Log failed command execution if there are more retry attempts left.

---
 nova/utils.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nova/utils.py b/nova/utils.py
index 829adfb9e..80e5b6bbe 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -166,6 +166,7 @@ def execute(cmd, process_input=None, addl_env=None, check_exit_code=True,
             if not attempts:
                 raise
             else:
+                LOG.debug(_("%r failed. Retrying."), cmd)
                 greenthread.sleep(random.randint(20, 200) / 100.0)
 
 
-- 
cgit 


From 4e9c570fbf8b3987d556da085b61f159f32c16f1 Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Mon, 7 Mar 2011 21:59:05 +0100
Subject: Use IptablesManager.semapahore from securitygroups driver to ensure
 we don't apply half a rule set.

---
 nova/virt/libvirt_conn.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index b900cb8eb..825bcb0d7 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1372,8 +1372,12 @@ class IptablesFirewallDriver(FirewallDriver):
 
     def refresh_security_group_rules(self, security_group):
         for instance in self.instances.values():
-            self.remove_filters_for_instance(instance)
-            self.add_filters_for_instance(instance)
+            # We use the semaphore to make sure noone applies the rule set
+            # after we've yanked the existing rules but before we've put in
+            # the new ones.
+            with self.iptables.semaphore:
+                self.remove_filters_for_instance(instance)
+                self.add_filters_for_instance(instance)
         self.iptables.apply()
 
     def _security_group_chain_name(self, security_group_id):
-- 
cgit 


From 11f2d788fd63c66af0e992f7b75b61273c059bcb Mon Sep 17 00:00:00 2001
From: Soren Hansen <soren@linux2go.dk>
Date: Thu, 10 Mar 2011 21:31:47 +0100
Subject: PEP8

---
 nova/utils.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/nova/utils.py b/nova/utils.py
index 3008a512e..87e726394 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -166,9 +166,9 @@ def execute(*cmd, **kwargs):
                                                 stdout=stdout,
                                                 stderr=stderr,
                                                 cmd=' '.join(cmd))
-            # NOTE(termie): this appears to be necessary to let the subprocess call
-            #               clean something up in between calls, without it two
-            #               execute calls in a row hangs the second one
+            # NOTE(termie): this appears to be necessary to let the subprocess
+            #               call clean something up in between calls, without
+            #               it two execute calls in a row hangs the second one
             greenthread.sleep(0)
             return result
         except ProcessExecutionError:
-- 
cgit