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