Change in vdsm[master]: [WIP] Added blockSD.getImageMap(), [block|file]SD.getAllVolu...

ilvovsky at redhat.com ilvovsky at redhat.com
Mon Apr 23 08:16:40 UTC 2012


Igor Lvovsky has posted comments on this change.

Change subject: [WIP] Added blockSD.getImageMap(), [block|file]SD.getAllVolumes
......................................................................


Patch Set 5: I would prefer that you didn't submit this

(10 inline comments)

Please split this patch and take to this one only things that you mentioned in commit message

....................................................
File vdsm/storage/blockSD.py
Line 139:                 break
How do you handle volumes with tags like:
  _remove_me_xxx  ?

Line 157:             res[vName]['imgs'].insert(0, vImg)
should it be sorted somehow as parent<->child ?

Line 159:             not vName.startswith(blockVolume.image.REMOVED_IMAGE_PREFIX) \
blockVolume.image.REMOVED_IMAGE_PREFIX  ???

Line 830:         TODO: Remove and use getAllVolumes().
????

....................................................
File vdsm/storage/fileSD.py
Line 247:         The parent can't be determined in file without reading the metadata.
... determined in file domain without reading the volume's metadata

Line 259:             if volumes.has_key(volUUID):
volumes hasn't volUUID as keys. they has a whole path from glob.

....................................................
File vdsm/storage/image.py
Line 460:         tSelfImage = tImgs[0]
what is tImgs ?
I thing you missing some call here?

Line 469:             os.link(os.path.join(self.repoPath, destDom.sdUUID, sd.DOMAIN_IMAGES, tSelfImage, volUUID), tLink)
How is this change related to your commit message.
I think you have a little mess here with rebases.

....................................................
File vdsm/storage/sd.py
Line 133: ImgsPar = namedtuple("ImgsPar", "imgs,parent")
Ah, such a bad name. what is it mean?

....................................................
File vdsm/storage/sp.py
Line 1768: 
How is this change related to your commit message.
I think you have a little mess here with rebases.
Dealing with deleteImage MUST be in separate patch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7eccf5ca100bd354aa09208ca60bb112fb697063
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>


More information about the vdsm-patches mailing list