Mark Wu has uploaded a new change for review. Change subject: Make vdsm/configNetwork.py PEP8 clean ......................................................................
Make vdsm/configNetwork.py PEP8 clean Change-Id: I02be4c0cdb1868a8518a73e22dfb77adedc08f3f Signed-off-by: Mark Wu <[email protected]> --- M Makefile.am M vdsm/configNetwork.py 2 files changed, 106 insertions(+), 42 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/58/7758/1 diff --git a/Makefile.am b/Makefile.am index 40d7658..4900cc5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -47,6 +47,7 @@ vdsm/blkid.py \ vdsm/caps.py \ vdsm/clientIF.py \ + vdsm/configNetwork.py \ vdsm/constants.py.in \ vdsm/debugPluginClient.py \ vdsm/define.py \ diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py index 74d19c7..df807bf 100755 --- a/vdsm/configNetwork.py +++ b/vdsm/configNetwork.py @@ -17,7 +17,11 @@ # Refer to the README and COPYING files for full details of the license # -import sys, subprocess, os, re, traceback +import sys +import subprocess +import os +import re +import traceback import pipes import pwd import time @@ -42,11 +46,13 @@ MAX_BRIDGE_NAME_LEN = 15 ILLEGAL_BRIDGE_CHARS = frozenset(':. \t') + class ConfigNetworkError(Exception): def __init__(self, errCode, message): self.errCode = errCode self.message = message Exception.__init__(self, self.errCode, self.message) + def ifdown(iface): "Bring down an interface" @@ -60,6 +66,7 @@ if not line.endswith(' does not exist!')])) return p.returncode + def ifup(iface): "Bring up an interface" p = subprocess.Popen([constants.EXT_IFUP, iface], stdout=subprocess.PIPE, @@ -69,6 +76,7 @@ logging.info(out) if err.strip(): logging.warn(err) + def ifaceUsers(iface): "Returns a list of entities using the interface" @@ -87,8 +95,12 @@ users.add(v) return users + def nicOtherUsers(bridge, vlan, bonding, nic): - "Returns a list of interfaces using a nic, other than the specified one (used for validation)" + """ + Returns a list of interfaces using a nic, + other than the specified one (used for validation) + """ if bonding: owner = bonding elif vlan: @@ -101,8 +113,12 @@ users.discard(owner) return users + def bondingOtherUsers(bridge, vlan, bonding): - "Return a list of nics/interfaces using a bonding, other than the specified one (used for validation)" + """ + Return a list of nics/interfaces using a bonding, + other than the specified one (used for validation) + """ if vlan: owner = bonding + '.' + vlan else: @@ -123,6 +139,8 @@ # nicSort(["p33p2", "p33p10"]) => ["p33p10", "p33p2"] # nicSort(["p331", "p33p1"]) => ["p33p1", "p331"] # + + def nicSort(nics): "Return a list of nics/interfaces ordered by name" @@ -141,6 +159,7 @@ nics_list.append((prefix, intidx, stridx + postfix)) return [x + z for x, y, z in sorted(nics_list)] + class ConfigWriter(object): NET_CONF_PREF = netinfo.NET_CONF_DIR + 'ifcfg-' @@ -274,7 +293,10 @@ self._persistentBackup(filename) def _atomicBackup(self, filename): - """Backs up configuration to memory, for a later rollback in case of error.""" + """ + Backs up configuration to memory, + for a later rollback in case of error. + """ if filename not in self._backups: try: @@ -378,8 +400,9 @@ 'disabled', exc_info=True) def _createConfFile(self, conf, name, ipaddr=None, netmask=None, - gateway=None, bootproto=None, mtu=None, onboot='yes', **kwargs): + gateway=None, bootproto=None, mtu=None, onboot='yes', **kwargs): """ Create ifcfg-* file with proper fields per device """ + cfg = """DEVICE=%s\nONBOOT=%s\n""" % (pipes.quote(name), pipes.quote(onboot)) cfg += conf @@ -454,8 +477,9 @@ bootproto, mtu, onboot, **kwargs) # create the bonding device to avoid initscripts noise - if bonding not in open('/sys/class/net/bonding_masters').read().split(): - open('/sys/class/net/bonding_masters', 'w').write('+%s\n' % bonding) + bondMastersPath = '/sys/class/net/bonding_masters' + if bonding not in open(bondMastersPath).read().split(): + open(bondMastersPath, 'w').write('+%s\n' % bonding) def addNic(self, nic, bonding=None, bridge=None, mtu=None, ipaddr=None, netmask=None, gateway=None, bootproto=None, @@ -489,8 +513,8 @@ cf = self.NET_CONF_PREF + nic self._backup(cf) try: - hwlines = [ line for line in open(cf).readlines() - if line.startswith('HWADDR=') ] + hwlines = [line for line in open(cf).readlines() + if line.startswith('HWADDR=')] l = ['DEVICE=%s\n' % nic, 'ONBOOT=yes\n'] + hwlines open(cf, 'w').writelines(l) except IOError: @@ -528,8 +552,8 @@ its value or None if not found """ with open(conffile) as f: - entries = [ line for line in f.readlines() - if line.startswith(entry + '=') ] + entries = [line for line in f.readlines() + if line.startswith(entry + '=')] if len(entries) != 0: value = entries[0].split('=', 1)[1] return value.strip() @@ -554,8 +578,8 @@ the configuration file """ with open(conffile) as f: - entries = [ line for line in f.readlines() - if not line.startswith(entry + '=') ] + entries = [line for line in f.readlines() + if not line.startswith(entry + '=')] if not delete: entries.append('\n' + entry + '=' + value) @@ -606,12 +630,14 @@ else: return - nics, delvlan, bonding = _netinfo.getNicsVlanAndBondingForNetwork(bridge) + nics, delvlan, bonding = (_netinfo + .getNicsVlanAndBondingForNetwork(bridge)) if delvlan is None: return if bonding: - iface_bridged = _netinfo.getBridgedNetworksAndVlansForIface(bonding) + iface_bridged = (_netinfo + .getBridgedNetworksAndVlansForIface(bonding)) vlans = [v for (_, v) in iface_bridged] iface = bonding else: @@ -641,14 +667,18 @@ cf = self.NET_CONF_PREF + nics[0] self._updateConfigValue(cf, 'MTU', str(newmtu), newmtu is None) + def isBridgeNameValid(bridgeName): return bridgeName and len(bridgeName) <= MAX_BRIDGE_NAME_LEN and \ len(set(bridgeName) & ILLEGAL_BRIDGE_CHARS) == 0 and \ not bridgeName.startswith('-') + def validateBridgeName(bridgeName): if not isBridgeNameValid(bridgeName): - raise ConfigNetworkError(ne.ERR_BAD_BRIDGE, "Bridge name isn't valid: %r"%bridgeName) + raise ConfigNetworkError(ne.ERR_BAD_BRIDGE, + "Bridge name isn't valid: %r" % bridgeName) + def _validateIpAddress(address): try: @@ -662,27 +692,36 @@ return False return True + def validateIpAddress(ipAddr): if not _validateIpAddress(ipAddr): - raise ConfigNetworkError(ne.ERR_BAD_ADDR, "Bad IP address: %r"%ipAddr) + raise ConfigNetworkError(ne.ERR_BAD_ADDR, + "Bad IP address: %r" % ipAddr) + def validateNetmask(netmask): if not _validateIpAddress(netmask): - raise ConfigNetworkError(ne.ERR_BAD_ADDR, "Bad netmask: %r"%netmask) + raise ConfigNetworkError(ne.ERR_BAD_ADDR, + "Bad netmask: %r" % netmask) + def validateGateway(gateway): if not _validateIpAddress(gateway): - raise ConfigNetworkError(ne.ERR_BAD_ADDR, "Bad gateway: %r"%gateway) + raise ConfigNetworkError(ne.ERR_BAD_ADDR, + "Bad gateway: %r" % gateway) + def validateBondingName(bonding): if not re.match('^bond[0-9]+$', bonding): - raise ConfigNetworkError(ne.ERR_BAD_BONDING, '%r is not a valid bonding device name' % bonding) + raise ConfigNetworkError(ne.ERR_BAD_BONDING, + '%r is not a valid bonding device name' % bonding) + def validateBondingOptions(bonding, bondingOptions): 'Example: BONDING_OPTS="mode=802.3ad miimon=150"' try: for option in bondingOptions.split(): - key,value = option.split('=') + key, value = option.split('=') if not os.path.exists('/sys/class/net/%(bonding)s/bonding/%(key)s' % locals()): raise ConfigNetworkError(ne.ERR_BAD_BONDING, @@ -691,12 +730,15 @@ raise ConfigNetworkError(ne.ERR_BAD_BONDING, "Error parsing bonding options: %r" % bondingOptions) + def validateVlanId(vlan): try: if not 0 <= int(vlan) <= MAX_VLAN_ID: - raise ConfigNetworkError(ne.ERR_BAD_VLAN, 'vlan id out of range: %r, must be 0..%s' % (vlan, MAX_VLAN_ID)) + raise ConfigNetworkError(ne.ERR_BAD_VLAN, + 'vlan id out of range: %r, must be 0..%s' % (vlan, MAX_VLAN_ID)) except ValueError: raise ConfigNetworkError(ne.ERR_BAD_VLAN, 'vlan id must be a number') + def _validateInterNetworkCompatibility(ni, vlan, iface, bridged): """ @@ -737,6 +779,7 @@ raise ConfigNetworkError(ne.ERR_BAD_PARAMS, "interface %r already has networks" % \ (iface)) + def _addNetworkValidation(_netinfo, network, vlan, bonding, nics, ipaddr, netmask, gateway, bondingOptions, bridged=True, @@ -817,9 +860,11 @@ else: _validateInterNetworkCompatibility(_netinfo, vlan, nic, bridged) + def addNetwork(network, vlan=None, bonding=None, nics=None, ipaddr=None, netmask=None, mtu=None, gateway=None, force=False, - configWriter=None, bondingOptions=None, bridged=True, **options): + configWriter=None, bondingOptions=None, bridged=True, + **options): nics = nics or () _netinfo = netinfo.NetInfo() bridged = utils.tobool(bridged) @@ -831,9 +876,9 @@ if not utils.tobool(force): logging.debug('validating network...') _addNetworkValidation(_netinfo, network=network, - vlan=vlan, bonding=bonding, nics=nics, ipaddr=ipaddr, - netmask=netmask, gateway=gateway, bondingOptions=bondingOptions, - bridged=bridged, **options) + vlan=vlan, bonding=bonding, nics=nics, ipaddr=ipaddr, + netmask=netmask, gateway=gateway, bondingOptions=bondingOptions, + bridged=bridged, **options) logging.info("Adding network %s with vlan=%s, bonding=%s, nics=%s," " bondingOptions=%s, mtu=%s, bridged=%s, options=%s", @@ -932,6 +977,7 @@ # add libvirt network configWriter.createLibvirtNetwork(network, bridged, iface) + def assertBridgeClean(bridge, vlan, bonding, nics): brifs = os.listdir('/sys/class/net/%s/brif/' % bridge) for nic in nics: @@ -949,7 +995,9 @@ pass if brifs: - raise ConfigNetworkError(ne.ERR_USED_BRIDGE, 'bridge %s has interfaces %s connected' % (bridge, brifs)) + raise ConfigNetworkError(ne.ERR_USED_BRIDGE, + 'bridge %s has interfaces %s connected' % (bridge, brifs)) + def showNetwork(network): _netinfo = netinfo.NetInfo() @@ -975,12 +1023,14 @@ print "vlan=%s, bonding=%s, nics=%s" % (vlan, bonding, nics) + def listNetworks(): _netinfo = netinfo.NetInfo() print "Networks:", _netinfo.networks.keys() print "Vlans:", _netinfo.vlans.keys() print "Nics:", _netinfo.nics.keys() print "Bondings:", _netinfo.bondings.keys() + def delNetwork(network, vlan=None, bonding=None, nics=None, force=False, configWriter=None, implicitBonding=True, **options): @@ -1069,6 +1119,7 @@ ifdown(name) ifup(name) + def clientSeen(timeout): start = time.time() while timeout >= 0: @@ -1079,19 +1130,23 @@ return False -def editNetwork(oldBridge, newBridge, vlan=None, bonding=None, nics=None, **options): +def editNetwork(oldBridge, newBridge, vlan=None, bonding=None, nics=None, + **options): configWriter = ConfigWriter() try: delNetwork(oldBridge, configWriter=configWriter, **options) - addNetwork(newBridge, vlan=vlan, bonding=bonding, nics=nics, configWriter=configWriter, **options) + addNetwork(newBridge, vlan=vlan, bonding=bonding, nics=nics, + configWriter=configWriter, **options) except: configWriter.restoreBackups() raise if utils.tobool(options.get('connectivityCheck', False)): - if not clientSeen(int(options.get('connectivityTimeout', CONNECTIVITY_TIMEOUT_DEFAULT))): + if not clientSeen(int(options.get('connectivityTimeout', + CONNECTIVITY_TIMEOUT_DEFAULT))): delNetwork(newBridge, force=True) configWriter.restoreBackups() return define.errCode['noConPeer']['status']['code'] + def _validateNetworkSetup(networks={}, bondings={}): _netinfo = netinfo.NetInfo() @@ -1099,7 +1154,8 @@ for network, networkAttrs in networks.iteritems(): if networkAttrs.get('remove', False): if set(networkAttrs) - set(['remove']): - raise ConfigNetworkError(ne.ERR_BAD_PARAMS, "Cannot specify any attribute when removing") + raise ConfigNetworkError(ne.ERR_BAD_PARAMS, + "Cannot specify any attribute when removing") for bonding, bondingAttrs in bondings.iteritems(): validateBondingName(bonding) @@ -1108,14 +1164,17 @@ if bondingAttrs.get('remove', False): if bonding not in _netinfo.bondings: - raise ConfigNetworkError(ne.ERR_BAD_BONDING, 'Cannot remove bonding %s: Doesn\'t exist' % bonding) + raise ConfigNetworkError(ne.ERR_BAD_BONDING, + 'Cannot remove bonding %s: Doesn\'t exist' % bonding) continue nics = bondingAttrs.get('nics', None) if not nics: - raise ConfigNetworkError(ne.ERR_BAD_PARAMS, "Must specify nics for bonding") + raise ConfigNetworkError(ne.ERR_BAD_PARAMS, + "Must specify nics for bonding") if not set(nics).issubset(set(_netinfo.nics)): - raise ConfigNetworkError(ne.ERR_BAD_NIC, "Unknown nics in: %r"%list(nics)) + raise ConfigNetworkError(ne.ERR_BAD_NIC, + "Unknown nics in: %r" % list(nics)) def _editBondings(bondings, configWriter): @@ -1166,6 +1225,7 @@ for nic in nicSort(bondAttrs['nics']): ifup(nic) + def _removeBondings(bondings, configWriter): """ Remove bond interface """ logger = logging.getLogger("_removeBondings") @@ -1191,7 +1251,7 @@ Params: networks - dict of key=network, value=attributes - where 'attributes' is a dict with the following optional items: + where 'attributes' is a dict with the following optional items: vlan=<id> bonding="<name>" | nic="<name>" (bonding and nics are mutually exclusive) @@ -1206,7 +1266,7 @@ remove=True (other attributes can't be specified) bondings - dict of key=bonding, value=attributes - where 'attributes' is a dict with the following optional items: + where 'attributes' is a dict with the following optional items: nics=["<nic1>" , "<nic2>", ...] options="<bonding-options>" -- OR -- @@ -1283,9 +1343,8 @@ elif d['bonding'] in _ni.bondings: logger.debug("Updating bond %r info", d['bonding']) d['nics'] = _ni.bondings[d['bonding']]['slaves'] - d['bondingOptions'] = \ - _ni.bondings[d['bonding']]['cfg'].get('BONDING_OPTS', - None) + d['bondingOptions'] = (_ni.bondings[d['bonding']] + ['cfg'].get('BONDING_OPTS', None)) else: d['nics'] = [d.pop('nic')] d['force'] = force @@ -1313,16 +1372,19 @@ logger.error(e, exc_info=True) raise + def setSafeNetworkConfig(): """Declare current network configuration as 'safe'""" subprocess.Popen([constants.EXT_VDSM_STORE_NET_CONFIG]) + def usage(): print """Usage: ./configNetwork.py add Network <attributes> <options> edit oldNetwork newNetwork <attributes> <options> del Network <options> - setup Network [None|attributes] [++ Network [None|attributes] [++ ...]] [:: <options>] + setup Network [None|attributes] \ +[++ Network [None|attributes] [++ ...]] [:: <options>] attributes = [vlan=...] [bonding=...] [nics=<nic1>,...] options = [Force=<True|False>] [bridged=<True|False>]... @@ -1336,6 +1398,7 @@ API.Global.translateNetOptionsToNew(kwargs) return kwargs + def main(): if len(sys.argv) <= 1: @@ -1367,9 +1430,9 @@ kwargs['nics'] = kwargs['nics'].split(',') editNetwork(oldBridge, newBridge, **kwargs) elif sys.argv[1] == 'setup': - batchCommands, options = utils.listSplit( sys.argv[2:], '::', 1 ) + batchCommands, options = utils.listSplit(sys.argv[2:], '::', 1) d = {} - for batchCommand in utils.listSplit( batchCommands, '++' ): + for batchCommand in utils.listSplit(batchCommands, '++'): d[batchCommand[0]] = _parseKwargs(batchCommand[1:]) or None setupNetworks(d, **_parseKwargs(options)) elif sys.argv[1] == 'show': -- To view, visit http://gerrit.ovirt.org/7758 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I02be4c0cdb1868a8518a73e22dfb77adedc08f3f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
