Eduardo has posted comments on this change.
Change subject: Create storage domain using command type 1
......................................................................
Patch Set 7: Code-Review-1
(6 comments)
http://gerrit.ovirt.org/#/c/23646/7/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 132: return LVM_ENC_ESCAPE.sub(lambda c: unichr(int(c.groups()[0])), s)
Line 133:
Line 134:
Line 135: def encodeVgTags(tagsDict):
Line 136: tags = [VGTagMetadataRW.METADATA_TAG_PREFIX +
More efficient will be to build the tuple directly.
Line 137: lvmTagEncode("%s=%s" % (k, v))
Line 138: for k, v in tagsDict.items()]
Line 139: return tuple(tags)
Line 140:
Line 570: # no one reads it from here anyway
Line 571: initialMetadata = {
Line 572: sd.DMDK_VERSION: version,
Line 573: sd.DMDK_SDUUID: sdUUID,
Line 574: sd.DMDK_TYPE: BLOCK_SD_MD_FIELDS[sd.DMDK_TYPE][1](storageType),
Add a comment please explaining the use of the MD encoding please.
Line 575: sd.DMDK_CLASS: BLOCK_SD_MD_FIELDS[sd.DMDK_CLASS][1](domClass),
Line 576: sd.DMDK_DESCRIPTION: domainName,
Line 577: sd.DMDK_ROLE: sd.REGULAR_DOMAIN,
Line 578: sd.DMDK_POOLS: BLOCK_SD_MD_FIELDS[sd.DMDK_POOLS][1]([]),
Line 599:
Line 600: # Mark VG with Storage Domain Tag
Line 601: try:
Line 602: lvm.replaceVGTag(vgName, STORAGE_UNREADY_DOMAIN_TAG,
Line 603: STORAGE_DOMAIN_TAG, safe=False)
safe=False? IMHO it is not required anymore (previous version). Unify all this tag changes
please.
Line 604: except se.StorageException:
Line 605: raise se.VolumeGroupUninitialized(vgName)
Line 606:
Line 607: bsd = BlockStorageDomain(sdUUID)
http://gerrit.ovirt.org/#/c/23646/7/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:
Line 1052: # Public Logical volume interface
Line 1053: #
Line 1054:
Line 1055:
Line 1056: def _completeCreateLV(rc, vgName, lvName, activate=True):
I really dislike the "complete" part of the name.
Line 1057: if rc == 0:
Line 1058: _lvminfo._invalidatevgs(vgName)
Line 1059: _lvminfo._invalidatelvs(vgName, lvName)
Line 1060: else:
Line 1090: rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )), rw=True)
Line 1091: _completeCreateLV(rc, vgName, lvName)
Line 1092:
Line 1093:
Line 1094: def createLV(vgName, lvName, size, activate=True, contiguous=False,
The contiguous flag can be dropped, unifiying the cmd part of the 3 .*createLV functions
in the internal part.
Line 1095: initialTag=None):
Line 1096: """
Line 1097: Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1098: """
Line 1091: _completeCreateLV(rc, vgName, lvName)
Line 1092:
Line 1093:
Line 1094: def createLV(vgName, lvName, size, activate=True, contiguous=False,
Line 1095: initialTag=None):
For regular LVs, not special, image related ones the initialTag is mandatory, simplifiying
the code.
Line 1096: """
Line 1097: Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1098: """
Line 1099: # WARNING! From man vgs:
--
To view, visit
http://gerrit.ovirt.org/23646
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes