Change in vdsm[master]: Multiple Gateways: SourceRoute now uses a configurator

amuller at redhat.com amuller at redhat.com
Sun Jun 9 16:59:26 UTC 2013


Assaf Muller has posted comments on this change.

Change subject: Multiple Gateways: SourceRoute now uses a configurator
......................................................................


Patch Set 4: (4 inline comments)

Replied to Guiseppe's review.

....................................................
File vdsm/netconf/ifcfg.py
Line 159:         return os.path.join(netinfo.NET_CONF_DIR, '%s-%s' % (type, device))
Line 160: 
Line 161:     def _removeSourceRouteFile(self, type, device):
Line 162:         filePath = self._getFilePath(type, device)
Line 163:         self.configWriter._backup(filePath)
Regarding the first point:
configSourceRoute and removeSourceRoute are the two methods I added to the API of the class. The rest of the methods are internal as far as I'm concerned so I added the underscore to signal that no one should use them outside of the class. That's my reasoning anyway, does that make sense?
Line 164:         self.configWriter._removeFile(filePath)
Line 165: 
Line 166:     def _writeConfFile(self, contents, type, device):
Line 167:         filePath = self._getFilePath(type, device)


....................................................
File vdsm/netconf/iproute2.py
Line 20: 
Line 21: import ipwrapper
Line 22: 
Line 23: 
Line 24: class Iproute2(object):
These methods will integrate into another patch by Mark Wu, which will implement a lot more methods into Iproute2. Still, you have a point.
Line 25:     def configureSourceRoute(self, sourceRoute):
Line 26:         for route in sourceRoute.routes:
Line 27:             ipwrapper.ipRouteAdd(route)
Line 28: 


....................................................
File vdsm/sourceRoute.py
Line 52:         return [ipwrapper.Rule(source=self.network, table=self.table),
Line 53:                 ipwrapper.Rule(destination=self.network, table=self.table)]
Line 54: 
Line 55:     def configure(self, ipaddr, mask, gateway):
Line 56:         if not self._isLibvirtInterface() or gateway == '0.0.0.0':
Sorry, I was about to push a new patchset that fixes this exact issue.
Line 57:             print "interface %s is not a libvirt interface" % self.device
Line 58:             return
Line 59: 
Line 60:         self.ipaddr = ipaddr


Line 75: 
Line 76:         self.configurator.configureSourceRoute(self)
Line 77: 
Line 78:     def _isLibvirtInterface(self):
Line 79:         networks = netinfo.networks()
I agree. Dan disagrees and says that it's not worth to add a new method to netinfo because no one else will use it. I still think it's a better idea.
Line 80:         trackedInterfaces = [network.get('bridge') or network.get('iface')
Line 81:                              for network in networks.itervalues()]
Line 82:         return self.device in trackedInterfaces
Line 83: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2bfba77799dd74923e30db082dbddcc64678c72
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller <amuller 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: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list