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

Eduardo has posted comments on this change.

Change subject: [WIP] Create storage domain using command type 1
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.ovirt.org/#/c/23646/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 516:             raise se.StorageDomainIsMadeFromTooManyPVs()
Line 517: 
Line 518:         # Create metadata service volume
Line 519:         metasize = cls.metaSize(vgName)
Line 520:         lvm.createLV(vgName, sd.METADATA, "%s" % (metasize),
safe=False)
Look at the lvm code comment.
Line 521:         # Create the mapping right now so the index 0 is guaranteed
Line 522:         # to belong to the metadata volume. Since the metadata is at
Line 523:         # least SDMETADATA/METASIZE units, we know we can use the first
Line 524:         # SDMETADATA bytes of the metadata volume for the SD metadata.


http://gerrit.ovirt.org/#/c/23646/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 654:     """ This fuction is so restrictive as we can
do."""
Line 655:     log.debug("### _current_lvmconf %s",
globals()["_current_lvmconf"])
Line 656:     globals()["_current_lvmconf"] =
_current_lvmconf.replace("locking_type=4",
Line 657:                                                             
"locking_type=1")
Line 658:     log.debug("### _current_lvmconf %s",
globals()["_current_lvmconf"])
These logs should be updated before production.
Line 659: 
Line 660: 
Line 661: def bootstrap(refreshlvs=()):
Line 662:     """


Line 1061: #
Line 1062: 
Line 1063: 
Line 1064: def createLV(vgName, lvName, size, activate=True, contiguous=False,
Line 1065:              initialTag=None, safe=True):
createLV operates on an existing VG making "safe" parameters misuse very
dangerous at this time.

If the intention is to use this parameter only when the SD is created may be an
alternative method should be found lioke spliting the function for createLV() for images
volumes and createSpecialVolumes which are created only at SD creation and never modified,
making this parameter redundant.
Line 1066:     """
Line 1067:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1068:     """
Line 1069:     # WARNING! From man vgs:


Line 1280:     """
Line 1281:     return os.path.exists(lvPath(vgName, lvName))
Line 1282: 
Line 1283: 
Line 1284: def changeVGTags(vgName, delTags=(), addTags=(), safe=True):
Again, this change is dangerous and very misleading.

IMHO separating the only valid case for a non-SPM doing this operation (create) will be
more in line with the purpose of this series.

The new function should be more simple than the actual changeVGTags since it operates in a
known set of tags.

Please note that actually createSD() is the only user of replaceVGtags . Therefore you can
change it there and the split is natural and you need not to change changeVGTags()
function.
Line 1285:     delTags = set(delTags)
Line 1286:     addTags = set(addTags)
Line 1287:     if delTags.intersection(addTags):
Line 1288:         raise se.VolumeGroupReplaceTagError(


Line 1321:     if rc != 0:
Line 1322:         raise se.VolumeGroupRemoveTagError(vgName)
Line 1323: 
Line 1324: 
Line 1325: def replaceVGTag(vg, oldTag, newTag, safe=True):
Please see above.
Line 1326:     changeVGTags(vg, [oldTag], [newTag], safe)
Line 1327: 
Line 1328: 
Line 1329: def addVGTags(vgName, tags):


-- 
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes