Avoid RPC calls while holding iptables lock.

This exhibitied itself as very slow instance starts on a Canonical
test cluster. This was because do_referesh_security_group_rules()
was making rpc calls while holding the iptables lock. This refactor
avoids that while making no functional changes (I hope).

This should resolve bug 1062314.

Change-Id: I36f805bd72f7bd06082cfe96c58d637203bcffb7

2 files changed, 30 insertions, 10 deletions

diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 8861eb8de..7af877f22 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -3142,11 +3142,23 @@ class IptablesFirewallTestCase(test.TestCase):
     def test_do_refresh_security_group_rules(self):
         instance_ref = self._create_instance_ref()
         self.mox.StubOutWithMock(self.fw,
+                                 'instance_rules')
+        self.mox.StubOutWithMock(self.fw,
                                  'add_filters_for_instance',
                                  use_mock_anything=True)
+
+        self.fw.instance_rules(instance_ref,
+                               mox.IgnoreArg()).AndReturn((None, None))
+        self.fw.add_filters_for_instance(instance_ref, mox.IgnoreArg(),
+                                         mox.IgnoreArg())
+        self.fw.instance_rules(instance_ref,
+                               mox.IgnoreArg()).AndReturn((None, None))
+        self.fw.add_filters_for_instance(instance_ref, mox.IgnoreArg(),
+                                         mox.IgnoreArg())
+        self.mox.ReplayAll()
+
         self.fw.prepare_instance_filter(instance_ref, mox.IgnoreArg())
         self.fw.instances[instance_ref['id']] = instance_ref
-        self.mox.ReplayAll()
         self.fw.do_refresh_security_group_rules("fake")
 
     def test_unfilter_instance_undefines_nwfilter(self):
diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py
index eb14a92c5..3e2ba5d33 100644
--- a/nova/virt/firewall.py
+++ b/nova/virt/firewall.py
@@ -182,7 +182,8 @@ class IptablesFirewallDriver(FirewallDriver):
 
         self.instances[instance['id']] = instance
         self.network_infos[instance['id']] = network_info
-        self.add_filters_for_instance(instance)
+        ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info)
+        self.add_filters_for_instance(instance, ipv4_rules, ipv6_rules)
         LOG.debug(_('Filters added to instance'), instance=instance)
         self.refresh_provider_fw_rules()
         LOG.debug(_('Provider Firewall Rules refreshed'), instance=instance)
@@ -218,7 +219,8 @@ class IptablesFirewallDriver(FirewallDriver):
             for rule in ipv6_rules:
                 self.iptables.ipv6['filter'].add_rule(chain_name, rule)
 
-    def add_filters_for_instance(self, instance):
+    def add_filters_for_instance(self, instance, inst_ipv4_rules,
+                                 inst_ipv6_rules):
         network_info = self.network_infos[instance['id']]
         chain_name = self._instance_chain_name(instance)
         if FLAGS.use_ipv6:
@@ -227,8 +229,7 @@ class IptablesFirewallDriver(FirewallDriver):
         ipv4_rules, ipv6_rules = self._filters_for_instance(chain_name,
                                                             network_info)
         self._add_filters('local', ipv4_rules, ipv6_rules)
-        ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info)
-        self._add_filters(chain_name, ipv4_rules, ipv6_rules)
+        self._add_filters(chain_name, inst_ipv4_rules, inst_ipv6_rules)
 
     def remove_filters_for_instance(self, instance):
         chain_name = self._instance_chain_name(instance)
@@ -430,15 +431,22 @@ class IptablesFirewallDriver(FirewallDriver):
         self.iptables.apply()
 
     @utils.synchronized('iptables', external=True)
+    def _inner_do_refresh_rules(self, instance, ipv4_rules,
+                                               ipv6_rules):
+        self.remove_filters_for_instance(instance)
+        self.add_filters_for_instance(instance, ipv4_rules, ipv6_rules)
+
     def do_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)
+            network_info = self.network_infos[instance['id']]
+            ipv4_rules, ipv6_rules = self.instance_rules(instance,
+                                                         network_info)
+            self._inner_do_refresh_rules(instance, ipv4_rules, ipv6_rules)
 
-    @utils.synchronized('iptables', external=True)
     def do_refresh_instance_rules(self, instance):
-        self.remove_filters_for_instance(instance)
-        self.add_filters_for_instance(instance)
+        network_info = self.network_infos[instance['id']]
+        ipv4_rules, ipv6_rules = self.instance_rules(instance, network_info)
+        self._inner_do_refresh_rules(instance, ipv4_rules, ipv6_rules)
 
     def refresh_provider_fw_rules(self):
         """See :class:`FirewallDriver` docs."""