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