Change in vdsm[master]: Revert "BZ#842631 Use domain proxies instead of actual domai...

[Federico Simoncelli]

Federico Simoncelli has uploaded a new change for review.

Change subject: Revert "BZ#842631 Use domain proxies instead of actual domain
references"
......................................................................

Revert "BZ#842631 Use domain proxies instead of actual domain references"

This reverts commit 942c2dc8d317b6180529ed7df4eee98fbf1b9836.

Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Change-Id: Id4b1e0600948ab24e144df94f8d02a101a87627a
---
M vdsm/storage/sdc.py
1 file changed, 11 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/7293/1

diff --git a/vdsm/storage/sdc.py b/vdsm/storage/sdc.py
index 2e8a6ff..9a7d09d 100644
--- a/vdsm/storage/sdc.py
+++ b/vdsm/storage/sdc.py
@@ -36,21 +36,6 @@
 DEFAULT_REFRESH_INTERVAL = 300
 
 
-class DomainProxy(object):
-    """Keeps domain references valid even when underlying domain object
changes
-    (due to format conversion for example"""
-    def __init__(self, cache, sdUUID):
-        self._sdUUID = sdUUID
-        self._cache = cache
-
-    def __getattr__(self, attrName):
-        dom = self.getRealDomain()
-        return getattr(dom, attrName)
-
-    def getRealDomain(self):
-        return self._cache._realProduce(self._sdUUID)
-
-
 class StorageDomainCache:
     """
     Storage Domain List keeps track of all the storage domains accessible by
@@ -59,9 +44,9 @@
     log = logging.getLogger('Storage.StorageDomainCache')
 
     def __init__(self, storage_repo):
-        self._syncroot = threading.RLock()
-        self.__proxyCache = {}
-        self.__domainCache = {}
+        self._syncroot = threading.Lock()
+        self.__cache = {}
+        self.__weakCache = {}
         self.storage_repo = storage_repo
         self.storageStale = True
 
@@ -79,14 +64,14 @@
         if self.storageStale == True:
             return None
         try:
-            return self.__proxyCache[sdUUID]()
+            return self.__weakCache[sdUUID]()
         except KeyError:
             return None
 
     def _cleanStaleWeakrefs(self):
-        for sdUUID, ref in self.__proxyCache.items():
+        for sdUUID, ref in self.__weakCache.items():
             if ref() is None:
-                del self.__proxyCache[sdUUID]
+                del self.__weakCache[sdUUID]
 
     def produce(self, sdUUID):
         dom = self._getDomainFromCache(sdUUID)
@@ -103,25 +88,11 @@
 
             self._cleanStaleWeakrefs()
 
-            dom = DomainProxy(self, sdUUID)
-            # This is needed to preserve the semantic where if the domain was
-            # absent from the cache and the domain cannot be found the
-            # operation would fail
-            dom.getRealDomain()
-            self.__proxyCache[sdUUID] = weakref.ref(dom)
-            return dom
-
-    def _realProduce(self, sdUUID):
-        with self._syncroot:
-            try:
-                return self.__domainCache[sdUUID]
-            except KeyError:
-                pass
-
             # _findDomain will raise StorageDomainDoesNotExist if sdUUID is not
             # found in storage.
             dom = self._findDomain(sdUUID)
-            self.__domainCache[sdUUID] = dom
+            self.__cache[sdUUID] = dom
+            self.__weakCache[sdUUID] = weakref.ref(dom)
             return dom
 
     def _findDomain(self, sdUUID):
@@ -158,15 +129,15 @@
 
     def refresh(self):
         self.invalidateStorage()
-        self.__domainCache.clear()
+        self.__cache.clear()
 
     def manuallyAddDomain(self, dom):
         with self._syncroot:
-            self.__domainCache[dom.sdUUID] = dom
+            self.__cache[dom.sdUUID] = dom
 
     def manuallyRemoveDomain(self, sdUUID):
         with self._syncroot:
-            del self.__domainCache[sdUUID]
+            del self.__cache[sdUUID]
 
 
 storage_repository = config.get('irs', 'repository')


--
To view, visit http://gerrit.ovirt.org/7293
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4b1e0600948ab24e144df94f8d02a101a87627a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com&gt;

Ayal Baron has posted comments on this change.

Change subject: Revert "BZ#842631 Use domain proxies instead of actual domain
references"
......................................................................


Patch Set 1: I would prefer that you didn't submit this

> From reviewing the following patch it looks like reverting is
> insufficient as you need to remove a call to getRealDomain in sp for this change not to
> break things. 
--
To view, visit http://gerrit.ovirt.org/7293
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4b1e0600948ab24e144df94f8d02a101a87627a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Revert "BZ#842631 Use domain proxies instead of actual domain
references"
......................................................................


Patch Set 1: (1 inline comment)

Don't get me wrong, I'm all for this patch, but you need to explain the
incentive...

....................................................
Commit Message
Line 3: AuthorDate: 2012-08-17 05:32:18 -0400
Line 4: Commit:     Federico Simoncelli <fsimonce(a)redhat.com&gt;
Line 5: CommitDate: 2012-08-17 10:48:42 -0400
Line 6: 
Line 7: Revert "BZ#842631 Use domain proxies instead of actual domain
references"
why?
Line 8: 
Line 9: This reverts commit 942c2dc8d317b6180529ed7df4eee98fbf1b9836.
Line 10: 
Line 11: Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com&gt;


--
To view, visit http://gerrit.ovirt.org/7293
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4b1e0600948ab24e144df94f8d02a101a87627a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Revert "BZ#842631 Use domain proxies instead of actual domain
references"
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/494/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/7293
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4b1e0600948ab24e144df94f8d02a101a87627a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has abandoned this change.

Change subject: Revert "BZ#842631 Use domain proxies instead of actual domain
references"
......................................................................


Patch Set 1: Abandoned

--
To view, visit http://gerrit.ovirt.org/7293
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Id4b1e0600948ab24e144df94f8d02a101a87627a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server