Ondřej Svoboda has posted comments on this change. Change subject: netinfo: Retrieve bonding options differing from defaults ......................................................................
Patch Set 32: (6 comments) http://gerrit.ovirt.org/#/c/24456/32/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 43: from .ipwrapper import Route Line 44: from .ipwrapper import routeGet Line 45: from .ipwrapper import routeShowGateways, routeShowAllDefaultGateways Line 46: from . import libvirtconnection Line 47: from .utils import execCmd, memoized, CommandPath > .utils should come after .netlink ( alphabetically) Done Line 48: from .netconfpersistence import RunningConfig Line 49: from .netlink import iter_addrs, iter_links Line 50: Line 51: Line 535: return ''.join(random.choice(CHARS) for _ in range(MAX_LENGTH)) Line 536: Line 537: Line 538: @memoized Line 539: def _getDefaultBondingOptions(): > This function still misses a docstring explained king its return value. Done Line 540: teeCmd = _TEE_BINARY.cmd Line 541: MAX_MODE = 6 Line 542: Line 543: bondName = _randomIfaceName() Line 593: Options having symbolic values, e.g. 'mode', are presented by sysfs in Line 594: the order symbolic name, numeric value, e.g. 'balance-rr 0'. Line 595: From a list given by bondOpts(), the numeric value is chosen. Line 596: Line 597: 'mode' is ordered the first so e.g. iproute2 configurator sets it first. > Iproute2 configurator should be fixed to take "mode" first. (in a separate After sending a new iteration of this patch, I will investigate. Line 598: ''' Line 599: mode = opts.pop('mode', None) Line 600: ifcfg = sorted(opts.iteritems()) Line 601: if mode: http://gerrit.ovirt.org/#/c/24456/32/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 1043: Line 1044: self.assertNetworkExists(NETWORK_NAME, bridged=bridged) Line 1045: self.assertBondExists(BONDING_NAME, nics) Line 1046: Line 1047: # Reduce bond size and create Network on detached NIC > please avoid whitespace noise. I thought I had already edited this out. Fixed :-) Line 1048: Line 1049: with nonChangingOperstate(BONDING_NAME): Line 1050: netName = NETWORK_NAME + '-2' Line 1051: networks = {netName: dict(nic=nics[0], http://gerrit.ovirt.org/#/c/24456/32/vdsm/network/configurators/__init__.py File vdsm/network/configurators/__init__.py: Line 152: self.configApplier.setIfaceMtu(iface.name, maxMtu) Line 153: return maxMtu Line 154: Line 155: def _waitForBondUp(self, bond): Line 156: # TODO: avoid polling > indeed. the easiest way to do this is Thanks! This was more of a proof-of-concept, written off the top of my head. Line 157: INTERVAL = 0.1 Line 158: for _ in range(50): Line 159: if netinfo.operstate(bond) == netinfo.OPERSTATE_UP: Line 160: return http://gerrit.ovirt.org/#/c/24456/32/vdsm/sudoers.vdsm.in File vdsm/sudoers.vdsm.in: Line 1: Cmnd_Alias VDSM_LIFECYCLE = \ Line 2: @DMIDECODE_PATH@, \ Line 3: @TEE_PATH@ /sys/class/net/bonding_masters, \ Line 4: @TEE_PATH@ /sys/class/net/*/bonding/mode, \ > Would you place that under a new Cmnd_Alias called VDSM_NETWORK ? Done Line 5: @VDSMDIR@/mk_sysprep_floppy, \ Line 6: @SERVICE_PATH@ ksmtuned *, \ Line 7: @SERVICE_PATH@ ksm * Line 8: Cmnd_Alias VDSM_STORAGE = @MOUNT_PATH@, @UMOUNT_PATH@, \ -- To view, visit http://gerrit.ovirt.org/24456 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ief6d366b1b761627c7203cf236b75ef538af3e26 Gerrit-PatchSet: 32 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches