Ido Barkan has uploaded a new change for review.

Change subject: net: SetupNetworks small cleanups.
......................................................................

net: SetupNetworks small cleanups.

Extracted the logic that alters network parameters for bonding and nics
to functions, did some internal renaming and few small cleanups.

Change-Id: Ib054e144dccbff40b40b5967dca0bf840bce8ab9
Signed-off-by: Ido Barkan <[email protected]>
---
M lib/vdsm/netconfpersistence.py
M vdsm/network/api.py
2 files changed, 55 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/42022/1

diff --git a/lib/vdsm/netconfpersistence.py b/lib/vdsm/netconfpersistence.py
index 656a588..cdb6d2e 100644
--- a/lib/vdsm/netconfpersistence.py
+++ b/lib/vdsm/netconfpersistence.py
@@ -413,5 +413,6 @@
     else:  # isolated bridged network
         return []
 
+
 def unicodize(d):
     return json.loads(json.dumps(d))
diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index 828d67c..3eee1e8 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -608,27 +608,6 @@
         configurator.configureBond(bond)
 
 
-def _buildBondOptions(bondName, bondings, _netinfo):
-    logger = logging.getLogger("_buildBondOptions")
-
-    bond = {}
-    if bondings.get(bondName):
-        bond['nics'] = bondings[bondName]['nics']
-        bond['bondingOptions'] = bondings[bondName].get('options', None)
-    elif bondName in _netinfo.bondings:
-        # We may not receive any information about the bonding device if it is
-        # unchanged. In this case check whether this bond exists on host and
-        # take its parameters.
-        logger.debug("Fetching bond %r info", bondName)
-        existingBond = _netinfo.bondings[bondName]
-        bond['nics'] = existingBond['slaves']
-        bond['bondingOptions'] = existingBond['cfg'].get('BONDING_OPTS', None)
-    else:
-        raise ConfigNetworkError(ne.ERR_BAD_PARAMS, "No bonding option given, "
-                                 "nor existing bond %s found." % bondName)
-    return bond
-
-
 def _buildSetupHookDict(req_networks, req_bondings, req_options):
 
     hook_dict = {'request': {'networks': dict(req_networks),
@@ -675,17 +654,14 @@
         if 'remove' in attrs:
             continue
 
-        d = dict(attrs)
-        if 'bonding' in d:
-            d.update(_buildBondOptions(d['bonding'], bondings, _netinfo))
-        else:
-            d['nics'] = [d.pop('nic')] if 'nic' in d else []
-        d['force'] = force
+        _verify_bond_params(attrs, bondings, _netinfo)
+        net_attr = update_underlying_device_params(attrs, bondings, _netinfo)
+        net_attr['force'] = force
 
         logger.debug("Adding network %r", network)
         try:
             _addNetwork(network, configurator=configurator,
-                        implicitBonding=True, _netinfo=_netinfo, **d)
+                        implicitBonding=True, _netinfo=_netinfo, **net_attr)
         except ConfigNetworkError as cne:
             if cne.errCode == ne.ERR_FAILED_IFUP:
                 logger.debug("Adding network %r failed. Running "
@@ -697,6 +673,56 @@
         _netinfo.updateDevices()  # Things like a bond mtu can change
 
 
+def _verify_bond_params(attrs, bondings, _netinfo):
+    if 'bonding' in attrs:
+        bondName = attrs['bonding']
+        if bondName not in bondings and bondName not in _netinfo.bondings:
+            raise ConfigNetworkError(
+                ne.ERR_BAD_PARAMS, "No bonding option given, nor existing "
+                                   "bond %s found." % bondName)
+
+
+def update_underlying_device_params(attrs, bondings, _netinfo):
+    """update a specific network attributes with relevant bonding/ nic
+    info."""
+    net_attr = dict(attrs)
+    if 'bonding' in net_attr:
+        net_attr.update(build_bond_options(
+            net_attr['bonding'], bondings, _netinfo))
+    else:
+        net_attr['nics'] = (
+            [net_attr.pop('nic')] if 'nic' in net_attr else [])
+    return net_attr
+
+
+def build_bond_options(bond_name, bondings, _netinfo):
+    """If VDSM is alsed to cinfigure a network over a bonding there are 2
+    options:
+    1. VDSM needs to create this bond. This is indicated by describing the bond
+       and it's nics in the 'bonding' dictionary.
+    2. Bonding already exists on the host. This is indicated bu just naming
+       the bonding name in the network attributes dictionary. In this case
+       VDSM tries to get bonding options and nics from the kernel. User might
+       add extra configuration to change an existing bond.
+    Etherway, bonding name, options and nics are places in the network
+    attributes before given to the configurator. This is also how the network
+    is persisted VDSM.
+    """
+    bond = {}
+    non_existing_bond = bondings.get(bond_name)
+    if non_existing_bond:
+        bond['nics'] = non_existing_bond['nics']
+        bond['bondingOptions'] = non_existing_bond.get('options')
+    else:
+        # bondName must be in _netinfo.bondings at this point
+        logging.debug("Fetching bond %r info", bond_name)
+        existing_bond = _netinfo.bondings[bond_name]
+        bond['nics'] = existing_bond['slaves']
+        bond['bondingOptions'] = existing_bond['cfg'].get('BONDING_OPTS')
+
+    return bond
+
+
 def _get_connectivity_timeout(options):
     return int(options.get('connectivityTimeout',
                            CONNECTIVITY_TIMEOUT_DEFAULT))


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib054e144dccbff40b40b5967dca0bf840bce8ab9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to