Change in vdsm[master]: Make getRoutes() return the default gateway of an interface, ...

mhuntxu(a)gmail.com has uploaded a new change for review.

Change subject: Make getRoutes() return the default gateway of an interface, otherwise the
gateway of an interface last occurs in /proc/net/route would be the result when multiple
gateways are assigned to the interface, which is clearly not we want.
......................................................................

Make getRoutes() return the default gateway of an interface,
otherwise the gateway of an interface last occurs in /proc/net/route
would be the result when multiple gateways are assigned to the
interface, which is clearly not we want.

Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
---
M vdsm/netinfo.py
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/01/1101/1
--
To view, visit http://gerrit.ovirt.org/1101
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mhuntxu(a)gmail.com

mhuntxu(a)gmail.com has posted comments on this change.

Change subject: Make getRoutes() return the default gateway of an interface, otherwise the
gateway of an interface last occurs in /proc/net/route would be the result when multiple
gateways are assigned to the interface, which is clearly not we want.
......................................................................


Patch Set 1: Verified

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mhuntxu(a)gmail.com
Gerrit-Reviewer: mhuntxu(a)gmail.com

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Make getRoutes() return the default gateway of an interface, otherwise the
gateway of an interface last occurs in /proc/net/route would be the result when multiple
gateways are assigned to the interface, which is clearly not we want.
......................................................................


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

(1 inline comment)

....................................................
Commit Message
Line 7: Make getRoutes() return the default gateway of an interface,
git commit messages shouls start with a short subject line, empty line, and then a long
story.

Please excuse me, I do not understand which was the routing table that broke this
function. Could you provide an example?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhuntxu(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Hunt Xu <mhuntxu(a)gmail.com&gt;

Hunt Xu has posted comments on this change.

Change subject: Make getRoutes() return the default gateway of an interface, otherwise the
gateway of an interface last occurs in /proc/net/route would be the result when multiple
gateways are assigned to the interface, which is clearly not we want.
......................................................................


Patch Set 1: No score

(1 inline comment)

....................................................
Commit Message
Line 7: Make getRoutes() return the default gateway of an interface,
Assuming "route -n" gives the following result:
  Kernel IP routing table
  Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
  0.0.0.0         192.168.1.1     0.0.0.0         UG    303    0        0 engine
  192.168.1.0     0.0.0.0         255.255.255.0   U     303    0        0 engine

Calling this function would get {'engine': '192.168.1.1'}, which is ok.
After adding a route to another subnet using "route add -net 192.168.10.0/24 x.x.x.x
dev engine", again calling the function we would now get {'engine':
'x.x.x.x'}.
So isn't the former result we are always expecting? Or whatever route of an interface
returned would be accepted?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhuntxu(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Hunt Xu <mhuntxu(a)gmail.com&gt;

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Make getRoutes() return the default gateway of an interface, otherwise the
gateway of an interface last occurs in /proc/net/route would be the result when multiple
gateways are assigned to the interface, which is clearly not we want.
......................................................................


Patch Set 1: (1 inline comment)

....................................................
Commit Message
Line 7: Make getRoutes() return the default gateway of an interface,
ovirt-engine expects us to return the default gateway of each bridge interface, so I
believe you're fixing a real bug.

Please fix the commit message to include `route -n` for which awrong gateway would be
reported.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhuntxu(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Hunt Xu <mhuntxu(a)gmail.com&gt;

Change in vdsm[master]: Make getRoutes() return the default gateway of each bridge i...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Make getRoutes() return the default gateway of each bridge interface
......................................................................


Patch Set 2: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhuntxu(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Hunt Xu <mhuntxu(a)gmail.com&gt;

Change in vdsm[master]: Make getRoutes() return the default gateway of each bridge i...

[Douglas Schilling Landgraf]

Douglas Schilling Landgraf has posted comments on this change.

Change subject: Make getRoutes() return the default gateway of each bridge interface
......................................................................


Patch Set 2: Verified; Looks good to me, but someone else must approve

Reproduced the report and the fix is correct.

Listing the currenct routes:
# route -n
Destination     Gateway
0.0.0.0         192.168.1.1
<snip>
192.168.1.0     0.0.0.0 

Adding a new route:
# route add -net 192.168.123.0/24 gw 192.168.1.101 dev ovirtmgmt

Listing routes:
# route -n
Destination     Gateway
0.0.0.0         192.168.1.1
<snip>
192.168.1.0     0.0.0.0 
192.168.123.0   192.168.1.101 
^ new route added

# python ./code-from-vdsm.py 
{'ovirtmgmt': '192.168.1.101'}
Got the last gateway added

Now executing with the changes:
# python ./code-from-vdsm.py 
{'ovirtmgmt': '192.168.1.1'}
Got the ovirtmgmt gateway

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhuntxu(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Hunt Xu <mhuntxu(a)gmail.com&gt;

Change in vdsm[master]: Make getRoutes() return the default gateway of each bridge i...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Make getRoutes() return the default gateway of each bridge interface
......................................................................


Patch Set 2: Looks good to me, approved

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhuntxu(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Hunt Xu <mhuntxu(a)gmail.com&gt;

Change in vdsm[master]: Make getRoutes() return the default gateway of each bridge i...

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Make getRoutes() return the default gateway of each bridge interface
......................................................................


Make getRoutes() return the default gateway of each bridge interface

Assuming `route -n` returns the followings:
Kernel IP routing table
Destination   Gateway     Genmask       Flags Metric Ref Use Iface
192.168.1.0   0.0.0.0     255.255.255.0 U     0      0   0   engine
0.0.0.0       192.168.1.1 0.0.0.0       UG    0      0   0   engine
192.168.10.0  192.168.1.2 255.255.255.0 UG    0      0   0   engine

Then the last shown gateway of engine '192.168.1.2' would be returned
by this functions since it latest occurs in /proc/net/route.
This commit makes this function always return the default gateway of
each bridge interface as ovirt-engine expects.

Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
---
M vdsm/netinfo.py
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Douglas Schilling Landgraf: Verified; Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved


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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6bceb3be2495bc80931c709e2132ff4542e6b763
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Hunt Xu <mhuntxu(a)gmail.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Hunt Xu <mhuntxu(a)gmail.com&gt;