[PATCH-next 04/40] lnst.Device.Device: improve cleanup data storage

From: Ondrej Lichtner

Storing cleanup data is moved into the NetTestSlave instead of the
device itself. This makes sure that the Device is returned to the state
it was in at the start of the recipe instead of at it's creation. This
has an impact on virtually created devices that can sometimes be created
with duplicated default values (e.g. name) that can conflict during
deconfiguration. But also on the generic sanity of the recipe in case
someone does some manual changes between recipes while the slave is
running, deconfiguration could have returned them back.

Finally, instead of storing cleanup variables individually, store them
in a single directory. This should also improve the implementation of
the cleanup method and make it easier to add more attributes when
necessary.

Signed-off-by: Ondrej Lichtner 
---
 lnst/Devices/Device.py     | 37 ++++++++++++++++++++++++-------------
 lnst/Slave/NetTestSlave.py |  6 ++++++
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py
index 5ee3a17..3782c95 100644
--- a/lnst/Devices/Device.py
+++ b/lnst/Devices/Device.py
@@ -62,6 +62,8 @@ class Device(object):
         self._nl_update = {}
         self._bulk_enabled = False

+        self._cleanup_data = None
+
     def _set_nl_attr(self, msg, value, name):
         msg[name] = value

@@ -178,7 +180,6 @@ class Device(object):
         self.ifindex = nl_msg['index']

self._nl_msg = nl_msg
-        self._store_cleanup_data()

def _update_netlink(self, nl_msg):
         if self.ifindex != nl_msg['index']:
@@ -255,22 +256,32 @@ class Device(object):
         for egress_pref in egress_prefs:
             exec_cmd("tc filter del dev %s pref %s" % (self.name, egress_pref))

-    def _store_cleanup_data(self):
+    def store_cleanup_data(self):
         """Stores initial configuration for later cleanup"""
-        self._orig_mtu = self.mtu
-        self._orig_name = self.name
-        self._orig_hwaddr = self.hwaddr
+        if self._cleanup_data:
+            logging.debug("Previous cleanup data present, possible deconfigration failure in the past?")
+
+        self._cleanup_data = {
+                "mtu": self.mtu,
+                "name": self.name,
+                "hwaddr": self.hwaddr}

-    def _restore_original_data(self):
+    def restore_original_data(self):
         """Restores initial configuration from stored values"""
-        if self.mtu != self._orig_mtu:
-            self.mtu = self._orig_mtu
+        if not self._cleanup_data:
+            logging.debug("No cleanup data present")
+            return
+
+        if self.mtu != self._cleanup_data["mtu"]:
+            self.mtu = self._cleanup_data["mtu"]
+
+        if self.name != self._cleanup_data["name"]:
+            self.name = self._cleanup_data["name"]

-        if self.name != self._orig_name:
-            self.name = self._orig_name
+        if self.hwaddr != self._cleanup_data["hwaddr"]:
+            self.hwaddr = self._cleanup_data["hwaddr"]

-        if self.hwaddr != self._orig_hwaddr:
-            self.hwaddr = self._orig_hwaddr
+        self._cleanup_data = None

def _create(self):
         """Creates a new netdevice of the corresponding type
@@ -302,7 +313,7 @@ class Device(object):
         self.ip_flush()
         self._clear_tc_qdisc()
         self._clear_tc_filters()
-        self._restore_original_data()
+        self.restore_original_data()

@property
     def link_header_type(self):
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
index 1bb8f23..d99fc54 100644
--- a/lnst/Slave/NetTestSlave.py
+++ b/lnst/Slave/NetTestSlave.py
@@ -129,6 +129,12 @@ class SlaveMethods:
                 logging.warning("Usage of NM is disabled!")
             logging.warning("=============================================")

+        for device in self._if_manager.get_devices():
+            try:
+                device.store_cleanup_data()
+            except DeviceDisabled:
+                pass
+
         return True

def bye(self):