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