Change in vdsm[master]: Create storage domain using command type 1

ewarszaw at redhat.com ewarszaw at redhat.com
Thu Mar 27 14:58:58 UTC 2014


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 at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list