Francesco Romani has posted comments on this change.

Change subject: hostdev: report physfn
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

seems OK. A couple of minor comments/questions inline.

http://gerrit.ovirt.org/#/c/35974/1/vdsm/hostdev.py
File vdsm/hostdev.py:

Line 44: 
Line 45: def _sriov_physfn(device_name):
Line 46:     return 'pci_' + _pci_path_to_name(
Line 47:         os.readlink('/sys/bus/pci/devices/{}/physfn'.format(
Line 48:             _name_to_pci_path(device_name))))[3:]
I have a distaste for code too packed, but this could safely considered 
personal taste. If you could make this a bit more sparse, fine. Otherwise, good 
enough.
Line 49: 
Line 50: 
Line 51: def _parse_device_params(device_xml):
Line 52:     """


Line 83: 
Line 84:     try:
Line 85:         params['physfn'] = _sriov_physfn(name)
Line 86:     except OSError:
Line 87:         # Device does not support sriov, we can safely go on
can we or do we need a fake key? Just wondering, I don't see actual problems in 
the code.
Line 88:         pass
Line 89: 
Line 90:     return params
Line 91: 


-- 
To view, visit http://gerrit.ovirt.org/35974
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id1c41b1fd7e0cab51c869ae4a7882072be2c3fad
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Martin Betak <[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

Reply via email to