Change in vdsm[master]: netinfo: Retrieve bonding options differing from defaults

osvoboda at redhat.com osvoboda at redhat.com
Sun Apr 20 16:39:44 UTC 2014


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 <osvoboda at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvoboda 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