Francesco Romani has posted comments on this change. Change subject: hostdev: add support for reporting pci addresses ......................................................................
Patch Set 10: (2 comments) looks OK, a couple of minor questions to improve my understanding http://gerrit.ovirt.org/#/c/35651/10//COMMIT_MSG Commit Message: Line 7: hostdev: add support for reporting pci addresses Line 8: Line 9: This patch adds addr field into hostdev tree that holds map of Line 10: address fields relevant to the device. This is required in order to Line 11: avoid parsing the pci name (such as pci_0000_05_fe_1). looks OK, can you please point me to the patch on which you'll use this new data? It is just to grasp better the whole picture. Line 12: Line 13: Change-Id: I8208af6b662550de426d8d015421e0ab2c3c617e http://gerrit.ovirt.org/#/c/35651/10/tests/hostdevTests.py File tests/hostdevTests.py: Line 309: 'capability': 'pci', Line 310: 'address': {'slot': '27', Line 311: 'bus': '0', Line 312: 'domain': '0', Line 313: 'function': '0'}}, It is most a matter of personal taste, so please consider it just an open point for possible improvement. Have you considered using a namedtuple? How does it look like? In general, address.bus seems nicer than addres["bus"], but I don't have yet a clear picture about how it will look in the end. May not make sense considering all the changes you planned. Line 314: u'scsi_0_0_0_0': {'capability': 'scsi', Line 315: 'parent': 'scsi_target0_0_0'}, Line 316: u'pci_0000_00_1a_0': {'product': '6 Series/C200 Series ' Line 317: 'Chipset Family USB Enhanced Host ' -- To view, visit http://gerrit.ovirt.org/35651 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8208af6b662550de426d8d015421e0ab2c3c617e Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[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
