Change in vdsm[master]: [WIP] Create storage pool using command type 1
ewarszaw at redhat.com
ewarszaw at redhat.com
Sun Feb 23 13:19:08 UTC 2014
Eduardo has posted comments on this change.
Change subject: [WIP] Create storage pool using command type 1
......................................................................
Patch Set 2: Code-Review-1
(6 comments)
http://gerrit.ovirt.org/#/c/23647/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 529: # to belong to the metadata volume. Since the metadata is at
Line 530: # least SDMETADATA/METASIZE units, we know we can use the first
Line 531: # SDMETADATA bytes of the metadata volume for the SD metadata.
Line 532: # pass metadata's dev to ensure it is the first mapping
Line 533: #mapping = cls.getMetaDataMapping(vgName)
> in final version comment needs to move with code and this needs to disappea
We already know that the mapping is redundant and it can be removed.
This is a different patch.
Line 534:
Line 535: # Create the rest of the BlockSD internal volumes
Line 536: lvm.createLV(vgName, sd.LEASES, sd.LEASES_SIZE, safe=False)
Line 537: lvm.createLV(vgName, sd.IDS, sd.IDS_SIZE, safe=False)
Line 593: }
Line 594:
Line 595: initialMetadata.update(mapping)
Line 596: toAdd = encodeVgTags(initialMetadata)
Line 597: lvm.changeVGTags(vgName, delTags=(), addTags=toAdd, safe=False)
> if it is not safe/possible to call md.update then need to explain this
IMHO is better to use the unused addVGTags() here. Since this function is actually unused can be safely converted to lock_type=1 only and assert that no new consumers are added. This is probably True since for any other option than a create will use changeVGTags() since a set of tags already exists.
Line 598:
Line 599: # Mark VG with Storage Domain Tag
Line 600: try:
Line 601: lvm.replaceVGTag(vgName, STORAGE_UNREADY_DOMAIN_TAG,
Line 1318: for k, v in params.iteritems():
Line 1319: enc = poolMD.getEncoder(k)
Line 1320: newParams[k] = enc(v)
Line 1321: toAdd = encodeVgTags(newParams)
Line 1322: lvm.changeVGTags(vgName, deltags=(), addTags=toAdd, safe=False)
Use addVGTags() instead as discussed.
Line 1323:
Line 1324: def setMasterDomainParams(self, spUUID, leaseParams):
Line 1325: vgUUID = self.getInfo()['vguuid']
Line 1326: vg = lvm.getVGbyUUID(vgUUID)
Line 1330: sd.DMDK_ROLE: sd.MASTER_DOMAIN})
Line 1331: toDel = encodeVgTags({sd.DMDK_ROLE: sd.REGULAR_DOMAIN,
Line 1332: sd.DMDK_POOLS: spUUID,
Line 1333: sd.DMDK_POOLS: ''})
Line 1334: lvm.changeVGTags(vgName, delTags=toDel, addTags=toAdd, safe=False)
IMHO simpler using the replaceVGTags() change already discussed.
Line 1335:
Line 1336: def refreshDirTree(self):
Line 1337: # create domain images folder
Line 1338: imagesPath = os.path.join(self.domaindir, sd.DOMAIN_IMAGES)
http://gerrit.ovirt.org/#/c/23647/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 941: misc.validateUUID(spUUID, 'spUUID')
Line 942: if masterDom not in domList:
Line 943: raise se.InvalidParameterException("masterDom", str(masterDom))
Line 944:
Line 945: if len(domList) > 1:
==1 semantics will be more strict.
Line 946: raise NotImplementedError("Create storage pool "
Line 947: "only with master domain")
Line 948:
Line 949: if len(poolName) > MAX_POOL_DESCRIPTION_SIZE:
http://gerrit.ovirt.org/#/c/23647/2/vdsm/storage/sd.py
File vdsm/storage/sd.py:
Line 768:
Line 769: def isMaster(self):
Line 770: return self.getMetaParam(DMDK_ROLE).capitalize() == MASTER_DOMAIN
Line 771:
Line 772: @classmethod
initMasterParams() is always called as an instance method. Then this should not be a classmethod.
Since this called during pool creation/reconstruction all this redundant calls, including the spbackend mess can be avoided IMHO.
Line 773: def initMasterParams(cls, poolMD, params):
Line 774: poolMD.update(params)
Line 775:
Line 776: def setMasterDomainParams(self, spUUID, leaseParams):
--
To view, visit http://gerrit.ovirt.org/23647
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia64f6dd2df38d2968f03ce66094f3ba7b4343503
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list