Use self.instances.pop in unfilter_instance to make the check/removal atomic.

Move the semaphore grab outside the for loop in refresh_security_group_rules to avoid reading a value from self.instances, blocking waiting for the semaphore, having the instance be removed in the mean time, and then add its rules back.

1 files changed, 7 insertions, 8 deletions

diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index b74ed25f9..d82b33ddd 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -1238,13 +1238,12 @@ class IptablesFirewallDriver(FirewallDriver):
         pass
 
     def unfilter_instance(self, instance):
-        if instance['id'] in self.instances:
-            del self.instances[instance['id']]
+        if self.instances.pop(instance['id'], False):
             self.remove_filters_for_instance(instance)
             self.iptables.apply()
         else:
             LOG.info(_('Attempted to unfilter instance %s which is not '
-                       'filtered'), instance['id'])
+                   'filtered'), instance['id'])
 
     def prepare_instance_filter(self, instance):
         self.instances[instance['id']] = instance
@@ -1387,11 +1386,11 @@ class IptablesFirewallDriver(FirewallDriver):
         pass
 
     def refresh_security_group_rules(self, security_group):
-        for instance in self.instances.values():
-            # 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:
+        # 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:
+            for instance in self.instances.values():
                 self.remove_filters_for_instance(instance)
                 self.add_filters_for_instance(instance)
         self.iptables.apply()