Change in vdsm[master]: net: flatten ifcfg.ConfigWriter
danken at redhat.com
danken at redhat.com
Wed Jul 8 14:37:26 UTC 2015
Dan Kenigsberg has posted comments on this change.
Change subject: net: flatten ifcfg.ConfigWriter
......................................................................
Patch Set 1:
(3 comments)
https://gerrit.ovirt.org/#/c/43338/1/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:
Line 699: for dev in reversed(deviceIfcfgs):
Line 700: ifdown(dev)
Line 701:
Line 702:
Line 703: def startDevices(deviceIfcfgs):
I think that we we make this function call sort_ifcfgs internally, we won't need to expose sort_ifcfgs nor exec_ifup, which gives us a cleaner API for this module.
Line 704: for dev in deviceIfcfgs:
Line 705: try:
Line 706: exec_ifup(dev)
Line 707: except ConfigNetworkError:
Line 708: logging.error('Failed to ifup device %s during rollback.', dev,
Line 709: exc_info=True)
Line 710:
Line 711:
Line 712: def sortDeviceIfcfgs(deviceIfcfgs):
since you are exposing these functions now, can you make them pep8-looking?
Line 713: devdict = {'Bridge': [],
Line 714: 'Vlan': [],
Line 715: 'Slave': [],
Line 716: 'Other': []}
Line 731:
Line 732: return devdict['Other'] + devdict['Vlan'] + devdict['Bridge']
Line 733:
Line 734:
Line 735: def devType(content):
this can be kept private even if we have to expose sortDeviceIfcfgs
Line 736: if re.search('^TYPE=Bridge$', content, re.MULTILINE):
Line 737: return "Bridge"
Line 738: elif re.search('^VLAN=yes$', content, re.MULTILINE):
Line 739: return "Vlan"
--
To view, visit https://gerrit.ovirt.org/43338
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I22676ee45dc7f4be7cd1b74f3f56305cc7e1a97a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list