Change in vdsm[master]: split network restoration from vdsmd.service
ibarkan at redhat.com
ibarkan at redhat.com
Thu Mar 12 08:03:08 UTC 2015
Ido Barkan has posted comments on this change.
Change subject: split network restoration from vdsmd.service
......................................................................
Patch Set 9:
(6 comments)
https://gerrit.ovirt.org/#/c/29441/9/init/systemd/vdsm-network.service.in
File init/systemd/vdsm-network.service.in:
Line 1: [Unit]
I think this unit file should be renamed to vdsm-network-restoration
Line 2: Description=Virtual Desktop Server Manager network restoration
Line 3: Wants=network.target
Line 4: Requires=vdsm-network-cleanup.service libvirtd.service
Line 5: After=libvirtd.service
Line 4: Requires=vdsm-network-cleanup.service libvirtd.service
Line 5: After=libvirtd.service
Line 6:
Line 7: [Service]
Line 8: Type=oneshot
should we use Type=forking here?
Line 9: EnvironmentFile=-/etc/sysconfig/vdsm
Line 10: ExecStartPre=@BINDIR@/vdsm-tool --vvverbose --append --logfile=@VDSMLOGDIR@/upgrade.log upgrade-unified-persistence
Line 11: ExecStartPre=@BINDIR@/vdsm-tool --vvverbose --append --logfile=@VDSMLOGDIR@/upgrade.log upgrade-3.0.0-networks
Line 12: ExecStart=@VDSMDIR@/vdsm-restore-net-config reconfigure --no-flush
Line 9: EnvironmentFile=-/etc/sysconfig/vdsm
Line 10: ExecStartPre=@BINDIR@/vdsm-tool --vvverbose --append --logfile=@VDSMLOGDIR@/upgrade.log upgrade-unified-persistence
Line 11: ExecStartPre=@BINDIR@/vdsm-tool --vvverbose --append --logfile=@VDSMLOGDIR@/upgrade.log upgrade-3.0.0-networks
Line 12: ExecStart=@VDSMDIR@/vdsm-restore-net-config reconfigure --no-flush
Line 13: RemainAfterExit=yes
why oneshot+ remainAfterExit is this for faster accomplishment of the restoration?
remainAfterExit behavioris very confusing
Line 14:
Line 15: # Allow systemctl reload
Line 16: ExecReload=/usr/share/vdsm/vdsm-restore-net-config reconfigure
Line 17:
https://gerrit.ovirt.org/#/c/29441/9/vdsm/vdsm-restore-net-config
File vdsm/vdsm-restore-net-config:
Line 59: for bond in runningConfig.bonds:
Line 60: removeBonds[bond] = {'remove': True}
Line 61: logging.debug('Removing all networks (%s) and bonds (%s) in running '
Line 62: 'config.', removeNetworks, removeBonds)
Line 63: if removeNetworks or removeBonds:
Is this just a small optimization?
Line 64: setupNetworks(removeNetworks, removeBonds, connectivityCheck=False,
Line 65: _inRollback=True)
Line 66:
Line 67: # Flush vdsm configurations left-overs from any configurator on the system
Line 75: nets, bonds = _filter_nets_bonds(persistentConfig.networks,
Line 76: persistentConfig.bonds)
Line 77: logging.debug('Calling setupNetworks with networks (%s) and bond (%s).',
Line 78: nets, bonds)
Line 79: if nets or bonds:
again, just an optimization?
Line 80: setupNetworks(nets, bonds, connectivityCheck=False, _inRollback=True)
Line 81:
Line 82:
Line 83: def _filter_nets_bonds(nets, bonds):
Line 129: if sys.modules[cls.__module__].is_available():
Line 130: yield cls
Line 131:
Line 132:
Line 133: def _configurator_flush(**opts):
is this really needs to be a part of this patch? or can it be separated?
Can you explain the purpose of it?
Line 134: """Flush vdsm configurations left-overs from any configurator on the system
Line 135: so that changes of configurator and persistence system are smooth. Only
Line 136: done if upgrade to unified persistence is not pending"""
Line 137: if (not upgrade._upgrade_needed(
--
To view, visit https://gerrit.ovirt.org/29441
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I70cdf1e8a1e7063ec850081ac6199728d4cde381
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <toni+ovirt at midokura.com>
Gerrit-Reviewer: Antoni Segura Puimedon <toni+ovirt at midokura.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list