Ayal Baron has posted comments on this change.

Change subject: sp: split metadata transaction in createMaster
......................................................................


Patch Set 2:

(3 comments)

....................................................
Commit Message
Line 21: The two side effects of this change are:
Line 22: - we always initialize the pool metadata even when the domain metadata
Line 23:   transaction fails
Line 24: - the createMaster operation takes now two storage operations instead
Line 25:   of one
are you sure it did not actually reduce the number of ops?
i.e. "with futurePoolMD.transaction():" really prevents domain.attach and the 
like to commit until transaction is finished?
(if it doesn't perform it in one operation today then the first side effect 
would not be correct either)
Line 26: 
Line 27: Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078


....................................................
File vdsm/storage/sd.py
Line 771:         pools = self.getPools()
Line 772: 
Line 773:         if len(pools) > 0:
Line 774:             self.log.info('initializing master domain %s detaching 
pools %s',
Line 775:                           self.sdUUID, ', '.join((str(x) for x in 
pools)))
why is this ok?
i.e. why is the check here not:
numPools = len(pools)
if numPools > 1 or (numPools == 1 and pools[0] != spUUID):
    raise ...
Line 776: 
Line 777:         with self._metadata.transaction():
Line 778:             self.changeLeaseParams(leaseParams)
Line 779:             self.setMetaParam(DMDK_POOLS, [spUUID])


....................................................
File vdsm/storage/sp.py
Line 722: 
Line 723:         if not misc.isAscii(poolName) and not 
domain.supportsUnicode():
Line 724:             raise se.UnicodeArgumentException()
Line 725: 
Line 726:         self.initMasterDomain(domain, poolName, masterVersion)
why not keep the same order of params?
Line 727:         domain.initMaster(self.spUUID, leaseParams)
Line 728: 
Line 729:     @unsecured
Line 730:     def reconstructMaster(self, hostId, poolName, msdUUID, domDict,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b22af92b1f9a481be9af844b1acc47ae513d078
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to