Change in vdsm[master]: sd: unify validateCreateVolumeParams

nsoffer at redhat.com nsoffer at redhat.com
Thu Jun 5 19:13:18 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: sd: unify validateCreateVolumeParams
......................................................................


Patch Set 4:

(2 comments)

Looks ok except the line break.

http://gerrit.ovirt.org/#/c/28128/4/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 630:     getVAllocSize = getVSize
Line 631: 
Line 632:     def validateCreateVolumeParams(self, volFormat, preallocate, srcVolUUID):
Line 633:         super(BlockStorageDomain, self) \
Line 634:             .validateCreateVolumeParams(volFormat, preallocate, srcVolUUID)
This is style is hard to read, please consider this:

    super(BlockStorageDomain, self).validateCreateVolumeParams(
        volFormat, preallocate, srcVolUUID)
Line 635:         # Sparse-Raw not supported for block volumes
Line 636:         if preallocate == volume.SPARSE_VOL and volFormat == volume.RAW_FORMAT:
Line 637:             raise se.IncorrectFormat(volume.type2name(volFormat))
Line 638: 


http://gerrit.ovirt.org/#/c/28128/4/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 415:             raise se.IncorrectType(volume.type2name(preallocate))
Line 416: 
Line 417:         # Volumes with a parent must be cow
Line 418:         if srcVolUUID != volume.BLANK_UUID and volFormat != volume.COW_FORMAT:
Line 419:             raise se.IncorrectFormat(volume.type2name(volFormat))
All these checks belong to the volume module, nothing here is related to StorageDomain class. But it seems useful to ask the domain about that, and overriding BlockStorageDomain version is also useful to add block specific validation.
Line 420: 
Line 421:     def createVolume(self, imgUUID, size, volFormat, preallocate, diskType,
Line 422:                      volUUID, desc, srcImgUUID, srcVolUUID):
Line 423:         """


-- 
To view, visit http://gerrit.ovirt.org/28128
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I082dc8b382af4a163fd4d10c67e696b1072e6b71
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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