Change in vdsm[master]: Don't down bonds and nics when adding vlan or resizing
wudxw at linux.vnet.ibm.com
wudxw at linux.vnet.ibm.com
Tue Aug 13 08:19:14 UTC 2013
Mark Wu has posted comments on this change.
Change subject: Don't down bonds and nics when adding vlan or resizing
......................................................................
Patch Set 7: Code-Review-1
(5 comments)
....................................................
File lib/vdsm/netinfo.py
Line 223: with open(path) as optFile:
Line 224: optName = os.path.basename(path)
Line 225: for optValue in optFile.read().rstrip().split(' '):
Line 226: opts.append((optName, optValue))
Line 227: return frozenset(opts)
I prefer this alternative.
Line 228:
Line 229:
Line 230: def ports(bridge):
Line 231: return os.listdir('/sys/class/net/' + bridge + '/brif')
Line 223: with open(path) as optFile:
Line 224: optName = os.path.basename(path)
Line 225: for optValue in optFile.read().rstrip().split(' '):
Line 226: opts.append((optName, optValue))
Line 227: return frozenset(opts)
There're many bonding options you can tune for a bonding. You could check them out under /sys/class/net/bondX/bonding. Even though only a few of them is frequently used, but we still need support the other less used options. So namedtuple is not a good choice.
Line 228:
Line 229:
Line 230: def ports(bridge):
Line 231: return os.listdir('/sys/class/net/' + bridge + '/brif')
....................................................
File vdsm/netconf/ifcfg.py
Line 102:
Line 103: # Create bond configuration in case it was a non ifcfg controlled bond
Line 104: # Needed to be before slave configuration for initscripts to add slave
Line 105: # to bond.
Line 106: is_ifcfg_controlled = netinfo.getIfaceCfg(bond.name)
Is it more clear to add another function to check if an interface is controlled?
you could simply check if the ifcfg exists.
Line 107: are_options_applied = bond.are_options_applied()
Line 108: if not is_ifcfg_controlled or not are_options_applied:
Line 109: bridgeName = _netinfo.getBridgedNetworkForIface(bond.name)
Line 110: if bridgeName:
Line 105: # to bond.
Line 106: is_ifcfg_controlled = netinfo.getIfaceCfg(bond.name)
Line 107: are_options_applied = bond.are_options_applied()
Line 108: if not is_ifcfg_controlled or not are_options_applied:
Line 109: bridgeName = _netinfo.getBridgedNetworkForIface(bond.name)
if it's not vdsm controlled, you could skip the check of bridge master.
Line 110: if bridgeName:
Line 111: bond.master = Bridge(bridgeName, self, port=bond)
Line 112: self.configApplier.addBonding(bond)
Line 113:
Line 116: ifdown(slave.name) # nics must be down to join a bond
Line 117: self.configApplier.addNic(slave)
Line 118: ifup(slave.name)
Line 119:
Line 120: if not is_ifcfg_controlled or not are_options_applied:
why does the case of is_ifcfg_controlled need re-activate the bond device?
Line 121: ifdown(bond.name)
Line 122: ifup(bond.name)
Line 123:
Line 124: def configureNic(self, nic, **opts):
--
To view, visit http://gerrit.ovirt.org/17491
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3075a2e65f06416c840c6a98689b77555b22e5d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Petr Ĺ ebek <psebek at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list