Change in vdsm[master]: WIP: Multiple Gateways Feature

danken at redhat.com danken at redhat.com
Thu May 30 20:04:24 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: WIP: Multiple Gateways Feature
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(6 inline comments)

partial review. there are plenty open questions, I am afraid.

....................................................
File vdsm/multipleGateways.py
Line 1: import os
Line 2: import sys
Line 3: import netaddr
Change spec to require this rpm.
Line 4: from vdsm import utils
Line 5: 
Line 6: 
Line 7: NETWORK_SCRIPTS = '/etc/sysconfig/network-scripts/'


Line 3: import netaddr
Line 4: from vdsm import utils
Line 5: 
Line 6: 
Line 7: NETWORK_SCRIPTS = '/etc/sysconfig/network-scripts/'
that's netinfo.NET_CONF_DIR
Line 8: 
Line 9: 
Line 10: class MultipleGatewaysConfigurator(object):
Line 11:     def __init__(self, isStatic, device):


Line 25:     def _buildRules(self):
Line 26:         return ["from %s table %s" % (self.network, self.table),
Line 27:                 "to %s table %s" % (self.network, self.table)]
Line 28: 
Line 29:     def _writeCommands(self, commands, type='route'):
this writing has to be performed via netconf.ifcfg.ConfigWriter, with all its backup glory.
Line 30:         filePath = NETWORK_SCRIPTS + '%s-%s' % (type, self.device)
Line 31:         with open(filePath, 'w') as file:
Line 32:             for command in commands:
Line 33:                 file.write(command + '\n')


Line 49:     def _addRulesDHCP(self, rules):
Line 50:         rules = ['ip rule add ' + rule for rule in rules]
Line 51:         self._runCommands(rules)
Line 52: 
Line 53:     def up(self, ip, mask, gateway):
"addr" is a more exact term than "ip".
Line 54:         try:
Line 55:             self.ip = netaddr.IPAddress(ip)
Line 56:             self.mask = netaddr.IPAddress(mask)
Line 57:             self.gateway = netaddr.IPAddress(gateway)


Line 57:             self.gateway = netaddr.IPAddress(gateway)
Line 58:         except netaddr.core.AddrFormatError:
Line 59:             print ("IP, subnet mask or gateway not properly formatted.")
Line 60:             return
Line 61:         self.table = self._generateTableId()
Python allows to add attributes on the fly, but that's not a good practice. It surprises other programmers.
Line 62:         network = netaddr.IPNetwork(str(self.ip) + '/' + str(self.mask))
Line 63:         self.network = "%s/%s" % (network.network, network.prefixlen)
Line 64: 
Line 65:         print (("Bringing up - ip: %s, network: %s, subnet: %s " +


Line 93:         return rule.split()[-1] if rule else None
Line 94: 
Line 95:     def _getRules(self, network):
Line 96:         _, output, _ = utils.execCmd("/sbin/ip rule".split())
Line 97:         relevantLines = [line for line in output if network in line]
yes, it would be nicer to have "ip rule" wrapped by a nice Python function, with proper parsing an unit tests.

However, this may be a bit more complicated for for the commands that are being run in dhcp, but needs to be written for static. I usually dislike the notion, but maybe you could wrap the `ip route via` command with a RouteCommand class, that has both call() and writecmd() methods.
Line 98:         rules = [line.split(":")[1].strip() for line in relevantLines]
Line 99:         return rules
Line 100: 
Line 101:     def _getNetwork(self, device):


--
To view, visit http://gerrit.ovirt.org/15207
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0224d896724b9cdc44215e92f0da0be71fd19038
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list