Francesco Romani has posted comments on this change. Change subject: clientIF: prepareVolumePath payload cleanup ......................................................................
Patch Set 4: (1 comment) Nir, a big thank you for the very detailed and in-depth review. You raised very valid point which I'll fix in the next changeset (and wipe away some uglyness introduced in my last uplod). http://gerrit.ovirt.org/#/c/24636/4/vdsm/clientIF.py File vdsm/clientIF.py: Line 337: return supervdsm.getProxy().mkIsoFs(vmId, payload['file'], Line 338: payload.get('volId')) Line 339: elif device == 'floppy': Line 340: return supervdsm.getProxy().mkFloppyFs(vmId, payload['file'], Line 341: payload.get('volId')) > The only difference in these calls is the function name but you repeat ever This is a very valid point. But Dan had a point also, I'd like to move somehow on the direction he suggested. Will try to accomodate both suggestions. Line 342: else: Line 343: raise vm.VolumeError("Unsupported 'device': %s" % device) Line 344: Line 345: def teardownVolumePath(self, drive): -- To view, visit http://gerrit.ovirt.org/24636 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I058206b7506ddbb5ec087c9ea0963a10ed57affb Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [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
