Change in vdsm[ovirt-3.6]: sp: update domain links on state change
ishaby at redhat.com
ishaby at redhat.com
Sun Jan 31 14:26:55 UTC 2016
Hello Nir Soffer,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/52902
to review the following change.
Change subject: sp: update domain links on state change
......................................................................
sp: update domain links on state change
In some situations (e.g. after a reconstructMaster command) the domain
links may be missing and it's impossible to recreate them on
connectStoragePool or refreshStoragePool since the storage may be
unreachable. The best we can do is recreate them as soon as the domains
are visible again.
This patch is based on a patch that was merged into ovirt-engine-3.4
but was never merged into master: https://gerrit.ovirt.org/#/c/27466/ .
It also deals with a few issues that patch had:
- It adds the "@unsecured" decoration to _domainStateChange, since
without it we get a SecureError in case that the host that runs it is
not the SPM.
- In the other patch, onDomainStateChange was registered in the
constructor of StoragePool.
This is a waste of resources since these events can be called only
after the connect method is called (specifically, after __rebuild is
called from within connect).
Also, onDomainStateChange is never unregistered, causing a leak of a
registration for every pool disconnection or unsuccessful connection.
This patch moves the registration to onDomainStateChange to be right
before it can actually be called (in connect), and calls unregister if
an error has occurred in __rebuild.
A call to unregister was also added to the disconnect method, so that
we won't be listening to events that cannot be called anymore.
- This patch locks the pool and the domain id's in _domainStateChange:
On the pool, a shared lock is taken since we query the pool's state.
On the domain, we take a new <domain_uuid>_repo exclusive lock to
prevent concurrent calls to_refreshDomainLinks.
Relates-To: https://bugzilla.redhat.com/show_bug.cgi?id=1091030
Bug-Url: https://bugzilla.redhat.com/1271771
Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee
Signed-off-by: Idan Shaby <ishaby at redhat.com>
Signed-off-by: Federico Simoncelli <fsimonce at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/51393
Reviewed-by: Nir Soffer <nsoffer at redhat.com>
Continuous-Integration: Jenkins CI
---
M vdsm/storage/sp.py
1 file changed, 47 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/52902/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 9318bf9..f5e69e8 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -89,6 +89,8 @@
self.domainMonitor = domainMonitor
self._upgradeCallback = partial(StoragePool._upgradePoolDomain,
proxy(self))
+ self._domainStateCallback = partial(
+ StoragePool._domainStateChange, proxy(self))
self._backend = None
def __is_secure__(self):
@@ -131,6 +133,24 @@
@unsecured
def getBackend(self):
return self._backend
+
+ @unsecured
+ def _domainStateChange(self, sdUUID, isValid):
+ if not isValid:
+ return
+
+ domain = sdCache.produce(sdUUID)
+ with rmanager.acquireResource(STORAGE, self.spUUID,
+ rm.LockType.shared):
+ if sdUUID not in self.getDomains(activeOnly=True):
+ self.log.debug("Domain %s is not an active pool domain, "
+ "skipping domain links refresh",
+ sdUUID)
+ return
+ with rmanager.acquireResource(STORAGE, sdUUID + "_repo",
+ rm.LockType.exclusive):
+ self.log.debug("Refreshing domain links for %s", sdUUID)
+ self._refreshDomainLinks(domain)
def _upgradePoolDomain(self, sdUUID, isValid):
# This method is called everytime the onDomainStateChange
@@ -628,11 +648,34 @@
# Make sure SDCache doesn't have stale data (it can be in case of FC)
sdCache.invalidateStorage()
sdCache.refresh()
- # Rebuild whole Pool
- self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
- self.__createMailboxMonitor()
+ # Since we start the monitor threads during the call to __rebuild,
+ # we should start watching the domains states right before we call
+ # __rebuild.
+ self._startWatchingDomainsState()
+ try:
+ # Rebuild whole Pool
+ self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
+ self.__createMailboxMonitor()
+ except Exception:
+ self._stopWatchingDomainsState()
+ raise
return True
+
+ @unsecured
+ def _startWatchingDomainsState(self):
+ self.log.debug("Start watching domains state")
+ self.domainMonitor.onDomainStateChange.register(
+ self._domainStateCallback)
+
+ @unsecured
+ def _stopWatchingDomainsState(self):
+ self.log.debug("Stop watching domains state")
+ try:
+ self.domainMonitor.onDomainStateChange.unregister(
+ self._domainStateCallback)
+ except KeyError:
+ self.log.warning("Domain state callback is not registered")
@unsecured
def stopMonitoringDomains(self):
@@ -660,6 +703,7 @@
fileUtils.cleanupdir(self.poolPath)
self.stopMonitoringDomains()
+ self._stopWatchingDomainsState()
return True
@unsecured
--
To view, visit https://gerrit.ovirt.org/52902
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
More information about the vdsm-patches
mailing list