Change in vdsm[master]: image: add a flag for preparing with illegal volumes

nsoffer at redhat.com nsoffer at redhat.com
Thu Mar 24 12:54:22 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: image: add a flag for preparing with illegal volumes
......................................................................


Patch Set 1:

(5 comments)

https://gerrit.ovirt.org/#/c/55199/1//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: Currently preparing an image is impossible if one of its volumes is
Line 10: illegal.
Line 11: 
Line 12: In the new image upload flow, an image which is uploaded must have the
... an image prepared for upload or random I/O using ovirt-imageio-daemon ...
Line 13: image prepared and have the image in ILLEGAL state. that means that if
Line 14: an upload is stopped by any reason- resuming it means preparing the
Line 15: image while it is in ILLEGAL state.
Line 16: 


Line 14: an upload is stopped by any reason- resuming it means preparing the
Line 15: image while it is in ILLEGAL state.
Line 16: 
Line 17: This patch adds a flag in the API for enabling the prepare process even
Line 18: if its volumes are ILLEGAL, for the upload image to use.
Please show here the image upload flow, explaining how we manage the illegal state during upload, and we want to use this state instead of adding a new state (.e.g "unsafe").
Line 19: 
Line 20: Change-Id: Ie18ab167635e6e099c3a12e03ea6620578c3103f


https://gerrit.ovirt.org/#/c/55199/1/lib/api/vdsmapi-schema.json
File lib/api/vdsmapi-schema.json:

Line 5308: #
Line 5309: # @volumeID:         The UUID of the Volume
Line 5310: #
Line 5311: # @allowIllegal:     #optional If set to True, prepare will succeed even if
Line 5312: #                    any of the image volumes are illegal.
Make it clear that this is false by default.

Also explain the context, making writable by untrusted source, and a warning about never using this when exposing an image to a vm.
Line 5313: #
Line 5314: # Returns:
Line 5315: # Information about prepared image
Line 5316: # (new in version 4.18)


https://gerrit.ovirt.org/#/c/55199/1/vdsm/API.py
File vdsm/API.py:

Line 925:     def download(self, methodArgs, volumeID=None):
Line 926:         return self._irs.downloadImage(
Line 927:             methodArgs, self._spUUID, self._sdUUID, self._UUID, volumeID)
Line 928: 
Line 929:     def prepare(self, volumeID, allowIllegal):
This is optional, so keep the allowIllegal=False
Line 930:         return self._irs.prepareImage(self._sdUUID, self._spUUID,
Line 931:                                       self._UUID, volumeID,
Line 932:                                       allowIllegal=allowIllegal)
Line 933: 


https://gerrit.ovirt.org/#/c/55199/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3190:         if not allowIllegal:
Line 3191:             for volUUID in imgVolumes:
Line 3192:                 legality = dom.produceVolume(imgUUID, volUUID).getLegality()
Line 3193:                 if legality == volume.ILLEGAL_VOL:
Line 3194:                     raise se.prepareIllegalVolumeError(volUUID)
This skips the verification silently. I would like to keep the verification, and log a debug message when we are preparing an illegal volume, something like:

    Preparing illegal volume vol_uuid

This will make it very clear if someone is using this option incorrectly.
Line 3195: 
Line 3196:         imgPath = dom.activateVolumes(imgUUID, imgVolumes)
Line 3197:         if spUUID and spUUID != sd.BLANK_UUID:
Line 3198:             runImgPath = dom.linkBCImage(imgPath, imgUUID)


-- 
To view, visit https://gerrit.ovirt.org/55199
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie18ab167635e6e099c3a12e03ea6620578c3103f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list