Change in vdsm[ovirt-3.5]: net: Drop default bonding options

ibarkan at redhat.com ibarkan at redhat.com
Sun Jun 28 20:50:29 UTC 2015


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/42959

to review the following change.

Change subject: net: Drop default bonding options
......................................................................

net: Drop default bonding options

On old engines, no bonding options were provided to VDSM and it
used to configure it's own default options. Nowadays, those options
are always provided by engine. If a user will setup a network on an
existing bond, VDSM takes ownership of the bond and configures the
options on it.
The code for the default options was very local and options were
set 'too late' to be persisted. Hence this code was incorrect and
is not needed anyway.
The change in behavior is that from now on, if a bond with no options
is explicitly requested, the kernel will be asked to create a bond
without any options making the default kernel options effective.

Change-Id: I9e4a8d832891f62d1fcc412606b014fa4f874062
Signed-off-by: Ido Barkan <ibarkan at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/42594
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/functional/networkTests.py
M vdsm/network/models.py
2 files changed, 9 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/59/42959/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 79a2200..6719346 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -1382,8 +1382,7 @@
                 self.assertEquals(status, SUCCESS, msg)
 
                 self.assertBondExists(BONDING_NAME, nics=nics[:1])
-                self._assert_exact_bond_opts(BONDING_NAME,
-                                             ['miimon=150', 'mode=4'])
+                self._assert_exact_bond_opts(BONDING_NAME, [])
 
                 # Increase bond size
                 bondings[BONDING_NAME]['nics'] = nics
@@ -1392,8 +1391,7 @@
                 self.assertEquals(status, SUCCESS, msg)
 
                 self.assertBondExists(BONDING_NAME, nics)
-                self._assert_exact_bond_opts(BONDING_NAME,
-                                             ['miimon=150', 'mode=4'])
+                self._assert_exact_bond_opts(BONDING_NAME, [])
 
                 # Reduce bond size
                 REQMODE_BROADCAST = '3'
diff --git a/vdsm/network/models.py b/vdsm/network/models.py
index 73839cb..f4b698f 100644
--- a/vdsm/network/models.py
+++ b/vdsm/network/models.py
@@ -188,11 +188,8 @@
         for slave in slaves:
             slave.master = self
         self.slaves = slaves
-        if options is None:
-            self.options = 'mode=802.3ad miimon=150'
-        else:
-            self.validateOptions(name, options)
-            self.options = options
+        self.options = options or ''
+        self.validateOptions(name, self.options)
         self.destroyOnMasterRemoval = destroyOnMasterRemoval
         super(Bond, self).__init__(name, configurator, ipconfig, mtu)
 
@@ -217,6 +214,11 @@
         self.configurator.configureBond(self, **opts)
 
     def areOptionsApplied(self):
+        # TODO: this method returns True iff self.options are a subset of the
+        # TODO: current bonding options. VDSM should probably compute if the
+        # TODO: non-default settings are equal to the non-default state.
+        if not self.options:
+            return True
         confOpts = [option.split('=', 1) for option in self.options.split(' ')]
         activeOpts = netinfo.bondOpts(self.name,
                                       (name for name, value in confOpts))


-- 
To view, visit https://gerrit.ovirt.org/42959
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9e4a8d832891f62d1fcc412606b014fa4f874062
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list