Change in vdsm[master]: storage: Search only the current image for children

danken at redhat.com danken at redhat.com
Sun Nov 30 21:30:00 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: storage: Search only the current image for children
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

http://gerrit.ovirt.org/#/c/35096/1//COMMIT_MSG
Commit Message:

Line 12: problem is that deleteVolumes takes an exclusive lock on the image
Line 13: namespace only.  If deletes are active on other images at the same time
Line 14: we'll get errors with missing files.
Line 15: 
Line 16: The code mistakenly claims that children can be in any image on the
> Feel free to elaborate on why bz1069610#c3 is wrong.
Answering for Eduardo:

* getChildren is raceful, but it was raceful before  317b957
* deleteImage does not generate any call to getChildren, as it is done in the storage domain level with sd.getVolsOfImage.

However, I think that parallel mergeSnapshots would show the problem, regardless of the live snapshot and live merge flows.
Line 17: storage domain.  It appears that they are in fact limited to the same
Line 18: image.  Therefore, the simplest resolution is to limit the search for
Line 19: children to the current image.
Line 20: 


http://gerrit.ovirt.org/#/c/35096/1/vdsm/storage/fileVolume.py
File vdsm/storage/fileVolume.py:

Line 360: 
Line 361:     def getChildren(self):
Line 362:         """ Return children volume UUIDs.
Line 363: 
Line 364:         Children will always belong to the same image as this volume.
> False for templates.
Eduardo, we have no intentions to reinroduce getChildren to getVolumeInfo()
http://gerrit.ovirt.org/#/c/20004/2/vdsm/storage/volume.py

Adam, would you please change the docstring of this function to match the new semantics?

 ... for a template's "self-volume", this function returns no children

or maybe even better

 ... this function should never be called on a template's base volume.
Line 365:         """
Line 366:         domPath = self.imagePath.split('images')[0]
Line 367:         metaPattern = os.path.join(domPath, 'images', self.imgUUID, '*.meta')
Line 368:         metaPaths = oop.getProcessPool(self.sdUUID).glob.glob(metaPattern)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ef9012cee3a8cb891926510c10ecc47b7cddaa1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce 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