Change in vdsm[master]: ifcfg: make conf file creation use device objects

asegurap at redhat.com asegurap at redhat.com
Thu Jun 26 09:42:47 UTC 2014


Antoni Segura Puimedon has uploaded a new change for review.

Change subject: ifcfg: make conf file creation use device objects
......................................................................

ifcfg: make conf file creation use device objects

Up until not the configuration file creation method was taking as
parameters several attributes of network models. That made the
calling methods have some needless code duplication for little
benefit. This patch turns that around and now it is possible for
configuration file creation to examine the object hierarchy in
case it needs to.

Change-Id: I3372cf088be58d178d2175715f5e08c80cd9e6d4
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
---
M vdsm/network/configurators/ifcfg.py
1 file changed, 50 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/43/29243/1

diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py
index e1c17a7..09b5ac8 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -548,63 +548,61 @@
             logging.debug('ignoring restorecon error in case '
                           'SElinux is disabled', exc_info=True)
 
-    @staticmethod
-    def _toIfcfgFormat(defaultRoute):
-        if defaultRoute is None:
-            return None
-        return 'yes' if defaultRoute else 'no'
+    _BLACKLIST = ('blockingdhcp', 'bondingOptions', 'connectivityCheck',
+                  'connectivityTimeout', 'custom', 'delay', 'DELAY', 'DEVICE',
+                  'force', 'forward_delay', 'implicitBonding', 'NAME',
+                  'onboot', 'ONBOOT', 'TYPE', 'VLAN')
 
-    def _createConfFile(self, conf, name, ipconfig, mtu=None, **kwargs):
-        """ Create ifcfg-* file with proper fields per device """
-
-        cfg = """DEVICE=%s\nONBOOT=%s\n""" % (
-            pipes.quote(name), 'no' if self.unifiedPersistence else 'yes')
-        cfg += conf
+    def _createConfFile(self, device, devconf='', ipconfig=None, mtu=None,
+                        **kwargs):
+        """Create the ifcfg file"""
+        if not ipconfig:
+            ipconfig = device.ipConfig
+        if not mtu:
+            mtu = device.mtu
+        cfg = ['DEVICE=%s' % pipes.quote(device.name),
+               'ONBOOT=%s' % 'no' if self.unifiedPersistence else 'yes',
+               'NM_CONTROLLED=no' % device]
+        if device.bridge:
+            cfg.append('BRIDGE=%s' % device.bridge.name)
         if ipconfig.ipaddr:
-            cfg = cfg + 'IPADDR=%s\n' % pipes.quote(ipconfig.ipaddr)
-            cfg = cfg + 'NETMASK=%s\n' % pipes.quote(ipconfig.netmask)
+            cfg += ['BOOTPROTO=none',
+                    'IPADDR=%s' % ipconfig.ipaddr,
+                    'NETMASK=%s' % ipconfig.netmask]
             if ipconfig.gateway:
-                cfg = cfg + 'GATEWAY=%s\n' % pipes.quote(ipconfig.gateway)
-            # According to manual the BOOTPROTO=none should be set
-            # for static IP
-            cfg = cfg + 'BOOTPROTO=none\n'
+                cfg.append('GATEWAY=%s' % ipconfig.gateway)
         elif ipconfig.bootproto:
-            cfg = cfg + 'BOOTPROTO=%s\n' % pipes.quote(ipconfig.bootproto)
+            cfg.append('BOOTPROTO=%s' % pipes.quote(ipconfig.bootproto))
             if (ipconfig.bootproto == 'dhcp' and
-                    os.path.exists(os.path.join(netinfo.NET_PATH, name))):
+                    os.path.exists(os.path.join(netinfo.NET_PATH,
+                                                device.name))):
                 # Ask dhclient to stop any dhclient running for the device
-                dhclient.kill_dhclient(name)
-
+                dhclient.kill_dhclient(device.name)
         if mtu:
-            cfg = cfg + 'MTU=%d\n' % mtu
+            cfg.append('MTU=%d')
         if ipconfig.defaultRoute:
-            cfg = cfg + 'DEFROUTE=%s\n' % ipconfig.defaultRoute
-        cfg += 'NM_CONTROLLED=no\n'
+            cfg.append('DEFROUTE=%s' %
+                       'yes' if ipconfig.defaultRoute else 'no')
         if ipconfig.ipv6addr or ipconfig.ipv6autoconf or ipconfig.dhcpv6:
-            cfg += 'IPV6INIT=yes\n'
+            cfg += ['IPV6INIT=yes',
+                    'IPV6_AUTOCONF=%s' %
+                    'yes' if ipconfig.ipv6autoconf else 'no']
             if ipconfig.ipv6addr is not None:
-                cfg += 'IPV6ADDR=%s\n' % pipes.quote(ipconfig.ipv6addr)
+                cfg.append('IPV6ADDR=%s' % pipes.quote(ipconfig.ipv6addr))
                 if ipconfig.ipv6gateway is not None:
-                    cfg += 'IPV6_DEFAULTGW=%s\n' % \
-                        pipes.quote(ipconfig.ipv6gateway)
+                    cfg.append('IPV6_DEFAULTGW=%s' %
+                               pipes.quote(ipconfig.ipv6gateway))
             elif ipconfig.dhcpv6:
-                cfg += 'DHCPV6C=yes\n'
-            if ipconfig.ipv6autoconf:
-                cfg += 'IPV6_AUTOCONF=%s\n' % 'yes'
-            else:
-                cfg += 'IPV6_AUTOCONF=%s\n' % 'no'
-        BLACKLIST = ['TYPE', 'NAME', 'DEVICE', 'VLAN', 'bondingOptions',
-                     'force', 'blockingdhcp', 'custom',
-                     'connectivityCheck', 'connectivityTimeout',
-                     'implicitBonding', 'delay', 'onboot', 'forward_delay',
-                     'DELAY', 'ONBOOT']
-        for k in set(kwargs.keys()).difference(set(BLACKLIST)):
-            if re.match('^[a-zA-Z_]\w*$', k):
-                cfg += '%s=%s\n' % (k.upper(), pipes.quote(kwargs[k]))
-            else:
-                logging.debug('ignoring variable %s', k)
+                cfg.append('DHCPV6C=yes')
 
-        self.writeConfFile(netinfo.NET_CONF_PREF + name, cfg)
+        for key in set(kwargs.keys()).difference(set(ConfigWriter._BLACKLIST)):
+            if re.match('^[a-zA-Z_]\w*$', key):
+                cfg.append('%s=%s\n' % (key.upper(), pipes.quote(kwargs[key])))
+            else:
+                logging.debug('ignoring variable %s', key)
+
+        self.writeConfFile(netinfo.NET_CONF_PREF + device.name,
+                           '\n'.join(cfg) + devconf)
 
     def addBridge(self, bridge, **opts):
         """ Create ifcfg-* file with proper fields for bridge """
@@ -612,34 +610,24 @@
         opts['hotplug'] = 'no'  # So that udev doesn't trigger an ifup
         if bridge.stp is not None:
             conf += 'STP=%s\n' % ('on' if bridge.stp else 'off')
-        ipconfig = bridge.ipConfig
-        defaultRoute = ConfigWriter._toIfcfgFormat(ipconfig.defaultRoute)
-        ipconfig = ipconfig._replace(defaultRoute=defaultRoute)
 
         if 'custom' in opts and 'bridge_opts' in opts['custom']:
             opts['bridging_opts'] = opts['custom']['bridge_opts']
-        self._createConfFile(conf, bridge.name, ipconfig, bridge.mtu, **opts)
+        self._createConfFile(bridge, devconf=conf, **opts)
 
     def addVlan(self, vlan, **opts):
         """ Create ifcfg-* file with proper fields for VLAN """
         conf = 'VLAN=yes\n'
         opts['hotplug'] = 'no'  # So that udev doesn't trigger an ifup
-        if vlan.bridge:
-            conf += 'BRIDGE=%s\n' % pipes.quote(vlan.bridge.name)
-        ipconfig = vlan.ipConfig
-        defaultRoute = ConfigWriter._toIfcfgFormat(ipconfig.defaultRoute)
-        ipconfig = ipconfig._replace(defaultRoute=defaultRoute)
-        self._createConfFile(conf, vlan.name, ipconfig, vlan.mtu, **opts)
+        self._createConfFile(vlan, devconf=conf, **opts)
 
     def addBonding(self, bond, **opts):
         """ Create ifcfg-* file with proper fields for bond """
         conf = 'BONDING_OPTS=%s\n' % pipes.quote(bond.options or '')
         opts['hotplug'] = 'no'  # So that udev doesn't trigger an ifup
-        if bond.bridge:
-            conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name)
-
-        ipconfig, mtu = self._getIfaceConfValues(bond)
-        self._createConfFile(conf, bond.name, ipconfig, mtu, **opts)
+        ipconf, mtu = self._getIfaceConfValues(bond)
+        self._createConfFile(bond, devconf=conf, ipconfig=ipconf, mtu=mtu,
+                             **opts)
 
         # create the bonding device to avoid initscripts noise
         if bond.name not in open(netinfo.BONDING_MASTERS).read().split():
@@ -655,8 +643,6 @@
                       _netinfo.nics[nic.name]['hwaddr'])
 
             conf += 'HWADDR=%s\n' % pipes.quote(hwaddr)
-        if nic.bridge:
-            conf += 'BRIDGE=%s\n' % pipes.quote(nic.bridge.name)
         if nic.bond:
             conf += 'MASTER=%s\nSLAVE=yes\n' % pipes.quote(nic.bond.name)
 
@@ -664,8 +650,9 @@
         if ethtool_opts:
             conf += 'ETHTOOL_OPTS=%s\n' % pipes.quote(ethtool_opts)
 
-        ipconfig, mtu = self._getIfaceConfValues(nic)
-        self._createConfFile(conf, nic.name, ipconfig, mtu, **opts)
+        ipconf, mtu = self._getIfaceConfValues(nic)
+        self._createConfFile(nic, devconf=conf, ipconfig=ipconf, mtu=mtu,
+                             **opts)
 
     @staticmethod
     def _getIfaceConfValues(iface):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3372cf088be58d178d2175715f5e08c80cd9e6d4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>


More information about the vdsm-patches mailing list