Change in vdsm[master]: Network: Flush all configurators before restoring networks

ibarkan at redhat.com ibarkan at redhat.com
Mon Mar 16 13:33:36 UTC 2015


Ido Barkan has uploaded a new change for review.

Change subject: Network: Flush all configurators before restoring networks
......................................................................

Network: Flush all configurators before restoring networks

Right now, when there are many networks network service takes time to start
up (think DHCP). After it is up, VDSM removes all networks and configures
it's own persistent networks, which can also take quite some time.
This patch teaches 'vdsm-restore-net-config' to conditionally flush the
network configuration and splits this behaviour to a unit that systemd will
start before the network service.

systemd will now manage the following chain of tasks:
1. vdsm-network-cleanup.service will cleanup system files that configurators may
   have written on boot before network configuration daemons run, so no time is
   lost by them configuring vdsm networks (and conflict avoidance).
2. after libvirt is up, reconfigure the persistent network configuration of vdsm
   using the configured net_configurator.
3. start vdsm and super-vdsm

Note that it would be possible with this change to remove from vdsmd.service
the after vdsm-network.service so that vdsm would start in parallel to the
networks being set up. This could be quite useful so that the engine could
communicate with vdsm as soon as the management network is restored. However,
for now, the engine would see the host unsynched and we have to think carefully
if that is okay and how long that would take.

Change-Id: I9e3d0fb35d608a8b019ad80f7e40f4399e7db479
Signed-off-by: Ido Barkan <ibarkan at redhat.com>
---
M .gitignore
M init/systemd/Makefile.am
A init/systemd/vdsm-network-cleanup.service.in.py
M init/systemd/vdsm-network.service.in
M init/sysvinit/vdsmd.init.in
M lib/vdsm/tool/restore_nets.py
M vdsm.spec.in
M vdsm/network/configurators/__init__.py
M vdsm/vdsm-restore-net-config
9 files changed, 56 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/38767/1

diff --git a/.gitignore b/.gitignore
index 54cbc46..4245b44 100644
--- a/.gitignore
+++ b/.gitignore
@@ -26,6 +26,7 @@
 init/systemd/systemd-vdsmd
 init/systemd/vdsm-tmpfiles.d.conf
 init/systemd/vdsmd.service
+init/systemd/vdsm-network-cleanup.service
 init/systemd/vdsm-network.service
 init/sysvinit/supervdsmd.init
 init/sysvinit/vdsmd.init
diff --git a/init/systemd/Makefile.am b/init/systemd/Makefile.am
index 07c4ae8..000e267 100644
--- a/init/systemd/Makefile.am
+++ b/init/systemd/Makefile.am
@@ -37,6 +37,7 @@
 	supervdsmd.service.in \
 	systemd-vdsmd.in \
 	vdsmd.service.in \
+	vdsm-network-cleanup.service.in \
 	vdsm-network.service.in \
 	vdsm-tmpfiles.d.conf.in \
 	$(NULL)
diff --git a/init/systemd/vdsm-network-cleanup.service.in.py b/init/systemd/vdsm-network-cleanup.service.in.py
new file mode 100644
index 0000000..b2c8dab
--- /dev/null
+++ b/init/systemd/vdsm-network-cleanup.service.in.py
@@ -0,0 +1,13 @@
+[Unit]
+Description=Virtual Desktop Server Manager network system files cleanup
+Wants=network.target
+Before=network.target network.service NetworkManager.service systemd-networkd.service
+
+[Service]
+Type=oneshot
+EnvironmentFile=-/etc/sysconfig/vdsm
+ExecStart=@VDSMDIR@/vdsm-restore-net-config flush
+RemainAfterExit=yes
+
+[Install]
+WantedBy=multi-user.target
\ No newline at end of file
diff --git a/init/systemd/vdsm-network.service.in b/init/systemd/vdsm-network.service.in
index 9eb3e5e..9570ef5 100644
--- a/init/systemd/vdsm-network.service.in
+++ b/init/systemd/vdsm-network.service.in
@@ -1,7 +1,7 @@
 [Unit]
 Description=Virtual Desktop Server Manager network restoration
 Wants=network.target
-Requires=libvirtd.service
+Requires=vdsm-network-cleanup.service libvirtd.service
 After=libvirtd.service
 
 [Service]
@@ -9,7 +9,7 @@
 EnvironmentFile=-/etc/sysconfig/vdsm
 ExecStartPre=@BINDIR@/vdsm-tool --vvverbose --append --logfile=@VDSMLOGDIR@/upgrade.log upgrade-unified-persistence
 ExecStartPre=@BINDIR@/vdsm-tool --vvverbose --append --logfile=@VDSMLOGDIR@/upgrade.log upgrade-3.0.0-networks
-ExecStart=@VDSMDIR@/vdsm-restore-net-config
+ExecStart=@VDSMDIR@/vdsm-restore-net-config restore --no-flush
 RemainAfterExit=yes
 
 [Install]
diff --git a/init/sysvinit/vdsmd.init.in b/init/sysvinit/vdsmd.init.in
index 1f8dc64..409ee48 100755
--- a/init/sysvinit/vdsmd.init.in
+++ b/init/sysvinit/vdsmd.init.in
@@ -142,7 +142,7 @@
 }
 
 restore_nets(){
-    "@PYTHON@" "@VDSMDIR@/vdsm-restore-net-config" || return 1
+    "@PYTHON@" "@VDSMDIR@/vdsm-restore-net-config" restore || return 1
     return 0
 }
 
diff --git a/lib/vdsm/tool/restore_nets.py b/lib/vdsm/tool/restore_nets.py
index 99ea9f2..861bf01 100644
--- a/lib/vdsm/tool/restore_nets.py
+++ b/lib/vdsm/tool/restore_nets.py
@@ -38,7 +38,7 @@
 
 def restore():
     rc, out, err = utils.execCmd(
-        [os.path.join(P_VDSM, 'vdsm-restore-net-config')], raw=True)
+        [os.path.join(P_VDSM, 'vdsm-restore-net-config'), 'restore'], raw=True)
     sys.stdout.write(out)
     sys.stderr.write(err)
     if rc != 0:
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 819178c..e369b33 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -762,6 +762,7 @@
 %if 0%{?with_systemd}
 install -Dm 0755 init/systemd/systemd-vdsmd %{buildroot}/usr/lib/systemd/systemd-vdsmd
 install -Dm 0644 init/systemd/vdsmd.service %{buildroot}%{_unitdir}/vdsmd.service
+install -Dm 0644 init/systemd/vdsm-network-cleanup.service %{buildroot}%{_unitdir}/vdsm-network-cleanup.service
 install -Dm 0644 init/systemd/vdsm-network.service %{buildroot}%{_unitdir}/vdsm-network.service
 install -Dm 0644 init/systemd/supervdsmd.service %{buildroot}%{_unitdir}/supervdsmd.service
 
@@ -868,6 +869,7 @@
 /bin/systemctl restart systemd-modules-load.service >/dev/null 2>&1 || :
 if [ "$1" -eq 1 ] ; then
     /bin/systemctl enable vdsmd.service >/dev/null 2>&1 || :
+    /bin/systemctl enable vdsm-network-cleanup.service >/dev/null 2>&1 || :
     /bin/systemctl enable vdsm-network.service >/dev/null 2>&1 || :
     /bin/systemctl enable supervdsmd.service >/dev/null 2>&1 || :
 fi
@@ -905,6 +907,7 @@
 %if 0%{?with_systemd}
 %systemd_preun vdsmd.service
 %systemd_preun vdsm-network.service
+%systemd_preun vdsm-network-cleanup.service
 %systemd_preun supervdsmd.service
 %else
 if [ "$1" -eq 0 ]; then
@@ -912,6 +915,7 @@
     /bin/systemctl --no-reload disable supervdsmd.service > /dev/null 2>&1 || :
     /bin/systemctl stop vdsmd.service > /dev/null 2>&1 || :
     /bin/systemctl stop vdsm-network.service > /dev/null 2>&1 || :
+    /bin/systemctl stop vdsm-network-cleanup.service > /dev/null 2>&1 || :
     /bin/systemctl stop supervdsmd.service > /dev/null 2>&1 || :
 fi
 exit 0
@@ -1015,6 +1019,7 @@
 %if 0%{?with_systemd}
 /usr/lib/systemd/systemd-vdsmd
 %{_unitdir}/vdsmd.service
+%{_unitdir}/vdsm-network-cleanup.service
 %{_unitdir}/vdsm-network.service
 %{_unitdir}/supervdsmd.service
 %else
diff --git a/vdsm/network/configurators/__init__.py b/vdsm/network/configurators/__init__.py
index 39ad9a6..ce6ce05 100644
--- a/vdsm/network/configurators/__init__.py
+++ b/vdsm/network/configurators/__init__.py
@@ -70,7 +70,7 @@
         return RunningConfig().diffFrom(self.runningConfig)
 
     def flush(self):
-        libvirt.flush()
+        pass
 
     def configureBridge(self, bridge, **opts):
         raise NotImplementedError
diff --git a/vdsm/vdsm-restore-net-config b/vdsm/vdsm-restore-net-config
index 8ffb880..bcd3924 100755
--- a/vdsm/vdsm-restore-net-config
+++ b/vdsm/vdsm-restore-net-config
@@ -28,6 +28,7 @@
 from vdsm.config import config
 from vdsm import netinfo
 from vdsm.constants import P_VDSM_RUN
+from vdsm.tool import unified_persistence, upgrade
 
 # Ifcfg persistence restoration
 from network.configurators import ifcfg
@@ -35,6 +36,7 @@
 # Unified persistence restoration
 from network.api import setupNetworks
 from network import configurators
+from network.configurators import libvirt
 from vdsm.netconfpersistence import RunningConfig, PersistentConfig
 import pkgutil
 
@@ -46,7 +48,7 @@
     configWriter.restorePersistentBackup()
 
 
-def unified_restoration():
+def unified_restoration(flush_configurators):
     """
     Builds a setupNetworks command from the persistent configuration to set it
     as running configuration.
@@ -65,8 +67,9 @@
 
     # Flush vdsm configurations left-overs from any configurator on the system
     # so that changes of configurator and persistence system are smooth.
-    for configurator_cls in _get_all_configurators():
-        configurator_cls().flush()
+    if flush_configurators:
+        _configurator_flush()
+    libvirt.flush()
 
     # Restore non-VDSM network devices (BZ#1188251)
     configWriter = ifcfg.ConfigWriter()
@@ -143,17 +146,27 @@
         os.utime(file_path, None)
 
 
-def restore(force_restore=False):
+def restore(force_restore=False, flush_configurators=True):
     if not force_restore and _nets_already_restored(_NETS_RESTORED_MARK):
         logging.info('networks already restored. doing nothing.')
         return
 
     if config.get('vars', 'net_persistence') == 'unified':
-        unified_restoration()
+        unified_restoration(flush_configurators)
     else:
         ifcfg_restoration()
 
     touch_file(_NETS_RESTORED_MARK)
+
+
+def _configurator_flush():
+    """Flush vdsm configurations left-overs from any configurator on the system
+    so that changes of configurator and persistence system are smooth. Only
+    done if upgrade to unified persistence is not pending"""
+    if (not upgrade._upgrade_needed(
+            unified_persistence.UpgradeUnifiedPersistence)):
+        for configurator_cls in _get_all_configurators():
+            configurator_cls().flush()
 
 
 if __name__ == '__main__':
@@ -166,14 +179,25 @@
                             level=logging.DEBUG)
         logging.error('Could not init proper logging', exc_info=True)
 
-    parser = optparse.OptionParser("usage: %prog [options] nets_restored_mark")
+    parser = optparse.OptionParser("usage: %prog restore|flush [options]")
     force_option_help = "the restore action first tests for an existence of " \
                         "a mark that is made after the last successful " \
                         "restore action. Unless this option is used, " \
                         "restore will be a no-op if this mark exists."
     parser.add_option('--force', action='store_true', default=False,
                       help=force_option_help)
+    parser.add_option('--no-flush', dest='flush_configurators', default=True,
+                      action='store_false')
 
     options, args = parser.parse_args()
 
-    restore(options.force)
+    if len(args) != 1 or args[0] not in ['restore', 'flush']:
+        parser.error("Wrong number of arguments. only restore or flush are "
+                     "allowed")
+    arg, = args
+    if arg == 'restore':
+        restore(options.force, options.flush_configurators)
+    elif arg == 'flush':
+        if not options.flush_configurators:
+            parser.error('flush cannot be used with --no-flush')
+        _configurator_flush()


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9e3d0fb35d608a8b019ad80f7e40f4399e7db479
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