Nir Soffer has posted comments on this change. Change subject: vm: Unify checks for vdsm image ......................................................................
Patch Set 2: (1 comment) .................................................... 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 I'm not arguing if the device == 'disk' check is needed or not - this patch is not about removing unneeded checks but about removing code duplication, make the code simpler to understand and easier to use. What you are trying to do in http://gerrit.ovirt.org/22363, remove one disk check, make the code worse, adding more places with different logic. We should first unify the checks, removing the code duplication. Then, we can remove unneeded checks in one place. 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
