Nir Soffer has posted comments on this change.
Change subject: storage: get currImgDir correctly in fileSD.deleteImage()
......................................................................
Patch Set 4:
(1 comment)
....................................................
File vdsm/storage/fileSD.py
Line 342: def deleteImage(self, sdUUID, imgUUID, volsImgs):
Line 343: if sdUUID == self.sdUUID:
Line 344: currImgDir = os.path.join(self.domaindir, 'images',
imgUUID)
Line 345: else:
Line 346: raise se.StorageDomainLayoutError(sdUUID)
The sdUUID validation is not related to creating the image path. This check
should be done before creating the path:
if sdUUID != self.sdUUID:
raise ...
I'm not sure what should we raise here - looks like caller error to me, there
is no storage layout error.
Even better, I don't see any reason to call a FileStorageDomain.deleteImage
with a sdUUID - domain knows its UUID. This may require fixing some callers,
and probably better done in a different patch.
deleteImage is not the place for building the path for the image. The proper
place for this is FileStorageDomain.getImagePath(imgUUID) method.
def getImagePath(self, imgUUID):
return os.path.join(self.domaindir, 'images', imgUUID)
Finally, do we have constant for "images"? If we don't It would be nice if you
add one :-)
Line 347: dirName, baseName = os.path.split(currImgDir)
Line 348: toDelDir = os.tempnam(dirName, sd.REMOVED_IMAGE_PREFIX +
baseName)
Line 349: try:
Line 350: self.oop.os.rename(currImgDir, toDelDir)
--
To view, visit http://gerrit.ovirt.org/22359
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8ee6cb963a428f18b6e43f0585d461d0975c377a
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhun...@gmail.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Eduardo <ewars...@redhat.com>
Gerrit-Reviewer: Hunt Xu <mhun...@gmail.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
