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

fromani at redhat.com fromani at redhat.com
Tue Jan 20 08:12:26 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: hostdev: add support for reporting pci addresses
......................................................................


Patch Set 5:

(6 comments)

few minors comments, mostly for my own understanding.

http://gerrit.ovirt.org/#/c/35651/5/tests/hostdevTests.py
File tests/hostdevTests.py:

Line 298:     </device>
Line 299:     """
Line 300: 
Line 301: DEVICES_PARSED = {u'pci_0000_00_1b_0': {'product': '6 Series/C200 Series '
Line 302:                                         'Chipset Family High Definition Audio '
nit: please avoid unrelated changes (or please explain why it is related). I don't really care in this case but as general rule we should keep patches minimal.
Line 303:                                         'Controller',
Line 304:                                         'vendor': 'Intel Corporation',
Line 305:                                         'product_id': '0x1c20',
Line 306:                                         'parent': 'computer',


Line 305:                                         'product_id': '0x1c20',
Line 306:                                         'parent': 'computer',
Line 307:                                         'vendor_id': '0x8086',
Line 308:                                         'capability': 'pci',
Line 309:                                         'addr': {'slot': '27',
minor: any reason not to use the full 'address' name?
Line 310:                                                  'bus': '0',
Line 311:                                                  'domain': '0',
Line 312:                                                  'function': '0'}},
Line 313:                   u'scsi_0_0_0_0': {'capability': 'scsi',


Line 338:                                                  'function': '2'}},
Line 339:                   u'scsi_target0_0_0': {'capability': 'scsi_target',
Line 340:                                         'parent': 'scsi_host0'},
Line 341:                   u'pci_0000_00_02_0': {'product': '2nd Generation Core '
Line 342:                                         'Processor Family Integrated Graphics '
same probably unrelated change
Line 343:                                         'Controller',
Line 344:                                         'vendor': 'Intel Corporation',
Line 345:                                         'product_id': '0x0126',
Line 346:                                         'parent': 'computer',


Line 351:                                                  'domain': '0',
Line 352:                                                  'function': '0'}},
Line 353:                   u'scsi_host0': {'capability': 'scsi_host',
Line 354:                                   'parent': 'pci_0000_00_1f_2'},
Line 355:                   u'pci_0000_00_19_0': {'product': '82579LM Gigabit Network '
same probably unrelated change
Line 356:                                         'Connection',
Line 357:                                         'vendor': 'Intel Corporation',
Line 358:                                         'product_id': '0x1502',
Line 359:                                         'parent': 'computer',


http://gerrit.ovirt.org/#/c/35651/5/vdsm/hostdev.py
File vdsm/hostdev.py:

Line 55:     return params
Line 56: 
Line 57: 
Line 58: def _parse_pci_address(caps):
Line 59:     return _parse_address(caps, ('domain', 'bus', 'slot', 'function'))
It is always all or nothing? I mean, it is possible an address doesn't have all of these childrens?
Line 60: 
Line 61: 
Line 62: def _parse_device_params(device_xml):
Line 63:     """


Line 102:     try:
Line 103:         params['addr'] = address_parser[params['capability']](caps)
Line 104:     except KeyError:
Line 105:         # We can somewhat safely ignore missing address as that means we're
Line 106:         # dealing with device that is not yet supported
ok, but we can have two possible KeyErrors here (from outside to inside): missing parser given capability, missing capability in params.
Both are possible? Can we safely ignore both of them?
Line 107:         pass
Line 108: 
Line 109:     return params
Line 110: 


-- 
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: 5
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: 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