Change in vdsm[ovirt-4.0]: net: canonicalize IPv4 prefix to netmask

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(a)redhat.com&gt;
---
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(


-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;

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

-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

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

-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

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

-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

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


-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

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

-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

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(a)redhat.com&gt;
Line 18: Reviewed-on: https://gerrit.ovirt.org/64494
Line 19: Continuous-Integration: Jenkins CI


-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: Yes

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

Dan Kenigsberg has posted comments on this change.

Change subject: net: canonicalize IPv4 prefix to netmask
......................................................................


Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Code Review]

From Dan Kenigsberg <danken(a)redhat.com&gt;:

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(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/64494
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
(cherry picked from commit 6929ce5f006cbc932f0e994a3268ef9a42a09d94)
Reviewed-on: https://gerrit.ovirt.org/64507
Reviewed-by: Edward Haas <edwardh(a)redhat.com&gt;
---
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



-- 
To view, visit https://gerrit.ovirt.org/64507
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I344eef120b4c19f90d4dd20c7857bb73ab259e9b
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;