Petr Horáček has uploaded a new change for review.
Change subject: net: configurators: persist custom bond option
......................................................................
net: configurators: persist custom bond option
Until now, custom bond property was exposed only to
before_network_setup hook and then dropped (so it was not
passed to configurators and persisted). But this is not how
custom *network* property behaves. We want these two custom
properties to be symmetric - they have to be persisted
(if unified persistence is used) and must not be shown
in netinfo.
This patch moves removal of custom property down to
configurators. Thanks to it, custom property is persisted,
but not set and bond option.
Custom property is not dropped only in configurators, but in
models.py:Bond:areOptionsApplied as well - we don't want
configurator to reconfigure bond when there is a custom
property defined while its value is not used by configurator.
Change-Id: Ic4f1564e19904cc1d53bc6d1cf732ca35375332e
Signed-off-by: Petr Horáček <phora...@redhat.com>
---
M tests/functional/networkTests.py
M vdsm/network/configurators/__init__.py
M vdsm/network/configurators/ifcfg.py
M vdsm/network/configurators/iproute2.py
M vdsm/network/configurators/pyroute_two.py
M vdsm/network/models.py
6 files changed, 38 insertions(+), 23 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/91/43891/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index e5bfe25..4d9f480 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -2648,8 +2648,12 @@
NOCHK, test_kernel_config=False)
self.assertEqual(status, SUCCESS, msg)
self.assertBondExists(BONDING_NAME, nics)
- opts = self.vdsm_net.config.bonds.get(BONDING_NAME).get('options')
- self.assertEquals(opts, 'mode=balance-rr')
+ # custom property have to be persisted (if unified persistence is
+ # used) and must not be shown in netinfo.
+ self._assert_exact_bond_opts(BONDING_NAME, ('mode=balance-rr',))
+ if vdsm.config.config.get('vars', 'net_persistence') == 'unified':
+ bond = self.vdsm_net.config.bonds.get(BONDING_NAME)
+ self.assertEquals(bond.get('options'), 'mode=balance-rr')
status, msg = self.setupNetworks(
{}, {BONDING_NAME: {'remove': True}}, NOCHK)
diff --git a/vdsm/network/configurators/__init__.py
b/vdsm/network/configurators/__init__.py
index e51d3ed..bc41119 100644
--- a/vdsm/network/configurators/__init__.py
+++ b/vdsm/network/configurators/__init__.py
@@ -178,3 +178,16 @@
if iface.blockingdhcp and rc:
raise ConfigNetworkError(ERR_FAILED_IFUP, 'dhclient%s failed',
family)
+
+
+def remove_custom_bond_option(options):
+ """ Removes 'custom' option from bond options string.
+
+ >>> remove_custom_bond_option('custom=foo=bar mode=1')
+ 'mode=1'
+ """
+ d_opts = dict(opt.split('=', 1) for opt in options.split())
+ if d_opts.pop('custom', None) is not None:
+ options = ' '.join(['%s=%s' % (key, value)
+ for key, value in d_opts.items()])
+ return options
diff --git a/vdsm/network/configurators/ifcfg.py
b/vdsm/network/configurators/ifcfg.py
index f2dbd5e..c5e2c0d 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -44,7 +44,8 @@
if utils.isOvirtNode():
from ovirt.node.utils import fs as node_fs
-from . import Configurator, dhclient, getEthtoolOpts, libvirt
+from . import (Configurator, dhclient, getEthtoolOpts, libvirt,
+ remove_custom_bond_option)
from ..errors import ConfigNetworkError, ERR_FAILED_IFUP
from ..models import Nic, Bridge, IPv4, IPv6
from ..sourceroute import StaticSourceRoute, DynamicSourceRoute
@@ -567,7 +568,9 @@
def addBonding(self, bond, **opts):
""" Create ifcfg-* file with proper fields for bond """
- conf = 'BONDING_OPTS=%s\n' % pipes.quote(bond.options or '')
+ options = (remove_custom_bond_option(bond.options)
+ if bond.options else '')
+ conf = 'BONDING_OPTS=%s\n' % pipes.quote(options)
opts['hotplug'] = 'no' # So that udev doesn't trigger an ifup
if bond.bridge:
conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name)
diff --git a/vdsm/network/configurators/iproute2.py
b/vdsm/network/configurators/iproute2.py
index 0b5b73e..0850212 100644
--- a/vdsm/network/configurators/iproute2.py
+++ b/vdsm/network/configurators/iproute2.py
@@ -29,7 +29,8 @@
from vdsm.utils import CommandPath
from vdsm.utils import execCmd
-from . import Configurator, runDhclient, getEthtoolOpts, libvirt
+from . import (Configurator, runDhclient, getEthtoolOpts, libvirt,
+ remove_custom_bond_option)
from .dhclient import DhcpClient
from ..errors import ConfigNetworkError, ERR_FAILED_IFUP, ERR_FAILED_IFDOWN
from ..models import Nic
@@ -340,7 +341,9 @@
def addBondOptions(self, bond):
logging.debug('Add bond options %s', bond.options)
- for option in bond.options.split():
+ options = (remove_custom_bond_option(bond.options)
+ if bond.options else '')
+ for option in options:
key, value = option.split('=')
with open(netinfo.BONDING_OPT % (bond.name, key), 'w') as f:
f.write(value)
diff --git a/vdsm/network/configurators/pyroute_two.py
b/vdsm/network/configurators/pyroute_two.py
index c7730d3..b194a03 100644
--- a/vdsm/network/configurators/pyroute_two.py
+++ b/vdsm/network/configurators/pyroute_two.py
@@ -24,7 +24,7 @@
from vdsm import ipwrapper
from vdsm.netconfpersistence import RunningConfig
-from . import libvirt, runDhclient
+from . import libvirt, runDhclient, remove_custom_bond_option
from .dhclient import DhcpClient
from .iproute2 import Iproute2
@@ -153,7 +153,9 @@
def addBondOptions(self, bond):
logging.debug('Add bond options %s', bond.options)
- for option in bond.options.split():
+ options = (remove_custom_bond_option(bond.options)
+ if bond.options else '')
+ for option in options:
key, value = option.split('=')
with open(netinfo.BONDING_OPT % (bond.name, key), 'w') as f:
f.write(value)
diff --git a/vdsm/network/models.py b/vdsm/network/models.py
index 6466917..19d0179 100644
--- a/vdsm/network/models.py
+++ b/vdsm/network/models.py
@@ -24,6 +24,7 @@
from vdsm import netinfo
+from .configurators import remove_custom_bond_option
from .errors import ConfigNetworkError
from . import errors as ne
@@ -241,9 +242,11 @@
# 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:
+ options = (remove_custom_bond_option(self.options)
+ if self.options else '')
+ if options == '':
return True
- confOpts = [option.split('=', 1) for option in self.options.split(' ')]
+ confOpts = [option.split('=', 1) for option in options.split(' ')]
activeOpts = netinfo.bondOpts(self.name,
(name for name, value in confOpts))
return all(value in activeOpts[name] for name, value in confOpts)
@@ -267,22 +270,9 @@
_netinfo=_netinfo))
return slaves
- @staticmethod
- def remove_custom_option(options):
- """ 'custom' property should not be exposed to configurator, it is
- only for purpose of hooks """
- d_opts = dict(opt.split('=', 1) for opt in options.split())
- if d_opts.pop('custom', None) is not None:
- options = ' '.join(['%s=%s' % (key, value)
- for key, value in d_opts.items()])
- return options
-
@classmethod
def objectivize(cls, name, configurator, options, nics, mtu, _netinfo,
destroyOnMasterRemoval=None):
-
- if options:
- options = cls.remove_custom_option(options)
if nics: # New bonding or edit bonding.
slaves = cls._objectivizeSlaves(name, configurator, _nicSort(nics),
--
To view, visit https://gerrit.ovirt.org/43891
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic4f1564e19904cc1d53bc6d1cf732ca35375332e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phora...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
