Change in vdsm[master]: Automatic reconnection to the Storage Pool

abaron at redhat.com abaron at redhat.com
Wed Oct 17 07:22:06 UTC 2012


Ayal Baron has posted comments on this change.

Change subject: Automatic reconnection to the Storage Pool
......................................................................


Patch Set 2: Do not submit

(5 inline comments)

After considering this some more, I think the solution should just be to get rid of the entire reconnect code (which is currently dead anyway and has been for quite a while so no real backwards compatibility issues here).

Going forward we'll get rid of the master domain concept and then we won't have all this complexity and reconnecting to domains will be simple.

....................................................
File vdsm/storage/hsm.py
Line 319:             dirList = os.listdir(_poolsTmpDir)
Line 320:             for spUUID in dirList:
Line 321:                 poolPath = os.path.join(self.storage_repository, spUUID)
Line 322:                 try:
Line 323:                     self._connectStoragePool(spUUID, None, None, None, None)
+1 to Edu's comment
Line 324:                     # TODO: once we support simultaneous connection to multiple
Line 325:                     #       pools, remove following line (break)
Line 326:                     break
Line 327:                 except Exception:


Line 321:                 poolPath = os.path.join(self.storage_repository, spUUID)
Line 322:                 try:
Line 323:                     self._connectStoragePool(spUUID, None, None, None, None)
Line 324:                     # TODO: once we support simultaneous connection to multiple
Line 325:                     #       pools, remove following line (break)
since this will never happen you can just remove the comment.
Line 326:                     break
Line 327:                 except Exception:
Line 328:                     self.log.error("Unexpected error", exc_info=True)
Line 329: 


Line 829: 
Line 830:         try:
Line 831:             self.getPool(spUUID)
Line 832:         except se.StoragePoolUnknown:
Line 833:             pass # Pool not connected yet
this try..except..else..return clause forces a funky structure where your exception handling code is actually done after it's finished.
Either move everything after the clause into its own method so that instead of pass you'd have a method call or get rid of this clause.
Right now it's downright confusing.
Line 834:         else:
Line 835:             with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.shared):
Line 836:                 pool = self.getPool(spUUID)
Line 837:                 if not msdUUID or not masterVersion:


Line 848:         with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive):
Line 849:             try:
Line 850:                 pool = self.getPool(spUUID)
Line 851:             except se.StoragePoolUnknown:
Line 852:                 pass # Pool not connected yet
same here.
Line 853:             else:
Line 854:                 if not msdUUID or not masterVersion:
Line 855:                     hostID, scsiKey, \
Line 856:                     msdUUID, masterVersion = pool.getStoredPoolInformation()


Line 850:                 pool = self.getPool(spUUID)
Line 851:             except se.StoragePoolUnknown:
Line 852:                 pass # Pool not connected yet
Line 853:             else:
Line 854:                 if not msdUUID or not masterVersion:
this should indeed be split
Line 855:                     hostID, scsiKey, \
Line 856:                     msdUUID, masterVersion = pool.getStoredPoolInformation()
Line 857: 
Line 858:                 misc.validateN(hostID, 'hostID')


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I682a12deff90dd78bb31b7368c003796ed241cd9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list