Change in vdsm[master]: net: add a resolv.conf writer

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: add a resolv.conf writer
..


Patch Set 3:

(9 comments)

https://gerrit.ovirt.org/#/c/62705/3/lib/vdsm/network/ip/resolv.py
File lib/vdsm/network/ip/resolv.py:

PS3, Line 33: , 'r'
> 'r' is default, we can drop it.
Done


PS3, Line 38: nameservers = []
: for words in _parse_lines(file_text):
: if words[0] == 'nameserver':
: nameservers.append(words[1])
: return nameservers
> return [words[1] for words in _parse_lines(file_test) if words[0] == 'names
I still have domains in mind and want to generalize this into 
_parse_resolver_configuration, which returns two lists: 
https://gerrit.ovirt.org/#/c/61794/


Line 51: yield words
Line 52: 
Line 53: 
Line 54: def update(nameservers):
Line 55: lines = [_conf_header()]
> header_lines
No need to treat a header in a special way.
Line 56: for ns in nameservers:
Line 57: lines.append('nameserver ' + ns)
Line 58: 
Line 59: for words in _parse_lines(_read_resolv_conf()):


PS3, Line 56: for ns in nameservers:
: lines.append('nameserver ' + ns)
> ns_lines = ['nameserver ' + ns for ns in nameservers]
Done


PS3, Line 59: for words in _parse_lines(_read_resolv_conf()):
: if words[0] != 'nameserver':
: lines.append(' '.join(words))
> other_lines = [' '.join(words) for words in _parse_lines(...) if words[0] !
Done


Line 59: for words in _parse_lines(_read_resolv_conf()):
Line 60: if words[0] != 'nameserver':
Line 61: lines.append(' '.join(words))
Line 62: 
Line 63: with open(DNS_CONF_FILE, 'w') as f:
> lines = + itertools.chain.from_iterable([header_lines, ns_lines, other_line
Lists can be simply concatenated.
Line 64: f.writelines(line + '\n' for line in lines)
Line 65: 
Line 66: 
Line 67: def _conf_header():


Line 63: with open(DNS_CONF_FILE, 'w') as f:
Line 64: f.writelines(line + '\n' for line in lines)
Line 65: 
Line 66: 
Line 67: def _conf_header():
> it can be defined as a constant or memoized.
I went for a constant.


https://gerrit.ovirt.org/#/c/62705/3/tests/network/resolv_test.py
File tests/network/resolv_test.py:

PS3, Line 56: test_update
> Please describe what is being tested: Replacing existing, appending new one
For now, we test replacement, to keep it simple. 

Resolver uses up to MAXNS (3) nameservers and ignores any extra nameservers.

One nameserver is in now way special currently (only zero is, in that case 
current nameservers are used).


In a separate patch, we could support initscripts-style 'update' (like in 
ifup-post), but for 3, not 2 entries:

In addition to defined nameservers, and up to MAXNS, append current nameservers 
which differ from the defined ones (as fallback nameservers). I had this in 
patch set 1 (_extend_with_unique).


PS3, Line 68: [
: resolv._conf_header() + '\n'] +
: ['nameserver {}\n'.format(ns) for ns in 
nameservers] +
: self.RESOLV_CONF[1:3])
> can we define it in another constant?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41b81b04fbe3e3e2398f28cb16fb0dbbc0382249
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net tests: separate a resolv.conf test from NetInfo tests

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net tests: separate a resolv.conf test from NetInfo tests
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/62723/1/tests/network/resolv_test.py
File tests/network/resolv_test.py:

Line 17: # 02110-1301  USA
Line 18: #
Line 19: # Refer to the README and COPYING files for full details of the license
Line 20: #
Line 21: from __future__ import absolute_import
> empty line after __future__
Done
Line 22: import os
Line 23: 
Line 24: from nose.plugins.attrib import attr
Line 25: 


PS1, Line 29: as TestCaseBase
> redundant
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I566f3450f849df65e8b0c427ff883d7e6c9223d2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: move name resolver functions from netinfo to ip

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: move name resolver functions from netinfo to ip
..


Patch Set 2:

(2 comments)

https://gerrit.ovirt.org/#/c/62704/2/lib/vdsm/network/ip/resolv.py
File lib/vdsm/network/ip/resolv.py:

> Why the name change? Please convince us that the name needs to change.
This module affects the client-side part of DNS, and that is name resolution. 
If it included, some day, other means of name resolution (configured in 
/etc/nsswitch.conf), e.g. file-based (/etc/hosts) or LAN-based (Avahi), its 
name would still be fitting.
Line 1: #
Line 2: # Copyright 2016 Red Hat, Inc.
Line 3: #
Line 4: # This program is free software; you can redistribute it and/or modify


PS2, Line 2: 2016
> 2015-2016
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc6732a4c0ab5aa6afd63b167834cc3942a62c15
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net tests: separate a resolv.conf test from NetInfo tests

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: net tests: separate a resolv.conf test from NetInfo tests
..

net tests: separate a resolv.conf test from NetInfo tests

Change-Id: I566f3450f849df65e8b0c427ff883d7e6c9223d2
Signed-off-by: Ondřej Svoboda 
---
M tests/network/netinfo_test.py
A tests/network/resolv_test.py
2 files changed, 54 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/62723/1

diff --git a/tests/network/netinfo_test.py b/tests/network/netinfo_test.py
index c4d21de..67f82d5 100644
--- a/tests/network/netinfo_test.py
+++ b/tests/network/netinfo_test.py
@@ -25,7 +25,6 @@
 from nose.plugins.attrib import attr
 
 from vdsm.network import ipwrapper
-from vdsm.network.ip import resolv
 from vdsm.network.netinfo import addresses, bonding, misc, nics, routes
 from vdsm.network.netinfo.cache import get
 from vdsm.utils import random_iface_name
@@ -44,27 +43,6 @@
 
 @attr(type='unit')
 class TestNetinfo(TestCaseBase):
-
-def testGetHostNameservers(self):
-RESOLV_CONF = (
-'# Generated by NetworkManager\n'
-'search example.com company.net\n'
-'domain example.com\n'
-'nameserver 192.168.0.100\n'
-'nameserver 8.8.8.8\n'
-'nameserver 8.8.4.4\n'
-)
-nameservers = ['192.168.0.100', '8.8.8.8', '8.8.4.4']
-with namedTemporaryDir() as temp_dir:
-file_path = os.path.join(temp_dir, 'resolv.conf')
-
-with mock.patch.object(resolv, 'DNS_CONF_FILE', file_path):
-for content in (RESOLV_CONF, RESOLV_CONF + '\n'):
-with open(file_path, 'w') as file_object:
-file_object.write(content)
-
-self.assertEqual(resolv.get_host_nameservers(),
- nameservers)
 
 def testNetmaskConversions(self):
 path = os.path.join(os.path.dirname(__file__), "netmaskconversions")
diff --git a/tests/network/resolv_test.py b/tests/network/resolv_test.py
new file mode 100644
index 000..2b734dc
--- /dev/null
+++ b/tests/network/resolv_test.py
@@ -0,0 +1,54 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301  USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+from __future__ import absolute_import
+import os
+
+from nose.plugins.attrib import attr
+
+from vdsm.network.ip import resolv
+
+from testlib import mock
+from testlib import VdsmTestCase as TestCaseBase, namedTemporaryDir
+
+
+@attr(type='unit')
+class ResolvTest(TestCaseBase):
+RESOLV_CONF = [
+'# Generated by NetworkManager\n',
+'search example.com company.net\n',
+'domain example.com\n',
+'nameserver 192.168.0.100\n',
+'nameserver 8.8.8.8\n',
+'nameserver 8.8.4.4\n',
+]
+
+def test_get_host_nameservers(self):
+nameservers = ['192.168.0.100', '8.8.8.8', '8.8.4.4']
+with namedTemporaryDir() as temp_dir:
+file_path = os.path.join(temp_dir, 'resolv.conf')
+
+with mock.patch.object(resolv, 'DNS_CONF_FILE', file_path):
+for content in (self.RESOLV_CONF, self.RESOLV_CONF + ['\n']):
+with open(file_path, 'w') as file_object:
+file_object.writelines(content)
+
+self.assertEqual(resolv.get_host_nameservers(),
+ nameservers)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I566f3450f849df65e8b0c427ff883d7e6c9223d2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: add a resolv.conf writer

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: add a resolv.conf writer
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/62705/1/lib/vdsm/network/ip/resolv.py
File lib/vdsm/network/ip/resolv.py:

Line 50: if len(words) >= 2:
Line 51: yield words
Line 52: 
Line 53: 
Line 54: def update(nameservers):
> can you add a unit test for this function?
Yes, but please note I will remove complex handling of "old_nameservers" first. 
It needs rethinking or to be moved to canonicalize.
Line 55: file_text = _read_resolv_conf()
Line 56: old_nameservers = _parse_nameservers(file_text)
Line 57: nameservers = list(nameservers)
Line 58: _extend_with_unique(nameservers, old_nameservers)


PS1, Line 60: Generated by VDSM version
> can this be turned into a constant, importable from ifcfg configurator?
Let us discuss where to put such a constant.

This literal is in ifcfg.py, unified_persistence(_test).py and 
vdsm-restore-net-config. So far, it is found only in network-related code.

Is network/__init__.py a good location, then?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41b81b04fbe3e3e2398f28cb16fb0dbbc0382249
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: support configuring nameservers by ovs

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: net: support configuring nameservers by ovs
..

net: support configuring nameservers by ovs

Change-Id: I5fe4f82fe88e24b43e5f40e96ae4292ebf933a8e
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/netswitch.py
M tests/network/func_dns_test.py
2 files changed, 9 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/62706/1

diff --git a/lib/vdsm/network/netswitch.py b/lib/vdsm/network/netswitch.py
index 6df8ed8..424b052 100644
--- a/lib/vdsm/network/netswitch.py
+++ b/lib/vdsm/network/netswitch.py
@@ -25,6 +25,7 @@
 from vdsm.network import ipwrapper
 from vdsm.network.ip import address
 from vdsm.network.ip import dhclient
+from vdsm.network.ip import resolv
 from vdsm.network.libvirt import networks as libvirt_nets
 from vdsm.network.netinfo.cache import (libvirtNets2vdsm, get as netinfo_get,
 CachingNetInfo)
@@ -201,6 +202,7 @@
 
 for net, attrs in six.iteritems(nets2add):
 _set_static_ip_config(net, attrs)
+_set_static_dns_config(attrs)
 _set_dhcp_config(net, attrs)
 
 
@@ -236,6 +238,12 @@
 attrs.get('dhcpv6'))
 
 
+def _set_static_dns_config(attrs):
+ipv4 = address.IPv4(*_ipv4_conf_params(attrs))
+if ipv4.defaultRoute and attrs['nameservers']:
+resolv.update(attrs['nameservers'])
+
+
 def _set_ovs_links_up(nets2add, bonds2add, bonds2edit):
 # TODO: Make this universal for legacy and ovs.
 for dev in _gather_ovs_ifaces(nets2add, bonds2add, bonds2edit):
diff --git a/tests/network/func_dns_test.py b/tests/network/func_dns_test.py
index aa4be68..84dddf2 100644
--- a/tests/network/func_dns_test.py
+++ b/tests/network/func_dns_test.py
@@ -91,7 +91,5 @@
 
 @attr(type='functional', switch='ovs')
 class NetworkDNSOvsTest(NetworkDNSTemplate):
-# TODO: Implement 'nameservers' in the iproute2 configurator to make the
-#   tests pass with the OVS switch.
-__test__ = False
+__test__ = True
 switch = 'ovs'


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5fe4f82fe88e24b43e5f40e96ae4292ebf933a8e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: move name resolver functions from netinfo to ip

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: net: move name resolver functions from netinfo to ip
..

net: move name resolver functions from netinfo to ip

This is a preparation to share resolv.conf reading code with
a writer in the next patch.

Change-Id: Ifc6732a4c0ab5aa6afd63b167834cc3942a62c15
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/canonicalize.py
R lib/vdsm/network/ip/resolv.py
M lib/vdsm/network/kernelconfig.py
M lib/vdsm/network/netinfo/Makefile.am
M lib/vdsm/network/netinfo/cache.py
M tests/network/netinfo_test.py
6 files changed, 14 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/62704/1

diff --git a/lib/vdsm/network/canonicalize.py b/lib/vdsm/network/canonicalize.py
index b0bc983..05ab2d0 100644
--- a/lib/vdsm/network/canonicalize.py
+++ b/lib/vdsm/network/canonicalize.py
@@ -21,8 +21,9 @@
 
 import six
 
-from .netinfo import bridges, mtus, bonding, dns
+from .netinfo import bridges, mtus, bonding
 from vdsm import utils
+from vdsm.network.ip import resolv
 
 from .errors import ConfigNetworkError
 from . import errors as ne
@@ -135,6 +136,6 @@
 if 'nameservers' not in data:
 # Nameservers are relevant only for the management network.
 if data['defaultRoute']:
-data['nameservers'] = dns.get_host_nameservers()
+data['nameservers'] = resolv.get_host_nameservers()
 else:
 data['nameservers'] = []
diff --git a/lib/vdsm/network/netinfo/dns.py b/lib/vdsm/network/ip/resolv.py
similarity index 97%
rename from lib/vdsm/network/netinfo/dns.py
rename to lib/vdsm/network/ip/resolv.py
index 3ca82da..5d9ce22 100644
--- a/lib/vdsm/network/netinfo/dns.py
+++ b/lib/vdsm/network/ip/resolv.py
@@ -1,5 +1,5 @@
 #
-# Copyright 2015 Hat, Inc.
+# Copyright 2016 Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
diff --git a/lib/vdsm/network/kernelconfig.py b/lib/vdsm/network/kernelconfig.py
index abb73cf..9a82ac4 100644
--- a/lib/vdsm/network/kernelconfig.py
+++ b/lib/vdsm/network/kernelconfig.py
@@ -23,10 +23,10 @@
 import string
 
 from vdsm import constants
+from vdsm.network.ip import resolv
 from vdsm.network.netinfo import addresses
 from vdsm.network.netinfo import bonding
 from vdsm.network.netinfo import bridges
-from vdsm.network.netinfo import dns
 from vdsm.network.netinfo import routes
 from vdsm.network.netconfpersistence import BaseConfig
 
@@ -160,7 +160,8 @@
 
 
 def _translate_nameservers(attributes):
-nservers = dns.get_host_nameservers() if attributes['defaultRoute'] else []
+nservers = (resolv.get_host_nameservers() if attributes['defaultRoute']
+else [])
 attributes['nameservers'] = nservers
 
 
diff --git a/lib/vdsm/network/netinfo/Makefile.am 
b/lib/vdsm/network/netinfo/Makefile.am
index 87ee25b..1637e32 100644
--- a/lib/vdsm/network/netinfo/Makefile.am
+++ b/lib/vdsm/network/netinfo/Makefile.am
@@ -27,7 +27,6 @@
bridges.py \
cache.py \
dhcp.py \
-   dns.py \
misc.py \
mtus.py \
nics.py \
diff --git a/lib/vdsm/network/netinfo/cache.py 
b/lib/vdsm/network/netinfo/cache.py
index 960a5d9..f25666c 100644
--- a/lib/vdsm/network/netinfo/cache.py
+++ b/lib/vdsm/network/netinfo/cache.py
@@ -28,7 +28,7 @@
 from vdsm.network import libvirt
 from vdsm.network import netinfo
 from vdsm.network.ip.address import ipv6_supported
-from vdsm.network.ip import dhclient
+from vdsm.network.ip import dhclient, resolv
 from vdsm.network.ipwrapper import getLinks
 from vdsm.network.netconfpersistence import RunningConfig
 from vdsm.network.netlink import link as nl_link
@@ -37,7 +37,6 @@
 from . import bonding
 from . import bridges
 from .dhcp import set_netdev_dhcp_info
-from .dns import get_host_nameservers
 from .mtus import getMtu
 from . import nics
 from . import vlans
@@ -60,7 +59,7 @@
 :return: Dict of networking devices with all their details.
 """
 networking = {'bondings': {}, 'bridges': {}, 'networks': {}, 'nics': {},
-  'vlans': {}, 'nameservers': get_host_nameservers()}
+  'vlans': {}, 'nameservers': resolv.get_host_nameservers()}
 paddr = bonding.permanent_address()
 ipaddrs = getIpAddrs()
 routes = get_routes()
diff --git a/tests/network/netinfo_test.py b/tests/network/netinfo_test.py
index f0c3a6f..c4d21de 100644
--- a/tests/network/netinfo_test.py
+++ b/tests/network/netinfo_test.py
@@ -25,7 +25,8 @@
 from nose.plugins.attrib import attr
 
 from vdsm.network import ipwrapper
-from vdsm.network.netinfo import addresses, bonding, dns, misc, nics, routes
+from vdsm.network.ip import resolv
+from vdsm.network.netinfo import addresses, bonding, misc, nics, routes
 from vdsm.network.netinfo.cache import get
 from vdsm.utils 

Change in vdsm[master]: net: add a resolv.conf writer

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: net: add a resolv.conf writer
..

net: add a resolv.conf writer

Change-Id: I41b81b04fbe3e3e2398f28cb16fb0dbbc0382249
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/ip/resolv.py
1 file changed, 46 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/05/62705/1

diff --git a/lib/vdsm/network/ip/resolv.py b/lib/vdsm/network/ip/resolv.py
index 5d9ce22..c21b43f 100644
--- a/lib/vdsm/network/ip/resolv.py
+++ b/lib/vdsm/network/ip/resolv.py
@@ -18,22 +18,60 @@
 # Refer to the README and COPYING files for full details of the license
 from __future__ import absolute_import
 
+from vdsm import dsaversion
+
 DNS_CONF_FILE = '/etc/resolv.conf'
+MAXNS = 3  # the resolver suppors no more than this number of nameservers
 
 
 def get_host_nameservers():
-"""Returns a list of nameservers listed in /etc/resolv.conf"""
-with open(DNS_CONF_FILE, 'r') as file_object:
-file_text = file_object.read()
-return _parse_nameservers(file_text)
+"""Return a list of nameservers listed in /etc/resolv.conf"""
+return _parse_nameservers(_read_resolv_conf())
+
+
+def _read_resolv_conf():
+with open(DNS_CONF_FILE, 'r') as f:
+return f.read()
 
 
 def _parse_nameservers(file_text):
 nameservers = []
-for line in file_text.splitlines():
-words = line.strip().split()
-if len(words) < 2:
-continue
+for words in _parse_lines(file_text):
 if words[0] == 'nameserver':
 nameservers.append(words[1])
 return nameservers
+
+
+def _parse_lines(file_text):
+for line in file_text.splitlines():
+if not line or line[0] in ';#':
+continue
+words = line.strip().split()
+if len(words) >= 2:
+yield words
+
+
+def update(nameservers):
+file_text = _read_resolv_conf()
+old_nameservers = _parse_nameservers(file_text)
+nameservers = list(nameservers)
+_extend_with_unique(nameservers, old_nameservers)
+
+lines = ['# Generated by VDSM version ' + dsaversion.raw_version_revision]
+for words in _parse_lines(file_text):
+if words[0] == 'nameserver':
+if nameservers:
+ns = nameservers.pop(0)
+lines.append('nameserver ' + ns)
+else:
+lines.append(' '.join(words))
+
+with open(DNS_CONF_FILE, 'w') as f:
+return f.writelines(line + '\n' for line in lines)
+
+
+def _extend_with_unique(nameservers, old_nameservers):
+while len(nameservers) < MAXNS and old_nameservers:
+ns = old_nameservers.pop(0)
+if ns not in nameservers:
+nameservers.append(ns)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I41b81b04fbe3e3e2398f28cb16fb0dbbc0382249
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: don't accept nameservers on a non-default network

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: don't accept nameservers on a non-default network
..


Patch Set 2: Verified+1

Legacy network functional tests saw three failing tests (even if run 
individually) in func_rollback_test. This was not a regression, as without this 
branch they failed as well. New DNS patches passed successfully. OVS tests 
passed as well.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net tests: add a 'status' parameter to SetupNetworksError

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net tests: add a 'status' parameter to SetupNetworksError
..


Patch Set 2: Verified+1

Legacy network functional tests saw three failing tests (even if run 
individually) in func_rollback_test. This was not a regression, as without this 
branch they failed as well. New DNS patches passed successfully. OVS tests 
passed as well.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ed5e4672474584364705f399493f357eba86586
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: write out nameservers to ifcfg files

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: write out nameservers to ifcfg files
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Ori Ben Sasson 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: add a 'nameservers' property to NetInfo

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: add a 'nameservers' property to NetInfo
..


Patch Set 2: Verified+1

Verified by Ori Ben Sasson.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id01107ce0983307def68c6f33a7ffe886ce0b0fb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: Introduce nameservers (dns) network api

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Introduce nameservers (dns) network api
..


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Ori Ben Sasson 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: Introduce nameservers (dns) network api

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Introduce nameservers (dns) network api
..


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Ori Ben Sasson 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: don't accept nameservers on a non-default network

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: net: don't accept nameservers on a non-default network
..

net: don't accept nameservers on a non-default network

Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/api.py
A lib/vdsm/network/ip/validator.py
M tests/network/func_dns_test.py
3 files changed, 52 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/62362/1

diff --git a/lib/vdsm/network/api.py b/lib/vdsm/network/api.py
index 7d96b1d..4afedbc 100644
--- a/lib/vdsm/network/api.py
+++ b/lib/vdsm/network/api.py
@@ -37,7 +37,7 @@
 from vdsm.network import libvirt
 from vdsm.network.ipwrapper import DUMMY_BRIDGE
 
-from . ip import address as ipaddress
+from . ip import address as ipaddress, validator as ipvalidator
 from . canonicalize import canonicalize_networks, canonicalize_bondings
 from . errors import RollbackIncomplete
 from . import netconfpersistence
@@ -265,6 +265,7 @@
 canonicalize_bondings(bondings)
 
 logging.debug('Validating configuration')
+ipvalidator.validate(networks)
 netswitch.validate(networks, bondings)
 
 bondings, networks, options = _apply_hook(bondings, networks, options)
diff --git a/lib/vdsm/network/ip/validator.py b/lib/vdsm/network/ip/validator.py
new file mode 100644
index 000..55e5d7e
--- /dev/null
+++ b/lib/vdsm/network/ip/validator.py
@@ -0,0 +1,34 @@
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from __future__ import absolute_import
+
+import six
+
+from vdsm.network import errors as ne
+
+
+def validate(nets):
+for net, attrs in six.iteritems(nets):
+if 'remove' in attrs:
+continue
+elif attrs['nameservers'] and not attrs['defaultRoute']:
+raise ne.ConfigNetworkError(
+ne.ERR_BAD_PARAMS,
+'Name servers may only be defined on the default host network')
diff --git a/tests/network/func_dns_test.py b/tests/network/func_dns_test.py
index 08d7c10..aa4be68 100644
--- a/tests/network/func_dns_test.py
+++ b/tests/network/func_dns_test.py
@@ -20,9 +20,11 @@
 
 from __future__ import absolute_import
 
+from vdsm.network.errors import ERR_BAD_PARAMS
+
 from nose.plugins.attrib import attr
 
-from .netfunctestlib import NetFuncTestCase, NOCHK
+from .netfunctestlib import NetFuncTestCase, NOCHK, SetupNetworksError
 from .nettestlib import dummy_device, restore_resolv_conf
 
 NETWORK_NAME = 'test-network'
@@ -67,6 +69,19 @@
 with self.setupNetworks(NETCREATE, {}, NOCHK):
 self.assertNameservers(original_nameservers)
 
+def test_set_nameservers_on_non_default_network(self):
+with dummy_device() as nic:
+NETCREATE = {NETWORK_NAME: {'nic': nic, 'switch': self.switch,
+'nameservers': NAMESERVERS,
+'defaultRoute': False,
+'ipaddr': IPv4_ADDRESS,
+'netmask': IPv4_NETMASK,
+'gateway': IPv4_GATEWAY,
+}}
+with self.assertRaises(SetupNetworksError) as err:
+self.setupNetworks(NETCREATE, {}, NOCHK)
+self.assertEqual(err.exception.status, ERR_BAD_PARAMS)
+
 
 @attr(type='functional', switch='legacy')
 class NetworkDNSLegacyTest(NetworkDNSTemplate):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net tests: add a 'status' parameter to SetupNetworksError

[osvoboda]

Hello Dan Kenigsberg, Edward Haas,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/62361

to review the following change.

Change subject: net tests: add a 'status' parameter to SetupNetworksError
..

net tests: add a 'status' parameter to SetupNetworksError

Change-Id: I1ed5e4672474584364705f399493f357eba86586
Signed-off-by: Ondřej Svoboda 
Reviewed-on: https://gerrit.ovirt.org/62166
Continuous-Integration: Jenkins CI
Reviewed-by: Edward Haas 
Reviewed-by: Dan Kenigsberg 
---
M tests/network/netfunctestlib.py
1 file changed, 6 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/61/62361/1

diff --git a/tests/network/netfunctestlib.py b/tests/network/netfunctestlib.py
index 54cd99f..fc5ae96 100644
--- a/tests/network/netfunctestlib.py
+++ b/tests/network/netfunctestlib.py
@@ -346,7 +346,10 @@
 
 
 class SetupNetworksError(Exception):
-pass
+def __init__(self, status, msg):
+super(SetupNetworksError, self).__init__(msg)
+self.status = status
+self.msg = msg
 
 
 class SetupNetworks(object):
@@ -361,7 +364,7 @@
 
 status, msg = self.vdsm_proxy.setupNetworks(networks, bonds, options)
 if status != SUCCESS:
-raise SetupNetworksError(msg)
+raise SetupNetworksError(status, msg)
 
 try:
 self.post_setup_hook()
@@ -378,7 +381,7 @@
 def __exit__(self, type, value, traceback):
 status, msg = self._cleanup()
 if type is None and status != SUCCESS:
-raise SetupNetworksError(msg)
+raise SetupNetworksError(status, msg)
 
 def _cleanup(self):
 networks_caps = self.vdsm_proxy.netinfo.networks


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ed5e4672474584364705f399493f357eba86586
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: write out nameservers to ifcfg files

[osvoboda]

Hello Dan Kenigsberg, Edward Haas,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/62360

to review the following change.

Change subject: net: write out nameservers to ifcfg files
..

net: write out nameservers to ifcfg files

This patch handles setupNetworks' recently introduced
'nameservers' key. It adds necessary wiring to propagate
a list of nameservers to configurators and lets the ifcfg
configurator write DNS1= and DNS2= entries accordingly.

Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Bug-Url: https://bugzilla.redhat.com/1351095
Signed-off-by: Ondřej Svoboda 
Reviewed-on: https://gerrit.ovirt.org/61184
Continuous-Integration: Jenkins CI
Reviewed-by: Edward Haas 
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/network/configurators/ifcfg.py
M lib/vdsm/network/legacy_switch.py
M lib/vdsm/network/models.py
A tests/network/func_dns_test.py
M tests/network/netfunctestlib.py
M tests/network/nettestlib.py
6 files changed, 123 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/60/62360/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index 3624451..243a224 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -515,7 +515,8 @@
 if self.unifiedPersistence and utils.isOvirtNode():
 node_fs.Config().persist(fileName)
 
-def _createConfFile(self, conf, name, ipv4, ipv6, mtu=None, **kwargs):
+def _createConfFile(self, conf, name, ipv4, ipv6, mtu, nameservers,
+**kwargs):
 """ Create ifcfg-* file with proper fields per device """
 cfg = 'DEVICE=%s\n' % pipes.quote(name)
 cfg += conf
@@ -549,6 +550,9 @@
 elif ipv6.dhcpv6:
 cfg += 'DHCPV6C=yes\n'
 cfg += 'IPV6_AUTOCONF=%s\n' % _to_ifcfg_bool(ipv6.ipv6autoconf)
+if nameservers:
+for i, nameserver in enumerate(nameservers[0:2], 1):
+cfg += 'DNS{}={}\n'.format(i, nameserver)
 
 ifcfg_file = NET_CONF_PREF + name
 hook_dict = {'ifcfg_file': ifcfg_file,
@@ -574,7 +578,7 @@
 conf += 'BRIDGING_OPTS="%s"\n' % opts['custom']['bridge_opts']
 
 self._createConfFile(conf, bridge.name, bridge.ipv4, bridge.ipv6,
- bridge.mtu, **opts)
+ bridge.mtu, bridge.nameservers, **opts)
 
 def addVlan(self, vlan, **opts):
 """ Create ifcfg-* file with proper fields for VLAN """
@@ -584,7 +588,7 @@
 conf += 'BRIDGE=%s\n' % pipes.quote(vlan.bridge.name)
 conf += 'ONBOOT=yes\n'
 self._createConfFile(conf, vlan.name, vlan.ipv4, vlan.ipv6, vlan.mtu,
- **opts)
+ vlan.nameservers, **opts)
 
 def addBonding(self, bond, **opts):
 """ Create ifcfg-* file with proper fields for bond """
@@ -596,8 +600,9 @@
 conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name)
 conf += 'ONBOOT=yes\n'
 
-ipv4, ipv6, mtu = self._getIfaceConfValues(bond)
-self._createConfFile(conf, bond.name, ipv4, ipv6, mtu, **opts)
+ipv4, ipv6, mtu, nameservers = self._getIfaceConfValues(bond)
+self._createConfFile(conf, bond.name, ipv4, ipv6, mtu, nameservers,
+ **opts)
 
 # create the bonding device to avoid initscripts noise
 with open(netinfo_bonding.BONDING_MASTERS) as info:
@@ -622,14 +627,16 @@
 if ethtool_opts:
 conf += 'ETHTOOL_OPTS=%s\n' % pipes.quote(ethtool_opts)
 
-ipv4, ipv6, mtu = self._getIfaceConfValues(nic)
-self._createConfFile(conf, nic.name, ipv4, ipv6, mtu, **opts)
+ipv4, ipv6, mtu, nameservers = self._getIfaceConfValues(nic)
+self._createConfFile(conf, nic.name, ipv4, ipv6, mtu, nameservers,
+ **opts)
 
 @staticmethod
 def _getIfaceConfValues(iface):
 ipv4 = copy.deepcopy(iface.ipv4)
 ipv6 = copy.deepcopy(iface.ipv6)
 mtu = iface.mtu
+nameservers = iface.nameservers
 if ifaceUsed(iface.name):
 confParams = misc.getIfaceCfg(iface.name)
 if not ipv4.address and ipv4.bootproto != 'dhcp':
@@ -649,7 +656,10 @@
 mtu = confParams.get('MTU')
 if mtu:
 mtu = int(mtu)
-return ipv4, ipv6, mtu
+if iface.nameservers is None:
+nameservers = [confParams[key] for key in ('DNS1', 'DNS2')
+   if key in confParams]
+return ipv4, ipv6, mtu, nameservers
 
 def removeNic(self, nic):
 cf = NET_CONF_PREF + nic
diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index c58450d..05a62c2 

Change in vdsm[ovirt-4.0]: net: add a 'nameservers' property to NetInfo

[osvoboda]

Hello Dan Kenigsberg, Edward Haas,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/62359

to review the following change.

Change subject: net: add a 'nameservers' property to NetInfo
..

net: add a 'nameservers' property to NetInfo

Functional tests in the following patch will use it.

Change-Id: Id01107ce0983307def68c6f33a7ffe886ce0b0fb
Bug-Url: https://bugzilla.redhat.com/1351095
Signed-off-by: Ondřej Svoboda 
Reviewed-on: https://gerrit.ovirt.org/62148
Continuous-Integration: Jenkins CI
Reviewed-by: Edward Haas 
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/network/netinfo/cache.py
M tests/network/config_network_test.py
M tests/network/models_test.py
M tests/network/unified_persistence_test.py
4 files changed, 9 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/59/62359/1

diff --git a/lib/vdsm/network/netinfo/cache.py 
b/lib/vdsm/network/netinfo/cache.py
index 9b7d67d..764b54e 100644
--- a/lib/vdsm/network/netinfo/cache.py
+++ b/lib/vdsm/network/netinfo/cache.py
@@ -225,6 +225,7 @@
 self.nics = _netinfo['nics']
 self.bondings = _netinfo['bondings']
 self.bridges = _netinfo['bridges']
+self.nameservers = _netinfo['nameservers']
 
 def del_network(self, network):
 del self.networks[network]
@@ -361,3 +362,4 @@
 self.nics = _netinfo['nics']
 self.bondings = _netinfo['bondings']
 self.bridges = _netinfo['bridges']
+self.nameservers = _netinfo['nameservers']
diff --git a/tests/network/config_network_test.py 
b/tests/network/config_network_test.py
index caf100d..09ba27f 100644
--- a/tests/network/config_network_test.py
+++ b/tests/network/config_network_test.py
@@ -101,7 +101,8 @@
 'fakebr1': {'ports': ['bond00']},
 'fakebr2': {'ports': ['eth7.1']}
 },
-'bondings': {'bond00': {'slaves': ['eth5', 'eth6']}}
+'bondings': {'bond00': {'slaves': ['eth5', 'eth6']}},
+'nameservers': [],
 }
 
 fakeInfo = netinfo.cache.CachingNetInfo(_netinfo)
diff --git a/tests/network/models_test.py b/tests/network/models_test.py
index e6991bc..b4abeb9 100644
--- a/tests/network/models_test.py
+++ b/tests/network/models_test.py
@@ -139,7 +139,7 @@
 def testTextualRepr(self):
 _netinfo = {'networks': {}, 'vlans': {},
 'nics': ['testnic1', 'testnic2'],
-'bondings': {}, 'bridges': {}}
+'bondings': {}, 'bridges': {}, 'nameservers': []}
 fakeInfo = CachingNetInfo(_netinfo)
 nic1 = Nic('testnic1', None, _netinfo=fakeInfo)
 nic2 = Nic('testnic2', None, _netinfo=fakeInfo)
@@ -175,7 +175,7 @@
 def testIterNetworkHierarchy(self):
 _netinfo = {'networks': {}, 'vlans': {},
 'nics': ['testnic1', 'testnic2'],
-'bondings': {}, 'bridges': {}}
+'bondings': {}, 'bridges': {}, 'nameservers': []}
 fakeInfo = CachingNetInfo(_netinfo)
 # Vlanned and bonded VM network
 nic1 = Nic('testnic1', configurator=None, _netinfo=fakeInfo)
diff --git a/tests/network/unified_persistence_test.py 
b/tests/network/unified_persistence_test.py
index a459cd4..a165cf5 100644
--- a/tests/network/unified_persistence_test.py
+++ b/tests/network/unified_persistence_test.py
@@ -88,7 +88,9 @@
 'bridged': True, 'ipv6addrs': ['fe80::b88d:23ff:fe4d:d741/64'],
 'gateway': '', 'bootproto4': 'none', 'netmask': '', 'stp': 'off',
 'ipv4addrs': [], 'mtu': '1500', 'ipv6gateway': '::', 'ports':
-[BOND_NAME + '.' + VLAN]}}}
+[BOND_NAME + '.' + VLAN]}},
+'nameservers': [],
+}
 
 
 FAKE_IFCFGS = {


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id01107ce0983307def68c6f33a7ffe886ce0b0fb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: don't accept nameservers on a non-default network

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: don't accept nameservers on a non-default network
..


Patch Set 7: Verified+1

No failed unit tests on EL7 and 1 failed on F24, though it is not a regression. 
'del_neigh' is not caught by test_events_keys (tests/network/link_test.py).

Old functional tests have seen no regression. test_ipv4_default_route and 
testSetupNetworksEmergencyDevicesCleanup(Bond|Vlan)Overwrite fail on both 
platforms. Additionally, tests using dhclient directly (5) result in errors on 
F24 because the '-timeout' parameter has been renamed to '--timeout'.

Among new functional tests, test_add_net_based_on_nic fails in operstate check 
but again, it is no regression: AssertionError: u'DOWN' not found in ('UP', 
'UNKNOWN')

The newly added functional tests work fine.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net tests: add a 'status' parameter to SetupNetworksError

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net tests: add a 'status' parameter to SetupNetworksError
..


Patch Set 3: Verified+1

No failed unit tests on EL7 and 1 failed on F24, though it is not a regression. 
'del_neigh' is not caught by test_events_keys (tests/network/link_test.py).

Old functional tests have seen no regression. test_ipv4_default_route and 
testSetupNetworksEmergencyDevicesCleanup(Bond|Vlan)Overwrite fail on both 
platforms. Additionally, tests using dhclient directly (5) result in errors on 
F24 because the '-timeout' parameter has been renamed to '--timeout'.

Among new functional tests, test_add_net_based_on_nic fails in operstate check 
but again, it is no regression: AssertionError: u'DOWN' not found in ('UP', 
'UNKNOWN')

The newly added functional tests work fine.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ed5e4672474584364705f399493f357eba86586
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: write out nameservers to ifcfg files

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: write out nameservers to ifcfg files
..


Patch Set 17: Verified+1

No failed unit tests on EL7 and 1 failed on F24, though it is not a regression. 
'del_neigh' is not caught by test_events_keys (tests/network/link_test.py).

Old functional tests have seen no regression. test_ipv4_default_route and 
testSetupNetworksEmergencyDevicesCleanup(Bond|Vlan)Overwrite fail on both 
platforms. Additionally, tests using dhclient directly (5) result in errors on 
F24 because the '-timeout' parameter has been renamed to '--timeout'.

Among new functional tests, test_add_net_based_on_nic fails in operstate check 
but again, it is no regression: AssertionError: u'DOWN' not found in ('UP', 
'UNKNOWN')

The newly added functional tests work fine.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: add a 'nameservers' property to NetInfo

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: add a 'nameservers' property to NetInfo
..


Patch Set 2: Verified+1

No failed unit tests on EL7 and 1 failed on F24, though it is not a regression. 
'del_neigh' is not caught by test_events_keys (tests/network/link_test.py).

Old functional tests have seen no regression. test_ipv4_default_route and 
testSetupNetworksEmergencyDevicesCleanup(Bond|Vlan)Overwrite fail on both 
platforms. Additionally, tests using dhclient directly (5) result in errors on 
F24 because the '-timeout' parameter has been renamed to '--timeout'.

Among new functional tests, test_add_net_based_on_nic fails in operstate check 
but again, it is no regression: AssertionError: u'DOWN' not found in ('UP', 
'UNKNOWN')

The newly added functional tests work fine.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id01107ce0983307def68c6f33a7ffe886ce0b0fb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: Report the default gateway flag through caps

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Report the default gateway flag through caps
..


Patch Set 12: Code-Review+1

Thanks!

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6dfbdd71310cbbb672a185e2f1ac6fdd9b495906
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: Report the default gateway flag through caps

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Report the default gateway flag through caps
..


Patch Set 11: Code-Review-1

(1 comment)

One nit/confusion.

https://gerrit.ovirt.org/#/c/58351/11/tests/network/func_static_ip_test.py
File tests/network/func_static_ip_test.py:

PS11, Line 86: nic1
Seeing nic1 made me wonder if there is 'nic2' in this test as well. Please make 
it just 'nic' :-)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6dfbdd71310cbbb672a185e2f1ac6fdd9b495906
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: don't accept nameservers on a non-default network

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: don't accept nameservers on a non-default network
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/62167/6/lib/vdsm/network/ip/validator.py
File lib/vdsm/network/ip/validator.py:

PS6, Line 24: from .. import errors as ne
> Please use 'from vdsm.network import errors as ne' and not the relative pat
Good point.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: don't accept nameservers on a non-default network

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: don't accept nameservers on a non-default network
..


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/62167/5/lib/vdsm/network/api.py
File lib/vdsm/network/api.py:

PS5, Line 270: validate_network_setup_common
> lets call it 'validate' like the others.
OK, the short name is better.


PS5, Line 270: validator
> Ahh.. it looks like no one owns this validator, putting it under ip will gi
Let's have an IP validator.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: tests: add a 'status' parameter to SetupNetworksError

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: tests: add a 'status' parameter to SetupNetworksError
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/62166/2//COMMIT_MSG
Commit Message:

PS2, Line 7: tests
> You better mention it is a net test.
I made it "net tests".


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ed5e4672474584364705f399493f357eba86586
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: don't accept a network with nameservers but default rou...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: don't accept a network with nameservers but default route 
unset
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/62167/1/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

PS1, Line 116: with a default route
> It is a bit confusing to link nameservers with the default route.
Okay, now I use the term "default network". Validation has been moved to api, 
with implementation in a new module (which may see some unified validation in 
the future).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: don't accept a network with nameservers but default rou...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: don't accept a network with nameservers but default route 
unset
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/62167/1/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

Line 100: ne.ERR_BAD_PARAMS,
Line 101: 'Mixing of legacy and OVS networks is not supported 
inside one '
Line 102: 'setupNetworks() call.')
Line 103: 
Line 104: _validate_network_setup_common(legacy_nets or ovs_nets)
> Please move this to the api.py level, it is agnostic to the switch type use
Hmm, DNS is certainly related to IP and the notion of a default network is 
again linked to (IP) routing...

Yes, we could have an ip.validate module. But 'validate.py' in this directory 
would work as well.
Line 105: if use_legacy_switch:
Line 106: legacy_switch.validate_network_setup(legacy_nets, 
legacy_bonds)
Line 107: elif use_ovs_switch:
Line 108: ovs_switch.validate_network_setup(ovs_nets, ovs_bonds)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: tests: add assertRaisesSetupNetworksError to netfunctestlib

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: tests: add assertRaisesSetupNetworksError to netfunctestlib
..


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/62166/1/tests/network/netfunctestlib.py
File tests/network/netfunctestlib.py:

Line 332: self.assertEqual(nameservers,
Line 333:  self.netinfo.nameservers[:len(nameservers)])
Line 334: 
Line 335: @contextmanager
Line 336: def assertRaisesSetupNetworksError(self, status):
> I'm not convinced that this error wrapper is needed.
Currently, yes. I thought more tests could use it in the future. But I don't 
mind dropping it for now.
Line 337: with self.assertRaises(SetupNetworksError) as sne_context:
Line 338: yield
Line 339: self.assertEqual(sne_context.exception.args[0], status)
Line 340: 


PS1, Line 337: sne_context
> 'ex' or 'exc' ?
I'll get rid of "context" (I took it from config_network_test.py). In the line 
below, I didn't want to duplicate "exception" so I would prefer "sne" or just 
"e".


PS1, Line 339: args[0]
> It will be better to add an attribute to the SetupNetowrkError class and na
I agree.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ed5e4672474584364705f399493f357eba86586
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: add a 'nameservers' property to NetInfo

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: add a 'nameservers' property to NetInfo
..


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/62148/1//COMMIT_MSG
Commit Message:

PS1, Line 7: netinfo
> net
Done


https://gerrit.ovirt.org/#/c/62148/1/tests/network/config_network_test.py
File tests/network/config_network_test.py:

PS1, Line 105: ,
> redundant
I took the liberty to keep the commas – so a new key (e.g. 'searchDomains' or 
just 'domains') can be added without the need to modify this line.


https://gerrit.ovirt.org/#/c/62148/1/tests/network/unified_persistence_test.py
File tests/network/unified_persistence_test.py:

PS1, Line 77: ,
> redundant
Please let me keep the comma :-)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id01107ce0983307def68c6f33a7ffe886ce0b0fb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: write out nameservers to ifcfg files

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: write out nameservers to ifcfg files
..


Patch Set 13:

(4 comments)

https://gerrit.ovirt.org/#/c/61184/13/tests/network/func_dns_test.py
File tests/network/func_dns_test.py:

Line 67: self.assertNameservers(NAMESERVERS)
Line 68: 
Line 69: 
Line 70: @attr(type='functional', switch='legacy')
Line 71: class NetworkDNSLegacyTest(NetworkDNSTemplate):
> Any reason you are not covering OVS?
As soon as I verify it works, I will add it.
Line 72: __test__ = True


https://gerrit.ovirt.org/#/c/61184/14/tests/network/func_dns_test.py
File tests/network/func_dns_test.py:

PS14, Line 53: ement_network_without_nameservers(se
> Lets assert this before the setup.
Done in patch set 16, thanks!


PS14, Line 54: e() as nic:
 : NETCREATE = {NETWORK_NAME: {'nic': nic, 'switch': sel
> If we move the previous 'if' to an assert before the setup, wouldn't this b
Indeed :-)


https://gerrit.ovirt.org/#/c/61184/15/tests/network/func_dns_test.py
File tests/network/func_dns_test.py:

PS15, Line 82: 
> False
I dropped the results of the tests from the TODO (in patch set 17).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: don't accept a network with nameservers but default rou...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: don't accept a network with nameservers but default route 
unset
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/62167/1/tests/network/func_dns_test.py
File tests/network/func_dns_test.py:

PS1, Line 77: 'ipaddr': IPv4_ADDRESS,
: 'netmask': IPv4_NETMASK,
: 'gateway': IPv4_GATEWAY,
This and 'nic' is unnecessary.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: don't accept a network with nameservers but default rou...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: net: don't accept a network with nameservers but default route 
unset
..

net: don't accept a network with nameservers but default route unset

Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/netswitch.py
M tests/network/func_dns_test.py
2 files changed, 23 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/62167/1

diff --git a/lib/vdsm/network/netswitch.py b/lib/vdsm/network/netswitch.py
index 6df8ed8..792f81d 100644
--- a/lib/vdsm/network/netswitch.py
+++ b/lib/vdsm/network/netswitch.py
@@ -101,12 +101,21 @@
 'Mixing of legacy and OVS networks is not supported inside one '
 'setupNetworks() call.')
 
+_validate_network_setup_common(legacy_nets or ovs_nets)
 if use_legacy_switch:
 legacy_switch.validate_network_setup(legacy_nets, legacy_bonds)
 elif use_ovs_switch:
 ovs_switch.validate_network_setup(ovs_nets, ovs_bonds)
 
 
+def _validate_network_setup_common(nets):
+for net, attrs in six.iteritems(nets):
+if ('remove' not in attrs and attrs['nameservers'] and
+not attrs['defaultRoute']):
+raise ne.ConfigNetworkError(ne.ERR_BAD_PARAMS, 'Name servers may '
+'only be defined with a default route')
+
+
 def setup(networks, bondings, options, in_rollback):
 legacy_nets, ovs_nets, legacy_bonds, ovs_bonds = _split_switch_type(
 networks, bondings)
diff --git a/tests/network/func_dns_test.py b/tests/network/func_dns_test.py
index 7abd2dc..b44d270 100644
--- a/tests/network/func_dns_test.py
+++ b/tests/network/func_dns_test.py
@@ -20,6 +20,8 @@
 
 from __future__ import absolute_import
 
+from vdsm.network.errors import ERR_BAD_PARAMS
+
 from nose.plugins.attrib import attr
 
 from .netfunctestlib import NetFuncTestCase, NOCHK
@@ -67,6 +69,18 @@
 with self.setupNetworks(NETCREATE, {}, NOCHK):
 self.assertNameservers(original_nameservers)
 
+def test_fail_with_nameservers_but_with_default_route_unset(self):
+with dummy_device() as nic:
+NETCREATE = {NETWORK_NAME: {'nic': nic, 'switch': self.switch,
+'nameservers': NAMESERVERS,
+'defaultRoute': False,
+'ipaddr': IPv4_ADDRESS,
+'netmask': IPv4_NETMASK,
+'gateway': IPv4_GATEWAY,
+}}
+with self.assertRaisesSetupNetworksError(ERR_BAD_PARAMS):
+self.setupNetworks(NETCREATE, {}, NOCHK)
+
 
 @attr(type='functional', switch='legacy')
 class NetworkDNSLegacyTest(NetworkDNSTemplate):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: tests: add assertRaisesSetupNetworksError to netfunctestlib

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: tests: add assertRaisesSetupNetworksError to netfunctestlib
..

tests: add assertRaisesSetupNetworksError to netfunctestlib

Change-Id: I1ed5e4672474584364705f399493f357eba86586
Signed-off-by: Ondřej Svoboda 
---
M tests/network/netfunctestlib.py
1 file changed, 9 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/62166/1

diff --git a/tests/network/netfunctestlib.py b/tests/network/netfunctestlib.py
index 5da4d07..7398db5 100644
--- a/tests/network/netfunctestlib.py
+++ b/tests/network/netfunctestlib.py
@@ -20,6 +20,7 @@
 
 from __future__ import absolute_import
 
+from contextlib import contextmanager
 from copy import deepcopy
 
 import six
@@ -331,6 +332,12 @@
 self.assertEqual(nameservers,
  self.netinfo.nameservers[:len(nameservers)])
 
+@contextmanager
+def assertRaisesSetupNetworksError(self, status):
+with self.assertRaises(SetupNetworksError) as sne_context:
+yield
+self.assertEqual(sne_context.exception.args[0], status)
+
 def assert_kernel_vs_running_config(self):
 """
 This is a special test, that checks setup integrity through
@@ -378,7 +385,7 @@
 
 status, msg = self.vdsm_proxy.setupNetworks(networks, bonds, options)
 if status != SUCCESS:
-raise SetupNetworksError(msg)
+raise SetupNetworksError(status, msg)
 
 try:
 self.post_setup_hook()
@@ -395,7 +402,7 @@
 def __exit__(self, type, value, traceback):
 status, msg = self._cleanup()
 if type is None and status != SUCCESS:
-raise SetupNetworksError(msg)
+raise SetupNetworksError(status, msg)
 
 def _cleanup(self):
 networks_caps = self.vdsm_proxy.netinfo.networks


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ed5e4672474584364705f399493f357eba86586
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: write out nameservers to ifcfg files

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: write out nameservers to ifcfg files
..


Patch Set 13:

(3 comments)

https://gerrit.ovirt.org/#/c/61184/13/lib/vdsm/network/netinfo/cache.py
File lib/vdsm/network/netinfo/cache.py:

Line 216: self.vlans = _netinfo['vlans']
Line 217: self.nics = _netinfo['nics']
Line 218: self.bondings = _netinfo['bondings']
Line 219: self.bridges = _netinfo['bridges']
Line 220: self._netinfo = _netinfo
> Please keep only what you need (nameservers)
Done
Line 221: 
Line 222: def del_network(self, network):
Line 223: del self.networks[network]
Line 224: 


https://gerrit.ovirt.org/#/c/61184/13/tests/network/netfunctestlib.py
File tests/network/netfunctestlib.py:

Line 327: # Links are sometimes marked as UNKNOWN after turned 
UP.
Line 328: self.assertIn(operstate(dev).upper(), ('UP', 
'UNKNOWN'))
Line 329: 
Line 330: def assertNameservers(self, nameservers):
Line 331: self.assertTrue(set(nameservers) <= set(
> I think that initscripts are modifying resolv.conf, but I did not check thi
Initscripts do a kind of a find on resolv.conf, they even keep "# 
Generated by NetworkManager" there.

More than two nameservers might be left there so equality check is not feasible.

But I think it makes sense to check that nameservers added by a test are listed 
first.
Line 332: self.netinfo._netinfo['nameservers']))
Line 333: 
Line 334: def assert_kernel_vs_running_config(self):
Line 335: """


PS13, Line 332: _
> We should not check against a private structure, only through the API layer
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: write out nameservers to ifcfg files

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: write out nameservers to ifcfg files
..


Patch Set 12: Verified+1

Two new DNS tests pass with the legacy switch.

One of them defines a management network with nameservers explicitly, and 
checks they are present in the system. The other add nameservers to 
/etc/resolv.conf manually and lets the management network adopt them.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: write out nameservers to ifcfg files

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: write out nameservers to ifcfg files
..


Patch Set 11:

(2 comments)

https://gerrit.ovirt.org/#/c/61184/11/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

PS11, Line 520: 
> removal of kwargs should not be part of this patch. are we sure that **opts
kwargs is only declared here and not used inside the function at all.


https://gerrit.ovirt.org/#/c/61184/8/lib/vdsm/network/legacy_switch.py
File lib/vdsm/network/legacy_switch.py:

PS8, Line 193: 
> Yes, just move it after configurator argument, without the default and chec
I made sure in interactive Python command line, it worked.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: network: rename reported 'dnss' to 'nameservers' for clarity

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: rename reported 'dnss' to 'nameservers' for clarity
..


Patch Set 2: Verified+1

In spite of Fedora 23 CI host having some troube in 'yum install' (?) unit 
tests passed on my F23. 'nameservers' are reported instead of 'dnss' now.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie66c0e8cdf86da38a8e1f4b35d284c174973389e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: network: rename reported 'dnss' to 'nameservers' for clarity

[osvoboda]

Hello Dan Kenigsberg, Edward Haas,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/61932

to review the following change.

Change subject: network: rename reported 'dnss' to 'nameservers' for clarity
..

network: rename reported 'dnss' to 'nameservers' for clarity

Change-Id: Ie66c0e8cdf86da38a8e1f4b35d284c174973389e
Signed-off-by: Ondřej Svoboda 
Reviewed-on: https://gerrit.ovirt.org/61712
Continuous-Integration: Jenkins CI
Reviewed-by: Edward Haas 
Reviewed-by: Dan Kenigsberg 
---
M lib/api/vdsm-api.yml
M lib/vdsm/network/netinfo/cache.py
M lib/vdsm/network/netinfo/dns.py
M tests/network/netinfo_test.py
4 files changed, 9 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/61932/1

diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index c9a74a6..11e5b46 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -6384,7 +6384,7 @@
 added: '4.0'
 
 -   description: Provides host DNS servers from /etc/resolve.conf
-name: dnss
+name: nameservers
 type:
 - string
 added: '4.0'
diff --git a/lib/vdsm/network/netinfo/cache.py 
b/lib/vdsm/network/netinfo/cache.py
index 9b7d67d..a9d0ef3 100644
--- a/lib/vdsm/network/netinfo/cache.py
+++ b/lib/vdsm/network/netinfo/cache.py
@@ -61,7 +61,7 @@
 :return: Dict of networking devices with all their details.
 """
 networking = {'bondings': {}, 'bridges': {}, 'networks': {}, 'nics': {},
-  'vlans': {}, 'dnss': get_host_nameservers()}
+  'vlans': {}, 'nameservers': get_host_nameservers()}
 paddr = bonding.permanent_address()
 ipaddrs = getIpAddrs()
 routes = get_routes()
diff --git a/lib/vdsm/network/netinfo/dns.py b/lib/vdsm/network/netinfo/dns.py
index e22714e..3ca82da 100644
--- a/lib/vdsm/network/netinfo/dns.py
+++ b/lib/vdsm/network/netinfo/dns.py
@@ -25,15 +25,15 @@
 """Returns a list of nameservers listed in /etc/resolv.conf"""
 with open(DNS_CONF_FILE, 'r') as file_object:
 file_text = file_object.read()
-return _parse_dnss(file_text)
+return _parse_nameservers(file_text)
 
 
-def _parse_dnss(file_text):
-dnss = []
+def _parse_nameservers(file_text):
+nameservers = []
 for line in file_text.splitlines():
 words = line.strip().split()
 if len(words) < 2:
 continue
 if words[0] == 'nameserver':
-dnss.append(words[1])
-return dnss
+nameservers.append(words[1])
+return nameservers
diff --git a/tests/network/netinfo_test.py b/tests/network/netinfo_test.py
index 8fde0a0..f217a65 100644
--- a/tests/network/netinfo_test.py
+++ b/tests/network/netinfo_test.py
@@ -58,7 +58,7 @@
 'nameserver 8.8.8.8\n'
 'nameserver 8.8.4.4\n'
 )
-dnss = ['192.168.0.100', '8.8.8.8', '8.8.4.4']
+nameservers = ['192.168.0.100', '8.8.8.8', '8.8.4.4']
 with namedTemporaryDir() as temp_dir:
 file_path = os.path.join(temp_dir, 'resolv.conf')
 
@@ -68,7 +68,7 @@
 file_object.write(content)
 
 self.assertEqual(
-dns.get_host_nameservers(), dnss)
+dns.get_host_nameservers(), nameservers)
 
 def testNetmaskConversions(self):
 path = os.path.join(os.path.dirname(__file__), "netmaskconversions")


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie66c0e8cdf86da38a8e1f4b35d284c174973389e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: net: Introduce nameservers (dns) network api

[osvoboda]

Hello Dan Kenigsberg, Edward Haas,

I'd like you to do a code review.  Please visit

https://gerrit.ovirt.org/61931

to review the following change.

Change subject: net: Introduce nameservers (dns) network api
..

net: Introduce nameservers (dns) network api

oVirt is required to preserve and manage the nameservers for the host.

This patch introduces the api layer through which nameservers may be
set.
Nameservers can be set through setupNetworks verb, using the 'nameservers'
key in the networks sub-verb where the value is a list of nameservers.

Canonicalization: In case the key is missing, it will be populated with
the current dns setting (from resolv.conf) for the management network
(identified by the defaultRoute flag).

Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Signed-off-by: Edward Haas 
Reviewed-on: https://gerrit.ovirt.org/61739
Continuous-Integration: Jenkins CI
Tested-by: Ondřej Svoboda 
Reviewed-by: Ondřej Svoboda 
Reviewed-by: Dan Kenigsberg 
---
M lib/api/vdsm-api.yml
M lib/vdsm/network/api.py
M lib/vdsm/network/canonicalize.py
M lib/vdsm/network/kernelconfig.py
4 files changed, 26 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/61931/1

diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index c9a74a6..654ed85 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -5261,6 +5261,13 @@
 type: boolean
 
 -   defaultvalue: no-default
+description: List of nameservers
+name: nameservers
+type:
+- string
+added: '4.0'
+
+-   defaultvalue: no-default
 description: Specify Switch. Legacy is used if not passed
 name: switch
 type: string
diff --git a/lib/vdsm/network/api.py b/lib/vdsm/network/api.py
index 7d96b1d..df858ae 100644
--- a/lib/vdsm/network/api.py
+++ b/lib/vdsm/network/api.py
@@ -217,6 +217,7 @@
 ipv6autoconf="0|1"
 dhcpv6="0|1"
 defaultRoute=True|False
+nameservers=[,  ...]"
 switch="legacy|..."
 (other options will be passed to the config file AS-IS)
 -- OR --
diff --git a/lib/vdsm/network/canonicalize.py b/lib/vdsm/network/canonicalize.py
index 3c2da55..b0bc983 100644
--- a/lib/vdsm/network/canonicalize.py
+++ b/lib/vdsm/network/canonicalize.py
@@ -21,7 +21,7 @@
 
 import six
 
-from .netinfo import (bridges, mtus, bonding)
+from .netinfo import bridges, mtus, bonding, dns
 from vdsm import utils
 
 from .errors import ConfigNetworkError
@@ -46,6 +46,7 @@
 _canonicalize_ipv6(attrs)
 _canonicalize_switch_type_net(attrs)
 _canonicalize_ip_default_route(attrs)
+_canonicalize_nameservers(attrs)
 
 
 def canonicalize_bondings(bonds):
@@ -128,3 +129,12 @@
 def _canonicalize_ip_default_route(data):
 if 'defaultRoute' not in data:
 data['defaultRoute'] = False
+
+
+def _canonicalize_nameservers(data):
+if 'nameservers' not in data:
+# Nameservers are relevant only for the management network.
+if data['defaultRoute']:
+data['nameservers'] = dns.get_host_nameservers()
+else:
+data['nameservers'] = []
diff --git a/lib/vdsm/network/kernelconfig.py b/lib/vdsm/network/kernelconfig.py
index a125e69..abb73cf 100644
--- a/lib/vdsm/network/kernelconfig.py
+++ b/lib/vdsm/network/kernelconfig.py
@@ -26,6 +26,7 @@
 from vdsm.network.netinfo import addresses
 from vdsm.network.netinfo import bonding
 from vdsm.network.netinfo import bridges
+from vdsm.network.netinfo import dns
 from vdsm.network.netinfo import routes
 from vdsm.network.netconfpersistence import BaseConfig
 
@@ -84,6 +85,7 @@
 _translate_ipaddr(attributes, net_attr)
 _translate_hostqos(attributes, net_attr)
 _translate_switch_type(attributes, net_attr)
+_translate_nameservers(attributes)
 
 return attributes
 
@@ -157,6 +159,11 @@
 attributes['switch'] = net_attr['switch']
 
 
+def _translate_nameservers(attributes):
+nservers = dns.get_host_nameservers() if attributes['defaultRoute'] else []
+attributes['nameservers'] = nservers
+
+
 def _remove_zero_values_in_net_qos(net_qos):
 """
 net_qos = {'out': {


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
___
vdsm-patches mailing list

Change in vdsm[master]: network: rename reported 'dnss' to 'nameservers' for clarity

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: rename reported 'dnss' to 'nameservers' for clarity
..


Patch Set 4: Verified+1

Netinfo now reports 'nameservers'. Unit tests passed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie66c0e8cdf86da38a8e1f4b35d284c174973389e
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 8:

(4 comments)

Let me add tests in a separate patch.

https://gerrit.ovirt.org/#/c/61184/8//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2016-08-01 17:11:46 +0200
Line 6: 
Line 7: ifcfg: write current DNS information to a management network's ifcfg 
file
Line 8: 
Line 9: This is to maintain the current DNS configuration.
> It is to set DNS server entries, not only to preserve them.
Done
Line 10: 
Line 11: DNS1= and DNS2= are used to carry 'nameserver' entries from 
/etc/resolv.conf
Line 12: which would otherwise be lost.
Line 13: 


https://gerrit.ovirt.org/#/c/61184/8/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

Line 551: cfg += 'IPV6_DEFAULTGW=%s\n' % 
pipes.quote(ipv6.gateway)
Line 552: elif ipv6.dhcpv6:
Line 553: cfg += 'DHCPV6C=yes\n'
Line 554: cfg += 'IPV6_AUTOCONF=%s\n' % 
_to_ifcfg_bool(ipv6.ipv6autoconf)
Line 555: # Preserve DNS configuration in a management network ifcfg 
file. Only
> I do not think the comment is needed, it is clear that this value is set if
Done
Line 556: # a top-level device (of any type) carries this information.
Line 557: if device.nameservers:
Line 558: for i, nameserver in enumerate(device.nameservers[0:2], 
1):
Line 559: cfg += 'DNS{}={}\n'.format(i, nameserver)


https://gerrit.ovirt.org/#/c/61184/8/lib/vdsm/network/legacy_switch.py
File lib/vdsm/network/legacy_switch.py:

PS8, Line 114: to be added to /etc/resolv.conf
> implementation detail, please remove.
Done


PS8, Line 193: =None
> No need for the default, it is populated by the canonicalize step.
Did you mean – make this a positional argument like 'network' and 
'configurator' because 'nameservers' is always populated (as well as _netinfo)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: Introduce nameservers (dns) network api

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Introduce nameservers (dns) network api
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: rename reported 'dnss' to 'nameservers' for clarity

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: rename reported 'dnss' to 'nameservers' for clarity
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/61712/3/lib/vdsm/network/netinfo/dns.py
File lib/vdsm/network/netinfo/dns.py:

PS3, Line 31: dnss
> what about this one?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie66c0e8cdf86da38a8e1f4b35d284c174973389e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: introduce DNS search domains to the API

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: network: introduce DNS search domains to the API
..

network: introduce DNS search domains to the API

This patch adds 'searchDomains' key to the 'setupNetworks'
verb. If the key is missing and the network in question is the
management network (bearing the default route), it is completed
with information from /etc/resolv.conf in the same manner as
the previously introduced 'nameservers' key.

Change-Id: Ib3849ebac81a07fbb3b9d1ebc5d517dc143e291f
Signed-off-by: Ondřej Svoboda 
---
M lib/api/vdsm-api.yml
M lib/vdsm/network/api.py
M lib/vdsm/network/canonicalize.py
M lib/vdsm/network/kernelconfig.py
4 files changed, 22 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/95/61795/1

diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index a6790c9..a49e071 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -5334,6 +5334,13 @@
 added: '4.0'
 
 -   defaultvalue: no-default
+description: List of DNS search domains
+name: searchDomains
+type:
+- string
+added: '4.0'
+
+-   defaultvalue: no-default
 description: Specify Switch. Legacy is used if not passed
 name: switch
 type: string
diff --git a/lib/vdsm/network/api.py b/lib/vdsm/network/api.py
index a09ebfb..0a4abd5 100644
--- a/lib/vdsm/network/api.py
+++ b/lib/vdsm/network/api.py
@@ -218,6 +218,7 @@
 dhcpv6="0|1"
 defaultRoute=True|False
 nameservers=[,  ...]"
+searchDomains=[,  ...]"
 switch="legacy|..."
 (other options will be passed to the config file AS-IS)
 -- OR --
diff --git a/lib/vdsm/network/canonicalize.py b/lib/vdsm/network/canonicalize.py
index 71a8f5a..c666f58 100644
--- a/lib/vdsm/network/canonicalize.py
+++ b/lib/vdsm/network/canonicalize.py
@@ -46,7 +46,7 @@
 _canonicalize_ipv6(attrs)
 _canonicalize_switch_type_net(attrs)
 _canonicalize_ip_default_route(attrs)
-_canonicalize_nameservers(attrs)
+_canonicalize_nameservers_and_search_domains(attrs)
 
 
 def canonicalize_bondings(bonds):
@@ -131,11 +131,14 @@
 data['defaultRoute'] = False
 
 
-def _canonicalize_nameservers(data):
+def _canonicalize_nameservers_and_search_domains(data):
 if 'nameservers' not in data:
-# Nameservers are relevant only for the management network.
+# Nameservers and search domains are relevant only for the management
+# network.
 if data['defaultRoute']:
-nameservers, _ = dns.get_resolver_config()
-data['nameservers'] = nameservers
+nameservers, search_domains = dns.get_resolver_config()
 else:
-data['nameservers'] = []
+nameservers = []
+search_domains = []
+data['nameservers'] = nameservers
+data['searchDomains'] = search_domains
diff --git a/lib/vdsm/network/kernelconfig.py b/lib/vdsm/network/kernelconfig.py
index e4e1fc4..3ed15cd 100644
--- a/lib/vdsm/network/kernelconfig.py
+++ b/lib/vdsm/network/kernelconfig.py
@@ -85,7 +85,7 @@
 _translate_ipaddr(attributes, net_attr)
 _translate_hostqos(attributes, net_attr)
 _translate_switch_type(attributes, net_attr)
-_translate_nameservers(attributes)
+_translate_nameservers_and_search_domains(attributes)
 
 return attributes
 
@@ -159,12 +159,14 @@
 attributes['switch'] = net_attr['switch']
 
 
-def _translate_nameservers(attributes):
+def _translate_nameservers_and_search_domains(attributes):
 if attributes['defaultRoute']:
-nameservers, _ = dns.get_resolver_config()
+nameservers, search_domains = dns.get_resolver_config()
 else:
 nameservers = []
+search_domains = []
 attributes['nameservers'] = nameservers
+attributes['searchDomains'] = search_domains
 
 
 def _remove_zero_values_in_net_qos(net_qos):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib3849ebac81a07fbb3b9d1ebc5d517dc143e291f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: netinfo: report DNS search domains from /etc/resolv.conf

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: netinfo: report DNS search domains from /etc/resolv.conf
..

netinfo: report DNS search domains from /etc/resolv.conf

The search domains are specified in /etc/resolv.conf either
using 'domain' or 'search' directives. The former takes
a single domain name, while the latter takes a list.

Only the last directive is used and serves to translate short
host names to fully qualified domain names (FQDN).

'searchDomains' is now a part of host capabilities, alongside
the recently renamed 'nameservers'.

Change-Id: I6305a8861e7e82441085bcb68c2c5cf1ec52ee51
Signed-off-by: Ondřej Svoboda 
---
M lib/api/vdsm-api.yml
M lib/vdsm/network/canonicalize.py
M lib/vdsm/network/kernelconfig.py
M lib/vdsm/network/netinfo/cache.py
M lib/vdsm/network/netinfo/dns.py
M tests/network/netinfo_test.py
6 files changed, 35 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/94/61794/1

diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index e794e77..a6790c9 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -6462,6 +6462,12 @@
 - string
 added: '4.0'
 
+-   description: DNS search domains from /etc/resolve.conf
+name: searchDomains
+type:
+- string
+added: '4.0'
+
 -   description: Distance information between each two numa nodes
 name: numaNodeDistance
 type: *NumaNodeDistanceMap
diff --git a/lib/vdsm/network/canonicalize.py b/lib/vdsm/network/canonicalize.py
index b0bc983..71a8f5a 100644
--- a/lib/vdsm/network/canonicalize.py
+++ b/lib/vdsm/network/canonicalize.py
@@ -135,6 +135,7 @@
 if 'nameservers' not in data:
 # Nameservers are relevant only for the management network.
 if data['defaultRoute']:
-data['nameservers'] = dns.get_host_nameservers()
+nameservers, _ = dns.get_resolver_config()
+data['nameservers'] = nameservers
 else:
 data['nameservers'] = []
diff --git a/lib/vdsm/network/kernelconfig.py b/lib/vdsm/network/kernelconfig.py
index abb73cf..e4e1fc4 100644
--- a/lib/vdsm/network/kernelconfig.py
+++ b/lib/vdsm/network/kernelconfig.py
@@ -160,8 +160,11 @@
 
 
 def _translate_nameservers(attributes):
-nservers = dns.get_host_nameservers() if attributes['defaultRoute'] else []
-attributes['nameservers'] = nservers
+if attributes['defaultRoute']:
+nameservers, _ = dns.get_resolver_config()
+else:
+nameservers = []
+attributes['nameservers'] = nameservers
 
 
 def _remove_zero_values_in_net_qos(net_qos):
diff --git a/lib/vdsm/network/netinfo/cache.py 
b/lib/vdsm/network/netinfo/cache.py
index 26c0b8d..5376b0b 100644
--- a/lib/vdsm/network/netinfo/cache.py
+++ b/lib/vdsm/network/netinfo/cache.py
@@ -37,7 +37,7 @@
 from . import bridges
 from .dhcp import (propose_updates_to_reported_dhcp,  update_reported_dhcp,
dhcp_status, dhcp_faked_status)
-from .dns import get_host_nameservers
+from .dns import get_resolver_config
 from .mtus import getMtu
 from . import nics
 from . import vlans
@@ -59,8 +59,10 @@
 retrieving data from libvirt.
 :return: Dict of networking devices with all their details.
 """
+nameservers, search_domains = get_resolver_config()
 networking = {'bondings': {}, 'bridges': {}, 'networks': {}, 'nics': {},
-  'vlans': {}, 'nameservers': get_host_nameservers()}
+  'vlans': {}, 'nameservers': nameservers,
+  'searchDomains': search_domains}
 paddr = bonding.permanent_address()
 ipaddrs = getIpAddrs()
 routes = get_routes()
diff --git a/lib/vdsm/network/netinfo/dns.py b/lib/vdsm/network/netinfo/dns.py
index 674f176..66f4966 100644
--- a/lib/vdsm/network/netinfo/dns.py
+++ b/lib/vdsm/network/netinfo/dns.py
@@ -21,19 +21,27 @@
 DNS_CONF_FILE = '/etc/resolv.conf'
 
 
-def get_host_nameservers():
-"""Returns a list of nameservers listed in /etc/resolv.conf"""
+def get_resolver_config():
+"""
+Return a list of nameservers and a list of search domains as listed in
+/etc/resolv.conf
+"""
 with open(DNS_CONF_FILE, 'r') as file_object:
 file_text = file_object.read()
-return _parse_dnss(file_text)
+return _parse_resolver_configuration(file_text)
 
 
-def _parse_dnss(file_text):
+def _parse_resolver_configuration(file_text):
 nameservers = []
+search_domains = []
 for line in file_text.splitlines():
 words = line.strip().split()
 if len(words) < 2:
 continue
 if words[0] == 'nameserver':
 nameservers.append(words[1])
-return nameservers
+elif words[0] in ('domain', 'search'):
+# According to resolv.conf(5) "the last instance [of a 'domain' or
+# 'search' line] 

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 8: Verified+1

The whole topic (at this time, 4 patches) successfully preserves DNS servers 
statically defined on a NIC. First, I created a bridgeless network, then I 
converted it to bridged and finally, I rebooted. During all the steps 
nameservers stayed unchanged even with NetworkManager running.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: simplify _update_from_ifcfg

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: simplify _update_from_ifcfg
..


Patch Set 3: Verified+1

The whole topic (at this time, 4 patches) successfully preserves DNS servers 
statically defined on a NIC. First, I created a bridgeless network, then I 
converted it to bridged and finally, I rebooted. During all the steps 
nameservers stayed unchanged even with NetworkManager running.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: If2dc4d17cfacd927b9fa0d8940f5a546db9c05b6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: pass a NetDevice to _createConfFile

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: pass a NetDevice to _createConfFile
..


Patch Set 3: Verified+1

The whole topic (at this time, 4 patches) successfully preserves DNS servers 
statically defined on a NIC. First, I created a bridgeless network, then I 
converted it to bridged and finally, I rebooted. During all the steps 
nameservers stayed unchanged even with NetworkManager running.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67db38cb7253bff14f86019a52fef3d736a65c1f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: Introduce nameservers (dns) network api

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Introduce nameservers (dns) network api
..


Patch Set 2: Verified+1

The whole topic (at this time, 4 patches) successfully preserves DNS servers 
statically defined on a NIC. First, I created a bridgeless network, then I 
converted it to bridged and finally, I rebooted. During all the steps 
nameservers stayed unchanged even with NetworkManager running.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: pass a NetDevice to _createConfFile

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: pass a NetDevice to _createConfFile
..


Patch Set 2:

(1 comment)

History.

https://gerrit.ovirt.org/#/c/61471/2/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

Line 627: self._createConfFile(conf, nic_updated_from_ifcfg)
Line 628: 
Line 629: @staticmethod
Line 630: def _update_from_ifcfg(iface):
Line 631: if ifaceUsed(iface.name):
> Can you please explain this code, I do not understand why this is here at a
For now, I tracked it down to https://gerrit.ovirt.org/7259 and 
https://bugzilla.redhat.com/846004

I will read these sources and try to answer the question: do we still need this?

It seems to me that upon adding a VLAN, IP configuration could be preserved on 
NICs/bondings by other means, principally in the API, not as (quite) a hack in 
this one configurator.
Line 632: ipv4 = copy.deepcopy(iface.ipv4)
Line 633: ipv6 = copy.deepcopy(iface.ipv6)
Line 634: mtu = iface.mtu
Line 635: confParams = misc.getIfaceCfg(iface.name)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67db38cb7253bff14f86019a52fef3d736a65c1f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: Introduce nameservers (dns) network api

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Introduce nameservers (dns) network api
..


Patch Set 1: Code-Review+1

This part looks good but please also consider adding 'searchDomains'.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: Introduce nameservers (dns) network api

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: Introduce nameservers (dns) network api
..


Patch Set 1:

https://gerrit.ovirt.org/#/c/61712/ does the renaming.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic9b0d0370951fe3985f61e550374eb63eb2cf161
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: preserve the current DNS search domains as well

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: preserve the current DNS search domains as well
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/61476/4/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

PS4, Line 6439: string
Shouldn't an asterisk denote this is a list (as well as 'dnss' is)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib001c04e527f1bdce94f56a15a0dbbfe88155754
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/61184/6/lib/vdsm/network/legacy_switch.py
File lib/vdsm/network/legacy_switch.py:

Line 160: top_net_dev.ipv6 = IPv6(
Line 161: ipv6addr, ipv6gateway, defaultRoute, ipv6autoconf, dhcpv6)
Line 162: top_net_dev.blockingdhcp = (configurator._inRollback or
Line 163: utils.tobool(blockingdhcp))
Line 164: # _netinfo doesn't update DNS configuration during network setup.
> The sync between netinfo content and actual is generic, not DNS specific. T
OK, I'll remove it.
Line 165: top_net_dev.nameservers = _netinfo.nameservers
Line 166: return top_net_dev
Line 167: 
Line 168: 


https://gerrit.ovirt.org/#/c/61184/6/lib/vdsm/network/netinfo/cache.py
File lib/vdsm/network/netinfo/cache.py:

Line 352: self.vlans = _netinfo['vlans']
Line 353: self.nics = _netinfo['nics']
Line 354: self.bondings = _netinfo['bondings']
Line 355: self.bridges = _netinfo['bridges']
Line 356: # self.nameservers must not be updated because
> Thank you for explaining the problem.
I heard you were thinking it through. But, isn't the new API meant for future 
versions, not 4.0.x? This patch is meant to fix static IP configuration which 
suffers from DNS configuration disappearing...

Anyway, you might want to distinguish two cases: if DHCP is enabled (on any 
network), simply leave DNS configuration to a DHCP client; if not, try to 
preserve current DNS configuration. In addition to this, also accept a list of 
DNS servers, which either replaces the current ones or extends their list (I 
think that manually declared DNS servers are usually prioritized over existing 
ones).

Back to the topic: to fix the current bug we simply need to obtain a list of 
nameservers before network configuration and write it out at the end. I thought 
this place was not that bad because I made 'updateDevices' update only devices, 
and not DNS servers and I wrote that explicitly in a comment. The other way is 
to keep such a list in netswitch._setup_legacy. Do you prefer the other one?
Line 357: # they may disappear when a network is temporarily removed 
during
Line 358: # configuration (such as an upgrade to bridged) with 
NetworkManager in


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/61184/6/lib/vdsm/network/legacy_switch.py
File lib/vdsm/network/legacy_switch.py:

Line 160: top_net_dev.ipv6 = IPv6(
Line 161: ipv6addr, ipv6gateway, defaultRoute, ipv6autoconf, dhcpv6)
Line 162: top_net_dev.blockingdhcp = (configurator._inRollback or
Line 163: utils.tobool(blockingdhcp))
Line 164: # _netinfo doesn't update DNS configuration during network setup.
> I do not understand this comment.
Yeah, rather I meant that the object doesn't refresh what it knows about DNS, 
and thanks to that, it remembers nameservers even though they are removed 
(presumably by NetworkManager) with a network they were defined with.
Line 165: top_net_dev.nameservers = _netinfo.nameservers
Line 166: return top_net_dev
Line 167: 
Line 168: 


https://gerrit.ovirt.org/#/c/61184/6/lib/vdsm/network/netinfo/cache.py
File lib/vdsm/network/netinfo/cache.py:

Line 352: self.vlans = _netinfo['vlans']
Line 353: self.nics = _netinfo['nics']
Line 354: self.bondings = _netinfo['bondings']
Line 355: self.bridges = _netinfo['bridges']
Line 356: # self.nameservers must not be updated because
> It better be reported as it is, this is the purpose of the method.
I noticed 'updateDevices' is not used for reporting to the world at all, rather 
it is only used during network configuration during which DNS configuration is 
lost (when a nonbridged network is deleted first, then 'updateDevices' is 
called a couple of times, and finally a bridged network is created).

I need to keep DNS configuration somewhere. If not in CachingNetInfo, then I 
would have to put it into an extra variable in netswitch._setup_legacy (and 
pass it to legacy_switch.add_missing_networks) or stuff it inside 'networks', 
as DNS configuration is in fact something we want to set up (later, even a user 
would like to configure DNS somehow).

So no, the purpose of this method is not to "report" anything, rather just 
refresh what we know about networks (solely) during configuration. And no, I 
cannot choose to use information from _netinfo after a nonbridged network is 
gone because DNS configuration is already lost at the time. The information is 
"stable" exactly at the time the constructor is called.
Line 357: # they may disappear when a network is temporarily removed 
during
Line 358: # configuration (such as an upgrade to bridged) with 
NetworkManager in


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: rename reported 'dnss' to 'nameservers' for clarity

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: network: rename reported 'dnss' to 'nameservers' for clarity
..

network: rename reported 'dnss' to 'nameservers' for clarity

Change-Id: Ie66c0e8cdf86da38a8e1f4b35d284c174973389e
Signed-off-by: Ondřej Svoboda 
---
M lib/api/vdsm-api.yml
M lib/vdsm/network/netinfo/cache.py
M lib/vdsm/network/netinfo/dns.py
M tests/network/config_network_test.py
M tests/network/models_test.py
M tests/network/unified_persistence_test.py
6 files changed, 10 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/61712/1

diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml
index a99c488..ee8d1f1 100644
--- a/lib/api/vdsm-api.yml
+++ b/lib/api/vdsm-api.yml
@@ -6428,7 +6428,7 @@
 added: '4.0'
 
 -   description: Provides host DNS servers from /etc/resolve.conf
-name: dnss
+name: nameservers
 type:
 - string
 added: '4.0'
diff --git a/lib/vdsm/network/netinfo/cache.py 
b/lib/vdsm/network/netinfo/cache.py
index 3cd32c3..9ec7de5 100644
--- a/lib/vdsm/network/netinfo/cache.py
+++ b/lib/vdsm/network/netinfo/cache.py
@@ -61,7 +61,7 @@
 """
 nameservers, search_domains = get_host_nameservers()
 networking = {'bondings': {}, 'bridges': {}, 'networks': {}, 'nics': {},
-  'vlans': {}, 'dnss': nameservers,
+  'vlans': {}, 'nameservers': nameservers,
   'searchDomains': search_domains}
 paddr = bonding.permanent_address()
 ipaddrs = getIpAddrs()
@@ -218,7 +218,7 @@
 self.nics = _netinfo['nics']
 self.bondings = _netinfo['bondings']
 self.bridges = _netinfo['bridges']
-self.nameservers = _netinfo['dnss']
+self.nameservers = _netinfo['nameservers']
 self.search_domains = _netinfo['searchDomains']
 
 def del_network(self, network):
diff --git a/lib/vdsm/network/netinfo/dns.py b/lib/vdsm/network/netinfo/dns.py
index 43a532c..c88c2f0 100644
--- a/lib/vdsm/network/netinfo/dns.py
+++ b/lib/vdsm/network/netinfo/dns.py
@@ -29,14 +29,14 @@
 
 
 def _parse_dnss(file_text):
-dnss = []
+nameservers = []
 search_domains = []
 for line in file_text.splitlines():
 words = line.strip().split()
 if len(words) < 2:
 continue
 if words[0] == 'nameserver':
-dnss.append(words[1])
+nameservers.append(words[1])
 elif words[0] == 'search':
 search_domains.extend(words[1:])
-return dnss, search_domains
+return nameservers, search_domains
diff --git a/tests/network/config_network_test.py 
b/tests/network/config_network_test.py
index 8fda600..323b211 100644
--- a/tests/network/config_network_test.py
+++ b/tests/network/config_network_test.py
@@ -102,7 +102,7 @@
 'fakebr2': {'ports': ['eth7.1']}
 },
 'bondings': {'bond00': {'slaves': ['eth5', 'eth6']}},
-'dnss': [],
+'nameservers': [],
 'searchDomains': [],
 }
 
diff --git a/tests/network/models_test.py b/tests/network/models_test.py
index e828e3d..5b8a5d5 100644
--- a/tests/network/models_test.py
+++ b/tests/network/models_test.py
@@ -139,7 +139,7 @@
 def testTextualRepr(self):
 _netinfo = {'networks': {}, 'vlans': {},
 'nics': ['testnic1', 'testnic2'],
-'bondings': {}, 'bridges': {}, 'dnss': [],
+'bondings': {}, 'bridges': {}, 'nameservers': [],
 'searchDomains': []}
 fakeInfo = CachingNetInfo(_netinfo)
 nic1 = Nic('testnic1', None, _netinfo=fakeInfo)
@@ -176,7 +176,7 @@
 def testIterNetworkHierarchy(self):
 _netinfo = {'networks': {}, 'vlans': {},
 'nics': ['testnic1', 'testnic2'],
-'bondings': {}, 'bridges': {}, 'dnss': [],
+'bondings': {}, 'bridges': {}, 'nameservers': [],
 'searchDomains': []}
 fakeInfo = CachingNetInfo(_netinfo)
 # Vlanned and bonded VM network
diff --git a/tests/network/unified_persistence_test.py 
b/tests/network/unified_persistence_test.py
index 8dd8211..a77568d 100644
--- a/tests/network/unified_persistence_test.py
+++ b/tests/network/unified_persistence_test.py
@@ -74,7 +74,7 @@
 'gateway': '', 'bootproto4': 'none', 'netmask': '', 'stp': 'off',
 'ipv4addrs': [], 'mtu': '1500', 'ipv6gateway': '::', 'ports':
 [BOND_NAME + '.' + VLAN]}},
-'dnss': [],
+'nameservers': [],
 'searchDomains': [],
 }
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie66c0e8cdf86da38a8e1f4b35d284c174973389e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master

Change in vdsm[master]: network: preserve the current 'search domains' as well

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: network: preserve the current 'search domains' as well
..

network: preserve the current 'search domains' as well

Change-Id: Ib001c04e527f1bdce94f56a15a0dbbfe88155754
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
M lib/vdsm/network/legacy_switch.py
M lib/vdsm/network/models.py
M lib/vdsm/network/netinfo/cache.py
M lib/vdsm/network/netinfo/dns.py
5 files changed, 14 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/76/61476/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index e926d2e..f9e6cbf 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -557,6 +557,9 @@
 if device.nameservers and ipv4.defaultRoute:
 for i, nameserver in enumerate(device.nameservers[0:2], 1):
 conf += 'DNS{}={}\n'.format(i, nameserver)
+if device.search_domains:
+conf += 'DOMAIN={}\n'.format(pipes.quote(
+' '.join(device.search_domains)))
 
 ifcfg_file = NET_CONF_PREF + name
 hook_dict = {'ifcfg_file': ifcfg_file,
diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index 796bc6b..abc6eaa 100644
--- a/lib/vdsm/network/legacy_switch.py
+++ b/lib/vdsm/network/legacy_switch.py
@@ -162,6 +162,7 @@
 top_net_dev.blockingdhcp = (configurator._inRollback or
 utils.tobool(blockingdhcp))
 top_net_dev.nameservers = _netinfo.nameservers
+top_net_dev.search_domains = _netinfo.search_domains
 return top_net_dev
 
 
diff --git a/lib/vdsm/network/models.py b/lib/vdsm/network/models.py
index ce08182..469f5ee 100644
--- a/lib/vdsm/network/models.py
+++ b/lib/vdsm/network/models.py
@@ -42,6 +42,7 @@
 self.master = None
 self.duid_source = None
 self.nameservers = None
+self.search_domains = None
 
 def __iter__(self):
 raise NotImplementedError
diff --git a/lib/vdsm/network/netinfo/cache.py 
b/lib/vdsm/network/netinfo/cache.py
index db01b97..d378f35 100644
--- a/lib/vdsm/network/netinfo/cache.py
+++ b/lib/vdsm/network/netinfo/cache.py
@@ -59,8 +59,10 @@
 retrieving data from libvirt.
 :return: Dict of networking devices with all their details.
 """
+nameservers, search_domains = get_host_nameservers()
 networking = {'bondings': {}, 'bridges': {}, 'networks': {}, 'nics': {},
-  'vlans': {}, 'dnss': get_host_nameservers()}
+  'vlans': {}, 'dnss': nameservers,
+  'searchdomains': search_domains}
 paddr = bonding.permanent_address()
 ipaddrs = getIpAddrs()
 routes = get_routes()
@@ -217,6 +219,7 @@
 self.bondings = _netinfo['bondings']
 self.bridges = _netinfo['bridges']
 self.nameservers = _netinfo['dnss']
+self.search_domains = _netinfo['searchdomains']
 
 def del_network(self, network):
 del self.networks[network]
@@ -354,3 +357,4 @@
 self.bondings = _netinfo['bondings']
 self.bridges = _netinfo['bridges']
 self.nameservers = _netinfo['dnss']
+self.search_domains = _netinfo['searchdomains']
diff --git a/lib/vdsm/network/netinfo/dns.py b/lib/vdsm/network/netinfo/dns.py
index e22714e..43a532c 100644
--- a/lib/vdsm/network/netinfo/dns.py
+++ b/lib/vdsm/network/netinfo/dns.py
@@ -30,10 +30,13 @@
 
 def _parse_dnss(file_text):
 dnss = []
+search_domains = []
 for line in file_text.splitlines():
 words = line.strip().split()
 if len(words) < 2:
 continue
 if words[0] == 'nameserver':
 dnss.append(words[1])
-return dnss
+elif words[0] == 'search':
+search_domains.extend(words[1:])
+return dnss, search_domains


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib001c04e527f1bdce94f56a15a0dbbfe88155754
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/61184/5/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

PS5, Line 556: of any type
In the previous revision, it could only be a bridge. With the move to 
_createConfFile we lost the distinction but the condition below still targets 
the ovirtmgmt bridge well.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: simplify _update_from_ifcfg

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: ifcfg: simplify _update_from_ifcfg
..

ifcfg: simplify _update_from_ifcfg

Change-Id: If2dc4d17cfacd927b9fa0d8940f5a546db9c05b6
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
1 file changed, 27 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/72/61472/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index 3487103..95e0927 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -597,8 +597,9 @@
 conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name)
 conf += 'ONBOOT=yes\n'
 
-bond_updated_from_ifcfg = self._update_from_ifcfg(bond)
-self._createConfFile(conf, bond_updated_from_ifcfg)
+if ifaceUsed(bond.name):
+bond = self._update_from_ifcfg(copy.deepcopy(bond))
+self._createConfFile(conf, bond)
 
 # create the bonding device to avoid initscripts noise
 with open(netinfo_bonding.BONDING_MASTERS) as info:
@@ -623,39 +624,33 @@
 if ethtool_opts:
 conf += 'ETHTOOL_OPTS=%s\n' % pipes.quote(ethtool_opts)
 
-nic_updated_from_ifcfg = self._update_from_ifcfg(nic)
-self._createConfFile(conf, nic_updated_from_ifcfg)
+if ifaceUsed(nic.name):
+nic = self._update_from_ifcfg(copy.deepcopy(nic))
+self._createConfFile(conf, nic)
 
 @staticmethod
 def _update_from_ifcfg(iface):
-if ifaceUsed(iface.name):
-ipv4 = copy.deepcopy(iface.ipv4)
-ipv6 = copy.deepcopy(iface.ipv6)
-mtu = iface.mtu
-confParams = misc.getIfaceCfg(iface.name)
-if not ipv4.address and ipv4.bootproto != 'dhcp':
-ipv4.address = confParams.get('IPADDR')
-ipv4.netmask = confParams.get('NETMASK')
-ipv4.gateway = confParams.get('GATEWAY')
-if not ipv4.bootproto:
-ipv4.bootproto = confParams.get('BOOTPROTO')
-if ipv4.defaultRoute is None and confParams.get('DEFROUTE'):
-ipv4.defaultRoute = _from_ifcfg_bool(confParams['DEFROUTE'])
-if confParams.get('IPV6INIT') == 'yes':
-ipv6.address = confParams.get('IPV6ADDR')
-ipv6.gateway = confParams.get('IPV6_DEFAULTGW')
-ipv6.ipv6autoconf = confParams.get('IPV6_AUTOCONF') == 'yes'
-ipv6.dhcpv6 = confParams.get('DHCPV6C') == 'yes'
-if not iface.mtu:
-mtu = confParams.get('MTU')
-if mtu:
-mtu = int(mtu)
-iface_updated_from_ifcfg = copy.deepcopy(iface)
-iface_updated_from_ifcfg.ipv4 = ipv4
-iface_updated_from_ifcfg.ipv6 = ipv6
-return iface_updated_from_ifcfg
-else:
-return iface
+confParams = misc.getIfaceCfg(iface.name)
+ipv4 = iface.ipv4
+ipv6 = iface.ipv6
+if not ipv4.address and ipv4.bootproto != 'dhcp':
+ipv4.address = confParams.get('IPADDR')
+ipv4.netmask = confParams.get('NETMASK')
+ipv4.gateway = confParams.get('GATEWAY')
+if not ipv4.bootproto:
+ipv4.bootproto = confParams.get('BOOTPROTO')
+if ipv4.defaultRoute is None and confParams.get('DEFROUTE'):
+ipv4.defaultRoute = _from_ifcfg_bool(confParams['DEFROUTE'])
+if confParams.get('IPV6INIT') == 'yes':
+ipv6.address = confParams.get('IPV6ADDR')
+ipv6.gateway = confParams.get('IPV6_DEFAULTGW')
+ipv6.ipv6autoconf = confParams.get('IPV6_AUTOCONF') == 'yes'
+ipv6.dhcpv6 = confParams.get('DHCPV6C') == 'yes'
+if not iface.mtu:
+mtu = confParams.get('MTU')
+if mtu:
+iface.mtu = int(mtu)
+return iface
 
 def removeNic(self, nic):
 cf = NET_CONF_PREF + nic


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If2dc4d17cfacd927b9fa0d8940f5a546db9c05b6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: pass a NetDevice to _createConfFile

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: ifcfg: pass a NetDevice to _createConfFile
..

ifcfg: pass a NetDevice to _createConfFile

If we want to handle DNS1/2 in this function, we should give
it all information stored in a NetDevice instance.

Change-Id: I67db38cb7253bff14f86019a52fef3d736a65c1f
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
1 file changed, 20 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/61471/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index e9f67e1..3487103 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -517,8 +517,9 @@
 if self.unifiedPersistence and utils.isOvirtNode():
 node_fs.Config().persist(fileName)
 
-def _createConfFile(self, conf, name, ipv4, ipv6, mtu=None, **kwargs):
+def _createConfFile(self, conf, device):
 """ Create ifcfg-* file with proper fields per device """
+name, ipv4, ipv6 = device.name, device.ipv4, device.ipv6
 cfg = 'DEVICE=%s\n' % pipes.quote(name)
 cfg += conf
 if ipv4.address:
@@ -536,8 +537,8 @@
 # Ask dhclient to stop any dhclient running for the device
 dhclient.kill(name)
 address.flush(name, family=4)
-if mtu:
-cfg += 'MTU=%d\n' % mtu
+if device.mtu:
+cfg += 'MTU=%d\n' % device.mtu
 if ipv4.defaultRoute is not None:
 cfg += 'DEFROUTE=%s\n' % _to_ifcfg_bool(ipv4.defaultRoute)
 cfg += 'NM_CONTROLLED=no\n'
@@ -575,8 +576,7 @@
 if 'custom' in opts and 'bridge_opts' in opts['custom']:
 conf += 'BRIDGING_OPTS="%s"\n' % opts['custom']['bridge_opts']
 
-self._createConfFile(conf, bridge.name, bridge.ipv4, bridge.ipv6,
- bridge.mtu, **opts)
+self._createConfFile(conf, bridge)
 
 def addVlan(self, vlan, **opts):
 """ Create ifcfg-* file with proper fields for VLAN """
@@ -585,8 +585,7 @@
 if vlan.bridge:
 conf += 'BRIDGE=%s\n' % pipes.quote(vlan.bridge.name)
 conf += 'ONBOOT=yes\n'
-self._createConfFile(conf, vlan.name, vlan.ipv4, vlan.ipv6, vlan.mtu,
- **opts)
+self._createConfFile(conf, vlan)
 
 def addBonding(self, bond, **opts):
 """ Create ifcfg-* file with proper fields for bond """
@@ -598,8 +597,8 @@
 conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name)
 conf += 'ONBOOT=yes\n'
 
-ipv4, ipv6, mtu = self._getIfaceConfValues(bond)
-self._createConfFile(conf, bond.name, ipv4, ipv6, mtu, **opts)
+bond_updated_from_ifcfg = self._update_from_ifcfg(bond)
+self._createConfFile(conf, bond_updated_from_ifcfg)
 
 # create the bonding device to avoid initscripts noise
 with open(netinfo_bonding.BONDING_MASTERS) as info:
@@ -624,15 +623,15 @@
 if ethtool_opts:
 conf += 'ETHTOOL_OPTS=%s\n' % pipes.quote(ethtool_opts)
 
-ipv4, ipv6, mtu = self._getIfaceConfValues(nic)
-self._createConfFile(conf, nic.name, ipv4, ipv6, mtu, **opts)
+nic_updated_from_ifcfg = self._update_from_ifcfg(nic)
+self._createConfFile(conf, nic_updated_from_ifcfg)
 
 @staticmethod
-def _getIfaceConfValues(iface):
-ipv4 = copy.deepcopy(iface.ipv4)
-ipv6 = copy.deepcopy(iface.ipv6)
-mtu = iface.mtu
+def _update_from_ifcfg(iface):
 if ifaceUsed(iface.name):
+ipv4 = copy.deepcopy(iface.ipv4)
+ipv6 = copy.deepcopy(iface.ipv6)
+mtu = iface.mtu
 confParams = misc.getIfaceCfg(iface.name)
 if not ipv4.address and ipv4.bootproto != 'dhcp':
 ipv4.address = confParams.get('IPADDR')
@@ -651,7 +650,12 @@
 mtu = confParams.get('MTU')
 if mtu:
 mtu = int(mtu)
-return ipv4, ipv6, mtu
+iface_updated_from_ifcfg = copy.deepcopy(iface)
+iface_updated_from_ifcfg.ipv4 = ipv4
+iface_updated_from_ifcfg.ipv6 = ipv6
+return iface_updated_from_ifcfg
+else:
+return iface
 
 def removeNic(self, nic):
 cf = NET_CONF_PREF + nic


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I67db38cb7253bff14f86019a52fef3d736a65c1f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 4:

(3 comments)

https://gerrit.ovirt.org/#/c/61184/4/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

PS4, Line 579: DOMAIN=
> I do not understand... we just need to keep the existing functionality of t
You are right that we do not copy DNS configuration across ifcfg files. I was 
going to do it in https://gerrit.ovirt.org/#/c/61157/1 but Dan came up with a 
simpler solution.

The referenced patch doesn't contain the necessary handling for 
NetworkManager-written ifcfg files which use a different naming "convention". I 
can push a more advanced version of the patch, if you are interested.

So, let's say an original ifcfg file (written by an installer or NetworkManager 
or anything) contained static DNS1=, DNS2= and DOMAIN= parameters, which ended 
up in /etc/resolv.conf. NetworkManager was in charge of eth0. Now VDSM comes 
and steals eth0 from NM and /etc/resolv.conf is emptied. Regardless of a 
method, we have to preserve what was in /etc/resolv.conf.


https://gerrit.ovirt.org/#/c/61184/4/lib/vdsm/network/legacy_switch.py
File lib/vdsm/network/legacy_switch.py:

Line 160: top_net_dev.ipv6 = IPv6(
Line 161: ipv6addr, ipv6gateway, defaultRoute, ipv6autoconf, dhcpv6)
Line 162: top_net_dev.blockingdhcp = (configurator._inRollback or
Line 163: utils.tobool(blockingdhcp))
Line 164: if defaultRoute:
> I understand the heuristic, but it does not need to appear here. It's enoug
I moved it there, thanks!
Line 165: top_net_dev.nameservers = _netinfo.nameservers
Line 166: return top_net_dev
Line 167: 
Line 168: 


https://gerrit.ovirt.org/#/c/61184/4/tests/network/models_test.py
File tests/network/models_test.py:

PS4, Line 142: 'dnss'
> Unfortunately, this may break comparability as it is reported back to Engin
I'll ask them. But 'dnss' is a new thing and probably not used yet (but still I 
would have to change the RPC schema).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 4:

(6 comments)

https://gerrit.ovirt.org/#/c/61184/4/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

Line 570: if bridge.duid_source and dhclient.supports_duid_file():
Line 571: duid_source_file = dhclient.LEASE_FILE.format(
Line 572: '', bridge.duid_source)
Line 573: conf += 'DHCLIENTARGS="-df %s"\n' % duid_source_file
Line 574: if bridge.nameservers:
> Why is it only under addBridge?
Yes, it is generic in principle and I'll move it there, if this is possible. I 
simply wanted to target the management network as the one to carry DNS 
configuration, but this should probably be apparent from the code itself.
Line 575: # TODO: PEERDNS=yes, to be on the safe side in the future?
Line 576: # Write DNS1= and DNS2=, if known
Line 577: for i, nameserver in enumerate(bridge.nameservers[0:2], 
1):
Line 578: conf += 'DNS{}={}\n'.format(i, nameserver)


PS4, Line 575: PEERDNS=yes, to be on the safe side in the future?
> This should have the same logic as DEFROUTE, this way we cover DHCP as well
I'll move this comment to a later patch, OK.

Do you mean to only disallow explicit DNS configuration ... if DHCP is used? Do 
you refer to "DEFROUTE logic", as in modelling the output of "PEERDNS" to look 
like the code below?

if ipv4.defaultRoute is not None:
cfg += 'DEFROUTE=%s\n' % _to_ifcfg_bool(ipv4.defaultRoute)


PS4, Line 579: DOMAIN=
> Why do we need to handle this?
For the same reason this patch exists: to preserve (full) DNS configuration, in 
a straightforward way (piggybacking it on a management network's ifcfg file).

Search domains seem important to me (if I imagine to be in charge of a large 
cluster), consider a deployment at "example.com" where all hosts can be 
specified with just "node01", node02" instead of their full 
"nodeXX.example.com" name.


https://gerrit.ovirt.org/#/c/61184/4/lib/vdsm/network/legacy_switch.py
File lib/vdsm/network/legacy_switch.py:

Line 160: top_net_dev.ipv6 = IPv6(
Line 161: ipv6addr, ipv6gateway, defaultRoute, ipv6autoconf, dhcpv6)
Line 162: top_net_dev.blockingdhcp = (configurator._inRollback or
Line 163: utils.tobool(blockingdhcp))
Line 164: if defaultRoute:
> This heuristic needs to be explained.
I wanted to add DNS information specifically (only) to the management network 
(which bears a default gateway, hence the heuristic).

The reason was to limit duplication of such information, to have DNS1 and DNS2 
entries only in a specific ifcfg file (or a few files, if there were multiple 
gateways), serving as a carrier for this information.

(At a quick glance, it seems that ifup-post is able to deal with duplicated 
DNS1, DNS2 well.)
Line 165: top_net_dev.nameservers = _netinfo.nameservers
Line 166: return top_net_dev
Line 167: 
Line 168: 


https://gerrit.ovirt.org/#/c/61184/4/tests/network/config_network_test.py
File tests/network/config_network_test.py:

Line 101: 'fakebr1': {'ports': ['bond00']},
Line 102: 'fakebr2': {'ports': ['eth7.1']}
Line 103: },
Line 104: 'bondings': {'bond00': {'slaves': ['eth5', 'eth6']}},
Line 105: 'dnss': ['10.20.30.40', '20.30.40.50', '30.40.50.60'],
> Why not an empty list?
I chose the wrong place to show what this list could contain.

The contents don't matter at all, I'll use an empty one.
Line 106: }
Line 107: 
Line 108: fakeInfo = netinfo.cache.CachingNetInfo(_netinfo)
Line 109: nic = 'eth2'


https://gerrit.ovirt.org/#/c/61184/4/tests/network/models_test.py
File tests/network/models_test.py:

PS4, Line 142: 'dnss'
Really, if I was new to the project, I would wonder -- this means DNS, right? 
And there may be multiple nameservers, hence "dns-s". Ooh, get it...

Let us rename this to 'nameservers', please :-)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: a legacy bridge now reads its NIC's original namese...

[osvoboda]

Ondřej Svoboda has abandoned this change.

Change subject: network: a legacy bridge now reads its NIC's original 
nameservers
..


Abandoned

Superseded by https://gerrit.ovirt.org/61184/

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ifda684a525a663a460e0d8e8d76aa73c4cb2cfff
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..


Patch Set 4: Verified+1

(4 comments)

DNS1=redhat.com appeared in ifcfg-ovirtmgmt with this patch, and initscripts 
used it in /etc/resolv.conf, also see Martin Tessun's confirmation: 
https://bugzilla.redhat.com/show_bug.cgi?id=1351095#c26

Unit tests passed.

https://gerrit.ovirt.org/#/c/61184/4/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

PS4, Line 575: PEERDNS=yes, to be on the safe side in the future?
PEERDNS is just like IPV6INIT. Currently, if is isn't present, it is assumed to 
be "yes", see ifup-post:

if [ "$PEERDNS" != "no" ] ...; then
  (all DNS1, DNS2 and DOMAIN handling)
fi


PS4, Line 579: DOMAIN=
"DOMAIN=ovirt.org redhat.com" handling will be added in another patch. Yes, it 
is a list.

The above translates to "search ovirt.org redhat.com" in /etc/resolv.conf and 
is necessary for looking up short names. If you add this to your 
/etc/resolv.conf, you can type "ping gerrit" instead of "ping 
gerrit.ovirt.org". If gerrit.ovirt.org didn't exist, gerrit.redhat.com would be 
tried next.


https://gerrit.ovirt.org/#/c/61184/4/lib/vdsm/network/netinfo/cache.py
File lib/vdsm/network/netinfo/cache.py:

PS4, Line 219: self.nameservers = _netinfo['dnss']
I would prefer to have the full _netinfo available.

This line and the other down below are not the last additions. I will be 
introducing 'search_domains' (which will contain a list of domains gathered 
from a "search domain1 domain2" line in /etc/resolv.conf).


PS4, Line 356: dnss
Additionally, 'dnss' works but 'nameservers' is better, in my opinion.

The last 's' in 'DNSs' is extraneous, as shown even by a real 'DNSS' 
(Department of Natural and Social Sciences), where the plural is simply 
swallowed, they don't write DNSSs.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: a legacy bridge now reads its NIC's original namese...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: a legacy bridge now reads its NIC's original 
nameservers
..


Patch Set 1: Code-Review-1

We don't need to read ifcfg files to learn about name servers.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifda684a525a663a460e0d8e8d76aa73c4cb2cfff
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: ifcfg: write current DNS information to a management network...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: ifcfg: write current DNS information to a management network's 
ifcfg file
..

ifcfg: write current DNS information to a management network's ifcfg file

Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Bug-Url: https://bugzilla.redhat.com/1351095
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
M lib/vdsm/network/legacy_switch.py
M lib/vdsm/network/models.py
M lib/vdsm/network/netinfo/cache.py
4 files changed, 11 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/61184/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index e9f67e1..6e41c96 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -571,6 +571,12 @@
 duid_source_file = dhclient.LEASE_FILE.format(
 '', bridge.duid_source)
 conf += 'DHCLIENTARGS="-df %s"\n' % duid_source_file
+if bridge.nameservers:
+# TODO: PEERDNS=yes, to be on the safe side in the future?
+# Write DNS1= and DNS2=, if known
+for i, nameserver in enumerate(bridge.nameservers[0:2], 1):
+conf += 'DNS{}={}'.format(i, nameserver)
+# TODO: DOMAIN= (search [domain] in /etc/resolv.conf)?
 
 if 'custom' in opts and 'bridge_opts' in opts['custom']:
 conf += 'BRIDGING_OPTS="%s"\n' % opts['custom']['bridge_opts']
diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index 97d5f0d..f7c80ef 100644
--- a/lib/vdsm/network/legacy_switch.py
+++ b/lib/vdsm/network/legacy_switch.py
@@ -161,6 +161,8 @@
 ipv6addr, ipv6gateway, defaultRoute, ipv6autoconf, dhcpv6)
 top_net_dev.blockingdhcp = (configurator._inRollback or
 utils.tobool(blockingdhcp))
+if defaultRoute:
+top_net_dev.nameservers = _netinfo.nameservers
 return top_net_dev
 
 
diff --git a/lib/vdsm/network/models.py b/lib/vdsm/network/models.py
index 0ab50b8..ce08182 100644
--- a/lib/vdsm/network/models.py
+++ b/lib/vdsm/network/models.py
@@ -41,6 +41,7 @@
 self.configurator = configurator
 self.master = None
 self.duid_source = None
+self.nameservers = None
 
 def __iter__(self):
 raise NotImplementedError
diff --git a/lib/vdsm/network/netinfo/cache.py 
b/lib/vdsm/network/netinfo/cache.py
index 96d2ff2..db01b97 100644
--- a/lib/vdsm/network/netinfo/cache.py
+++ b/lib/vdsm/network/netinfo/cache.py
@@ -216,6 +216,7 @@
 self.nics = _netinfo['nics']
 self.bondings = _netinfo['bondings']
 self.bridges = _netinfo['bridges']
+self.nameservers = _netinfo['dnss']
 
 def del_network(self, network):
 del self.networks[network]
@@ -352,3 +353,4 @@
 self.nics = _netinfo['nics']
 self.bondings = _netinfo['bondings']
 self.bridges = _netinfo['bridges']
+self.nameservers = _netinfo['dnss']


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: network: a legacy bridge now reads its NIC's original namese...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: network: a legacy bridge now reads its NIC's original 
nameservers
..

network: a legacy bridge now reads its NIC's original nameservers

Change-Id: Ifda684a525a663a460e0d8e8d76aa73c4cb2cfff
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/legacy_switch.py
M lib/vdsm/network/models.py
M lib/vdsm/network/netinfo/misc.py
3 files changed, 16 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/57/61157/1

diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index 97d5f0d..f80698c 100644
--- a/lib/vdsm/network/legacy_switch.py
+++ b/lib/vdsm/network/legacy_switch.py
@@ -32,6 +32,7 @@
 from vdsm.network.netinfo import NET_PATH
 from vdsm.network.netinfo import addresses
 from vdsm.network.netinfo import bridges
+from vdsm.network.netinfo import misc
 from vdsm.network.netinfo import mtus
 from vdsm.network.netinfo import nics as netinfo_nics
 from vdsm.network.netinfo.cache import CachingNetInfo
@@ -153,6 +154,10 @@
 # bridge gets the same IP address. (BZ#1219429)
 if top_net_dev.port and bootproto == 'dhcp':
 top_net_dev.duid_source = top_net_dev.port.name
+# Inherit the list of nameservers from a NIC's ifcfg file.
+if top_net_dev.port:
+top_net_dev.nameservers = misc.nameservers_from_ifcfg(
+top_net_dev.port.name)
 if top_net_dev is None:
 raise ConfigNetworkError(ne.ERR_BAD_PARAMS, 'Network defined without '
  'devices.')
diff --git a/lib/vdsm/network/models.py b/lib/vdsm/network/models.py
index 0ab50b8..ce08182 100644
--- a/lib/vdsm/network/models.py
+++ b/lib/vdsm/network/models.py
@@ -41,6 +41,7 @@
 self.configurator = configurator
 self.master = None
 self.duid_source = None
+self.nameservers = None
 
 def __iter__(self):
 raise NotImplementedError
diff --git a/lib/vdsm/network/netinfo/misc.py b/lib/vdsm/network/netinfo/misc.py
index c4227c3..c67994a 100644
--- a/lib/vdsm/network/netinfo/misc.py
+++ b/lib/vdsm/network/netinfo/misc.py
@@ -50,3 +50,13 @@
 predicate is True"""
 return [dev.name for dev in getLinks() if predicate(dev) and
 not dev.isHidden()]
+
+
+def nameservers_from_ifcfg(device):
+nameservers = []
+ifcfg = getIfaceCfg(device)
+for key in ('DNS1', 'DNS2'):
+nameserver = ifcfg.get(key)
+if nameserver:
+nameservers.append(nameserver)
+return nameservers


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifda684a525a663a460e0d8e8d76aa73c4cb2cfff
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: netinfo: tighten exception handling in getIfaceCfg

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: netinfo: tighten exception handling in getIfaceCfg
..


Patch Set 2: -Verified

Only rebased and adressing the two comments from patch set 2 now.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6db32b1ad8aaa5e3543e8a19e76a347409811b0b
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: net: report OVS netinfo in caps

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: report OVS netinfo in caps
..


Patch Set 19: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53200891e292a0d9c8a347cc14344281d3ace65
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: report OVS netinfo in caps

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: report OVS netinfo in caps
..


Patch Set 19:

(1 comment)

Another name nit.

https://gerrit.ovirt.org/#/c/57102/19/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:

PS19, Line 143: _netinfo
Why not call this legacy_netinfo?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53200891e292a0d9c8a347cc14344281d3ace65
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: verify that a bond gets the same address once adopted...

[osvoboda]

Ondřej Svoboda has abandoned this change.

Change subject: tests: verify that a bond gets the same address once adopted 
from NM
..


Abandoned

Merged into the commit extending the original integration test into a 
functional one.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I881bed7142551bdadec2b41ac54aa7f8c2aff0a9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: generalize wait_for_ipv6() into wait_for_address()

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: tests: generalize wait_for_ipv6() into wait_for_address()
..


Patch Set 6:

(3 comments)

https://gerrit.ovirt.org/#/c/56983/6/tests/network/nettestlib.py
File tests/network/nettestlib.py:

PS6, Line 419: address
> ip or ipaddress
I went for "ip".


PS6, Line 419: families=(6,), wait_for_scopes=None
> There is problem when introducing multiple families.
For now, I simplified this to only one IP family. But I think a general 
approach is still possible if a distinction is made between the addresses 
obtained ('event' contains a 'family': 'inet' field for IPv4 addresses) in 
addition to the scope.


Line 420: """
Line 421: Wait for the iface to get IP addresses of a specified family with 
netlink
Line 422: Monitor.
Line 423: """
Line 424: event_types = ['ipv{}-ifaddr'.format(family) for family in 
families]
> tuple([...])
Noted, in case we want to implement waiting for scope+family combinations in 
the future.
Line 425: if not wait_for_scopes:
Line 426: wait_for_scopes = ['global', 'link']
Line 427: try:
Line 428: with monitor.Monitor(groups=event_types, timeout=timeout) as 
mon:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iefe2632973e13db07309bdedde20227de06bd17a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: dhclient: consider NetworkManager lease files for DUID too

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: dhclient: consider NetworkManager lease files for DUID too
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/57151/2/lib/vdsm/network/configurators/dhclient.py
File lib/vdsm/network/configurators/dhclient.py:

Line 154: lease_file_names = [LEASE_FILE.format(dhclient_suffix, 
device_name)]
Line 155: lease_file_names.extend(
Line 156: iglob(NM_LEASE_FILE_GLOB.format(dhclient_suffix, 
device_name)))
Line 157: dated_lease_file_names = []
Line 158: for name in lease_file_names:
> define a helper function, and use max(key=) to find the newest file
Done
Line 159: try:
Line 160: modified_date = os.stat(name).st_mtime
Line 161: except OSError as e:
Line 162: if e.errno == errno.ENOENT:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a102a2964ed5e5864ff516c71752a23c4c55cf
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: report OVS netinfo in caps

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: report OVS netinfo in caps
..


Patch Set 10: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53200891e292a0d9c8a347cc14344281d3ace65
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: access network caps via network API

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: net: access network caps via network API
..


Patch Set 8: Code-Review+1

(1 comment)

Just a name nit, so you can ignore.

https://gerrit.ovirt.org/#/c/57402/8/vdsm/caps.py
File vdsm/caps.py:

PS8, Line 211: type
'caps_name' instead of a semi-reserved word?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icfce0d2fe741e89a3ca54b58c9738ce33495ec92
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: ad_actor_system can not be reset to its default value

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: ad_actor_system can not be reset to its default value
..


Patch Set 2: Verified+1

With this patch, only testSetupNetworksAddDelDhcp(True, (4, 6)) randomly failed 
(MonitorError(E_NOT_RUNNING)), and test_bond_mode4_caps_aggregator_id (which I 
am looking into) failed on Fedora 23.

test_getVdsStats is still "skipped because of #1261457".

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f307b64546c3efa73ca202e97ced49eeb9670e9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: don't fail with firewalld if a veth is gone already

[osvoboda]

Ondřej Svoboda has abandoned this change.

Change subject: tests: don't fail with firewalld if a veth is gone already
..


Abandoned

I haven't had issues with firewalld so I assume this is not necessary to have.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I02b2651cba09e74321e9b9ad658979798eb4458a
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: allow to run a DHCP server on multiple interfaces

[osvoboda]

Ondřej Svoboda has abandoned this change.

Change subject: tests: allow to run a DHCP server on multiple interfaces
..


Abandoned

The test that was going to use this ended up setting up a bond on the router 
side.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I67819397603fc311a781fadc41fe5de88b14ed7c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: verify that a bond gets the same address once adopted...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: tests: verify that a bond gets the same address once adopted 
from NM
..


Patch Set 4: Verified+1

test_consume_nm_bond_dhcpv4 now passes even with added checks ensuring we get 
the same DHCP lease over the adopted bond.

The NM+bond+DHCP deployment scenario should work. But I cannot run it due to 
"ovirt-engine: ERROR run:532 Error: cannot find 'getstring'" (python-cheetah) 
problems.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I881bed7142551bdadec2b41ac54aa7f8c2aff0a9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: restore MAC addresses of bonded NICs when adopting a ...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: restore MAC addresses of bonded NICs when adopting a bond
..


Patch Set 3: Verified+1

Verified as a part of the whole branch: ifcfg files for bonded NICs now 
preserve their MAC addresses.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad73e6aa016bcd97e50f58295df9ecbeacd0aa17
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: dhclient: consider NetworkManager lease files for DUID too

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: dhclient: consider NetworkManager lease files for DUID too
..


Patch Set 2: Verified+1

Verified as a part of the whole branch: lease files written by NetworkManager 
are successfully found and used to inherit DUID.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I62a102a2964ed5e5864ff516c71752a23c4c55cf
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: let VDSM consume a bond created by NetworkManager

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: tests: let VDSM consume a bond created by NetworkManager
..


Patch Set 23: Verified+1

Verified as a part of the whole branch (the three following patches fix DHCP 
inheritance).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7047ce59a515d0b8ed2c4c5307b4c0d47d4aa92b
Gerrit-PatchSet: 23
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: tests: verify that a bond gets the same address once adopted...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: tests: verify that a bond gets the same address once adopted 
from NM
..

tests: verify that a bond gets the same address once adopted from NM

Change-Id: I881bed7142551bdadec2b41ac54aa7f8c2aff0a9
Signed-off-by: Ondřej Svoboda 
---
M tests/functional/networkTests.py
M tests/network/networkmanager.py
2 files changed, 41 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/57517/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index cf185fb..7336080 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -3015,34 +3015,39 @@
 # Verify successfully obtained DHCP configuration.
 conn_info = networkmanager.connection_show(
 conn_name, ('connection.interface-name',
-'bond.options', 'IP4.GATEWAY'))
+'bond.options', 'IP4.ADDRESS', 'IP4.GATEWAY'))
 self.assert_nm_bond_exists(conn_info, BONDING_NAME,
dict(miimon='200',
 mode='broadcast'))
 self.assertEqual(conn_info['IP4.GATEWAY'], IP_GATEWAY)
+original_ipv4_addrs = conn_info['IP4.ADDRESS']
 
-# Let VDSM+initscripts adopt the bond.
-status, msg = self.setupNetworks(
-{NETWORK_NAME: {'bonding': BONDING_NAME, 'bridged': True,
-'bootproto': 'dhcp', 'blockingdhcp': True}
- },
-{BONDING_NAME: {'nics': [local1, local2],
-'options': 'mode=broadcast miimon=200'}},
-NOCHK, test_kernel_config=False)  # ignore bond10
-self.assertEqual(status, SUCCESS, msg)
-self.assertNetworkExists(NETWORK_NAME)
-self.assertBondExists(BONDING_NAME, [local1, local2],
-  'mode=3 miimon=200')
-network_info = self.vdsm_net.netinfo.networks[NETWORK_NAME]
-self.assertEqual(network_info['dhcpv4'], True)
-# TODO: assert that the address was preserved once a fix (to
-# let a bridge inherit DUID also from NetworkManager-created
-# leases) is merged.
+try:
+# Let VDSM+initscripts adopt the bond.
+status, msg = self.setupNetworks(
+{NETWORK_NAME: {'bonding': BONDING_NAME,
+'bridged': True, 'bootproto': 'dhcp',
+'blockingdhcp': True}
+ },
+{BONDING_NAME: {'nics': [local1, local2],
+'options': 'mode=broadcast miimon=200'
+}},
+NOCHK, test_kernel_config=False)  # ignore bond10
+self.assertEqual(status, SUCCESS, msg)
+self.assertNetworkExists(NETWORK_NAME)
+self.assertBondExists(BONDING_NAME, [local1, local2],
+  'mode=3 miimon=200')
+network_info = self.vdsm_net.netinfo.networks[NETWORK_NAME]
+self.assertEqual(network_info['dhcpv4'], True)
+self.assertEqual(set(network_info['ipv4adds']),
+ set(original_ipv4_addrs))
 
-status, msg = self.setupNetworks(
-{NETWORK_NAME: {'remove': True}},
-{BONDING_NAME: {'remove': True}},
-NOCHK, test_kernel_config=False)
-self.assertEqual(status, SUCCESS, msg)
-self.assertNetworkDoesntExist(NETWORK_NAME)
-self.assertBondDoesntExist(BONDING_NAME, [local1, local2])
+status, msg = self.setupNetworks(
+{NETWORK_NAME: {'remove': True}},
+{BONDING_NAME: {'remove': True}},
+NOCHK, test_kernel_config=False)
+self.assertEqual(status, SUCCESS, msg)
+self.assertNetworkDoesntExist(NETWORK_NAME)
+self.assertBondDoesntExist(BONDING_NAME, [local1, local2])
+finally:
+dhcp.delete_dhclient_leases(NETWORK_NAME, dhcpv4=True)
diff --git a/tests/network/networkmanager.py b/tests/network/networkmanager.py
index a786aab..a299396 100644
--- a/tests/network/networkmanager.py
+++ b/tests/network/networkmanager.py
@@ -110,6 +110,15 @@
 if rc:
 raise NetworkManagerError('nmcli failed (rc: {}): {}'.format(rc, err))
 
-# TODO: 

Change in vdsm[master]: ifcfg: restore MAC addresses of bonded NICs when adopting a ...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: ifcfg: restore MAC addresses of bonded NICs when adopting a bond
..

ifcfg: restore MAC addresses of bonded NICs when adopting a bond

The bond is downed during adoption if its ifcfg file is not found (this
applies to NetworkManager-written ifcfg files because it names them
differently from initscripts).

When a slave is detached from the bond, the kernel may change its MAC
address to prevent address conflict, as the bond may still be keeping
the NIC's MAC address.

We don't want the slave's MAC address to change because it may impede
the process of reacquiring a DHCP lease previously lent to the bond,
because at least a certain DHCP server (dnsmasq) requires MAC addresses
to be the same as before, even if DUID is correctly sent in the DHCP
request.

Change-Id: Iad73e6aa016bcd97e50f58295df9ecbeacd0aa17
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
M lib/vdsm/network/models.py
2 files changed, 14 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/57515/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index 1793b66..35c2b49 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -615,6 +615,12 @@
 if ethtool_opts:
 conf += 'ETHTOOL_OPTS=%s\n' % pipes.quote(ethtool_opts)
 
+# If the NIC was enslaved to a bond now being adopted to VDSM (e.g.
+# from NetworkManager), we have to restore its MAC address, as the
+# kernel may have changed it during detachment from the bond.
+if nic.permhwaddr:
+conf += 'MACADDR=%s\n' % pipes.quote(nic.permhwaddr)
+
 ipv4, ipv6, mtu = self._getIfaceConfValues(nic)
 self._createConfFile(conf, nic.name, ipv4, ipv6, mtu, **opts)
 
diff --git a/lib/vdsm/network/models.py b/lib/vdsm/network/models.py
index f91f010..341df11 100644
--- a/lib/vdsm/network/models.py
+++ b/lib/vdsm/network/models.py
@@ -32,7 +32,7 @@
 
 class NetDevice(object):
 def __init__(self, name, configurator, ipv4=None, ipv6=None,
- blockingdhcp=False, mtu=None):
+ blockingdhcp=False, mtu=None, permhwaddr=None):
 self.name = name
 self.ipv4 = ipv4 if ipv4 is not None else IPv4()
 self.ipv6 = ipv6 if ipv6 is not None else IPv6()
@@ -41,6 +41,7 @@
 self.configurator = configurator
 self.master = None
 self.duid_source = None
+self.permhwaddr = permhwaddr
 
 def __iter__(self):
 raise NotImplementedError
@@ -76,7 +77,7 @@
 
 class Nic(NetDevice):
 def __init__(self, name, configurator, ipv4=None, ipv6=None,
- blockingdhcp=False, mtu=None, _netinfo=None):
+ blockingdhcp=False, mtu=None, permhwaddr=None, _netinfo=None):
 if _netinfo is None:
 _netinfo = CachingNetInfo()
 if name not in _netinfo.nics:
@@ -85,8 +86,12 @@
 if _netinfo.ifaceUsers(name):
 mtu = max(mtu, mtus.getMtu(name))
 
+if permhwaddr is None:
+# remember a real MAC address of the NIC, if it is currently bonded
+permhwaddr = _netinfo.nics[name].get('permhwaddr')
+
 super(Nic, self).__init__(name, configurator, ipv4, ipv6, blockingdhcp,
-  mtu)
+  mtu, permhwaddr)
 
 def configure(self, **opts):
 # in a limited condition, we should not touch the nic config


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iad73e6aa016bcd97e50f58295df9ecbeacd0aa17
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: ad_actor_system may not be reset to its default value

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: ad_actor_system may not be reset to its default value
..


Patch Set 1: -Verified

A simpler version of this patch (an interim solution) would simply add 
ad_actor_system to EXCLUDED_BONDING_ENTRIES and say in the commit message that 
"the user set it in the first place so he/she knows how to reset it (by 
providing another valid MAC address)".

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f307b64546c3efa73ca202e97ced49eeb9670e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: ad_actor_system may not be reset to its default value

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: ad_actor_system may not be reset to its default value
..


Patch Set 1:

(1 comment)

Why I was dealing with it differently (it is semantically different from 
EXCLUDED_BONDING_ENTRIES).

https://gerrit.ovirt.org/#/c/57490/1/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

Line 880: for k, v in default_opts.iteritems():
Line 881: if k != 'mode' and k in current_opts and v != current_opts[k]:
Line 882: diff[k] = default_opts[k]
Line 883: for k, v in diff.iteritems():
Line 884: if k in (
> did we not have a means to exclude read-only options?
This was just a quick hack, to show an option of a different kind (settable, 
but not resettable) and perhaps (as said in the commit message) find a way to 
reset it.

If I understand the kernel documentation correctly, we could reset it to the 
bond's MAC address. But we would also have to play tricks not to report it so 
it probably isn't worth the effort.

I'll add ad_actor_system to EXCLUDED_BONDING_ENTRIES and document it properly. 
It will also have to be removed from bonding-defaults.json.
Line 885: # bonding.txt: The value can not be either NULL or 
Multicast.
Line 886: 'ad_actor_system',):
Line 887: continue
Line 888: with open(netinfo_bonding.BONDING_OPT % (bond_name, k), 'w') 
as f:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f307b64546c3efa73ca202e97ced49eeb9670e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: ad_actor_system may not be reset to its default value

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: ad_actor_system may not be reset to its default value
..


Patch Set 1: Verified+1

testSetupNetworksActiveSlave now passes on Fedora 23.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f307b64546c3efa73ca202e97ced49eeb9670e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: ad_actor_system may not be reset to its default value

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: ifcfg: ad_actor_system may not be reset to its default value
..

ifcfg: ad_actor_system may not be reset to its default value

_restore_default_bond_options tries to reset this option to
00:00:00:00:00:00, which (on Fedora 23) causes "IOError: [Errno 22]
Invalid argument", during testSetupNetworksActiveSlave.

Linux documentation (bonding.txt) says that "The value can not
be either NULL or Multicast". Stop resetting the value for now.

From more digging:

bond_option_ad_actor_system_set in bond_options.c doesn't look like it
is possible to reset the value (only valid MAC address is accepted).
The address (if set) persists bond mode changes.

The documentation also says that "If the value is not given then
system defaults to using the masters' mac address as actors' system
address." so the only sane way to reset the option (if we wanted)
would be to the bond's MAC address.

Change-Id: I3f307b64546c3efa73ca202e97ced49eeb9670e9
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
1 file changed, 4 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/57490/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index 384ee25..9532577 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -881,6 +881,10 @@
 if k != 'mode' and k in current_opts and v != current_opts[k]:
 diff[k] = default_opts[k]
 for k, v in diff.iteritems():
+if k in (
+# bonding.txt: The value can not be either NULL or Multicast.
+'ad_actor_system',):
+continue
 with open(netinfo_bonding.BONDING_OPT % (bond_name, k), 'w') as f:
 f.write(' '.join(v))
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f307b64546c3efa73ca202e97ced49eeb9670e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: legacy_switch: quote bonding options and stop repeating "bon...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: legacy_switch: quote bonding options and stop repeating "bond 
Bond()"
..


Patch Set 1: Verified+1

Strings only.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie34b67a74f9411b956376eb0ef0c1debc5d8ff72
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: network: point to vars.hidden_nics when refusing to bond them

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: network: point to vars.hidden_nics when refusing to bond them
..


Patch Set 2: Verified+1

Strings only.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I83130813bb6e168c5148cf58d01ef97de39394bd
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: netconfpersistence: clarify logging messages, use explicit t...

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: netconfpersistence: clarify logging messages, use explicit terms
..


Patch Set 1: Verified+1

Strings only.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I78a15d6218273619f96364ef6fcb77b910998ebb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: explain why we have to reset bonding options to default

[osvoboda]

Ondřej Svoboda has posted comments on this change.

Change subject: ifcfg: explain why we have to reset bonding options to default
..


Patch Set 1: Verified+1

Documentation-only commit.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72bea05213c8561367351749fd6b3831ebe77b9b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: ifcfg: remove duplicated handling of extraneous slaves

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: ifcfg: remove duplicated handling of extraneous slaves
..

ifcfg: remove duplicated handling of extraneous slaves

legacy_switch._detach_extraneous_slaves already does the same job.

Change-Id: I438f93d3002a2371920c23dba2de510559759ce0
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
1 file changed, 0 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/57489/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index 433b2fa..0f66710 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -153,11 +153,6 @@
 self.configApplier.addBonding(bond)
 bondIfcfgWritten = True
 
-# Detach excessive slaves (TODO: hasn't _bonds_edit done this already?)
-for nic in currentNics - nicsToSet:
-ifdown(nic)  # So that no users will be detected for it.
-Nic(nic, self, _netinfo=_netinfo).remove()
-
 for slave in bond.slaves:
 if slave.name in nicsToAdd:
 ifdown(slave.name)  # nics must be down to join a bond


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I438f93d3002a2371920c23dba2de510559759ce0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: legacy_switch: split _detach_extraneous_slaves off _bonds_edit

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: legacy_switch: split _detach_extraneous_slaves off _bonds_edit
..

legacy_switch: split _detach_extraneous_slaves off _bonds_edit

Change-Id: I0a5008d0c801312ab93d14cdaef7674cdf8739f8
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/legacy_switch.py
1 file changed, 4 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/57488/1

diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index e722659..ea5080d 100644
--- a/lib/vdsm/network/legacy_switch.py
+++ b/lib/vdsm/network/legacy_switch.py
@@ -522,6 +522,8 @@
 remove, edit, add = _bonds_classification(bonds, _netinfo)
 _bonds_remove(remove, configurator, _netinfo, in_rollback)
 _netinfo.updateDevices()
+_detach_extraneous_slaves()
+_netinfo.updateDevices()
 _bonds_edit(edit, configurator, _netinfo)
 _bonds_add(add, configurator, _netinfo)
 
@@ -576,7 +578,7 @@
 return True
 
 
-def _bonds_edit(bonds, configurator, _netinfo):
+def _detach_extraneous_slaves(bonds, configurator, _netinfo):
 for name, attrs in six.iteritems(bonds):
 excessive_slaves = (set(_netinfo.bondings[name]['slaves']) -
 set(attrs.get('nics')))
@@ -585,8 +587,8 @@
   excessive_slaves, name)
 _remove_slaves(excessive_slaves, configurator, _netinfo)
 
-_netinfo.updateDevices()
 
+def _bonds_edit(bonds, configurator, _netinfo):
 for name, attrs in six.iteritems(bonds):
 bond = Bond.objectivize(
 name, configurator, attrs.get('options'), attrs.get('nics'),


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0a5008d0c801312ab93d14cdaef7674cdf8739f8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: legacy_switch: make _bonds_edit more readable; and mark a po...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: legacy_switch: make _bonds_edit more readable; and mark a 
possible duplicity in ifcfg
..

legacy_switch: make _bonds_edit more readable; and mark a possible duplicity in 
ifcfg

Change-Id: Ic6f8f3de793087b836885d8afac9256f36f1d9b3
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/configurators/ifcfg.py
M lib/vdsm/network/legacy_switch.py
2 files changed, 7 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/57487/1

diff --git a/lib/vdsm/network/configurators/ifcfg.py 
b/lib/vdsm/network/configurators/ifcfg.py
index 384ee25..433b2fa 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -153,6 +153,7 @@
 self.configApplier.addBonding(bond)
 bondIfcfgWritten = True
 
+# Detach excessive slaves (TODO: hasn't _bonds_edit done this already?)
 for nic in currentNics - nicsToSet:
 ifdown(nic)  # So that no users will be detected for it.
 Nic(nic, self, _netinfo=_netinfo).remove()
diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index 21c45d4..e722659 100644
--- a/lib/vdsm/network/legacy_switch.py
+++ b/lib/vdsm/network/legacy_switch.py
@@ -521,6 +521,7 @@
 _netinfo.updateDevices()
 remove, edit, add = _bonds_classification(bonds, _netinfo)
 _bonds_remove(remove, configurator, _netinfo, in_rollback)
+_netinfo.updateDevices()
 _bonds_edit(edit, configurator, _netinfo)
 _bonds_add(add, configurator, _netinfo)
 
@@ -576,16 +577,14 @@
 
 
 def _bonds_edit(bonds, configurator, _netinfo):
-_netinfo.updateDevices()
-
 for name, attrs in six.iteritems(bonds):
-slaves_to_remove = (set(_netinfo.bondings[name]['slaves']) -
+excessive_slaves = (set(_netinfo.bondings[name]['slaves']) -
 set(attrs.get('nics')))
-logging.debug('Editing bond %r, removing slaves %s', name,
-  slaves_to_remove)
-_remove_slaves(slaves_to_remove, configurator, _netinfo)
+if excessive_slaves:
+logging.debug('Detaching extra slaves %s from %r',
+  excessive_slaves, name)
+_remove_slaves(excessive_slaves, configurator, _netinfo)
 
-# we need bonds to be slaveless in _netinfo
 _netinfo.updateDevices()
 
 for name, attrs in six.iteritems(bonds):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic6f8f3de793087b836885d8afac9256f36f1d9b3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: legacy_switch: quote bonding options and stop repeating "bon...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: legacy_switch: quote bonding options and stop repeating "bond 
Bond()"
..

legacy_switch: quote bonding options and stop repeating "bond Bond()"

Change-Id: Ie34b67a74f9411b956376eb0ef0c1debc5d8ff72
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/legacy_switch.py
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/86/57486/1

diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index 21c45d4..c84c478 100644
--- a/lib/vdsm/network/legacy_switch.py
+++ b/lib/vdsm/network/legacy_switch.py
@@ -592,7 +592,7 @@
 bond = Bond.objectivize(
 name, configurator, attrs.get('options'), attrs.get('nics'),
 mtu=None, _netinfo=_netinfo)
-logging.debug('Editing bond %r with options %s', bond, bond.options)
+logging.debug('Editing %r with options %r', bond, bond.options)
 configurator.editBonding(bond, _netinfo)
 
 
@@ -607,7 +607,7 @@
 bond = Bond.objectivize(
 name, configurator, attrs.get('options'), attrs.get('nics'),
 mtu=None, _netinfo=_netinfo)
-logging.debug('Creating bond %r with options %s', bond, bond.options)
+logging.debug('Creating %r with options %r', bond, bond.options)
 configurator.configureBond(bond)
 
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie34b67a74f9411b956376eb0ef0c1debc5d8ff72
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: network: point to vars.hidden_nics when refusing to bond them

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: network: point to vars.hidden_nics when refusing to bond them
..

network: point to vars.hidden_nics when refusing to bond them

Saying that dummy0 is "unknown" can mean either that it in fact
doesn't exist, or worse, that it is hidden from netinfo, and this
may confuse someone not aware of this configuration option.

Change-Id: I83130813bb6e168c5148cf58d01ef97de39394bd
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/legacy_switch.py
M lib/vdsm/network/models.py
2 files changed, 5 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/57485/1

diff --git a/lib/vdsm/network/legacy_switch.py 
b/lib/vdsm/network/legacy_switch.py
index 21c45d4..1ee61aa 100644
--- a/lib/vdsm/network/legacy_switch.py
+++ b/lib/vdsm/network/legacy_switch.py
@@ -632,8 +632,9 @@
 raise ConfigNetworkError(ne.ERR_BAD_PARAMS,
  'Must specify nics for bonding')
 if not set(nics).issubset(currentNicsSet):
-raise ConfigNetworkError(ne.ERR_BAD_NIC,
- 'Unknown nics in: %r' % list(nics))
+raise ConfigNetworkError(
+ne.ERR_BAD_NIC, 'Unknown nics in: %r (see vars.hidden_nics in '
+'vdsm.conf)' % list(nics))
 
 
 def _validate_network_remove(networkAttrs):
diff --git a/lib/vdsm/network/models.py b/lib/vdsm/network/models.py
index f91f010..51205af 100644
--- a/lib/vdsm/network/models.py
+++ b/lib/vdsm/network/models.py
@@ -80,7 +80,8 @@
 if _netinfo is None:
 _netinfo = CachingNetInfo()
 if name not in _netinfo.nics:
-raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %s' % name)
+raise ConfigNetworkError(ne.ERR_BAD_NIC, 'unknown nic: %s (see '
+'vars.hidden_nics in vdsm.conf)' % name)
 
 if _netinfo.ifaceUsers(name):
 mtu = max(mtu, mtus.getMtu(name))


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I83130813bb6e168c5148cf58d01ef97de39394bd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: netconfpersistence: clarify logging messages, use explicit t...

[osvoboda]

Ondřej Svoboda has uploaded a new change for review.

Change subject: netconfpersistence: clarify logging messages, use explicit terms
..

netconfpersistence: clarify logging messages, use explicit terms

In supervdsm.log, an unattentive reader may easily and often become
confused about the original "Adding/Removing bond/network" messages.

Change-Id: I78a15d6218273619f96364ef6fcb77b910998ebb
Signed-off-by: Ondřej Svoboda 
---
M lib/vdsm/network/netconfpersistence.py
1 file changed, 7 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/57484/1

diff --git a/lib/vdsm/network/netconfpersistence.py 
b/lib/vdsm/network/netconfpersistence.py
index fbf7676..39edb63 100644
--- a/lib/vdsm/network/netconfpersistence.py
+++ b/lib/vdsm/network/netconfpersistence.py
@@ -53,25 +53,26 @@
   ('configurator', '_netinfo', 'force',
'bondingOptions', 'implicitBonding'))
 self.networks[network] = cleanAttrs
-logging.info('Adding network %s(%s)', network, cleanAttrs)
+logging.info('Persisting network %s(%s)', network, cleanAttrs)
 
 def removeNetwork(self, network):
 try:
 del self.networks[network]
-logging.info('Removing network %s', network)
+logging.info('Removing network %s from persistence', network)
 except KeyError:
-logging.debug('Network %s not found for removal', network)
+logging.debug('Network %s not found for removal from persistence',
+  network)
 
 def setBonding(self, bonding, attributes):
 self.bonds[bonding] = attributes
-logging.info('Adding %s(%s)', bonding, attributes)
+logging.info('Persisting %s(%s)', bonding, attributes)
 
 def removeBonding(self, bonding):
 try:
 del self.bonds[bonding]
-logging.info('Removing %s', bonding)
+logging.info('Removing %s from persistence', bonding)
 except KeyError:
-logging.debug('%s not found for removal', bonding)
+logging.debug('%s not found for removal from persistence', bonding)
 
 def diffFrom(self, other):
 """Returns a diff Config that shows the what should be changed for


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I78a15d6218273619f96364ef6fcb77b910998ebb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches