Yeela Kaplan has posted comments on this change. Change subject: Create storage domain using command type 1 ......................................................................
Patch Set 7: (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. Done 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. Done 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 t Done 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. Done 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 .*creat Why is that? 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 mandator Done 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 <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: [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
