Change in vdsm[master]: DomainMonitor should use use real domains (no proxy)

[Federico Simoncelli]

Federico Simoncelli has uploaded a new change for review.

Change subject: DomainMonitor should use use real domains (no proxy)
......................................................................

DomainMonitor should use use real domains (no proxy)

In this patch:
* produce the domain when refreshing
* remove the remaining getRealDomain (DomainProxy) calls
* fix manuallyRemoveDomain for non-existent domains

Signed-off-by: Federico Simoncelli 
Change-Id: Ibbf67fc050658e3418aa666e8fcef1e1244571e9
---
M vdsm/storage/domainMonitor.py
M vdsm/storage/sdc.py
M vdsm/storage/sp.py
3 files changed, 32 insertions(+), 26 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/7294/1

diff --git a/vdsm/storage/domainMonitor.py b/vdsm/storage/domainMonitor.py
index 80b6ed3..0272cb1 100644
--- a/vdsm/storage/domainMonitor.py
+++ b/vdsm/storage/domainMonitor.py
@@ -75,13 +75,14 @@
     def monitoredDomains(self):
         return self._domains.keys()

-    def startMonitoring(self, domain, hostId):
-        if domain.sdUUID in self._domains:
+    def startMonitoring(self, sdUUID, hostId):
+        if sdUUID in self._domains:
             return

-        self._domains[domain.sdUUID] = DomainMonitorThread(
-                                        domain, hostId, self._interval)
-        self._domains[domain.sdUUID].start()
+        domainThread = DomainMonitorThread(sdUUID, hostId, self._interval)
+        domainThread.start()
+        # The domain should be added only after it succesfully started
+        self._domains[sdUUID] = domainThread

def stopMonitoring(self, sdUUID):
         # The domain monitor issues events that might become raceful if
@@ -107,22 +108,25 @@
 class DomainMonitorThread(object):
     log = logging.getLogger('Storage.DomainMonitorThread')

-    def __init__(self, domain, hostId, interval):
+    def __init__(self, sdUUID, hostId, interval):
         self.thread = Thread(target=self._monitorLoop)
         self.thread.setDaemon(True)

self.stopEvent = Event()
-        self.domain = domain
+        self.domain = None
+        self.sdUUID = sdUUID
         self.hostId = hostId
         self.interval = interval
         self.status = DomainMonitorStatus()
         self.nextStatus = DomainMonitorStatus()
-        self.isIsoDomain = domain.isISO()
+        self.isIsoDomain = None
         self.lastRefresh = time()
         self.refreshTime = \
                     config.getint("irs", "repo_stats_cache_refresh_timeout")

def start(self):
+        self.domain = sdCache.produce(self.sdUUID)
+        self.isIsoDomain = self.domain.isISO()
         self.thread.start()

def stop(self, wait=True):
@@ -134,17 +138,17 @@
         return self.status.copy()

def _monitorLoop(self):
-        self.log.debug("Starting domain monitor for %s", self.domain.sdUUID)
+        self.log.debug("Starting domain monitor for %s", self.sdUUID)

while not self.stopEvent.is_set():
             try:
                 self._monitorDomain()
             except:
                 self.log.error("The domain monitor for %s failed unexpectedly",
-                               self.domain.sdUUID, exc_info=True)
+                               self.sdUUID, exc_info=True)
             self.stopEvent.wait(self.interval)

-        self.log.debug("Stopping domain monitor for %s", self.domain.sdUUID)
+        self.log.debug("Stopping domain monitor for %s", self.sdUUID)

# If this is an ISO domain we didn't acquire the host id and releasing
         # it is superfluous.
@@ -153,16 +157,17 @@
                 self.domain.releaseHostId(self.hostId, unused=True)
             except:
                 self.log.debug("Unable to release the host id %s for domain "
-                       "%s", self.hostId, self.domain.sdUUID, exc_info=True)
+                               "%s", self.hostId, self.sdUUID, exc_info=True)

def _monitorDomain(self):
         self.nextStatus.clear()

-        # Refreshing the domain object in order to pick up changes as,
-        # for example, the domain upgrade.
         if time() - self.lastRefresh > self.refreshTime:
-            self.log.debug("Refreshing domain %s", self.domain.sdUUID)
-            sdCache.manuallyRemoveDomain(self.domain.sdUUID)
+            # Refreshing the domain object in order to pick up changes as,
+            # for example, the domain upgrade.
+            self.log.debug("Refreshing domain %s", self.sdUUID)
+            sdCache.manuallyRemoveDomain(self.sdUUID)
+            self.domain = sdCache.produce(self.sdUUID)
             self.lastRefresh = time()

try:
@@ -188,20 +193,19 @@

except Exception, e:
             self.log.error("Error while collecting domain %s monitoring "
-                           "information", self.domain.sdUUID, exc_info=True)
+                           "information", self.sdUUID, exc_info=True)
             self.nextStatus.error = e

self.nextStatus.lastCheck = time()
         self.nextStatus.valid = (self.nextStatus.error is None)

if self.status.valid != self.nextStatus.valid:
-            self.log.debug("Domain %s changed its status to %s",
-                           self.domain.sdUUID,
+            self.log.debug("Domain %s changed its status to %s", self.sdUUID,
                            "Valid" if self.nextStatus.valid else "Invalid")

try:
                 self.onDomainConnectivityStateChange.emit(
-                                    self.domain.sdUUID, self.nextStatus.valid)
+                                        self.sdUUID, self.nextStatus.valid)
             except:
                 self.log.warn("Could not emit domain state change event",
                               exc_info=True)
@@ -213,7 +217,7 @@
                 self.domain.acquireHostId(self.hostId, async=True)
             except:
                 self.log.debug("Unable to issue the acquire host id %s "
-                    "request for domain %s", self.hostId, self.domain.sdUUID,
+                    "request for domain %s", self.hostId, self.sdUUID,
                     exc_info=True)

self.status.update(self.nextStatus)
diff --git a/vdsm/storage/sdc.py b/vdsm/storage/sdc.py
index 9a7d09d..eee8d73 100644
--- a/vdsm/storage/sdc.py
+++ b/vdsm/storage/sdc.py
@@ -137,7 +137,10 @@

def manuallyRemoveDomain(self, sdUUID):
         with self._syncroot:
-            del self.__cache[sdUUID]
+            try:
+                del self.__cache[sdUUID]
+            except KeyError:
+                pass

storage_repository = config.get('irs', 'repository')
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index ed2b893..da403d9 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -1022,8 +1022,8 @@
         if targetFormat is None:
             targetFormat = self.getFormat()

-        self._formatConverter.convert(repoPath, self.id, domain.getRealDomain(),
-                                      isMsd, targetFormat)
+        self._formatConverter.convert(repoPath, self.id, domain, isMsd,
+                                      targetFormat)
         sdCache.manuallyRemoveDomain(sdUUID)

@unsecured
@@ -1557,8 +1557,7 @@
         for sdUUID in activeDomains:
             if sdUUID not in monitoredDomains:
                 try:
-                    self.domainMonitor \
-                            .startMonitoring(sdCache.produce(sdUUID), self.id)
+                    self.domainMonitor.startMonitoring(sdUUID, self.id)
                     self.log.debug("Storage Pool `%s` started monitoring "
                                    "domain `%s`", self.spUUID, sdUUID)
                 except se.StorageException:

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibbf67fc050658e3418aa666e8fcef1e1244571e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: DomainMonitor should use use real domains (no proxy)
......................................................................

Patch Set 1:

Build Successful

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbf67fc050658e3418aa666e8fcef1e1244571e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: DomainMonitor should use use real domains (no proxy)
......................................................................

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

(1 inline comment)

What's the difference between this patch and Eduardo's patch which moved the produce into domain monitor?

....................................................
File vdsm/storage/sp.py
Line 1021: 
Line 1022:         if targetFormat is None:
Line 1023:             targetFormat = self.getFormat()
Line 1024: 
Line 1025:         self._formatConverter.convert(repoPath, self.id, domain, isMsd,
wasn't getRealDomain method removed in previous patch? if so then previous patch is not self contained and breaks things? i.e. this fix should have been there as well, no?
Line 1026:                                       targetFormat)
Line 1027:         sdCache.manuallyRemoveDomain(sdUUID)
Line 1028: 
Line 1029:     @unsecured

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbf67fc050658e3418aa666e8fcef1e1244571e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: DomainMonitor should use use real domains (no proxy)
......................................................................

Patch Set 1: (1 inline comment)

....................................................
Commit Message
Line 3: AuthorDate: 2012-08-17 06:55:54 -0400
Line 4: Commit:     Federico Simoncelli 
Line 5: CommitDate: 2012-08-17 10:48:55 -0400
Line 6: 
Line 7: DomainMonitor should use use real domains (no proxy)
s/use use/use/

Why? (again, I'm all for this, but need to explain)
Line 8: 
Line 9: In this patch:
Line 10: * produce the domain when refreshing
Line 11: * remove the remaining getRealDomain (DomainProxy) calls

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbf67fc050658e3418aa666e8fcef1e1244571e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has abandoned this change.

Change subject: DomainMonitor should use use real domains (no proxy)
......................................................................

Patch Set 1: Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ibbf67fc050658e3418aa666e8fcef1e1244571e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: oVirt Jenkins CI Server