Eduardo Warszawski has posted comments on this change.
Change subject: BZ#750528 - pool refresh should not change metadatata. ......................................................................
Patch Set 2: (3 inline comments)
May be separated from the 1st one but why. The 2nd proevents unnecesary produce's.
.................................................... File vdsm/storage/sp.py Line 343: self.masterDomain.mountMaster() Line 344: self.masterDomain.createMasterTree(log=True) Line 345: self.tasksDir = os.path.join(self.poolPath, POOL_MASTER_DOMAIN, sd.MASTER_FS_DIR, sd.TASKS_DIR) Line 346: Line 347: #Domain private care I put this n a separate function and received comments why this function makes trees for backup domains and reset role and version for data domains. It can be split in three separate functions (produce should be common) but seems too much. Simply define wich criteria we use, inline or not since this too long function is mostly inline, as in the case of msd or mailboxes.
checkBackupDomain() was a very infortunate name since not checked anything and instead build trees. Line 348: domUUIDs = self.getDomains(activeOnly=True).keys() Line 349: for sdUUID in domUUIDs: Line 350: if sdUUID == self.masterDomain.sdUUID: Line 351: continue
Line 352: dom = sdCache.produce(sdUUID) Line 353: domMD = dom.getMetadata() Line 354: if domMD[sd.DMDK_CLASS] == sd.BACKUP_DOMAIN: Line 355: dom.mountMaster() Line 356: # Master tree should be exist in this point In the original. Line 357: # Recreate it if not. Line 358: dom.createMasterTree() Line 359: #Stale master domain? Line 360: elif domMD[sd.DMDK_CLASS] == sd.DATA_DOMAIN \
Line 1006: dom.acquireClusterLock(self.id) Line 1007: try: Line 1008: domMD = dom.getMetadata() Line 1009: #If you remove this condition, remove it from public_createStoragePool too. Line 1010: if domMD[sd.DMDK_CLASS] == sd.DATA_DOMAIN and domMD[sd.DMDK_VERSION] != self.masterDomain.getVersion(): isData(), getVersion(), getRole() all of them access the metadata independently using getMetadata() and suffering caching issues. The metadata should be accessed once for the sake of consistency and security. Eventual optimizations are benefitial side effects. Line 1011: raise se.MixedSDVersionError(dom.sdUUID, domMD[sd.DMDK_VERSION], self.masterDomain.sdUUID, self.masterDomain.getVersion()) Line 1012: Line 1013: dom.attach(self.spUUID) Line 1014: if domMD[sd.DMDK_CLASS] == sd.DATA_DOMAIN and domMD[sd.DMDK_ROLE] == sd.MASTER_DOMAIN:
-- To view, visit http://gerrit.usersys/1082 To unsubscribe, visit http://gerrit.usersys/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I12e3e700ff67a527c367533bf9f5654e8760a118 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo Warszawski ewarszaw@redhat.com Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo Warszawski ewarszaw@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com
vdsm-patches@lists.fedorahosted.org