Change in vdsm[master]: hostdev: add support for reporting pci addresses

fromani at redhat.com fromani at redhat.com
Thu Jan 22 08:10:32 UTC 2015


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 <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list