Change in vdsm[master]: hsm: deleteImage fails because of wrong dictionary use

fsimonce at redhat.com fsimonce at redhat.com
Mon Aug 19 12:56:40 UTC 2013


Federico Simoncelli has posted comments on this change.

Change subject: hsm: deleteImage fails because of wrong dictionary use
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

....................................................
File vdsm/storage/hsm.py
Line 1525:             # hence no need to create fake template if postZero is true.
Line 1526:             self._spmSchedule(spUUID, "zeroImage_%s" % imgUUID, dom.zeroImage,
Line 1527:                               sdUUID, imgUUID, volsByImg)
Line 1528:         else:
Line 1529:             dom.deleteImage(sdUUID, imgUUID, volsByImg)
Do we want to optimize in case the image is already a fake template? 

 vol.getLegality() == volume.FAKE_VOL

In such case we can avoid to delete and recreate another fake volume.
Line 1530:             # This is a hack to keep the interface consistent
Line 1531:             # We currently have race conditions in delete image, to quickly fix
Line 1532:             # this we delete images in the "synchronous" state. This only works
Line 1533:             # because Engine does not send two requests at a time. This hack is


Line 1536:             # an eliminate all race conditions
Line 1537:             if needFake:
Line 1538:                 img = image.Image(os.path.join(self.storage_repository,
Line 1539:                                                spUUID))
Line 1540:                 tName = volsByImg.keys()[0]
This might work but it's potentially wrong.
It works only because you have a dictionary with 1 key (but this is rather implicit) otherwise a random key would be selected.

We could try to do some cleanup, remove the (unnecessary) isTemplateWithChildren/needFake and get the fake volume uuid from the loop at line 1508:

 fakeVolUUID = None
 for k, v in volsByImg.iteritems():
     if len(v.imgs) > 1 and v.imgs[0] == imgUUID:
         if dom.isBackup():
             fakeVolUUID = k
         else:
             raise se.CannotDeleteSharedVolume(...)

 ...
     if fakeVolUUID is not None:
         ...
         tParams = dom.produceVolume(imgUUID, fakeVolUUID).getVolumeParams()
         ...
Line 1541:                 tParams = dom.produceVolume(imgUUID, tName).getVolumeParams()
Line 1542:                 img.createFakeTemplate(sdUUID=sdUUID, volParams=tParams)
Line 1543:             self._spmSchedule(spUUID, "deleteImage_%s" % imgUUID, lambda: True)
Line 1544: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I81f9a5aa63c0914e3b934046454df64ccd39c269
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list