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