Change in vdsm[master]: net: only backup ifcfg files if they are not already persisted.

ibarkan at redhat.com ibarkan at redhat.com
Wed Jul 15 14:54:53 UTC 2015


Ido Barkan has uploaded a new change for review.

Change subject: net: only backup ifcfg files if they are not already persisted.
......................................................................

net: only backup ifcfg files if they are not already persisted.

d6454e introduced backing up of ifcfg files that were not owned by
vdsm before the current call. Now, after selective restoration was
introduced, when a network is restored because it is different
from it's persistence, it is edited. This is done by removing it
(and it's ifcfg files) an re-add it. The result is that ifcfgs get
backed up also because they were just deleted by the current call,
and are now re-added. We end up with file placeholders that make vdsm
ifdown them during restoration. This defeats the whole selective
restoration feature.
In order to preserve the original meaning of _backup, we now
compute all the possible ifcfg files that are part of unified
persistence and backup only the ones that are not.

Change-Id: I8d99efb8141cf7c6409050170c24714899c64be9
Signed-off-by: Ido Barkan <ibarkan at redhat.com>
---
M tests/functional/networkTests.py
M vdsm/network/configurators/ifcfg.py
2 files changed, 96 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/43684/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 6ccd114..efbc54b 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -35,7 +35,7 @@
 from vdsm.netinfo import (bridges, operstate, getRouteDeviceTo,
                           _get_dhclient_ifaces, BONDING_SLAVES,
                           BONDING_MASTERS, NET_CONF_PREF, OPERSTATE_UNKNOWN,
-                          OPERSTATE_UP)
+                          OPERSTATE_UP, NET_CONF_BACK_DIR)
 from vdsm.netlink import monitor
 from vdsm import sysctl
 from vdsm.utils import CommandPath, RollbackContext, execCmd, pgrep, running
@@ -1565,6 +1565,15 @@
                     self.vdsm_net.restoreNetConfig()
 
                 _assert_all_nets_exist()
+                # no ifcfg backups should be left now that all ifcfgs are owned
+                # by vdsm
+                self.assertEqual([], os.listdir(NET_CONF_BACK_DIR))
+                # another 'boot' should restore nothing
+                RunningConfig().delete()
+                with nonChangingOperstate(NET_UNCHANGED):
+                    with nonChangingOperstate(NET_CHANGED):
+                        with nonChangingOperstate(NET_MISSING):
+                            self.vdsm_net.restoreNetConfig()
             finally:
                 self.setupNetworks(
                     {NET_UNCHANGED: {'remove': True},
@@ -1623,24 +1632,33 @@
             self.assertBondExists(BOND_UNCHANGED, [nic_b])
             self.assertBondExists(BOND_CHANGED, [nic_c], options='mode=4')
 
-        def _simulate_boot_after_upgrade():
-            # all non-management devices are down and have ONBOOT=no from older
-            # vdsm versions.
+        def _simulate_boot(change_bond=False, after_upgrade=False):
             device_names = (NET_UNCHANGED, BOND_UNCHANGED, nic_b, NET_CHANGED,
                             BOND_CHANGED, nic_c)
-            stop_devices((NET_CONF_PREF + name for name in device_names))
+            if after_upgrade:
+                stop_devices((NET_CONF_PREF + name for name in device_names))
             for dev in device_names:
                 with open(NET_CONF_PREF + dev) as f:
                     content = f.read()
-                # also test the case that a bond is different from it's
-                # backup
-                content = re.sub('ONBOOT=yes', 'ONBOOT=no', content)
-                if dev == BOND_CHANGED:
+                if after_upgrade:
+                    # all non-management devices are down and have ONBOOT=no
+                    # from older vdsm versions.
+                    content = re.sub('ONBOOT=yes', 'ONBOOT=no', content)
+                if change_bond and dev == BOND_CHANGED:
+                    # also test the case that a bond is different from it's
+                    # backup
                     content = re.sub('mode=4', 'mode=0', content)
                 with open(NET_CONF_PREF + dev, 'w') as f:
                     f.write(content)
             # we don't have running config during boot
             RunningConfig().delete()
+
+        def _verify_running_config_intact():
+            self.assertEqual({NET_MGMT, NET_CHANGED, NET_UNCHANGED,
+                              NET_ADDITIONAL},
+                             set(self.vdsm_net.config.networks.keys()))
+            self.assertEqual({BOND_CHANGED, BOND_UNCHANGED},
+                             set(self.vdsm_net.config.bonds.keys()))
 
         with dummyIf(4) as nics:
             nic_a, nic_b, nic_c, nic_d = nics
@@ -1655,21 +1673,32 @@
             try:
                 self.vdsm_net.save_config()
 
-                _simulate_boot_after_upgrade()
+                _simulate_boot(change_bond=True, after_upgrade=True)
 
                 with nonChangingOperstate(NET_MGMT):
                     self.vdsm_net.restoreNetConfig()
+                # no ifcfg backups should be left now that all ifcfgs are owned
+                # by vdsm
+                self.assertEqual([], os.listdir(NET_CONF_BACK_DIR))
 
                 status, msg = self.setupNetworks(
                     {NET_ADDITIONAL: net_additional_attrs}, {}, NOCHK)
                 self.assertEquals(status, SUCCESS, msg)
                 _assert_all_nets_exist()
-                # verify that the running config now has all desired networks
-                self.assertEqual({NET_MGMT, NET_CHANGED, NET_UNCHANGED,
-                                  NET_ADDITIONAL},
-                                 set(self.vdsm_net.config.networks.keys()))
-                self.assertEqual({BOND_CHANGED, BOND_UNCHANGED},
-                                 set(self.vdsm_net.config.bonds.keys()))
+                _verify_running_config_intact()
+                self.assertEqual(['ifcfg-%s' % NET_ADDITIONAL,
+                                  'ifcfg-%s' % nic_d],
+                                 os.listdir(NET_CONF_BACK_DIR))
+
+                # another 'boot' should restore nothing
+                _simulate_boot()
+                with nonChangingOperstate(NET_MGMT):
+                    with nonChangingOperstate(NET_UNCHANGED):
+                        with nonChangingOperstate(NET_CHANGED):
+                            self.vdsm_net.restoreNetConfig()
+
+                _assert_all_nets_exist()
+                self.assertEqual([], os.listdir(NET_CONF_BACK_DIR))
 
             finally:
                 status, msg = self.setupNetworks(
diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py
index e9ef7cc..0b7d725 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -39,7 +39,7 @@
 from vdsm import netinfo
 from vdsm import sysctl
 from vdsm import utils
-from vdsm.netconfpersistence import RunningConfig
+from vdsm.netconfpersistence import RunningConfig, PersistentConfig
 
 if utils.isOvirtNode():
     from ovirt.node.utils import fs as node_fs
@@ -359,10 +359,7 @@
 
     def _backup(self, filename):
         self._atomicBackup(filename)
-        if config.get('vars', 'net_persistence') != 'unified':
-            self._persistentBackup(filename)
-        elif not self._ownedIfcfg(filename):
-            # Backup non-VDSM network devices (BZ#1188251)
+        if filename not in _get_unified_persistence_ifcfg():
             self._persistentBackup(filename)
 
     def _atomicBackup(self, filename):
@@ -855,3 +852,52 @@
 
 def _to_ifcfg_bool(value):
     return 'yes' if value else 'no'
+
+
+def _get_unified_persistence_ifcfg():
+    """generate the set of ifcfg files that result of the current unified
+    persistent networks"""
+    persistent_config = PersistentConfig()
+    if not persistent_config:
+        return set()
+
+    ROUTE_PREFIX = 'route-'
+    RULE_PREFIX = 'rule-'
+    ifcfgs = set()
+
+    for bonding, bonding_attr in persistent_config.bonds.iteritems():
+        bond_nics = set(bonding_attr.get('nics', []))
+        ifcfgs.add(bonding)
+        ifcfgs |= bond_nics
+
+    for network, network_attr in persistent_config.networks.iteritems():
+        top_level_device = None
+
+        nic = network_attr.get('nic')
+        if nic:
+            ifcfgs.add(nic)
+            top_level_device = nic
+
+        network_bonding = network_attr.get('bonding', None)
+        if network_bonding:
+            top_level_device = network_bonding
+
+        vlan_id = network_attr.get('vlan', '')
+        if vlan_id:
+            underlying_device = network_bonding or network_attr.get('nic', '')
+            vlan_device = '.'.join([underlying_device, str(vlan_id)])
+            top_level_device = vlan_device
+            ifcfgs.add(vlan_device)
+
+        if utils.tobool(network_attr.get('bridged', True)):
+            ifcfgs.add(network)
+            top_level_device = network
+
+        ifcfgs.add((ROUTE_PREFIX + '%s') % top_level_device)
+        ifcfgs.add((RULE_PREFIX + '%s') % top_level_device)
+
+    return set(
+        netinfo.NET_CONF_DIR + ifcfg
+        if (ifcfg.startswith(ROUTE_PREFIX) or ifcfg.startswith(RULE_PREFIX))
+        else netinfo.NET_CONF_PREF + ifcfg
+        for ifcfg in ifcfgs)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8d99efb8141cf7c6409050170c24714899c64be9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>


More information about the vdsm-patches mailing list