3:51 a.m.
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/65056
to review the following change.
Change subject: net: remove default route before adding a new one
......................................................................
net: remove default route before adding a new one
Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Signed-off-by: Petr Horáček <phoracek(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/63220
Continuous-Integration: Jenkins CI
Reviewed-by: Edward Haas <edwardh(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/network/ip/address.py
1 file changed, 22 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/65056/1
diff --git a/lib/vdsm/network/ip/address.py b/lib/vdsm/network/ip/address.py
index 634925a..04aec93 100644
--- a/lib/vdsm/network/ip/address.py
+++ b/lib/vdsm/network/ip/address.py
@@ -18,6 +18,8 @@
#
from __future__ import absolute_import
+from contextlib import contextmanager
+import logging
import socket
import struct
@@ -173,28 +175,42 @@
def add(iface, ipv4, ipv6):
if ipv4.address:
- ipwrapper.addrAdd(iface, ipv4.address, ipv4.netmask)
- if ipv4.gateway and ipv4.defaultRoute:
- ipwrapper.routeAdd(['default', 'via', ipv4.gateway])
+ _add_ipv4_address(iface, ipv4)
if ipv6:
_add_ipv6_address(iface, ipv6)
elif ipv6_supported():
sysctl.disable_ipv6(iface)
+def _add_ipv4_address(iface, ipv4):
+ ipwrapper.addrAdd(iface, ipv4.address, ipv4.netmask)
+ if ipv4.gateway and ipv4.defaultRoute:
+ _set_default_route(ipv4.gateway, family=4)
+
+
def _add_ipv6_address(iface, ipv6):
if ipv6.address:
ipv6addr, ipv6netmask = ipv6.address.split('/')
ipwrapper.addrAdd(iface, ipv6addr, ipv6netmask, family=6)
- if ipv6.gateway:
- ipwrapper.routeAdd(['default', 'via', ipv6.gateway],
- dev=iface, family=6)
+ if ipv6.gateway and ipv6.defaultRoute:
+ _set_default_route(ipv6.gateway, family=6, dev=iface)
if ipv6.ipv6autoconf is not None:
with open('/proc/sys/net/ipv6/conf/%s/autoconf' % iface,
'w') as ipv6_autoconf:
ipv6_autoconf.write('1' if ipv6.ipv6autoconf else '0')
+@contextmanager
+def _set_default_route(gateway, family, dev=None):
+ try:
+ ipwrapper.routeAdd(['default', 'via', gateway], family=family,
dev=dev)
+ except ipwrapper.IPRoute2Error: # there already is a default route
+ logging.warning(
+ 'Existing default route will be removed so a new one can be set.')
+ ipwrapper.routeDel('default', family=family)
+ ipwrapper.routeAdd(['default', 'via', gateway], family=family,
dev=dev)
+
+
def flush(iface, family='both'):
ipwrapper.addrFlush(iface, family)
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
3:52 a.m.
gerrit-hooks has posted comments on this change.
Change subject: net: remove default route before adding a new one
......................................................................
Patch Set 1: Verified-1
* #63220::Update tracker: OK
* Check Bug-Url::ERROR, At least one bug-url is required for the stable branch
* Check merged to previous::OK, change not open on any previous branch
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
4:04 a.m.
gerrit-hooks has posted comments on this change.
Change subject: net: remove default route before adding a new one
......................................................................
Patch Set 2: -Verified
* #1381147::Update tracker: OK
* #63220::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1381147::OK, public bug
* Check Product::#1381147::OK, Correct classification oVirt
* Check TM::#1381147::ERROR, wrong target milestone for stable branch, --- should match
^.*4.0.*
* Check merged to previous::OK, change not open on any previous branch
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
4:05 a.m.
Petr Horáček has posted comments on this change.
Change subject: net: remove default route before adding a new one
......................................................................
Patch Set 2: Verified+1
Passed network/*_test.py OK.
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
4:23 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: net: remove default route before adding a new one
......................................................................
Patch Set 2:
oh, this could explain the problem that mburman reports
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7:44 a.m.
Petr Horáček has posted comments on this change.
Change subject: net: remove default route before adding a new one
......................................................................
Patch Set 2:
If you are talking about the 'setting default route for non-management networks'
problem, it is caused by missing implementation of source routing. this takes care only of
management networks static default route.
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
3:49 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: net: remove default route before adding a new one
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
3:50 a.m.
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: net: remove default route before adding a new one
......................................................................
net: remove default route before adding a new one
Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Signed-off-by: Petr Horáček <phoracek(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/63220
Continuous-Integration: Jenkins CI
Reviewed-by: Edward Haas <edwardh(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/1381147
Reviewed-on: https://gerrit.ovirt.org/65056
---
M lib/vdsm/network/ip/address.py
1 file changed, 22 insertions(+), 6 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Petr Horáček: Verified
Dan Kenigsberg: Looks good to me, approved
Edward Haas: Looks good to me, but someone else must approve
--
To view, visit https://gerrit.ovirt.org/65056
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie59c07845898630404ce517e5cc52048e50ed2dd
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
2740
days inactive
2770
days old
vdsm-patches@lists.fedorahosted.org
7 comments
4 participants
participants (4)
-
automation＠ovirt.org -
Code Review -
Dan Kenigsberg -
phoracek＠redhat.com