Dan Kenigsberg has posted comments on this change.
Change subject: sourcerouting: fix _getRoute not to include local routes ......................................................................
Patch Set 1: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/27262/1//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2014-04-30 16:38:34 +0200 Line 6: Line 7: sourcerouting: fix _getRoute not to include local routes Line 8: Line 9: _getRoute, a method used to find the routes of a vdsm created table s/vdsm created/vdsm-created/ Line 10: in Dynamic Source Routing was listing the routes of the table and Line 11: matching by device, the problem is that such matching would wrongly Line 12: include local scope routes. Having such route would generate an Line 13: IPRoute2 exception when removing routes and rules would never be
Line 7: sourcerouting: fix _getRoute not to include local routes Line 8: Line 9: _getRoute, a method used to find the routes of a vdsm created table Line 10: in Dynamic Source Routing was listing the routes of the table and Line 11: matching by device, the problem is that such matching would wrongly s/device,/device./ Line 12: include local scope routes. Having such route would generate an Line 13: IPRoute2 exception when removing routes and rules would never be Line 14: removed, thus leaving behind trash in the rule list. Line 15:
Line 10: in Dynamic Source Routing was listing the routes of the table and Line 11: matching by device, the problem is that such matching would wrongly Line 12: include local scope routes. Having such route would generate an Line 13: IPRoute2 exception when removing routes and rules would never be Line 14: removed, thus leaving behind trash in the rule list. Since we are missing tests for sourcerouting, I need much more detailed commit message to understand the problem and its solution. Could you supply and example routing table that would cause matching a local-scope routes?
Please explain how the changed code solves the problem. Adding a test with a sample output of routeShowTable('all') may do the trick for me. Line 15: Line 16: Change-Id: I5b3d43c8a2077e40b8b4314f02ea17bc3968c42b