Change in vdsm[ovirt-4.0]: ovs: fix bond removal validation

2016-07-25 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: ovs: fix bond removal validation
..


Patch Set 3:

* #1195208::Update tracker: OK
* Set MODIFIED::bug 1195208#1195208FAILED, illegal change from ON_QA

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
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
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: ovs: fix bond removal validation

2016-07-25 Thread fromani 
Francesco Romani has submitted this change and it was merged.

Change subject: ovs: fix bond removal validation
..


ovs: fix bond removal validation

We should allow removal of a bond which network-user will use nic
instead.

Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Signed-off-by: Petr Horáček 
Bug-Url: https://bugzilla.redhat.com/1195208
Reviewed-on: https://gerrit.ovirt.org/58723
Reviewed-by: Edward Haas 
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg 
Reviewed-on: https://gerrit.ovirt.org/61108
---
M lib/vdsm/network/ovs/validator.py
M tests/network/ovs_test.py
2 files changed, 49 insertions(+), 9 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Petr Horáček: Verified
  Dan Kenigsberg: Looks good to me, approved
  Edward Haas: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
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[ovirt-4.0]: ovs: fix bond removal validation

2016-07-25 Thread danken 
Dan Kenigsberg has posted comments on this change.

Change subject: ovs: fix bond removal validation
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
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
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: ovs: fix bond removal validation

2016-07-21 Thread phoracek 
Petr Horáček has posted comments on this change.

Change subject: ovs: fix bond removal validation
..


Patch Set 2: Verified+1

Passed functional/networkTests.py OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
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
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[ovirt-4.0]: ovs: fix bond removal validation

2016-07-21 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: ovs: fix bond removal validation
..


Patch Set 2:

* #1195208::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1195208::OK, public bug
* Check Product::#1195208::OK, Correct classification oVirt
* Check TM::#1195208::OK, correct target milestone ovirt-4.0.3
* Check merged to previous::OK, change not open on any previous branch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
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]: ovs: fix bond removal validation

2016-07-21 Thread edwardh 
Edward Haas has posted comments on this change.

Change subject: ovs: fix bond removal validation
..


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
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]: ovs: fix bond removal validation

2016-07-20 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: ovs: fix bond removal validation
..


Patch Set 1:

* #1195208::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1195208::OK, public bug
* Check Product::#1195208::OK, Correct classification oVirt
* Check TM::#1195208::OK, correct target milestone ovirt-4.0.3
* Check merged to previous::OK, change not open on any previous branch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
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]: ovs: fix bond removal validation

2016-07-20 Thread phoracek 
Hello Dan Kenigsberg, Edward Haas,

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

https://gerrit.ovirt.org/61108

to review the following change.

Change subject: ovs: fix bond removal validation
..

ovs: fix bond removal validation

We should allow removal of a bond which network-user will use nic
instead.

Change-Id: I5fd0a51fca056c04949f1916d0af435d1a125090
Signed-off-by: Petr Horáček 
Bug-Url: https://bugzilla.redhat.com/1195208
Reviewed-on: https://gerrit.ovirt.org/58723
Reviewed-by: Edward Haas 
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg 
---
M lib/vdsm/network/ovs/validator.py
M tests/network/ovs_test.py
2 files changed, 49 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/61108/1

diff --git a/lib/vdsm/network/ovs/validator.py 
b/lib/vdsm/network/ovs/validator.py
index 5715a5e..1039902 100644
--- a/lib/vdsm/network/ovs/validator.py
+++ b/lib/vdsm/network/ovs/validator.py
@@ -55,6 +55,7 @@
 ne.ERR_BAD_VLAN, 'Vlan device requires a nic/bond')
 
 
+# TODO: Pass all nets and bonds to validator at once, not one by one.
 def validate_bond_configuration(bond, attrs, nets, running_nets, kernel_nics):
 """Validate bonding parameters which are not verified by OVS itself.
 
@@ -82,12 +83,32 @@
 
 
 def _validate_bond_removal(bond, nets, running_nets):
-for net in _nets_with_bond(running_nets, bond):
-to_be_removed = 'remove' in nets.get(net, {})
-if not to_be_removed:
-raise ne.ConfigNetworkError(
-ne.ERR_USED_BOND, 'Cannot remove bonding %s: used by '
-'network %s' % (bond, net))
+running_nets_with_bond = set([
+net for net, attrs in six.iteritems(running_nets)
+if attrs['bond'] == bond])
+
+add_nets_with_bond = set()
+remove_nets_with_bond = set()
+for net, attrs in six.iteritems(nets):
+if 'remove' in attrs:
+running_bond = running_nets.get(net, {}).get('bond')
+if running_bond == bond:
+remove_nets_with_bond.add(net)
+elif net in running_nets:
+running_bond = running_nets[net].get('bond')
+if running_bond == bond:
+remove_nets_with_bond.add(net)
+if attrs.get('bonding') == bond:
+add_nets_with_bond.add(net)
+else:
+if attrs.get('bonding') == bond:
+add_nets_with_bond.add(net)
+
+if (add_nets_with_bond or
+(running_nets_with_bond - remove_nets_with_bond)):
+raise ne.ConfigNetworkError(
+ne.ERR_USED_BOND,
+'Cannot remove bonding {}: used by network.'.format(bond))
 
 
 def _nets_with_bond(running_nets, bond):
diff --git a/tests/network/ovs_test.py b/tests/network/ovs_test.py
index 9a5fdc1..a7c072d 100644
--- a/tests/network/ovs_test.py
+++ b/tests/network/ovs_test.py
@@ -136,7 +136,7 @@
 running_nets = {}
 with self.assertNotRaises():
 ovs_validator.validate_bond_configuration(
-'bond1', {'remove': True, 'switch': 'ovs'}, nets, running_nets,
+'bond1', {'remove': True}, nets, running_nets,
 fake_kernel_nics)
 
 def test_remove_bond_attached_to_network_that_was_removed(self):
@@ -145,7 +145,7 @@
 running_nets = {'net1': {'bond': 'bond1'}}
 with self.assertNotRaises():
 ovs_validator.validate_bond_configuration(
-'bond1', {'remove': True, 'switch': 'ovs'}, nets, running_nets,
+'bond1', {'remove': True}, nets, running_nets,
 fake_kernel_nics)
 
 def test_remove_bond_attached_to_network_that_was_not_removed(self):
@@ -154,7 +154,26 @@
 running_nets = {'net1': {'bond': 'bond1'}}
 with self.assertRaises(ne.ConfigNetworkError) as e:
 ovs_validator.validate_bond_configuration(
-'bond1', {'remove': True, 'switch': 'ovs'}, nets, running_nets,
+'bond1', {'remove': True}, nets, running_nets,
+fake_kernel_nics)
+self.assertEquals(e.exception[0], ne.ERR_USED_BOND)
+
+def test_remove_bond_attached_to_network_that_will_use_nic(self):
+fake_kernel_nics = ['eth0', 'eth1']
+nets = {'net1': {'nic': 'eth0'}}
+running_nets = {'net1': {'bond': 'bond1'}}
+with self.assertNotRaises():
+ovs_validator.validate_bond_configuration(
+'bond1', {'remove': True}, nets, running_nets,
+fake_kernel_nics)
+
+def test_remove_bond_reattached_to_another_network(self):
+fake_kernel_nics = ['eth0', 'eth1', 'eth2']
+nets = {'net1': {'nic': 'eth0'}, 'net2': {'bonding': 'bond1'}}
+running_nets = {'net1': {'bond': 'bond1'}}
+with self.assertRaises(ne.ConfigNetworkError) as e:
+ovs_validator.validate_bond_configuration(
+