Sergey Gotliv has posted comments on this change. Change subject: vm: Unify checks for vdsm image ......................................................................
Patch Set 2: Code-Review-1 (1 comment) I realized that if I am out my -1 is out with me :-). .................................................... File vdsm/vm.py Line 88: :type drive: dict or vm.Drive Line 89: :return: bool Line 90: """ Line 91: if drive['device'] != 'disk': Line 92: return False Currently vdsm image is a PDIV, it has nothing to do with device == disk. What other devices you know that contain poolID, domainID, imageID and volumeID? This check of device is unnecessary and earlier today I did a first step to remove that check here http://gerrit.ovirt.org/#/c/22363/. @Author knows that because he already reviewed it :-). So what is this? Putting this check in that place won't allow me to remove it step by step, now its like all or nothing and as I explained there "all" is a time consuming in the matter of tests. Line 93: required = ('volumeID', 'domainID', 'imageID', 'poolID') Line 94: return all(k in drive for k in required) Line 95: Line 96: -- To view, visit http://gerrit.ovirt.org/22370 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaa109a19aeec56f09ed2e6d64dee636c7779d426 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
