Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
gerrit-hooks has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 6: * Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Dan Kenigsberg has submitted this change and it was merged. Change subject: net: Setup validation for OVS - Check nics usage .. net: Setup validation for OVS - Check nics usage OVS is not properly failing when adding a port that is already slaved to a different device (bond as an example). Therefore, there is a need to extend the validation checks to include a check that looks for nics that may have multiple usages. Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Signed-off-by: Edward Haas Reviewed-on: https://gerrit.ovirt.org/64212 Continuous-Integration: Jenkins CI Reviewed-by: Petr Horáček Reviewed-by: Dan Kenigsberg --- M lib/vdsm/network/netswitch.py M lib/vdsm/network/ovs/validator.py 2 files changed, 43 insertions(+), 4 deletions(-) Approvals: Jenkins CI: Passed CI tests Petr Horáček: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved Edward Haas: Verified -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Dan Kenigsberg has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 5: Code-Review+2 raising -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Petr Horáček has posted comments on this change.
Change subject: net: Setup validation for OVS - Check nics usage
..
Patch Set 5: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/64212/4/lib/vdsm/network/ovs/validator.py
File lib/vdsm/network/ovs/validator.py:
PS4, Line 80: get('nic')
> Nope, that will not work when key exists with value of None.
aha, sorry, i see now
--
To view, visit https://gerrit.ovirt.org/64212
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas
Gerrit-Reviewer: Edward Haas
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Edward Haas has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 5: Verified+1 -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
gerrit-hooks has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Edward Haas has posted comments on this change.
Change subject: net: Setup validation for OVS - Check nics usage
..
Patch Set 4:
(3 comments)
https://gerrit.ovirt.org/#/c/64212/4/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:
PS4, Line 182: ovs_nets =
ovs_info.create_netinfo(_ovs_info)['networks']
: ovs_switch.validator.validate_nic_usage(
: nets2add, bonds2add,
: _get_kernel_nets_nics(ovs_nets),
_get_kernel_bonds_slaves())
> cant we move most if this to validator and keep just 'ovs_switch.validator.
ovs_nets can be regenerated only here and the logic on how to interpret the
info is done here.
the validator is currently nicely independent and does not depend on too much
external stuff. It expects the info to flow in already.
I think we should do an overall cleanup in this module (netswitch) and move
things under the relevant switch, keeping here only switch common stuff.
https://gerrit.ovirt.org/#/c/64212/4/lib/vdsm/network/ovs/validator.py
File lib/vdsm/network/ovs/validator.py:
PS4, Line 43: new_bond
> this matches not only new bonds, but also ones we want to edit.
Right, I can call it setup_bond or request_bond.
It is related to the validation of usages in the sense that if a bond was
mentioned for removal, but also mentioned in a network SB, we would not have
detected this error.
The addition is this: and 'remove' not in bonds[bond]
PS4, Line 80: get('nic')
> or get('nic', []), but i guess your code is more explicit
Nope, that will not work when key exists with value of None.
(Failed in the tests)
--
To view, visit https://gerrit.ovirt.org/64212
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas
Gerrit-Reviewer: Edward Haas
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Petr Horáček has posted comments on this change.
Change subject: net: Setup validation for OVS - Check nics usage
..
Patch Set 4: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/64212/4/lib/vdsm/network/netswitch.py
File lib/vdsm/network/netswitch.py:
PS4, Line 182: ovs_nets =
ovs_info.create_netinfo(_ovs_info)['networks']
: ovs_switch.validator.validate_nic_usage(
: nets2add, bonds2add,
: _get_kernel_nets_nics(ovs_nets),
_get_kernel_bonds_slaves())
cant we move most if this to validator and keep just
'ovs_switch.validator.validate_nic_usage(nets2add, bonds2add) here? it's a mess
https://gerrit.ovirt.org/#/c/64212/4/lib/vdsm/network/ovs/validator.py
File lib/vdsm/network/ovs/validator.py:
PS4, Line 43: new_bond
this matches not only new bonds, but also ones we want to edit.
is this change related to validate_nic_usage?
PS4, Line 80: get('nic')
or get('nic', []), but i guess your code is more explicit
--
To view, visit https://gerrit.ovirt.org/64212
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas
Gerrit-Reviewer: Edward Haas
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Edward Haas has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 4: -Code-Review Verified+1 -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Edward Haas has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 4: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
gerrit-hooks has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
gerrit-hooks has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 3: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
gerrit-hooks has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
gerrit-hooks has posted comments on this change. Change subject: net: Setup validation for OVS - Check nics usage .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/64212 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Edward Haas Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
Change in vdsm[master]: net: Setup validation for OVS - Check nics usage
Edward Haas has uploaded a new change for review.
Change subject: net: Setup validation for OVS - Check nics usage
..
net: Setup validation for OVS - Check nics usage
OVS is not properly failing when adding a port that is already slaved
to a different device (bond as an example).
Therefore, there is a need to extend the validation checks to
include a check that looks for nics that may have multiple usages.
Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1
Signed-off-by: Edward Haas
---
M lib/vdsm/network/ovs/switch.py
M lib/vdsm/network/ovs/validator.py
2 files changed, 31 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/64212/1
diff --git a/lib/vdsm/network/ovs/switch.py b/lib/vdsm/network/ovs/switch.py
index 7f84168..599dfc6 100644
--- a/lib/vdsm/network/ovs/switch.py
+++ b/lib/vdsm/network/ovs/switch.py
@@ -38,7 +38,13 @@
def validate_network_setup(nets, bonds):
ovs_networks = info.create_netinfo(info.OvsInfo())['networks']
kernel_nics = nics()
+
kernel_bonds = Bond.bonds()
+kernel_bonds_slaves = set()
+for bond_name in kernel_bonds:
+kernel_bonds_slaves |= Bond(bond_name).slaves
+validator.validate_nic_usage(nets, bonds, kernel_bonds_slaves)
+
for net, attrs in six.iteritems(nets):
validator.validate_net_configuration(
net, attrs, bonds, kernel_bonds, kernel_nics)
diff --git a/lib/vdsm/network/ovs/validator.py
b/lib/vdsm/network/ovs/validator.py
index 4911a71..3df65f2 100644
--- a/lib/vdsm/network/ovs/validator.py
+++ b/lib/vdsm/network/ovs/validator.py
@@ -23,8 +23,7 @@
from vdsm.network import errors as ne
-def validate_net_configuration(net, attrs, to_be_configured_bonds,
- running_bonds, kernel_nics):
+def validate_net_configuration(net, attrs, bonds, running_bonds, kernel_nics):
"""Test if network meets logical Vdsm requiremets.
Bridgeless networks are allowed in order to support Engine requirements.
@@ -40,8 +39,9 @@
if nic and nic not in kernel_nics:
raise ne.ConfigNetworkError(
ne.ERR_BAD_NIC, 'Nic %s does not exist' % nic)
-if bond and (bond not in running_bonds and
- bond not in to_be_configured_bonds):
+running_bond = bond in running_bonds
+new_bond = bond in bonds and 'remove' not in bonds[bond]
+if bond and not running_bond and not new_bond:
raise ne.ConfigNetworkError(
ne.ERR_BAD_BONDING, 'Bond %s does not exist' % bond)
else:
@@ -65,6 +65,27 @@
raise ne.ConfigNetworkError(ne.ERR_BAD_NIC, 'Missing nics attribute')
+def validate_nic_usage(nets, bonds, kernel_bonds_slaves):
+request_bonds_slaves = set()
+for bond_attr in six.itervalues(bonds):
+if 'remove' in bond_attr:
+continue
+request_bonds_slaves |= set(bond_attr['nics'])
+
+request_nets_nics = set()
+for net_attr in six.itervalues(nets):
+if 'remove' in net_attr:
+continue
+request_nets_nics |= set(net_attr.get('nic', ()))
+
+shared_nics = ((request_bonds_slaves | kernel_bonds_slaves) &
+ request_nets_nics)
+if shared_nics:
+raise ne.ConfigNetworkError(ne.ERR_USED_NIC,
+'Nic/s with multiple usages: ' +
+shared_nics)
+
+
def _validate_bond_addition(nics, kernel_nics):
for nic in nics:
if nic not in kernel_nics:
--
To view, visit https://gerrit.ovirt.org/64212
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie1ed8805f7e1b84e784feef14c5378b41af1cbf1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org
