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(a)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(a)redhat.com>
Xavi Francisco has uploaded a new change for review.
Change subject: vm: Modify and save state after hotunplugging the disk
......................................................................
vm: Modify and save state after hotunplugging the disk
The rationale behind this patch is to modify the way the hotunplug
process is executed. Now when the hotunplug command is called we first
remove the disk from the internal state and after that the libvirt
hotunplug command is executed.The problem with that approach is
that if VDSM restarts between the modification of the internal
state and the actual unplugging of the disk, the disk never gets
unplugged leaving the system in an inconsistent state.
This patch solves the issue by executing the modification of the
internal state after the actual unplugging has successfully happened.
This state is later saved so if either the disk has been unplugged or
not VDSM is able to recover its state in case of a restart.
Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Signed-off-by: Xavi Francisco <xfrancis(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 12 insertions(+), 20 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/28187/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index bd670b9..1eb2adc 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -3449,20 +3449,6 @@
driveXml = drive.getXML().toprettyxml(encoding='utf-8')
self.log.debug("Hotunplug disk xml: %s", driveXml)
# Remove found disk from vm's drives list
- if isVdsmImage(drive):
- self.sdIds.remove(drive.domainID)
- self._devices[DISK_DEVICES].remove(drive)
- # Find and remove disk device from vm's conf
- diskDev = None
- for dev in self.conf['devices'][:]:
- if (dev['type'] == DISK_DEVICES and
- dev['path'] == drive.path):
- with self._confLock:
- self.conf['devices'].remove(dev)
- diskDev = dev
- break
-
- self.saveState()
hooks.before_disk_hotunplug(driveXml, self.conf,
params=drive.custom)
@@ -3472,19 +3458,25 @@
self.log.error("Hotunplug failed", exc_info=True)
if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
return errCode['noVM']
- self._devices[DISK_DEVICES].append(drive)
- # Restore disk device in vm's conf and _devices
- if diskDev:
- with self._confLock:
- self.conf['devices'].append(diskDev)
- self.saveState()
return {
'status': {'code': errCode['hotunplugDisk']['status']['code'],
'message': e.message}}
else:
+ if isVdsmImage(drive):
+ self.sdIds.remove(drive.domainID)
+ self._devices[DISK_DEVICES].remove(drive)
+ # Find and remove disk device from vm's conf
+ for dev in self.conf['devices'][:]:
+ if (dev['type'] == DISK_DEVICES and
+ dev['path'] == drive.path):
+ with self._confLock:
+ self.conf['devices'].remove(dev)
+ break
hooks.after_disk_hotunplug(driveXml, self.conf,
params=drive.custom)
self._cleanupDrives(drive)
+ finally:
+ self.saveState()
return {'status': doneCode, 'vmList': self.status()}
--
To view, visit http://gerrit.ovirt.org/28187
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com>