Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.

Mark Wu has uploaded a new change for review.

Change subject: netconf: Improve unified persistence's rollback in memory.
......................................................................

netconf: Improve unified persistence's rollback in memory.

This patch removes the dependency on ifcfg's code for the unified
persistence's rollback in memory. It also move transaction management
implementaion from ifcfg to its base class Configurator. With this
change, the rollback function be reused for the iproute2 configuration.

Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668
Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
---
M lib/vdsm/constants.py.in
M vdsm/netconf/__init__.py
M vdsm/netconf/ifcfg.py
M vdsm/vdsm-restore-net-config
M vdsm/vdsm-restore-net-config.init.in
5 files changed, 62 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/41/19541/1

diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index cfea347..9f4dae6 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -138,6 +138,7 @@
 EXT_UNPERSIST = '@UNPERSIST_PATH@'
 
 EXT_VDSM_STORE_NET_CONFIG = '@VDSMDIR@/vdsm-store-net-config'
+EXT_VDSM_RESTORE_NET_CONFIG = '@VDSMDIR@/vdsm-restore-net-config'
 
 EXT_WGET = '@WGET_PATH@'
 
diff --git a/vdsm/netconf/__init__.py b/vdsm/netconf/__init__.py
index c0e3ee9..9dc0f70 100644
--- a/vdsm/netconf/__init__.py
+++ b/vdsm/netconf/__init__.py
@@ -18,13 +18,16 @@
 #
 
 import logging
+import pickle
 
 from netmodels import Bond, Bridge
 from sourceRoute import DynamicSourceRoute
 from sourceRoute import StaticSourceRoute
+from vdsm import constants
 from vdsm import netinfo
 from vdsm.config import config
 from vdsm.netconfpersistence import RunningConfig
+from vdsm.utils import execCmd
 
 
 class Configurator(object):
@@ -46,6 +49,28 @@
         else:
             self.rollback()
 
+    def begin(self, configApplierClass):
+        if self.configApplier is None:
+            self.configApplier = configApplierClass()
+        if self.unifiedPersistence and self.runningConfig is None:
+            self.runningConfig = RunningConfig()
+
+    def rollback(self):
+        if self.unifiedPersistence:
+            liveConfig = pickle.dumps(self.runningConfig)
+            execCmd([constants.EXT_VDSM_RESTORE_NET_CONFIG, '--memory',
+                     liveConfig])
+            self.runningConfig = None
+        else:
+            self.configApplier.restoreBackups()
+        self.configApplier = None
+
+    def commit(self):
+        self.configApplier = None
+        if self.unifiedPersistence:
+            self.runningConfig.save()
+            self.runningConfig = None
+
     def configureBridge(self, bridge, **opts):
         raise NotImplementedError
 
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index ec3966a..e3a2131 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -36,7 +36,6 @@
 from vdsm import constants
 from vdsm import netinfo
 from vdsm import utils
-from vdsm.netconfpersistence import RunningConfig
 import dsaversion
 import libvirtCfg
 import neterrors as ne
@@ -48,22 +47,7 @@
         super(Ifcfg, self).__init__(ConfigWriter())
 
     def begin(self):
-        if self.configApplier is None:
-            self.configApplier = ConfigWriter()
-        if self.unifiedPersistence and self.runningConfig is None:
-            self.runningConfig = RunningConfig()
-
-    def rollback(self):
-        self.configApplier.restoreBackups()
-        self.configApplier = None
-        if self.unifiedPersistence:
-            self.runningConfig = None
-
-    def commit(self):
-        self.configApplier = None
-        if self.unifiedPersistence:
-            self.runningConfig.save()
-            self.runningConfig = None
+        super(Ifcfg, self).begin(ConfigWriter)
 
     def configureBridge(self, bridge, **opts):
         self.configApplier.addBridge(bridge, **opts)
@@ -274,8 +258,8 @@
                       "(until next 'set safe config')", backup)
 
     def _networkBackup(self, network):
-        self._atomicNetworkBackup(network)
         if config.get('vars', 'persistence') != 'unified':
+            self._atomicNetworkBackup(network)
             self._persistentNetworkBackup(network)
 
     def _atomicNetworkBackup(self, network):
@@ -316,8 +300,8 @@
             logging.info('Restored %s', network)
 
     def _backup(self, filename):
-        self._atomicBackup(filename)
         if config.get('vars', 'persistence') != 'unified':
+            self._atomicBackup(filename)
             self._persistentBackup(filename)
 
     def _atomicBackup(self, filename):
diff --git a/vdsm/vdsm-restore-net-config b/vdsm/vdsm-restore-net-config
index 62045f3..0b2f11b 100755
--- a/vdsm/vdsm-restore-net-config
+++ b/vdsm/vdsm-restore-net-config
@@ -19,8 +19,11 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
+import getopt
 import logging
 import logging.config
+import pickle
+import sys
 
 from netconf import ifcfg
 from vdsm.config import config
@@ -33,23 +36,27 @@
     configWriter.restorePersistentBackup()
 
 
-def unified_restoration():
+def unified_restoration(fromConfig, toConfig):
     """
-    Builds a setupNetworks command from the persistent configuration to set it
-    as running configuration.
+    Builds a setupNetworks command to restore live network configuration to
+    the original state. For the transaction on disk, it restores configuration
+    from running config to persistence config. And for the transaction in
+    memory, it restores from all configured interface in a transaction to the
+    running config becore this transaction.
     """
-    runningConfig = RunningConfig()
-    networks = runningConfig.networks
-    removeNetworks = {}
-    for network in networks:
-        removeNetworks[network] = {'remove': True}
-    logging.debug("Removing all networks in running config: %s" %
-                  removeNetworks)
-    setupNetworks(removeNetworks, {}, connectivityCheck=False)
+    def _getNewConfigs(fromItems, toItems, isMemTrans):
+        itemsRemoved = dict((item, {'remove': True})
+                            for item in set(fromItems) - set(toItems))
+        items = itemsRemoved
+        if isMemTrans:
+            for name in toItems:
+                if name not in fromItems or fromItems[name] != toItems[name]:
+                    items[name] = toItems[name]
+        return items
 
-    persistentConfig = PersistentConfig()
-    nets = persistentConfig.networks
-    bonds = persistentConfig.bonds
+    isMemTrans = isinstance(toConfig, RunningConfig)
+    nets = _getNewConfigs(fromConfig.networks, toConfig.networks, isMemTrans)
+    bonds = _getNewConfigs(fromConfig.bonds, toConfig.bonds, isMemTrans)
     logging.debug("Calling setupNetworks with networks: %s, bondings: %s" %
                   (nets, bonds))
     setupNetworks(nets, bonds, connectivityCheck=False)
@@ -63,10 +70,17 @@
                             level=logging.DEBUG)
         logging.error("Could not init proper logging", exc_info=True)
 
-    if config.get('vars', 'persistence') == 'unified':
-        unified_restoration()
-    else:
-        ifcfg_restoration()
+    opts, args = getopt.getopt(sys.argv[1:], "dm:", ["disk",
"memory="])
+    for opt, arg in opts:
+        if opt in ('-m', '--memory'):
+            unified_restoration(pickle.loads(arg), RunningConfig())
+        elif opt in ('-d', '--disk'):
+            if config.get('vars', 'persistence') == 'unified':
+                unified_restoration(RunningConfig(), PersistentConfig())
+            else:
+                ifcfg_restoration()
+        else:
+            assert False, "Unhandled option"
 
 
 if __name__ == '__main__':
diff --git a/vdsm/vdsm-restore-net-config.init.in b/vdsm/vdsm-restore-net-config.init.in
index 83a437a..e26b89a 100644
--- a/vdsm/vdsm-restore-net-config.init.in
+++ b/vdsm/vdsm-restore-net-config.init.in
@@ -33,7 +33,7 @@
     # commands required in all Fedora initscripts
     start|restart|reload|force-reload|condrestart|try-restart)
         echo -n $"Running $prog $1: "
-        @VDSMDIR@/vdsm-restore-net-config
+        @VDSMDIR@/vdsm-restore-net-config --disk
         retval=$?
         echo
         ;;


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;

Mark Wu has posted comments on this change.

Change subject: netconf: Improve unified persistence's rollback in memory.
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

....................................................
File vdsm/vdsm-restore-net-config.init.in
Line 32: case "$1" in
Line 33:     # commands required in all Fedora initscripts
Line 34:     start|restart|reload|force-reload|condrestart|try-restart)
Line 35:         echo -n $"Running $prog $1: "
Line 36:         @VDSMDIR@/vdsm-restore-net-config --disk
also need this change for vdsm-restore-net-config.service. will fix it in next patch.
Line 37:         retval=$?
Line 38:         echo
Line 39:         ;;
Line 40:     stop|status)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: netconf: Improve unified persistence's rollback in memory.
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4619/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4541/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3734/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Mark Wu has abandoned this change.

Change subject: netconf: Improve unified persistence's rollback in memory.
......................................................................


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;
Gerrit-Reviewer: Assaf Muller <amuller(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server