Change in vdsm[master]: Bond destroying

Petr Šebek has uploaded a new change for review.

Change subject: Bond destroying
......................................................................

Bond destroying

If there is deleted network with bond other than bond[0-4] and no
other network is using that bond, the bond is destroyed. It prevents
accumulation poorly named bonds.

Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Bug-Url: https://bugzilla.redhat.com/928442
Signed-off-by: Petr Sebek 
---
M vdsm/configNetwork.py
1 file changed, 4 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/13752/1

diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 2a85e34..dd362c1 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -516,6 +516,10 @@
     def removeBonding(self, bonding):
         self._backup(netinfo.NET_CONF_PREF + bonding)
         self._removeFile(netinfo.NET_CONF_PREF + bonding)
+        if bonding not in ['bond0', 'bond1', 'bond2', 'bond3', 'bond4']:
+            with open(BONDING_MASTERS, 'w') as f:
+                f.write("-%s\n" % bonding)
+

def removeBridge(self, bridge):
         ifdown(bridge)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1931/ (1/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1880/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 1: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1880/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1931/ : FAILURE

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: oVirt Jenkins CI Server

Antoni Segura Puimedon has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 1:

You are getting some unit tests errors, but they don't see related to your changes. I'll check what's up with that. (Ask David Caro as well, if he knows something about it).

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(2 inline comments)

Yeela, do you have a clue by any chance?

....................................................
Commit Message
Line 7: Bond destroying
Line 8: 
Line 9: If there is deleted network with bond other than bond[0-4] and no
Line 10: other network is using that bond, the bond is destroyed. It prevents
Line 11: accumulation poorly named bonds.
"poorly named" bonds are not the problem.

The issue that we must keep bond0-4 for backward compatibility. But we do not want to accumulate any other bond.

Please fix the commit message to show that.
Line 12: 
Line 13: Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Line 14: Bug-Url: https://bugzilla.redhat.com/928442

....................................................
File vdsm/configNetwork.py
Line 517:         self._backup(netinfo.NET_CONF_PREF + bonding)
Line 518:         self._removeFile(netinfo.NET_CONF_PREF + bonding)
Line 519:         if bonding not in ['bond0', 'bond1', 'bond2', 'bond3', 'bond4']:
Line 520:             with open(BONDING_MASTERS, 'w') as f:
Line 521:                 f.write("-%s\n" % bonding)
wouldn't ifdown(bonding) be enough? Isn't it required anyway?
Line 522: 
Line 523: 
Line 524:     def removeBridge(self, bridge):
Line 525:         ifdown(bridge)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Yeela Kaplan has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 1:

The build should be successful after rebase
(will include http://gerrit.ovirt.org/#/c/13777/)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1969/ (1/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1918/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1969/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1918/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Antoni Segura Puimedon has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)

Just a minor comment.

....................................................
File vdsm/netconf/ifcfg.py
Line 468: 
Line 469:     def removeBonding(self, bonding):
Line 470:         self._backup(netinfo.NET_CONF_PREF + bonding)
Line 471:         self._removeFile(netinfo.NET_CONF_PREF + bonding)
Line 472:         if bonding not in ['bond0', 'bond1', 'bond2', 'bond3', 'bond4']:
it should be a tuple instead of a list, since we are not adding anything to it.
Line 473:             with open(netinfo.BONDING_MASTERS, 'w') as f:
Line 474:                 f.write("-%s\n" % bonding)
Line 475: 
Line 476:     def removeBridge(self, bridge):

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/netconf/ifcfg.py
Line 468: 
Line 469:     def removeBonding(self, bonding):
Line 470:         self._backup(netinfo.NET_CONF_PREF + bonding)
Line 471:         self._removeFile(netinfo.NET_CONF_PREF + bonding)
Line 472:         if bonding not in ['bond0', 'bond1', 'bond2', 'bond3', 'bond4']:
right. and a frozenset() could have been even nicer.

But please look at lib/vdsm/tool/load_needed_modules.py.in

it has the same set of bonds, with the same meaning. It might be a good idea to define it in netinfo once, and use it from both places.
Line 473:             with open(netinfo.BONDING_MASTERS, 'w') as f:
Line 474:                 f.write("-%s\n" % bonding)
Line 475: 
Line 476:     def removeBridge(self, bridge):

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1974/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1918/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1974/ : FAILURE

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: No score

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1975/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1918/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1975/ : FAILURE

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: No score

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1976/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 2: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1918/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1976/ : FAILURE

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Petr Šebek has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/configNetwork.py
Line 517:         self._backup(netinfo.NET_CONF_PREF + bonding)
Line 518:         self._removeFile(netinfo.NET_CONF_PREF + bonding)
Line 519:         if bonding not in ['bond0', 'bond1', 'bond2', 'bond3', 'bond4']:
Line 520:             with open(BONDING_MASTERS, 'w') as f:
Line 521:                 f.write("-%s\n" % bonding)
But ifdown(bonding) is already called in configNetwork.delNetwork() at line 1137 before configWriter.removeBonding().
Line 522: 
Line 523: 
Line 524:     def removeBridge(self, bridge):
Line 525:         ifdown(bridge)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1983/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1929/ (1/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 3: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1983/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1929/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Šebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Šebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 3: No score

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1986/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 3: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1929/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1986/ : FAILURE

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 3: No score

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1987/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 3: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1929/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1987/ : FAILURE

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1933/ (1/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1989/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1989/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1933/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Antoni Segura Puimedon has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 4: (1 inline comment)

Just one very minor comment. But I think the patch can be taken even without that change.

....................................................
File lib/vdsm/tool/load_needed_modules.py.in
Line 41: 
Line 42: 
Line 43: def _enable_bond_dev():
Line 44:     # @ENGINENAME@ currently assumes that all bonding devices pre-exist
Line 45:     existing = set(file(netinfo.BONDING_MASTERS).read().split())
I know you didn't write the "file", but since you are modifying this line... could you change it to use open? file is deprecated and we should get rid of it in the whole codebase.
Line 46:     for bond in netinfo.REQUIRED_BONDINGS - existing:
Line 47:         with open(netinfo.BONDING_MASTERS, 'w') as f:
Line 48:             f.write('+%s\n' % bond)
Line 49:

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 4: I would prefer that you didn't submit this

(1 inline comment)

Thanks, Petr, but I do not think that this patch is ready yet.

....................................................
File lib/vdsm/tool/load_needed_modules.py.in
Line 18: #
Line 19: 
Line 20: 
Line 21: import subprocess
Line 22: import netinfo
* imports of vdsm-specific modules should come after stdlib ones.
* this import works "by chance". I'd rather that you use a more explicit one:

from vdsm inport netinfo
Line 23: 
Line 24: from vdsm.tool import expose
Line 25: 
Line 26:

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1981/ (1/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2038/ (2/2)

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 5:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2038/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1981/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Petr Sebek has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 5: Verified

I have tested
# vdsm-tool load-needed-modules
and everything seems to work properly.

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Antoni Segura Puimedon has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 5: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Bond destroying
......................................................................

Patch Set 5: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Bond destroying
......................................................................

Bond destroying

If there is deleted network with bond other than bond[0-4] and no
other network is using that bond, the bond is destroyed. bond[0-4]
must be kept for backward compatibility, but we don't want to accumulate
any other bond.

Because we need same set of bondings in load_needed_modules.py.in, set of
bondings was added to netinfo and imported from there. The path to
bonding masters now also comes from netinfo.BONDING_MASTERS.

Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Bug-Url: https://bugzilla.redhat.com/928442
Signed-off-by: Petr Sebek 
---
M lib/vdsm/netinfo.py
M lib/vdsm/tool/load_needed_modules.py.in
M vdsm/netconf/ifcfg.py
3 files changed, 10 insertions(+), 6 deletions(-)

Approvals:
  Antoni Segura Puimedon: Looks good to me, but someone else must approve
  Petr Sebek: Verified
  Dan Kenigsberg: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/13752
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I0cd27f1ff9a62e202c3513e54df5481d1b72ed7c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Sebek 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Petr Sebek 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server