Francesco Romani has posted comments on this change. Change subject: hostdev: report additional information in 'scsi' device ......................................................................
Patch Set 6: Code-Review+1 (2 comments) cosmetic comments inside, not worth a resubmit. Full review later. https://gerrit.ovirt.org/#/c/56038/6/lib/vdsm/hostdev.py File lib/vdsm/hostdev.py: PS6, Line 192: def is_parent(device, parent_name): : actual_parent = None : : try: : actual_parent = device['params']['parent'] : except KeyError: : pass : : return actual_parent == parent_name only in case you resubmit for other reasons, consider this simplification def is_parent(device, parent_name): try: return parent_name == device['params']['parent'] except KeyError: return False What makes the code harder than necessary to read is IMO mostly the nesting of attribute/item access. In this case the temporary is helping a little, so it could be removed. However this is minor and not worth a resubmit alone. PS6, Line 243: if strict: : raise UnsuitableSCSIDevice I'm not really happy about this, let's think about a *future* cleanup patch. Maybe swallowing this exception in the hostdevListByCaps is simpler? -- To view, visit https://gerrit.ovirt.org/56038 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I789f49c9abca2dff1487acf3e8a04ae1407956f4 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Milan Zamazal <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
