From Dan Kenigsberg <[email protected]>: Dan Kenigsberg has posted comments on this change.
Change subject: api: modify VF MAC addresses on creation to a valid address ...................................................................... Patch Set 12: Code-Review-1 (4 comments) https://gerrit.ovirt.org/#/c/71029/12/lib/vdsm/network/api.py File lib/vdsm/network/api.py: Line 70: time.sleep(0.5) Line 71: udevadm.settle(timeout=10) Line 72: Line 73: Line 74: # igb driver forbids resetting VF MAC address back to 00:00:00:00:00:00, rephrase as a doc text? Line 75: # which was its original value. By setting the MAC addresses to a valid Line 76: # value (TARGET_MAC, for example), upon restoration the valid address will Line 77: # be accepted. Line 78: # (BZ: https://bugzilla.redhat.com/1341248) Line 76: # value (TARGET_MAC, for example), upon restoration the valid address will Line 77: # be accepted. Line 78: # (BZ: https://bugzilla.redhat.com/1341248) Line 79: def _set_valid_macs_for_igb_vfs(pci_path): Line 80: def modify_mac_addresses(pci_path): I'd prefer to see this auxiliary function defined on the module level, instead of in each function call. Line 81: TARGET_MAC = '00:00:00:00:00:01' Line 82: Line 83: pf = os.listdir('/sys/bus/pci/devices/{}/net/'.format(pci_path))[0] Line 84: vfs = glob.iglob('/sys/class/net/{}/device/virtfn*'.format(pf)) PS12, Line 86: vf[-1] this would break horribly if you have more than 10 VFs. There must be a better way to find the VF number. I'd guess that range(sriov_numvfs) would do the trick. Line 86: vf_num = vf[-1] Line 87: ipwrapper.linkSet(pf, ['vf', vf_num, 'mac', TARGET_MAC]) Line 88: Line 89: drivers = os.listdir('/sys/bus/pci/drivers/') Line 90: if 'igb' in drivers: I think if os.path.exists('/sys/bus/pci/drivers/igb') is shorter and clearer Line 91: driver_content = os.listdir('/sys/bus/pci/drivers/igb') Line 92: if pci_path in driver_content: Line 93: modify_mac_addresses(pci_path) Line 94: -- To view, visit https://gerrit.ovirt.org/71029 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib740d45206d41b330fd1b33f31765050cadbd679 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Leon Goldberg <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Leon Goldberg <[email protected]> Gerrit-Reviewer: Meni Yakove <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- [email protected] To unsubscribe send an email to [email protected]
