Antoni Segura Puimedon has uploaded a new change for review.

Change subject: ethtool_opts: provide a way to apply to all slaves
......................................................................

ethtool_opts: provide a way to apply to all slaves

There was a hole in the feature that this hook services in that it
was not possible to apply some opts to all the slaves in the
cases when the combination of settings was not appliable to all
the slaves in a single ethtool command.

This patch solves the bug by adding a special device name that
the admin can use when defining the custom network opts, i.e.,
'vdsm_bond_slaves' that will mean that the special device name
will be replaced with each of the slave names, one at a time,
and run as many ethtool procs as there are slaves in the bond.

Bug-Url: https://bugzilla.redhat.com/1126478
Change-Id: Idddbc167be4ac8ba7c4b4ab10da0275b4caf2a58
Signed-off-by: Antoni S. Puimedon <asegu...@redhat.com>
---
M vdsm_hooks/ethtool_options/README
M vdsm_hooks/ethtool_options/ethtool_options.py
2 files changed, 38 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/32508/1

diff --git a/vdsm_hooks/ethtool_options/README 
b/vdsm_hooks/ethtool_options/README
index e15b71b..48f9cfe 100644
--- a/vdsm_hooks/ethtool_options/README
+++ b/vdsm_hooks/ethtool_options/README
@@ -11,5 +11,17 @@
 set the command line parameters that one would pass to the ethtool command
 line application. E.g.:
     '--coalesce ethX rx-usecs 14 sample_interval 3 --offload ethX rx on lro on 
tso off --change ethX speed 1000 duplex half'
-If it is for a bond with em1 and em2, it could look like:
+
+bonding
+--------
+For bondings there are two options:
+a) Pick which devices to apply something on (subject to the command actually
+   being appliable with a single ethtool call):
+   If it is for a bond with em1 and em2, it could look like:
     '--offload em2 rx on --offload em1 tx on'
+b) Apply to all the bond slaves:
+    '--coalesce vdsm_bond_slaves rx-usecs 14 sample_interval 3'
+   This would execute an ethtool process for each slave.
+
+NOTE: It is a hidden lazy admin feature that if one uses vdsm_bond_slaves with
+a non-bonded network it will apply to the only nic, if any.
diff --git a/vdsm_hooks/ethtool_options/ethtool_options.py 
b/vdsm_hooks/ethtool_options/ethtool_options.py
index 58f19bf..80c3323 100644
--- a/vdsm_hooks/ethtool_options/ethtool_options.py
+++ b/vdsm_hooks/ethtool_options/ethtool_options.py
@@ -32,6 +32,8 @@
     '/usr/bin/ethtool',  # Arch
 )
 
+ALL_SLAVES = 'vdsm_bond_slaves'  # Too long to be a device name, so we're safe
+
 Subcommand = namedtuple('Subcommand', ('name', 'device', 'flags'))
 
 
@@ -40,10 +42,10 @@
                  'custom': ethtool_opts,
                  'bootproto': 'dhcp', 'STP': 'no', 'bridged': 'true'}
 
-    _validate_dev_ownership(nics, 'test_net',
-                            (item for item in
-                             net_attrs['custom']['ethtool_opts'].split(' ') if
-                             item))
+    _validate_dev_ownership(
+        nics, 'test_net',
+        (item for item in _parse_into_subcommands(
+            net_attrs['custom']['ethtool_opts'].split(' ')) if item))
 
 
 def test():
@@ -88,8 +90,21 @@
     options = attrs['custom'].get('ethtool_opts')
     if options is not None:
         tokens = options.split(' ')
-        _validate_dev_ownership(_net_nics(attrs), network, tokens)
-        _set_ethtool_opts(network, tokens)
+        nics = _net_nics(attrs)
+        subcommands = list(_parse_into_subcommands(tokens))
+        if any(subcmd.device == ALL_SLAVES for subcmd in subcommands):
+            if all(subcmd.device == ALL_SLAVES for subcmd in subcommands):
+                for nic in nics:  # Set for each slave
+                    _set_ethtool_opts(network,
+                                      [nic if token == ALL_SLAVES else token
+                                       for token in tokens])
+            else:
+                raise RuntimeError(
+                    'Trying to apply ethtool opts for mixing all slaves '
+                    'expression and specific slaves is not valid')
+        else:  # No bonding or specific slaves case
+            _validate_dev_ownership(_net_nics(attrs), network, subcommands)
+            _set_ethtool_opts(network, tokens)
 
 
 def _net_nics(attrs):
@@ -99,13 +114,13 @@
         return [attrs.pop('nic')] if 'nic' in attrs else ()
 
 
-def _validate_dev_ownership(nics, name, tokens):
-    """Generator that takes ethtool cmdline arguments and raises an exception
-    if there is a device that does not belong to the network"""
+def _validate_dev_ownership(nics, name, subcommands):
+    """Takes ethtool subcommands and raises an exception if there is a device
+    that does not belong to the network"""
     if not nics:
         raise RuntimeError('Network %s has no nics.' % name)
 
-    for cmd in _parse_into_subcommands(tokens):
+    for cmd in subcommands:
         if cmd.device not in nics:
             raise RuntimeError('Trying to apply ethtool opts for dev: %s, not '
                                'in the net nics: %s' % (cmd.device, nics))


-- 
To view, visit http://gerrit.ovirt.org/32508
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idddbc167be4ac8ba7c4b4ab10da0275b4caf2a58
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegu...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to