Petr Horáček has uploaded a new change for review.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
net: canonicalize IPv4 prefix to netmask
In order to fix OVS switch, this patch moves canonicalization of passed IPv4 prefix into netmask from legacy_switch.py to canonicalize.py.
It also adds missing 'prefix' to setupNetworks documentation.
Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b Bug-Url: https://bugzilla.redhat.com/1374194 Signed-off-by: Petr Horáček phoracek@redhat.com --- M lib/vdsm/network/api.py M lib/vdsm/network/canonicalize.py M lib/vdsm/network/legacy_switch.py M tests/network/func_static_ip_test.py M tests/network/netfunctestlib.py 5 files changed, 30 insertions(+), 14 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/64507/1
diff --git a/lib/vdsm/network/api.py b/lib/vdsm/network/api.py index 799686f..1b5ab48 100644 --- a/lib/vdsm/network/api.py +++ b/lib/vdsm/network/api.py @@ -209,7 +209,7 @@ bonding="<name>" | nic="<name>" (bonding and nics are mutually exclusive) ipaddr="<ipv4>" - netmask="<ipv4>" + netmask="<ipv4>" | prefix=<prefixlen> gateway="<ipv4>" bootproto="..." ipv6addr="<ipv6>[/<prefixlen>]" diff --git a/lib/vdsm/network/canonicalize.py b/lib/vdsm/network/canonicalize.py index b0bc983..3468796 100644 --- a/lib/vdsm/network/canonicalize.py +++ b/lib/vdsm/network/canonicalize.py @@ -22,6 +22,7 @@ import six
from .netinfo import bridges, mtus, bonding, dns +from .netinfo.addresses import prefix2netmask from vdsm import utils
from .errors import ConfigNetworkError @@ -47,6 +48,7 @@ _canonicalize_switch_type_net(attrs) _canonicalize_ip_default_route(attrs) _canonicalize_nameservers(attrs) + _canonicalize_prefix(attrs)
def canonicalize_bondings(bonds): @@ -138,3 +140,15 @@ data['nameservers'] = dns.get_host_nameservers() else: data['nameservers'] = [] + + +def _canonicalize_prefix(data): + prefix = data.pop('prefix', None) + if prefix: + if 'netmask' in data: + raise ConfigNetworkError( + ne.ERR_BAD_PARAMS, 'Both PREFIX and NETMASK supplied') + try: + data['netmask'] = prefix2netmask(int(prefix)) + except ValueError as ve: + raise ConfigNetworkError(ne.ERR_BAD_ADDR, 'Bad prefix: %s' % ve) diff --git a/lib/vdsm/network/legacy_switch.py b/lib/vdsm/network/legacy_switch.py index 05a62c2..8d426ec 100644 --- a/lib/vdsm/network/legacy_switch.py +++ b/lib/vdsm/network/legacy_switch.py @@ -30,7 +30,6 @@ from vdsm.network import kernelconfig from vdsm.network import libvirt from vdsm.network.netinfo import NET_PATH -from vdsm.network.netinfo import addresses from vdsm.network.netinfo import bridges from vdsm.network.netinfo import mtus from vdsm.network.netinfo import nics as netinfo_nics @@ -187,7 +186,7 @@ @_alter_running_config def _add_network(network, configurator, nameservers, vlan=None, bonding=None, nic=None, ipaddr=None, - netmask=None, prefix=None, mtu=None, gateway=None, + netmask=None, mtu=None, gateway=None, dhcpv6=None, ipv6addr=None, ipv6gateway=None, ipv6autoconf=None, bridged=True, _netinfo=None, hostQos=None, defaultRoute=None, blockingdhcp=False, **options): @@ -201,16 +200,6 @@ if network == '': raise ConfigNetworkError(ne.ERR_BAD_BRIDGE, 'Empty network names are not valid') - if prefix: - if netmask: - raise ConfigNetworkError(ne.ERR_BAD_PARAMS, - 'Both PREFIX and NETMASK supplied') - else: - try: - netmask = addresses.prefix2netmask(int(prefix)) - except ValueError as ve: - raise ConfigNetworkError(ne.ERR_BAD_ADDR, 'Bad prefix: %s' % - ve)
logging.debug('Validating network...') if network in _netinfo.networks: diff --git a/tests/network/func_static_ip_test.py b/tests/network/func_static_ip_test.py index 136a44c..e9bfdb2 100644 --- a/tests/network/func_static_ip_test.py +++ b/tests/network/func_static_ip_test.py @@ -108,6 +108,17 @@ with self.setupNetworks(netcreate, bondcreate, NOCHK): self.assertNetworkIp(NETWORK_NAME, netcreate[NETWORK_NAME])
+ def test_add_net_with_prefix(self): + with dummy_device() as nic: + network_attrs = {'nic': nic, + 'ipaddr': IPv4_ADDRESS, + 'prefix': IPv4_PREFIX_LEN, + 'switch': self.switch} + netcreate = {NETWORK_NAME: network_attrs} + + with self.setupNetworks(netcreate, {}, NOCHK): + self.assertNetworkIp(NETWORK_NAME, netcreate[NETWORK_NAME]) +
@attr(type='functional', switch='legacy') class NetworkStaticIpBasicLegacyTest(NetworkStaticIpBasicTemplate): diff --git a/tests/network/netfunctestlib.py b/tests/network/netfunctestlib.py index 7b59fa8..9e9b14f 100644 --- a/tests/network/netfunctestlib.py +++ b/tests/network/netfunctestlib.py @@ -30,6 +30,7 @@ from vdsm.network import kernelconfig from vdsm.network.ip import dhclient from vdsm.network.ip.address import ipv6_supported +from vdsm.network.netinfo.addresses import prefix2netmask from vdsm.network.netinfo.nics import operstate
from testlib import VdsmTestCase @@ -283,7 +284,8 @@ def assertStaticIPv4(self, netattrs, ipinfo): requires_ipaddress() address = netattrs['ipaddr'] - netmask = netattrs['netmask'] + netmask = (netattrs.get('netmask') or + prefix2netmask(int(netattrs.get('prefix')))) self.assertEqual(address, ipinfo['addr']) self.assertEqual(netmask, ipinfo['netmask']) ipv4 = ipaddress.IPv4Interface(
gerrit-hooks has posted comments on this change.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
Patch Set 1: Verified-1
* update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1374194::OK, public bug * Check Product::#1374194::OK, Correct classification oVirt * Check TM::#1374194::OK, correct target milestone ovirt-4.0.6 * Check merged to previous::WARN, Still open on branches master
gerrit-hooks has posted comments on this change.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
Patch Set 2:
* update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1374194::OK, public bug * Check Product::#1374194::OK, Correct classification oVirt * Check TM::#1374194::OK, correct target milestone ovirt-4.0.6 * Check merged to previous::WARN, Still open on branches master
Petr Horáček has posted comments on this change.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
Patch Set 2: Verified+1
Passed network/*_test.py and functional/networkTests.py
Edward Haas has posted comments on this change.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
The -1 is mainly for the commit message. The scenario where both prefix and mask are supplied is missing, but it should appear in a unit test not in an expensive func test. We should open a task to cover the canonicalize module with unit tests.
https://gerrit.ovirt.org/#/c/64507/2//COMMIT_MSG Commit Message:
Line 11: canonicalize.py. Line 12: Line 13: It also adds missing 'prefix' to setupNetworks documentation. Line 14: Line 15: Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b Missing some lines to indicate it is a backport. Line 16: Bug-Url: https://bugzilla.redhat.com/1374194
gerrit-hooks has posted comments on this change.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
Patch Set 3:
* #1374194::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1374194::OK, public bug * Check Product::#1374194::WARN, wrong product: ovirt-engine * Check TM::#1374194::OK, correct target milestone ovirt-4.0.6 * Check merged to previous::OK, change not open on any previous branch
Petr Horáček has posted comments on this change.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
Patch Set 3: Verified+1
(1 comment)
Passed network/*_test.py and functional/networkTests.py OK.
I added task to implement unit tests for canonicalization.
https://gerrit.ovirt.org/#/c/64507/2//COMMIT_MSG Commit Message:
Line 11: canonicalize.py. Line 12: Line 13: It also adds missing 'prefix' to setupNetworks documentation. Line 14: Line 15: Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Missing some lines to indicate it is a backport.
Done Line 16: Bug-Url: https://bugzilla.redhat.com/1374194 Line 17: Signed-off-by: Petr Horáček phoracek@redhat.com Line 18: Reviewed-on: https://gerrit.ovirt.org/64494 Line 19: Continuous-Integration: Jenkins CI
From Dan Kenigsberg danken@redhat.com:
Dan Kenigsberg has posted comments on this change.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
Patch Set 3: Code-Review+2
From Dan Kenigsberg danken@redhat.com:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: net: canonicalize IPv4 prefix to netmask ......................................................................
net: canonicalize IPv4 prefix to netmask
In order to fix OVS switch, this patch moves canonicalization of passed IPv4 prefix into netmask from legacy_switch.py to canonicalize.py.
It also adds missing 'prefix' to setupNetworks documentation.
Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b Bug-Url: https://bugzilla.redhat.com/1374194 Signed-off-by: Petr Horáček phoracek@redhat.com Reviewed-on: https://gerrit.ovirt.org/64494 Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg danken@redhat.com (cherry picked from commit 6929ce5f006cbc932f0e994a3268ef9a42a09d94) Reviewed-on: https://gerrit.ovirt.org/64507 Reviewed-by: Edward Haas edwardh@redhat.com --- M lib/vdsm/network/api.py M lib/vdsm/network/canonicalize.py M lib/vdsm/network/legacy_switch.py M tests/network/func_static_ip_test.py M tests/network/netfunctestlib.py 5 files changed, 30 insertions(+), 14 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
vdsm-patches@lists.fedorahosted.org