Federico Simoncelli has uploaded a new change for review.
Change subject: [WIP] Add a releaseHostId option to stop the DomainMonitorThread ......................................................................
[WIP] Add a releaseHostId option to stop the DomainMonitorThread
Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=851151 Change-Id: I83458fb4146de7e402606916615533da305bd867 Signed-off-by: Federico Simoncelli fsimonce@redhat.com --- M vdsm/storage/domainMonitor.py M vdsm/storage/sp.py 2 files changed, 19 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/81/7581/1
diff --git a/vdsm/storage/domainMonitor.py b/vdsm/storage/domainMonitor.py index 95e2f7b..c1ec5a9 100644 --- a/vdsm/storage/domainMonitor.py +++ b/vdsm/storage/domainMonitor.py @@ -84,14 +84,14 @@ # The domain should be added only after it succesfully started self._domains[sdUUID] = domainThread
- def stopMonitoring(self, sdUUID): + def stopMonitoring(self, sdUUID, releaseHostId=False): # The domain monitor issues events that might become raceful if # stopMonitoring doesn't stop until the thread exits. # Eg: when a domain is detached the domain monitor is stopped and # the host id is released. If the monitor didn't actually exit it # might respawn a new acquire host id. try: - self._domains[sdUUID].stop() + self._domains[sdUUID].stop(releaseHostId=releaseHostId) except KeyError: return
@@ -100,13 +100,15 @@ def getStatus(self, sdUUID): return self._domains[sdUUID].getStatus()
- def close(self): + def close(self, releaseHostId=False): for sdUUID in self._domains.keys(): - self.stopMonitoring(sdUUID) + self.stopMonitoring(sdUUID, releaseHostId=releaseHostId)
class DomainMonitorThread(object): log = logging.getLogger('Storage.DomainMonitorThread') + + RELEASE_HOSTID_DEFAULT = False
def __init__(self, sdUUID, hostId, interval): self.thread = Thread(target=self._monitorLoop) @@ -121,16 +123,20 @@ self.nextStatus = DomainMonitorStatus() self.isIsoDomain = None self.lastRefresh = time() + self.releaseHostId = self.RELEASE_HOSTID_DEFAULT self.refreshTime = \ config.getint("irs", "repo_stats_cache_refresh_timeout")
def start(self): self.thread.start()
- def stop(self, wait=True): + def stop(self, wait=True, releaseHostId): + self.releaseHostId = releaseHostId + self.stopEvent.set() if wait: self.thread.join() + self.domain = None
def getStatus(self): @@ -151,13 +157,17 @@
# If this is an ISO domain we didn't acquire the host id and releasing # it is superfluous. - if not self.isIsoDomain: + if not self.isIsoDomain and self.releaseHostId: try: self.domain.releaseHostId(self.hostId, unused=True) except: self.log.debug("Unable to release the host id %s for domain " "%s", self.hostId, self.sdUUID, exc_info=True)
+ # Resetting the releaseHostId value to its default just to be sure in + # case in the future we want to recycle the DomainMonitor objects. + self.releaseHostId = self.RELEASE_HOSTID_DEFAULT + def _monitorDomain(self): self.nextStatus.clear()
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 40de20a..e8c59f3 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -647,8 +647,8 @@
@unsecured - def stopMonitoringDomains(self): - self.domainMonitor.close() + def stopMonitoringDomains(self, releaseHostId=False): + self.domainMonitor.close(releaseHostId=releaseHostId) return True
@@ -675,7 +675,7 @@ if os.path.exists(self.poolPath): fileUtils.cleanupdir(self.poolPath)
- self.stopMonitoringDomains() + self.stopMonitoringDomains(releaseHostId=True) return True
-- To view, visit http://gerrit.ovirt.org/7581 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I83458fb4146de7e402606916615533da305bd867 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimonce@redhat.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: [WIP] Add a releaseHostId option to stop the DomainMonitorThread ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/752/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/7581 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I83458fb4146de7e402606916615533da305bd867 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Federico Simoncelli has posted comments on this change.
Change subject: [WIP] Add a releaseHostId option to stop the DomainMonitorThread ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(1 inline comment)
.................................................... File vdsm/storage/sp.py Line 598: finally: Line 599: self._setUnsafe() Line 600: Line 601: self._releaseTemporaryClusterLock(msdUUID) Line 602: self.stopMonitoringDomains() Testing this patch I discovered that it introduces a race around here (I didn't have time to dig more into details). Line 603: Line 604: return True Line 605: Line 606: @unsecured
-- To view, visit http://gerrit.ovirt.org/7581 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I83458fb4146de7e402606916615533da305bd867 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Yeela Kaplan ykaplan@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Itamar Heim has posted comments on this change.
Change subject: [WIP] Add a releaseHostId option to stop the DomainMonitorThread ......................................................................
Patch Set 1:
ping
Nir Soffer has posted comments on this change.
Change subject: [WIP] Add a releaseHostId option to stop the DomainMonitorThread ......................................................................
Patch Set 1:
(3 comments)
Looks ok after fixing stop parameters - but not clear why we need this. What was wrong by always releasing the host id?
.................................................... Commit Message Line 3: AuthorDate: 2012-08-29 10:28:42 -0400 Line 4: Commit: Federico Simoncelli fsimonce@redhat.com Line 5: CommitDate: 2012-08-29 10:32:45 -0400 Line 6: Line 7: [WIP] Add a releaseHostId option to stop the DomainMonitorThread Why? Line 8: Line 9: Bug-Id: https://bugzilla.redhat.com/show_bug.cgi?id=851151 Line 10: Change-Id: I83458fb4146de7e402606916615533da305bd867
.................................................... File vdsm/storage/domainMonitor.py Line 129: Line 130: def start(self): Line 131: self.thread.start() Line 132: Line 133: def stop(self, wait=True, releaseHostId): Should be:
def stop(self, wait=True, releaseHostId=RELEASE_HOSTID_DEFAULT): ... Line 134: self.releaseHostId = releaseHostId Line 135: Line 136: self.stopEvent.set() Line 137: if wait:
Line 165: "%s", self.hostId, self.sdUUID, exc_info=True) Line 166: Line 167: # Resetting the releaseHostId value to its default just to be sure in Line 168: # case in the future we want to recycle the DomainMonitor objects. Line 169: self.releaseHostId = self.RELEASE_HOSTID_DEFAULT Should use try-finally:
try: loop... finally: self.releaseHostId = self.RELEASE_HOSTID_DEFAULT Line 170: Line 171: def _monitorDomain(self): Line 172: self.nextStatus.clear() Line 173:
Itamar Heim has posted comments on this change.
Change subject: [WIP] Add a releaseHostId option to stop the DomainMonitorThread ......................................................................
Patch Set 1:
ping
Itamar Heim has abandoned this change.
Change subject: [WIP] Add a releaseHostId option to stop the DomainMonitorThread ......................................................................
Abandoned
abandoning per no reply. please restore if still relevant.
vdsm-patches@lists.fedorahosted.org