Change in vdsm[master]: New getChildrenList implementation.

abaron at redhat.com abaron at redhat.com
Wed Oct 9 07:41:48 UTC 2013


Ayal Baron has posted comments on this change.

Change subject: New getChildrenList implementation.
......................................................................


Patch Set 8:

(4 comments)

....................................................
Commit Message
Line 7: New getChildrenList implementation.
Line 8: 
Line 9: This implementation returns children of any image on the SD.
Line 10: Avoids to produce each volume in the domain.
Line 11: Implemented like one shot operation.
what do you mean implemented like one shot operation?
Line 12: 
Line 13: Change-Id: I584cd5d1b03d3965457f12c3d67de95455d1de24
Line 14: Related-to: BZ#960952


....................................................
File lib/vdsm/utils.py
Line 166: 
Line 167: def grepCmd(pattern, paths):
Line 168:     cmd = [constants.EXT_GREP, '-E', '-H', pattern]
Line 169:     cmd.extend(paths)
Line 170:     rc, out, err = execCmd(cmd)
I thought you agree with Saggi that execCmd should not be called directly?
Line 171:     if rc == 0:
Line 172:         matches = out  # A list of matching lines
Line 173:     elif rc == 1:
Line 174:         matches = []  # pattern not found


....................................................
File vdsm/storage/fileVolume.py
Line 374: 
Line 375:         Children can be found in any image of the volume SD.
Line 376:         """
Line 377:         domPath = self.imagePath.split('images')[0]
Line 378:         gPath = os.path.join(domPath, 'images', '*', '*.meta')
gPath? (I hate repeating comments).
If it wasn't clear, I have no idea what this name means or represents
Line 379:         metaPaths = oop.getProcessPool(self.sdUUID).glob.glob(gPath)
Line 380:         pattern = "%s.*%s" % (volume.PUUID, self.volUUID)
Line 381:         matches = grepCmd(pattern, metaPaths)
Line 382:         if matches:


Line 378:         gPath = os.path.join(domPath, 'images', '*', '*.meta')
Line 379:         metaPaths = oop.getProcessPool(self.sdUUID).glob.glob(gPath)
Line 380:         pattern = "%s.*%s" % (volume.PUUID, self.volUUID)
Line 381:         matches = grepCmd(pattern, metaPaths)
Line 382:         if matches:
you can remove the 'if matches'.  i.e.:

s/if matches.*/

children = []

for line ...

return tuple(children)
Line 383:             children = []
Line 384:             for line in matches:
Line 385:                 volMeta = os.path.basename(line.split(':')[0])
Line 386:                 children.append(os.path.splitext(volMeta)[0])  # volUUID


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I584cd5d1b03d3965457f12c3d67de95455d1de24
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw 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: Elad Ben Aharon <eladba1990 at gmail.com>
Gerrit-Reviewer: Gadi Ickowicz <gickowic at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei 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