Change in vdsm[master]: sp: improve domainStateChange event handling

nsoffer at redhat.com nsoffer at redhat.com
Wed Jan 6 08:25:15 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: sp: improve domainStateChange event handling
......................................................................


Patch Set 3:

(4 comments)

https://gerrit.ovirt.org/#/c/51393/3//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: onDomainStateChange is registered in the constructor of StoragePool.
Line 10: This is a waste of resources since these events can be called only after
Line 11: the connect method is called (specifically, after __rebuild is called
Line 12: from within connect).
> The main issue is that it is never unregistered, so each time you connect t
The main issue (leaking registration) is non-issue, multiple registration replace the same registration and does not leak anything.
Line 13: 
Line 14: This patch moves the registration to onDomainStateChange to be right
Line 15: before it can actually be called (in connect), and calls unregister if
Line 16: an error has occurred in __rebuild.


https://gerrit.ovirt.org/#/c/51393/3/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 635
Line 636
Line 637
Line 638
Line 639
> This should be reverted if next steps fail, and raise if pool is already co
We can wait with this for later patch, connect is called only if pool is not connected, under exclusive pool lock. Raising if pool is connect is good idea anyway but not required for this patch.


Line 641
Line 642
Line 643
Line 644
Line 645
If this raises, the pool is not considered connected, and we should stop watching domain state changes.

So the try expect should extended to contain also __createMailboxMonitor.


Line 649:         self.id = hostID
Line 650:         # Make sure SDCache doesn't have stale data (it can be in case of FC)
Line 651:         sdCache.invalidateStorage()
Line 652:         sdCache.refresh()
Line 653:         self._startWatchingDomainsState()
> what will happen if we are conencted to the pool and issue connect call aga
I checked misc.Event - multiple registrations with same function replace the function in the dict and are safe.

We always register the same function - self._domainStageChangeCallback.
Line 654:         try:
Line 655:             # Rebuild whole Pool
Line 656:             self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion)
Line 657:         except Exception:


-- 
To view, visit https://gerrit.ovirt.org/51393
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list