[virt-tools-list] [virt-manager PATCH] connection: change blacklist from array to dict

Pavel Hrdina phrdina at redhat.com
Fri Sep 8 08:32:12 UTC 2017


If initialization of new object fails we put it into blacklist and
newer parse it again until virt-manager is restarted.  This helps to
reduce number of failures if some object fails initialization every
time.

However, there are some cases where we put object into blacklist
incorrectly.  One of the cases is while creating new storage pool.
If the storage pool requires to be build before started but user
doesn't check to build it as well the start of the new storage pool
fails.  The issue is that at first we define that object which triggers
a lifecycle event for storage pool and queues new poll operation over
storage pools.  Before the poll operation starts the starting of the
storage pools fails and we undefine that storage pool before it is
initialized.  The initialization fails and the storage pool is never
managed from that point.

This patch modifies the blacklist to allow 3 failures before we give up
on a specific object and if the object is initialized without error
we remove it from blacklist completely.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1446486
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1093394

Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
---
 virtManager/connection.py | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/virtManager/connection.py b/virtManager/connection.py
index 797c6c2a..70da9220 100644
--- a/virtManager/connection.py
+++ b/virtManager/connection.py
@@ -51,11 +51,14 @@ class _ObjectList(vmmGObject):
     """
     Class that wraps our internal list of libvirt objects
     """
+
+    BLACKLIST_COUNT = 3
+
     def __init__(self):
         vmmGObject.__init__(self)
 
         self._objects = []
-        self._blacklist = []
+        self._blacklist = {}
         self._lock = threading.Lock()
 
     def _cleanup(self):
@@ -80,19 +83,30 @@ class _ObjectList(vmmGObject):
         choose not to poll, because they threw an error at init time
 
         :param obj: vmmLibvirtObject to blacklist
-        :returns: True if object added, False if object was already in list
+        :returns: number of added object to list
         """
+        key = self._blacklist_key(obj)
         if self.in_blacklist(obj):
-            return False
-        self._blacklist.append(self._blacklist_key(obj))
-        return True
+            self._blacklist[key] += 1
+        self._blacklist[key] = 1
+        return self._blacklist[key]
+
+    def remove_blacklist(self, obj):
+        """
+        :param obj: vmmLibvirtObject to remove from blacklist
+        :returns: True if object was blacklisted or False otherwise.
+        """
+        return bool(self._blacklist.pop(self._blacklist_key(obj), 0))
 
     def in_blacklist(self, obj):
         """
+        If an object is in list only once don't consider it blacklisted,
+        give it one more chance.
+
         :param obj: vmmLibvirtObject to check
-        :returns: True if object is in the blacklist
+        :returns: True if object is blacklisted
         """
-        return self._blacklist_key(obj) in self._blacklist
+        return self._blacklist.get(self._blacklist_key(obj), 0) > _ObjectList.BLACKLIST_COUNT
 
     def remove(self, obj):
         """
@@ -107,11 +121,7 @@ class _ObjectList(vmmGObject):
             # Identity check is sufficient here, since we should never be
             # asked to remove an object that wasn't at one point in the list.
             if obj not in self._objects:
-                if self.in_blacklist(obj):
-                    self._blacklist.remove(self._blacklist_key(obj))
-                    return True
-
-                return False
+                return self.remove_blacklist(obj)
 
             self._objects.remove(obj)
             return True
@@ -1166,9 +1176,14 @@ class vmmConnection(vmmGObject):
 
             if initialize_failed:
                 logging.debug("Blacklisting %s=%s", class_name, obj.get_name())
-                if self._objects.add_blacklist(obj) is False:
+                count = self._objects.add_blacklist(obj)
+                if count <= _ObjectList.BLACKLIST_COUNT:
+                    logging.debug("Object added in blacklist, count=%d", count)
+                else:
                     logging.debug("Object already blacklisted?")
                 return
+            else:
+                self._objects.remove_blacklist(obj)
 
             if not self._objects.add(obj):
                 logging.debug("New %s=%s requested, but it's already tracked.",
-- 
2.13.5




More information about the virt-tools-list mailing list