Ayal Baron has posted comments on this change.

Change subject: getAllVolumes returns partially deleted volumes too.
......................................................................


Patch Set 3: (1 inline comment)

....................................................
File vdsm/storage/blockSD.py
Line 1025:                     ip.imgs[0].startswith(sd.REMOVED_IMAGE_PREFIX)):
Line 1026:                         remnants[volName] = ip
Line 1027:             else:
Line 1028:                 vols[volName] = ip
Line 1029:         return vols, remnants
I don't see the logic in doing the split outside of this method.  Seems like 
redundant code and layer to me.  You always consume the partial and complete 
images separately and in any event this method would return all images, just 
split into 2 lists to avoid having to run 'if's on them in upper layers (or 
worse, forgetting to do so)
Line 1030: 
Line 1031:     def getAllVolumes(self):
Line 1032:         vols, rems = self.getAllVolumesImages()
Line 1033:         return vols


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8514236a5d4793f66709e9daf546fb46047414f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewars...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Paikov <pai...@gmail.com>
Gerrit-Reviewer: Eduardo <ewars...@redhat.com>
Gerrit-Reviewer: Gadi Ickowicz <gicko...@redhat.com>
Gerrit-Reviewer: Haim Ateya <hat...@redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to