Change in vdsm[master]: net: only backup ifcfg files if they are not already persisted.

ibarkan at redhat.com ibarkan at redhat.com
Wed Jul 15 16:57:51 UTC 2015


Ido Barkan has posted comments on this change.

Change subject: net: only backup ifcfg files if they are not already persisted.
......................................................................


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/43684/2/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:

Line 882:         if network_bonding:
Line 883:             top_level_device = network_bonding
Line 884: 
Line 885:         vlan_id = network_attr.get('vlan', '')
Line 886:         if vlan_id:
> I'd like to be more prudent in the future, and make sure vlan_id variables 
Done
Line 887:             underlying_device = network_bonding or network_attr.get('nic', '')
Line 888:             vlan_device = '.'.join([underlying_device, str(vlan_id)])
Line 889:             top_level_device = vlan_device
Line 890:             ifcfgs.add(vlan_device)


Line 896:         ifcfgs.add((ROUTE_PREFIX + '%s') % top_level_device)
Line 897:         ifcfgs.add((RULE_PREFIX + '%s') % top_level_device)
Line 898: 
Line 899:     return set(
Line 900:         netinfo.NET_CONF_DIR + ifcfg
> we've had painful bugs due to trailing slashes. Use os.path.join() when pos
Done
Line 901:         if (ifcfg.startswith(ROUTE_PREFIX) or ifcfg.startswith(RULE_PREFIX))
Line 902:         else netinfo.NET_CONF_PREF + ifcfg


Line 898: 
Line 899:     return set(
Line 900:         netinfo.NET_CONF_DIR + ifcfg
Line 901:         if (ifcfg.startswith(ROUTE_PREFIX) or ifcfg.startswith(RULE_PREFIX))
Line 902:         else netinfo.NET_CONF_PREF + ifcfg
> I think that adding ROUTE/RULE files into ifcfgs only adds complexity. Inst
not sure if this is exactly what you meant but string formatting can make it simpler.


-- 
To view, visit https://gerrit.ovirt.org/43684
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8d99efb8141cf7c6409050170c24714899c64be9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list