Change in vdsm[master]: Fix assert bug in testBrokenBridgelessNetReplacement
Mark Wu has posted comments on this change. Change subject: Fix assert bug in testBrokenBridgelessNetReplacement .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/22710 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie15ae32d83dbc771c675306b9aa8816162e9d7e3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix assert bug in testBrokenBridgelessNetReplacement
Mark Wu has posted comments on this change. Change subject: Fix assert bug in testBrokenBridgelessNetReplacement .. Patch Set 1: I am fine for both resolution. I just find that it's very bad to not closely follow the update of community. I didn't know that we already patch for this issue. A minor issue consume a lot time because I didn't see it complained the network still in config. I thought it's still in netinfo. ... -- To view, visit http://gerrit.ovirt.org/22710 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie15ae32d83dbc771c675306b9aa8816162e9d7e3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add config option for network configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 13: Thanks a lot for the help!! -- To view, visit http://gerrit.ovirt.org/18210 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add dhcp support for iproute2 configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add dhcp support for iproute2 configurator .. Patch Set 13: Dan, I didn't notice you have helped to update it. Please merge patch v12. I don't include other changes except you asked. I am sorry for the noise. Thanks a lot for the help! -- To view, visit http://gerrit.ovirt.org/15492 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea88e8693e47fa51edb89c33344332c88c5c964d Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add config option for network configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 13: Dan, I didn't notice you have helped to update it. Please merge patch v12. I don't include other changes except you asked. -- To view, visit http://gerrit.ovirt.org/18210 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1 Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix assert bug in testBrokenBridgelessNetReplacement
Mark Wu has uploaded a new change for review.
Change subject: Fix assert bug in testBrokenBridgelessNetReplacement
..
Fix assert bug in testBrokenBridgelessNetReplacement
If the underlying device is missing, vdsm will not report
the network, but its config in vdsm is still there before
the broken network is removed.
Change-Id: Ie15ae32d83dbc771c675306b9aa8816162e9d7e3
Signed-off-by: Mark Wu
---
M tests/functional/networkTests.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/10/22710/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 45bfaa5..a2096af 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -1635,7 +1635,7 @@
self.assertNetworkExists(NETWORK_NAME)
ipwrapper.linkDel(nic + '.' + VLAN_ID)
self.vdsm_net.refreshNetinfo()
-self.assertNetworkDoesntExist(NETWORK_NAME)
+self.assertNotIn(NETWORK_NAME, self.vdsm_net.netinfo.networks)
status, msg = self.vdsm_net.setupNetworks(network, {},
{'connectivityCheck': 0})
self.assertEqual(status, SUCCESS, msg)
--
To view, visit http://gerrit.ovirt.org/22710
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie15ae32d83dbc771c675306b9aa8816162e9d7e3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add dhcp support for iproute2 configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add dhcp support for iproute2 configurator .. Patch Set 11: IIRC, I was asked to move it to lib/vdsm before. I agree with you thought. I am going to move it back. -- To view, visit http://gerrit.ovirt.org/15492 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea88e8693e47fa51edb89c33344332c88c5c964d Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add config option for network configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 11: Hi Dan, Yes, 5 test cases can't pass with iproute2 configurator, including the case you pointed out. We don't want to resolve the following problem in that patch: 1. source routing support. (Assaf has a patch for it) 2. ipv6 support 3. persist the dhcp option in the unified persistence. The failure you see is caused by 3. I have get agreement on that with Antoni. Are you fine with it? -- To view, visit http://gerrit.ovirt.org/18210 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add dhcp support for iproute2 configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add dhcp support for iproute2 configurator .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.ovirt.org/15492 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea88e8693e47fa51edb89c33344332c88c5c964d Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add config option for network configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.ovirt.org/18210 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 25: Verified+1 -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add dhcp support for iproute2 configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add dhcp support for iproute2 configurator .. Patch Set 10: Verified+1 -- To view, visit http://gerrit.ovirt.org/15492 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iea88e8693e47fa51edb89c33344332c88c5c964d Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add config option for network configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 10: Verified+1 -- To view, visit http://gerrit.ovirt.org/18210 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 24: Verified+1 -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 21: Verified+1 -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has abandoned this change. Change subject: netconf: Improve unified persistence's rollback in memory. .. Abandoned Resolved in Toni's series. -- To view, visit http://gerrit.ovirt.org/20032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: define rollback contract at internal API
Mark Wu has posted comments on this change. Change subject: netconf: define rollback contract at internal API .. Patch Set 10: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/21702 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I29c9411ce3f1ede6557c529bb4d984f74ed8f8ad Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: provide a default rollback for configurators
Mark Wu has posted comments on this change. Change subject: netconf: provide a default rollback for configurators .. Patch Set 4: it needs a rebase on d4737dd40 to resolve the 'defaultdict' regression. -- To view, visit http://gerrit.ovirt.org/21739 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I267e7d682e5075695fbc16fd45b241c4137296da Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 19: It can pass all the tests except followings: testAddVlanedBridgeless testAddVlanedBridgeless_oneCommand testIPv6ConfigNetwork testStaticSourceRouting(kwargs=False) testStaticSourceRouting(kwargs=True) The first two is caused by getIfaceCfg doesn't work with unified persistence. The third one is caused the lack of ipv6 support staticRouting support is not included this patch either. -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change.
Change subject: netconf: Improve unified persistence's rollback in memory.
..
Patch Set 6:
(1 comment)
File vdsm/netconf/__init__.py
Line 65: if self.unifiedPersistence and self.runningConfig is None:
Line 66: self.runningConfig = RunningConfig()
Line 67:
Line 68: def rollback(self):
Line 69: if self.unifiedPersistence:
sorry for the unclear comments. I mean the memory restore code also need call
'setupNetworks', if we move it to RunningConfig, then we're are making a
dependency of configNetwork.py for vdsm.netconfpersistence
Line 70: restore_net = imp.load_source('restore_net',
Line 71: EXT_VDSM_RESTORE_NET_CONFIG)
Line 72: restore_net.restore_from_memory(self.runningConfig)
Line 73: self.runningConfig = None
--
To view, visit http://gerrit.ovirt.org/20032
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change.
Change subject: netconf: Improve unified persistence's rollback in memory.
..
Patch Set 6:
(1 comment)
File vdsm/netconf/__init__.py
Line 65: if self.unifiedPersistence and self.runningConfig is None:
Line 66: self.runningConfig = RunningConfig()
Line 67:
Line 68: def rollback(self):
Line 69: if self.unifiedPersistence:
I am find to move the memory restore code to RunningConfig class. but we
still need use imp.load to avoid circular importing issue. Can you accept that?
Line 70: restore_net = imp.load_source('restore_net',
Line 71: EXT_VDSM_RESTORE_NET_CONFIG)
Line 72: restore_net.restore_from_memory(self.runningConfig)
Line 73: self.runningConfig = None
--
To view, visit http://gerrit.ovirt.org/20032
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change. Change subject: netconf: Improve unified persistence's rollback in memory. .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.ovirt.org/20032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Skip monitoring the usage of tmpfs filesystems in diskStats
Mark Wu has abandoned this change. Change subject: Skip monitoring the usage of tmpfs filesystems in diskStats .. Abandoned It should be done on engine side -- To view, visit http://gerrit.ovirt.org/11675 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Idb0a4ae2cf7ceb6297e348d9e90c166373461ca1 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Deepak C Shetty Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Zhou Zheng Sheng Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change.
Change subject: netconf: Improve unified persistence's rollback in memory.
..
Patch Set 3:
(4 comments)
Thanks a lot for your insightful review! I will post a new patch in a few days.
File tests/functional/networkTests.py
Line 1514: self.assertNetworkExists(networks[0])
Line 1515:
Line 1516: # Submit a transaction composed of removing the
created network
Line 1517: # above, creating two new networks with good params
and bad
Line 1518: # params separately.
Done
Line 1519: status, msg =
self.vdsm_net.setupNetworks({networks[0]:
Line 1520:
dict(remove=True),
Line 1521:
networks[1]:
Line 1522:attrs[1],
File vdsm/netconf/__init__.py
Line 71: self.runningConfig.save()
Line 72: self.runningConfig = None
Line 73:
Line 74: def flush(self):
Line 75: libvirtCfg.flush()
Done
Line 76:
Line 77: def configureBridge(self, bridge, **opts):
Line 78: raise NotImplementedError
Line 79:
File vdsm/vdsm-restore-net-config
Line 67:
Line 68:
Line 69: def restore_from_memory(liveConfig):
Line 70: assert config.get('vars', 'persistence') == 'unified'
Line 71: lastConfig = RunningConfig() # snapshot of running config on disk
Done
Line 72:
Line 73: def get_restore_configs(liveConfig, lastConfig):
Line 74: restoreConfigs = {}
Line 75: for name in liveConfig:
Line 84: nets = get_restore_configs(liveConfig.networks,
lastConfig.networks)
Line 85: bonds = get_restore_configs(liveConfig.bonds, lastConfig.bonds)
Line 86: logging.debug("Calling setupNetworks with networks: %s, bondings:
%s" %
Line 87: (nets, bonds))
Line 88: setupNetworks(nets, bonds, connectivityCheck=False)
My thought of solving this problem is adding a new arg to setupNetworks to
indicate it's called in a normal api flow or the context of except handling,
and do the final restore like you suggest in Configurator.__exit__. Maybe the
new arg is not necessary, we can tell the exception depth by exploring its
stack frames. I am going to look into it.
Line 89:
Line 90:
Line 91: if __name__ == '__main__':
Line 92: try:
--
To view, visit http://gerrit.ovirt.org/20032
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change. Change subject: netconf: Improve unified persistence's rollback in memory. .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.ovirt.org/20032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change. Change subject: netconf: Improve unified persistence's rollback in memory. .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/20032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix vm block stats lost after recovery
Mark Wu has abandoned this change. Change subject: Fix vm block stats lost after recovery .. Abandoned -- To view, visit http://gerrit.ovirt.org/6056 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I59a90cf909a07967f642d348e47b4928f7516c61 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm.utils: drop unused ImagePathStatus and getPidNiceness
Mark Wu has posted comments on this change. Change subject: vdsm.utils: drop unused ImagePathStatus and getPidNiceness .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/20674 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I78a732c6204385a3c33c4a1aea2cc78e3f404bb6 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: drop unused checkPathStat
Mark Wu has posted comments on this change. Change subject: drop unused checkPathStat .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/20673 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d3017b2cc13134253284248258879c6140d26de Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: Sorry for no update on this patch in a long period. Actually, this patch is pending on two problems: 1, a race problem in the unified persistence patch merged in a few days before. Antoni and I are working on it now. 2. The live rollback support for unified persistence (http://gerrit.ovirt.org/#/c/20032/). I get a offline comment on patch from Antoni: using imp.load instead of pickle. I am going to update that patch in a couple of days. -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Itamar Heim Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change. Change subject: netconf: Improve unified persistence's rollback in memory. .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/20032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has uploaded a new change for review.
Change subject: netconf: Improve unified persistence's rollback in memory.
..
netconf: Improve unified persistence's rollback in memory.
This patch removes the dependency on ifcfg's code for the unified
persistence's rollback in memory. It also move transaction management
implementaion from ifcfg to its base class Configurator. With this
change, the rollback function be reused for the iproute2 configuration.
This patch also adds a functional test for live rollback. It passes with
ifcfg persistence and unified persistence.
Change-Id: I7436976d8bacbfaa1c4b059c71c4abb46192f99a
Signed-off-by: Mark Wu
---
M lib/vdsm/constants.py.in
M tests/functional/networkTests.py
M vdsm/netconf/__init__.py
M vdsm/netconf/ifcfg.py
M vdsm/vdsm-restore-net-config
5 files changed, 95 insertions(+), 19 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/20032/1
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 1fb729b..a1a0dd6 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -139,6 +139,7 @@
EXT_UNPERSIST = '@UNPERSIST_PATH@'
EXT_VDSM_STORE_NET_CONFIG = '@VDSMDIR@/vdsm-store-net-config'
+EXT_VDSM_RESTORE_NET_CONFIG = '@VDSMDIR@/vdsm-restore-net-config'
EXT_WGET = '@WGET_PATH@'
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index b6c747f..d1f2329 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -1425,3 +1425,36 @@
status, msg = self.vdsm_net.setupNetworks(delete_networks,
{}, {})
self.assertEqual(status, SUCCESS, msg)
+
+@cleanupNet
+@permutations([[True], [False]])
+@RequireDummyMod
+@ValidateRunningAsRoot
+def testSetupNetworksRollback(self, bridged):
+with dummyIf(1) as nics:
+with self.vdsm_net.pinger():
+nic, = nics
+tags = (10, 11, 12000)
+networks = [NETWORK_NAME + str(tag) for tag in tags]
+attrs = [dict(vlan=tag, nic=nic, bridged=bridged)
+ for tag in tags]
+status, msg = self.vdsm_net.setupNetworks({networks[0]:
+ attrs[0]}, {}, {})
+
+self.assertEqual(status, SUCCESS, msg)
+self.assertTrue(self.vdsm_net.networkExists(networks[0]))
+
+status, msg = self.vdsm_net.setupNetworks({networks[0]:
+ dict(remove=True),
+ networks[1]:
+ attrs[1],
+ networks[2]:
+ attrs[2]}, {}, {})
+self.assertFalse(self.vdsm_net.networkExists(networks[1]))
+self.assertFalse(self.vdsm_net.networkExists(networks[2]))
+self.assertTrue(self.vdsm_net.networkExists(networks[0]))
+
+status, msg = self.vdsm_net.setupNetworks({networks[0]:
+ dict(remove=True)},
+ {}, {})
+self.assertEqual(status, SUCCESS, msg)
diff --git a/vdsm/netconf/__init__.py b/vdsm/netconf/__init__.py
index c0e3ee9..9dc0f70 100644
--- a/vdsm/netconf/__init__.py
+++ b/vdsm/netconf/__init__.py
@@ -18,13 +18,16 @@
#
import logging
+import pickle
from netmodels import Bond, Bridge
from sourceRoute import DynamicSourceRoute
from sourceRoute import StaticSourceRoute
+from vdsm import constants
from vdsm import netinfo
from vdsm.config import config
from vdsm.netconfpersistence import RunningConfig
+from vdsm.utils import execCmd
class Configurator(object):
@@ -46,6 +49,28 @@
else:
self.rollback()
+def begin(self, configApplierClass):
+if self.configApplier is None:
+self.configApplier = configApplierClass()
+if self.unifiedPersistence and self.runningConfig is None:
+self.runningConfig = RunningConfig()
+
+def rollback(self):
+if self.unifiedPersistence:
+liveConfig = pickle.dumps(self.runningConfig)
+execCmd([constants.EXT_VDSM_RESTORE_NET_CONFIG, '--memory',
+ liveConfig])
+self.runningConfig = None
+else:
+self.configApplier.restoreBackups()
+self.configApplier = None
+
+def commit(self):
+self.configApplier = None
+if self.unifiedPersistence:
+self.runningConfig.save()
+self.runningConfig = None
+
def configureBridge(self, bridge, **opts):
raise NotImplementedError
diff -
Change in vdsm[master]: Bump kernel requires version to pass testSetupNetworksMtus o...
Mark Wu has posted comments on this change. Change subject: Bump kernel requires version to pass testSetupNetworksMtus on F19 .. Patch Set 1: It looks the jekins failure is caused by missing dependency of glusterfs, not relevant to this patch. -- To view, visit http://gerrit.ovirt.org/19974 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I161b34b0a67c88fe18c21e9857841639f7001098 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Bump kernel requires version to pass testSetupNetworksMtus o...
Mark Wu has posted comments on this change. Change subject: Bump kernel requires version to pass testSetupNetworksMtus on F19 .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/19974 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I161b34b0a67c88fe18c21e9857841639f7001098 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Mark Wu Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Bump kernel requires version to pass testSetupNetworksMtus o...
Mark Wu has uploaded a new change for review.
Change subject: Bump kernel requires version to pass testSetupNetworksMtus on
F19
..
Bump kernel requires version to pass testSetupNetworksMtus on F19
It's found that testSetupNetworksMtus fails on F19 because of the
broken promiscuity reference counting issue in kernel. It has been
fixed in kernel-3.11.3-201.fc19. With the updated kernel, the test
testSetupNetworksMtus can pass. For detailed information, please see:
https://bugzilla.redhat.com/show_bug.cgi?id=1005567
This problem doesn't exist in RHEL6 because it's triggered by a new
kernel change. So we don't need any fix for rhel kernel.
Change-Id: I161b34b0a67c88fe18c21e9857841639f7001098
Signed-off-by: Mark Wu
---
M vdsm.spec.in
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/19974/1
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 0b1727f..be6475b 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -164,9 +164,11 @@
%if 0%{?fedora} >= 19
Requires: python-pthreading >= 0.1.2
Requires: fence-agents-all
+Requires: kernel >= 3.11.3-201
%else
Requires: python-pthreading
Requires: fence-agents
+Requires: kernel >= 3.6
%endif
# Subprocess and thread bug was found on python 2.7.2
Requires: python >= 2.7.3
@@ -179,7 +181,6 @@
Requires: iscsi-initiator-utils >= 6.2.0.872-14
Requires: device-mapper-multipath >= 0.4.9-18
Requires: e2fsprogs >= 1.41.14
-Requires: kernel >= 3.6
Requires: sanlock >= 2.4-2, sanlock-python
Requires: policycoreutils-python
Requires: sed >= 4.2.1-10
--
To view, visit http://gerrit.ovirt.org/19974
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I161b34b0a67c88fe18c21e9857841639f7001098
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Let dummy interfaces destroied after cleaning up netw...
Mark Wu has abandoned this change. Change subject: tests: Let dummy interfaces destroied after cleaning up networks. .. Abandoned fixed in http://gerrit.ovirt.org/19650 -- To view, visit http://gerrit.ovirt.org/19609 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I8dd6c75794244069b035676ea67641f8a3d3964f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Sort bond's slave when objectivizing
Mark Wu has posted comments on this change.
Change subject: netconf: Sort bond's slave when objectivizing
..
Patch Set 3:
(1 comment)
File vdsm/netmodels.py
Line 389: to bonding in the same order that initscripts does it. Then it can
Line 390: obtain the same master mac address by iproute2 as ifcfg.
Line 391: """
Line 392:
Line 393: nics_list = []
Good catch! I will fix it in next patch.
Line 394: nics_rexp = re.compile("^(\D*)(\d*)(.*)$")
Line 395:
Line 396: for nic_name in nics:
Line 397: nic_sre = nics_rexp.match(nic_name)
--
To view, visit http://gerrit.ovirt.org/19591
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5db20ca598414ff29a368d99156b04b7f61961ae
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has abandoned this change. Change subject: netconf: Improve unified persistence's rollback in memory. .. Abandoned -- To view, visit http://gerrit.ovirt.org/19541 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Let dummy interfaces destroied after cleaning up netw...
Mark Wu has posted comments on this change. Change subject: tests: Let dummy interfaces destroied after cleaning up networks. .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/19609 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8dd6c75794244069b035676ea67641f8a3d3964f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server 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 dummy interfaces destroied after cleaning up netw...
Mark Wu has uploaded a new change for review.
Change subject: tests: Let dummy interfaces destroied after cleaning up
networks.
..
tests: Let dummy interfaces destroied after cleaning up networks.
If dummy interfaces are destroied before cleaning up newtowrks, it
will fail to restore network with this error:
'Missing required nics for bonding device.'
Change-Id: I8dd6c75794244069b035676ea67641f8a3d3964f
Signed-off-by: Mark Wu
---
M tests/functional/dummy.py
M tests/functional/networkTests.py
M tests/functional/utils.py
3 files changed, 3 insertions(+), 58 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/19609/1
diff --git a/tests/functional/dummy.py b/tests/functional/dummy.py
index f285cc4..74d8c94 100644
--- a/tests/functional/dummy.py
+++ b/tests/functional/dummy.py
@@ -21,6 +21,7 @@
from nose.plugins.skip import SkipTest
+from utils import restoreNetConfig
from vdsm.ipwrapper import linkAdd, linkDel, addrAdd, linkSet, IPRoute2Error
@@ -88,6 +89,7 @@
dummies = [create() for _ in range(num)]
yield dummies
except Exception:
+restoreNetConfig()
raise
finally:
for dummy in dummies:
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index beb5ae7..87ab8d0 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -28,7 +28,7 @@
import dummy
from dummy import dummyIf
-from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy, cleanupRules
+from utils import restoreNetConfig, SUCCESS, VdsProxy, cleanupRules
from vdsm.ipwrapper import (ruleAdd, ruleDel, routeAdd, routeDel, routeExists,
ruleExists, Route, Rule)
@@ -104,7 +104,6 @@
def setUp(self):
self.vdsm_net = VdsProxy()
-@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -139,7 +138,6 @@
self.vdsm_net.vlanExists(BONDING_NAME + '.' +
networks[vlan_net]['vlan']))
-@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -160,7 +158,6 @@
self.assertEqual(status, SUCCESS, msg)
self.assertFalse(self.vdsm_net.networkExists(NETWORK_NAME))
-@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -192,7 +189,6 @@
{BONDING_NAME: {'remove': True}}, {'connectivityCheck': False})
self.assertEqual(status, SUCCESS, msg)
-@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -211,7 +207,6 @@
self.assertEqual(status, SUCCESS, msg)
self.assertFalse(self.vdsm_net.networkExists(NETWORK_NAME))
-@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -229,7 +224,6 @@
self.assertEqual(status, SUCCESS, msg)
self.assertFalse(self.vdsm_net.networkExists(NETWORK_NAME))
-@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -244,14 +238,12 @@
bridged})
self.assertEqual(status, neterrors.ERR_BAD_BONDING, msg)
-@cleanupNet
def testFailWithInvalidBridgeName(self):
invalid_bridge_names = ('a' * 16, 'a b', 'a\tb', 'a.b', 'a:b')
for bridge_name in invalid_bridge_names:
status, msg = self.vdsm_net.addNetwork(bridge_name)
self.assertEqual(status, neterrors.ERR_BAD_BRIDGE, msg)
-@cleanupNet
def testFailWithInvalidIpConfig(self):
invalid_ip_configs = (dict(IPADDR='1.2.3.4'), dict(NETMASK='1.2.3.4'),
dict(GATEWAY='1.2.3.4'),
@@ -267,7 +259,6 @@
opts=ipconfig)
self.assertEqual(status, neterrors.ERR_BAD_ADDR, msg)
-@cleanupNet
@permutations([[True], [False]])
def testFailWithInvalidNic(self, bridged):
status, msg = self.vdsm_net.addNetwork(NETWORK_NAME,
@@ -276,7 +267,6 @@
self.assertEqual(status, neterrors.ERR_BAD_NIC, msg)
-@cleanupNet
@permutations([[True], [False]])
def testFailWithInvalidParams(self, bridged):
status, msg = self.vdsm_net.addNetwork(NETWORK_NAME, VLAN_ID,
@@ -288,7 +278,6 @@
opts={'bridged': bridged})
self.assertEqual(status, neterrors.ERR_BAD_PARAMS, msg)
-@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -319,7 +308,6 @@
self.vdsm_net.delNetwork(netVlan)
self.assertEquals(status, SUCCESS, msg)
-@cleanupNet
@permutations([[True], [False]
Change in vdsm[master]: netconf: Sort bond's slave when objectivizing
Mark Wu has posted comments on this change. Change subject: netconf: Sort bond's slave when objectivizing .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.ovirt.org/19591 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5db20ca598414ff29a368d99156b04b7f61961ae Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Sort bond's slave when objectivizing
Mark Wu has posted comments on this change. Change subject: netconf: Sort bond's slave when objectivizing .. Patch Set 1: Toni, thanks for the quick response! Actually I thought fixing in iproute2, but we need sort the nic object both in configureBond and editBonding. So I think bond's objectivize is a better place to fix it. I will add a comments as you suggested. -- To view, visit http://gerrit.ovirt.org/19591 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5db20ca598414ff29a368d99156b04b7f61961ae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Sort bond's slave when objectivizing
Mark Wu has posted comments on this change. Change subject: netconf: Sort bond's slave when objectivizing .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/19591 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5db20ca598414ff29a368d99156b04b7f61961ae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Sort bond's slave when objectivizing
Mark Wu has uploaded a new change for review. Change subject: netconf: Sort bond's slave when objectivizing .. netconf: Sort bond's slave when objectivizing To make iproute2 configurator pass testBondHwAddress, we need keep the bond's slaves sorted. Otherwise bond's hwaddr will vary with the order of enslaving. This work is done by initscript for ifcfg configurator. Change-Id: I5db20ca598414ff29a368d99156b04b7f61961ae Signed-off-by: Mark Wu --- M vdsm/netmodels.py 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/91/19591/1 diff --git a/vdsm/netmodels.py b/vdsm/netmodels.py index 50a5ba3..8cb2d3d 100644 --- a/vdsm/netmodels.py +++ b/vdsm/netmodels.py @@ -234,8 +234,8 @@ def objectivize(cls, name, configurator, options, nics, mtu, _netinfo, destroyOnMasterRemoval=None): if name and nics: # New bonding or edit bonding. -slaves = cls._objectivizeSlaves(name, configurator, nics, mtu, -_netinfo) +slaves = cls._objectivizeSlaves(name, configurator, sorted(nics), +mtu, _netinfo) if name in _netinfo.bondings: if _netinfo.ifaceUsers(name): mtu = max(mtu, netinfo.getMtu(name)) -- To view, visit http://gerrit.ovirt.org/19591 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5db20ca598414ff29a368d99156b04b7f61961ae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: Fix testBondHwAddress for fedora kernel
Mark Wu has posted comments on this change. Change subject: tests: Fix testBondHwAddress for fedora kernel .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/19590 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieb4158e678b6e5bfaa7ac5f7e12834cbeeab9572 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server 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: Fix testBondHwAddress for fedora kernel
Mark Wu has uploaded a new change for review.
Change subject: tests: Fix testBondHwAddress for fedora kernel
..
tests: Fix testBondHwAddress for fedora kernel
In relatively new kernel, the hwaddr of bonding will be set to a
random address when the last slave is released. You can find it in:
https://github.com/torvalds/linux/blob/master/drivers/net/bonding/bond_main.c#L1824
So to make the testBondHwAddress pass on fedora, we need backup the
hwaddr before destroy it.
Change-Id: Ieb4158e678b6e5bfaa7ac5f7e12834cbeeab9572
Signed-off-by: Mark Wu
---
M tests/functional/networkTests.py
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/19590/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index beb5ae7..d839df7 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -1161,11 +1161,12 @@
opts={'bridged':
bridged})
self.assertEquals(status, SUCCESS, msg)
+hwaddr = self.vdsm_net.netinfo.bondings[BONDING_NAME]['hwaddr']
status, msg = self.vdsm_net.delNetwork(NETWORK_NAME)
self.assertEquals(status, SUCCESS, msg)
-return self.vdsm_net.netinfo.bondings[BONDING_NAME]['hwaddr']
+return hwaddr
macAddress1 = _getBondHwAddress(nics[0], nics[1])
macAddress2 = _getBondHwAddress(nics[1], nics[0])
--
To view, visit http://gerrit.ovirt.org/19590
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb4158e678b6e5bfaa7ac5f7e12834cbeeab9572
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has posted comments on this change. Change subject: netconf: Improve unified persistence's rollback in memory. .. Patch Set 1: Code-Review-1 (1 comment) File vdsm/vdsm-restore-net-config.init.in Line 32: case "$1" in Line 33: # commands required in all Fedora initscripts Line 34: start|restart|reload|force-reload|condrestart|try-restart) Line 35: echo -n $"Running $prog $1: " Line 36: @VDSMDIR@/vdsm-restore-net-config --disk also need this change for vdsm-restore-net-config.service. will fix it in next patch. Line 37: retval=$? Line 38: echo Line 39: ;; Line 40: stop|status) -- To view, visit http://gerrit.ovirt.org/19541 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: (1 comment) File vdsm/netconf/iproute2.py Line 121: if toBeRemoved: Line 122: if iface.master is None: Line 123: self.configApplier.removeIpConfig(iface) Line 124: Line 125: if destroy: Done Line 126: destroyAction(iface) Line 127: else: Line 128: self.configApplier.setIfaceMtu(iface.name, Line 129:netinfo.DEFAULT_MTU) -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Improve unified persistence's rollback in memory.
Mark Wu has uploaded a new change for review.
Change subject: netconf: Improve unified persistence's rollback in memory.
..
netconf: Improve unified persistence's rollback in memory.
This patch removes the dependency on ifcfg's code for the unified
persistence's rollback in memory. It also move transaction management
implementaion from ifcfg to its base class Configurator. With this
change, the rollback function be reused for the iproute2 configuration.
Change-Id: Ie0a7c7eb6091fa85029a1baebf10fc1ea6e22668
Signed-off-by: Mark Wu
---
M lib/vdsm/constants.py.in
M vdsm/netconf/__init__.py
M vdsm/netconf/ifcfg.py
M vdsm/vdsm-restore-net-config
M vdsm/vdsm-restore-net-config.init.in
5 files changed, 62 insertions(+), 38 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/41/19541/1
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index cfea347..9f4dae6 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -138,6 +138,7 @@
EXT_UNPERSIST = '@UNPERSIST_PATH@'
EXT_VDSM_STORE_NET_CONFIG = '@VDSMDIR@/vdsm-store-net-config'
+EXT_VDSM_RESTORE_NET_CONFIG = '@VDSMDIR@/vdsm-restore-net-config'
EXT_WGET = '@WGET_PATH@'
diff --git a/vdsm/netconf/__init__.py b/vdsm/netconf/__init__.py
index c0e3ee9..9dc0f70 100644
--- a/vdsm/netconf/__init__.py
+++ b/vdsm/netconf/__init__.py
@@ -18,13 +18,16 @@
#
import logging
+import pickle
from netmodels import Bond, Bridge
from sourceRoute import DynamicSourceRoute
from sourceRoute import StaticSourceRoute
+from vdsm import constants
from vdsm import netinfo
from vdsm.config import config
from vdsm.netconfpersistence import RunningConfig
+from vdsm.utils import execCmd
class Configurator(object):
@@ -46,6 +49,28 @@
else:
self.rollback()
+def begin(self, configApplierClass):
+if self.configApplier is None:
+self.configApplier = configApplierClass()
+if self.unifiedPersistence and self.runningConfig is None:
+self.runningConfig = RunningConfig()
+
+def rollback(self):
+if self.unifiedPersistence:
+liveConfig = pickle.dumps(self.runningConfig)
+execCmd([constants.EXT_VDSM_RESTORE_NET_CONFIG, '--memory',
+ liveConfig])
+self.runningConfig = None
+else:
+self.configApplier.restoreBackups()
+self.configApplier = None
+
+def commit(self):
+self.configApplier = None
+if self.unifiedPersistence:
+self.runningConfig.save()
+self.runningConfig = None
+
def configureBridge(self, bridge, **opts):
raise NotImplementedError
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index ec3966a..e3a2131 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -36,7 +36,6 @@
from vdsm import constants
from vdsm import netinfo
from vdsm import utils
-from vdsm.netconfpersistence import RunningConfig
import dsaversion
import libvirtCfg
import neterrors as ne
@@ -48,22 +47,7 @@
super(Ifcfg, self).__init__(ConfigWriter())
def begin(self):
-if self.configApplier is None:
-self.configApplier = ConfigWriter()
-if self.unifiedPersistence and self.runningConfig is None:
-self.runningConfig = RunningConfig()
-
-def rollback(self):
-self.configApplier.restoreBackups()
-self.configApplier = None
-if self.unifiedPersistence:
-self.runningConfig = None
-
-def commit(self):
-self.configApplier = None
-if self.unifiedPersistence:
-self.runningConfig.save()
-self.runningConfig = None
+super(Ifcfg, self).begin(ConfigWriter)
def configureBridge(self, bridge, **opts):
self.configApplier.addBridge(bridge, **opts)
@@ -274,8 +258,8 @@
"(until next 'set safe config')", backup)
def _networkBackup(self, network):
-self._atomicNetworkBackup(network)
if config.get('vars', 'persistence') != 'unified':
+self._atomicNetworkBackup(network)
self._persistentNetworkBackup(network)
def _atomicNetworkBackup(self, network):
@@ -316,8 +300,8 @@
logging.info('Restored %s', network)
def _backup(self, filename):
-self._atomicBackup(filename)
if config.get('vars', 'persistence') != 'unified':
+self._atomicBackup(filename)
self._persistentBackup(filename)
def _atomicBackup(self, filename):
diff --git a/vdsm/vdsm-restore-net-config b/vdsm/vdsm-restore-net-config
index 62045f3..0b2f11b 100755
--- a/vdsm/vdsm-restore-net-config
+++ b/vdsm/vdsm-restore-net-config
@@ -19,8 +19,11 @@
# Refer to the README and COPYING files for full details of the license
#
+import getopt
import logging
import logging.config
+import pickle
+import sys
from netconf impo
Change in vdsm[master]: Unified network persistence [1/4] - Save running config
Mark Wu has posted comments on this change. Change subject: Unified network persistence [1/4] - Save running config .. Patch Set 24: Code-Review+1 (1 comment) File lib/vdsm/netconfpersistence.py Line 80: @staticmethod Line 81: def _setConfig(config, path): Line 82: dirPath = os.path.dirname(path) Line 83: try: Line 84: os.makedirs(dirPath) I guess it's because the dir is only needed when unified persistence is configured. If we choose the static approach, vdsm-tmpfiles.d.conf needs be updated accordingly to make it persist across reboot. Line 85: except OSError as ose: Line 86: if errno.EEXIST != ose.errno: Line 87: raise Line 88: with open(path, 'w') as configurationFile: -- To view, visit http://gerrit.ovirt.org/16699 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7137a96f84abd2c5e532c6c37737e36ef17567a9 Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Report bond speed as function of slaved nics speed and bond ...
Mark Wu has posted comments on this change.
Change subject: Report bond speed as function of slaved nics speed and bond mode
..
Patch Set 1: Code-Review-1
(4 comments)
File lib/vdsm/netinfo.py
Line 297: s = int(speedFile.read())
Line 298: if s not in (2 ** 16 - 1, 2 ** 32 - 1) or s > 0:
Line 299: return s
Line 300:
Line 301: # bondings() lists bond devices
I think the name bondings() is self explanatory, so this comment is not
necessary.
Line 302: if dev in bondings():
Line 303: bond_options = bondOpts(dev)
Line 304: # only bonds with slaved nics will be probed
Line 305: if bond_options['slaves']:
Line 299: return s
Line 300:
Line 301: # bondings() lists bond devices
Line 302: if dev in bondings():
Line 303: bond_options = bondOpts(dev)
the options you care about should just be 'slaves', 'active_slave', and 'mode',
so you could pass the list of keys into bondOpts. Then it can avoid reading
other options.
Line 304: # only bonds with slaved nics will be probed
Line 305: if bond_options['slaves']:
Line 306: # failover modes
Line 307: if bond_options['mode'][1] in ['1', '3']:
Line 303: bond_options = bondOpts(dev)
Line 304: # only bonds with slaved nics will be probed
Line 305: if bond_options['slaves']:
Line 306: # failover modes
Line 307: if bond_options['mode'][1] in ['1', '3']:
You could define constants for the failover mode and balance mode. With that,
you could remove the comments.
Line 308: s = speed(bond_options['active_slave'][0])
Line 309: # load balance modes
Line 310: elif bond_options['mode'][1] in ['0', '2', '4', '5',
'6']:
Line 311: s = 0
Line 309: # load balance modes
Line 310: elif bond_options['mode'][1] in ['0', '2', '4', '5',
'6']:
Line 311: s = 0
Line 312: for slave in bond_options['slaves']:
Line 313: s += speed(slave)
s = sum(speed(slave) for slave in bond_options['slaves']:)
Line 314: return s
Line 315:
Line 316: except Exception:
Line 317: logging.exception('cannot read %s speed', dev)
--
To view, visit http://gerrit.ovirt.org/19297
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide077846e49ac5e4d759a85ff9d5c6cec653aa18
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Amador Pahim
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Ryan Harper
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Unified network persistence [1/3] - Save running config
Mark Wu has posted comments on this change. Change subject: Unified network persistence [1/3] - Save running config .. Patch Set 22: Code-Review-1 (2 comments) File lib/vdsm/unifiedpersistence.py Line 34: def __init__(self, savePath): Line 35: self.networksPath = savePath + 'nets/' Line 36: self.bondingsPath = savePath + 'bonds/' Line 37: self.networks = self._getConfigs(self.networksPath) Line 38: self.bonds = self._getConfigs(self.bondingsPath) I think we should not load configuration from disk in __init__ for running config. We could move them into a separate function named loadConfig. It needs to be called in PersistentConfig.__init__() and vdsm-restore-net-config.unified_restoration() Then all configuration contained in self.networks and self.bonds are the network interfaces already configure but without configuration stored in running config dir. With this change it could be easier to rollback the network configuration. We could add a new function restore() to RunningConfig. It just remove all the networks and bonds contained in self.networks and self.bonds, and then restore those network from the running config on disk. It can perform the rollback without bringing impact on the untouched networks. Line 39: Line 40: def __eq__(self, other): Line 41: return self.networks == other.networks and self.bonds == other.bonds Line 42: File vdsm/netconf/ifcfg.py Line 56: def rollback(self): Line 57: self.configApplier.restoreBackups() Line 58: self.configApplier = None Line 59: if self.unifiedPersistence: Line 60: self.runningConfig = None We can call self.runningConfig.restore() as what I suggest in unifiedpersistence.py to restore live networks instead of still relying on restoreBackups. We can make _networkBackup and _backup totally empty if using unified persistence. With his proposal, the unified persistence can work independently of ifcfg's transaction management. Line 61: Line 62: def commit(self): Line 63: self.configApplier = None Line 64: if self.unifiedPersistence: -- To view, visit http://gerrit.ovirt.org/16699 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7137a96f84abd2c5e532c6c37737e36ef17567a9 Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: Add iproute2 configurator
..
Patch Set 18:
(4 comments)
File vdsm/netconf/iproute2.py
Line 39: def __init__(self):
Line 40: super(Iproute2, self).__init__(ConfigApplier())
Line 41:
Line 42: def begin(self):
Line 43: # To be implemented in a following patch.
Currently, the network configuration code is executed in a context manager
implemented in class Configurator itself. It aims to implement the transaction
management. Please see Configurator.__enter__() in netconf/__init__.py
The transaction management of iproute2 depends on unified persistentence
(http://gerrit.ovirt.org/#/c/16699/)
That's why I leave the API here. I will rebase on the series of unified
persistence patches or resolve it in a separate patch.
Line 44: pass
Line 45:
Line 46: def commit(self):
Line 47: # To be implemented in a following patch.
Line 81: if bond.areOptionsApplied():
Line 82: nicsToAdd = nicsToSet - currentNics
Line 83: nicsToRemove = currentNics - nicsToSet
Line 84: else:
Line 85: nicsToAdd = nicsToSet
Done
Line 86: nicsToRemove = currentNics
Line 87:
Line 88: for nic in nicsToRemove:
Line 89: slave = Nic(nic, self, _netinfo=_netinfo)
Line 121: if toBeRemoved:
Line 122: if iface.master is None:
Line 123: self.configApplier.removeIpConfig(iface)
Line 124:
Line 125: if destroy:
it's not a public API, but an internal function in Iproute2 class. It's used by
the API removeBond and removeNic. Do you still think it could lead bugs?
Line 126: destroyAction(iface)
Line 127: else:
Line 128: self.configApplier.setIfaceMtu(iface.name,
Line 129:netinfo.DEFAULT_MTU)
Line 217:
Line 218: def removeVlan(self, vlan):
Line 219: ipwrapper.linkDel(vlan.name)
Line 220:
Line 221: def addBond(self, bond):
Yes, I agree with your comments. I aggregate them under a class just to make
the code organized the same as ifcfg configurator. Then the two configurators
can share the common code as much as possible.
Line 222: if bond.name not in netinfo.bondings():
Line 223: logging.debug('Add new bonding %s', bond)
Line 224: with open(netinfo.BONDING_MASTERS, 'w') as f:
Line 225: f.write('+%s' % bond.name)
--
To view, visit http://gerrit.ovirt.org/15301
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: Saggi Mizrahi
Gerrit-Reviewer: oVirt Jenkins CI Server
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: bondHwAddress, safeNetworkConfig, volatileConfig
Mark Wu has posted comments on this change. Change subject: tests: bondHwAddress, safeNetworkConfig, volatileConfig .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/17640 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2b035155ef715d8456d9c4658ad2cb7ee76c80d0 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Fix the mtu of bond and nic can't be set for new ne...
Mark Wu has posted comments on this change. Change subject: netconf: Fix the mtu of bond and nic can't be set for new network .. Patch Set 2: Saggi, this patch just follow the pattern of existing test cases. So I would like to leave the refactoring in a separate patch. -- To view, visit http://gerrit.ovirt.org/18966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7a1f5b628261ae4ebf400494bcdb2939a9cd534 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Fix the mtu of bond and nic can't be set for new ne...
Mark Wu has posted comments on this change. Change subject: netconf: Fix the mtu of bond and nic can't be set for new network .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/18966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7a1f5b628261ae4ebf400494bcdb2939a9cd534 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Fix the mtu of bond and nic can't be set for new ne...
Mark Wu has posted comments on this change. Change subject: netconf: Fix the mtu of bond and nic can't be set for new network .. Patch Set 2: Saggi, thanks for the review! I will resolve your comments in next patch. -- To view, visit http://gerrit.ovirt.org/18966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7a1f5b628261ae4ebf400494bcdb2939a9cd534 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 18: Verified+1 -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add config option for network configurator
Mark Wu has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.ovirt.org/18210 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Fix the mtu of bond and nic can't be set for new ne...
Mark Wu has posted comments on this change. Change subject: netconf: Fix the mtu of bond and nic can't be set for new network .. Patch Set 1: Verified+1 It's verified on RHEL6. But the test case fails on Fedora19 because of bug https://bugzilla.redhat.com/show_bug.cgi?id=1005567 -- To view, visit http://gerrit.ovirt.org/18966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7a1f5b628261ae4ebf400494bcdb2939a9cd534 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Fix the mtu of bond and nic can't be set for new ne...
Mark Wu has uploaded a new change for review.
Change subject: netconf: Fix the mtu of bond and nic can't be set for new
network
..
netconf: Fix the mtu of bond and nic can't be set for new network
The configuring should not be skipped if the new vlaned network
over the bond or nic has a bigger mtu than current.
Change-Id: Id7a1f5b628261ae4ebf400494bcdb2939a9cd534
Signed-off-by: Mark Wu
---
M tests/functional/networkTests.py
M vdsm/netmodels.py
2 files changed, 120 insertions(+), 27 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/66/18966/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 94dd0e8..431dfae 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -1013,48 +1013,62 @@
{})
self.assertEquals(status, SUCCESS, msg)
+def _createBondedNetAndCheck(self, netNum, bondDict, bridged,
+ **networkOpts):
+netName = NETWORK_NAME + str(netNum)
+networks = {netName: dict(bonding=BONDING_NAME, bridged=bridged,
+ vlan=str(int(VLAN_ID) + netNum),
+ **networkOpts)}
+status, msg = self.vdsm_net.setupNetworks(networks,
+ {BONDING_NAME: bondDict},
+ {})
+self.assertEquals(status, SUCCESS, msg)
+self.vdsm_net.networkExists(netName, bridged=bridged)
+self.vdsm_net.bondExists(BONDING_NAME, bondDict['nics'])
+if 'mtu' in networkOpts:
+self.assertEquals(networkOpts['mtu'],
+ self.vdsm_net.getMtu(netName))
+
@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
def testSetupNetworksStableBond(self, bridged):
-def createBondedNetAndCheck(netNum, bondDict):
-netName = NETWORK_NAME + str(netNum)
-networks = {netName: dict(bonding=BONDING_NAME, bridged=bridged,
- vlan=str(int(VLAN_ID) + netNum))}
-status, msg = self.vdsm_net.setupNetworks(networks,
- {BONDING_NAME: bondDict},
- {})
-self.assertEquals(status, SUCCESS, msg)
-self.vdsm_net.networkExists(netName, bridged=bridged)
-self.vdsm_net.bondExists(BONDING_NAME, bondDict['nics'])
-
with dummyIf(3) as nics:
with self.vdsm_net.pinger():
# Add initial vlanned net over bond
-createBondedNetAndCheck(0, {'nics': nics[:2],
-'options': 'mode=3 miimon=250'})
+self._createBondedNetAndCheck(0, {'nics': nics[:2],
+ 'options': 'mode=3 miimon=250'},
+ bridged)
with nonChangingOperstate(BONDING_NAME):
# Add additional vlanned net over the bond
-createBondedNetAndCheck(1,
-{'nics': nics[:2],
- 'options': 'mode=3 miimon=250'})
+self._createBondedNetAndCheck(1,
+ {'nics': nics[:2],
+ 'options':
+ 'mode=3 miimon=250'},
+ bridged)
# Add additional vlanned net over the increasing bond
-createBondedNetAndCheck(2,
-{'nics': nics,
- 'options': 'mode=3 miimon=250'})
+self._createBondedNetAndCheck(2,
+ {'nics': nics,
+ 'options':
+ 'mode=3 miimon=250'},
+ bridged)
# Add additional vlanned net over the changing bond
-createBondedNetAndCheck(3,
-{'nics': nics[1:],
- 'options': 'mode=3 miimon=250'})
+self._createBondedNetAndCheck(3,
+ {'nics': nics[1:],
+ 'options':
+ 'mode=3 miimon=250'},
+ bridged)
# Add a network changing bond option
Change in vdsm[master]: Unified network persistence [1/3] - Save running config
Mark Wu has posted comments on this change.
Change subject: Unified network persistence [1/3] - Save running config
..
Patch Set 20: Code-Review-1
(5 comments)
File vdsm/configNetwork.py
Line 166: isolatedCommand = 'configurator' not in attrs
Line 167: configurator = attrs.get('configurator', None)
Line 168: # If we are running an isolated command (not part of
setupNetworks or
Line 169: # editNetwork we set up a transaction without rollback but
with commit.
Line 170: if isolatedCommand or configurator is None:
why not just let
isolatedCommand = 'configurator' not in attrs or configurator is None
in line 166?
Line 171: attrs['configurator'] = configurator = Ifcfg()
Line 172: configurator.begin()
Line 173:
Line 174: ret = func(**attrs)
Line 173:
Line 174: ret = func(**attrs)
Line 175:
Line 176: nics = attrs.pop('nics', None)
Line 177: if not attrs.get('bonding'): # Bond config handled in
configurator.
I really don't understand why you leave bond config to configurator. How about
filtering out bonding related arguments in the unifiedpersistence module, and
call setBonding there?
Line 178: if nics:
Line 179: attrs['nic'], = nics
Line 180:
Line 181: if func.__name__ == 'delNetwork':
File vdsm/netconf/ifcfg.py
Line 53: if self.unifiedPersistence and self.runningConfig is None:
Line 54: self.runningConfig = RunningConfig()
Line 55:
Line 56: def rollback(self):
Line 57: self.configApplier.restoreBackups()
what's the point of calling restoreBackups() when it use unified persistence?
Line 58: self.configApplier = None
Line 59: if self.unifiedPersistence:
Line 60: self.runningConfig = None
Line 61:
Line 56: def rollback(self):
Line 57: self.configApplier.restoreBackups()
Line 58: self.configApplier = None
Line 59: if self.unifiedPersistence:
Line 60: self.runningConfig = None
the rollback still not fully implemented, right? we still need add a method to
runningConfig, like restore(), to restore the live state of devices, and call
it here.
Line 61:
Line 62: def commit(self):
Line 63: self.configApplier = None
Line 64: if self.unifiedPersistence:
Line 62: def commit(self):
Line 63: self.configApplier = None
Line 64: if self.unifiedPersistence:
Line 65: self.runningConfig.save()
Line 66: self.runningConfig = None
the commit method is not specific configurator related, so we could move it to
the base class to make it available for iproute2. I am also fine to change it
when I rebase iproute2 configurator on it.
Line 67:
Line 68: def configureBridge(self, bridge, **opts):
Line 69: self.configApplier.addBridge(bridge, **opts)
Line 70: ifdown(bridge.name)
--
To view, visit http://gerrit.ovirt.org/16699
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7137a96f84abd2c5e532c6c37737e36ef17567a9
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: Add tuneBlockDevIo interface
Mark Wu has posted comments on this change. Change subject: vdsm: Add tuneBlockDevIo interface .. Patch Set 14: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/14394 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb33510a081d221af0f69d4dd2d55adf0b79efd2 Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mei Liu Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Doron Fediuck Gerrit-Reviewer: Eduardo Gerrit-Reviewer: Eli Mesika Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Mei Liu Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: Yeela Kaplan Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: fix testDelNetworkWithMTU
Mark Wu has posted comments on this change. Change subject: fix testDelNetworkWithMTU .. Patch Set 2: Code-Review+1 Thanks for the fix! This is what I am going to fix with the same idea :) -- To view, visit http://gerrit.ovirt.org/18714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I151447250159df389a159d3823a9bd41cb51c211 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: Add tuneBlockDevIo interface
Mark Wu has posted comments on this change.
Change subject: vdsm: Add tuneBlockDevIo interface
..
Patch Set 13: Code-Review-1
(16 comments)
I am sorry for not pointing out those problems in previous reviews.
File lib/vdsm/define.py
Line 146: ERROR = 1
Line 147: NORMAL = 0
Line 148:
Line 149:
Line 150: def translateErrCode(errKey, errMsg=None):
I don't think it's kind of translation. so you should give it a better name,
vdsmErrCode or customizedErrCode? I don't know.
Line 151: error = errCode[errKey].copy()
Line 152: if errMsg:
Line 153: error['status'].update({'message': errMsg})
Line 149:
Line 150: def translateErrCode(errKey, errMsg=None):
Line 151: error = errCode[errKey].copy()
Line 152: if errMsg:
Line 153: error['status'].update({'message': errMsg})
is error['status']['message'] = errMsg simpler?
File vdsm/vm.py
Line 4313: self.log.error(errLogMsg, exc_info=True)
Line 4314: return translateErrCode('balloonErr',
Line 4315: 'An integer is required for
target')
Line 4316: except libvirt.libvirtError as e:
Line 4317: self.log.error(errLogMsg, exc_info=True)
you could just use self.log.exception(errLogMsg) instead.
Line 4318: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 4319: return translateErrCode('noVM')
Line 4320: return translateErrCode('balloonErr', e.message)
Line 4321: else:
Line 4315: 'An integer is required for
target')
Line 4316: except libvirt.libvirtError as e:
Line 4317: self.log.error(errLogMsg, exc_info=True)
Line 4318: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 4319: return translateErrCode('noVM')
it doesn't do any translation on noVM, right? so it's not a good name.
Line 4320: return translateErrCode('balloonErr', e.message)
Line 4321: else:
Line 4322: for dev in self.conf['devices']:
Line 4323: if dev['type'] == BALLOON_DEVICES and \
Line 4334: if device.get('device') == 'disk' and device.get('name'):
Line 4335: devName = device['name']
Line 4336: result = (device.get('specParams', {}).get('ioTune',
{}))
Line 4337: if result:
Line 4338: results[devName] = result
try:
results[devName] = device['specParams']['ioTune']
except KeyError:
continue
is it better?
Line 4339: return results
Line 4340:
Line 4341: def _checkIoTuneInvalidParams(self, params):
Line 4342: validKeys = set(('total_bytes_sec', 'read_bytes_sec',
Line 4345: paramKeys = set(params)
Line 4346:
Line 4347: invalidParams = paramKeys - validKeys
Line 4348: invalidParamNames = ', '.join(invalidParams)
Line 4349: return invalidParamNames
I think we can use raise exception to replace passing back:
if invalidParamNames:
raise ValueError('tuneBlkDevIoErr',
'Parameter %s name(s) are invalid' %
invalidParamNames)
Line 4350:
Line 4351: def _checkIoTuneCategories(self, params):
Line 4352: categories = ("bytes", "iops")
Line 4353: for category in categories:
Line 4353: for category in categories:
Line 4354: if params.get('total_' + category + '_sec', 0) and \
Line 4355: (params.get('read_' + category + '_sec', 0) or
Line 4356: params.get('write_' + category + '_sec', 0)):
Line 4357: return category
raise ValueError('A non-zero total and non-zero read'
'/write value for %s_sec can not be'
' set at the same time' % category)
Line 4358:
Line 4359: def _validateIoTuneParams(self, params):
Line 4360: errLogMsg = 'Tuning block device I/O param is invalid'
Line 4361:
Line 4363: if invalidParamNames:
Line 4364: self.log.error(errLogMsg)
Line 4365: return translateErrCode('tuneBlkDevIoErr',
Line 4366: 'Parameter %s name(s) are
invalid' %
Line 4367: invalidParamNames)
just call self._checkIoTuneInvalidParams(params) here
Line 4368:
Line 4369: conflictCategory = self._checkIoTuneCategories(params)
Line 4370: if conflictCategory:
Line 4371: self.log.error(errLogMsg)
Line 4372: return translateErrCode('tuneBlkDevIoErr',
Line 4373: 'A non-zero total and non-zero
read'
Line 4374: '/write value for %s_sec can not
be'
Line 4375:
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 17: (7 comments) Ayal, Thanks a lot for the insightful review! Some problems doesn't exist in the patch 16, which should be the latest change, and the other problems will be fixed in next change. File vdsm/netconf/iproute2.py Line 84: @contextmanager Line 85: def _removeIface(self, iface): Line 86: _netinfo = netinfo.NetInfo() Line 87: toBeRemoved = not _netinfo.ifaceUsers(iface.name) Line 88: destroyed = yield not toBeRemoved this problem was already fixed in patch 16. Actually, patch 16 is the latest change of this patch. It's overrode by an old change unintentionally when Assaf updated his patch which is based on this one. Line 89: Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 90: if toBeRemoved: Line 91: if iface.master is None: Line 92: self.configApplier.removeIpConfig(iface) Line 93: Line 94: if not destroyed: Done Line 95: self.configApplier.setIfaceMtu(iface.name, Line 96:netinfo.DEFAULT_MTU) Line 97: self.configApplier.ifdown(iface) Line 98: else: Line 105: self.configApplier.removeBond(bonding) Line 106: for slave in bonding.slaves: Line 107: self.configApplier.removeBondSlave(bonding, slave) Line 108: slave.remove() Line 109: return True Done Line 110: Line 111: def removeNic(self, nic): Line 112: with self._removeIface(nic): Line 113: return False Line 140: ipConfig.netmask) Line 141: if ipConfig.gateway and ipConfig.defaultRoute: Line 142: ipwrapper.routeAdd(['default', 'via', ipConfig.gateway]) Line 143: except ipwrapper.IPRoute2Error as e: Line 144: if 'File exists' in e.message[0]: Done Line 145: pass Line 146: else: Line 147: raise Line 148: Line 149: def removeIpConfig(self, iface): Line 150: ipwrapper.addrFlush(iface.name) Line 151: Line 152: def setIfaceMtu(self, iface, mtu): Line 153: if mtu: in former implementation, setIfaceMtu use an instance of NetDevice as argument, and it just use iface.mtu as the mtu value if not extra mtu passed it. But we just use a plain str to represent the interface, so it doesn't make sense to call it without mtu. I am going to remove the statement 'if mtu:', the validation is already done in the netmodels layer. Line 154: ipwrapper.linkSet(iface, ['mtu', str(mtu)]) Line 155: Line 156: def setBondingMtu(self, iface, mtu): Line 157: self.setIfaceMtu(iface, mtu) Line 168: self.setIfaceMtu(iface.name, iface.mtu) Line 169: self.ifup(iface) Line 170: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Done Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) Line 176: Line 171: def addBridge(self, bridge): Line 172: execCmd(['brctl', 'addbr', bridge.name]) Line 173: Line 174: def addBridgePort(self, bridge): Line 175: execCmd(['brctl', 'addif', bridge.name, bridge.port.name]) Done Line 176: Line 177: def removeBridge(self, bridge): Line 178: execCmd([constants.EXT_BRCTL, 'delbr', bridge.name]) Line 179: -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Ayal Baron Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Saggi Mizrahi Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Remove vdsm directories on uninstallation
Mark Wu has posted comments on this change. Change subject: Remove vdsm directories on uninstallation .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/16764 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I119c691cfdf6a487f36aef6ab451073af0230143 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Douglas Schilling Landgraf Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Noam Slomianko Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Added required ipwrapper functions for iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Added required ipwrapper functions for iproute2 configurator .. Patch Set 1: (2 comments) File lib/vdsm/ipwrapper.py Line 265: command = [_IP_BINARY.cmd, 'addr', 'flush', 'dev', dev] Line 266: _execCmd(command) Line 267: Line 268: Line 269: def linkAdd(link=None, name=None, type=None, linkArgs=None): the word 'name' is optional in command line as what described in the manual of 'ip-link' Line 270: command = [_IP_BINARY.cmd, 'link', 'add'] Line 271: if link: Line 272: command += ['link', link] Line 273: if name: Line 265: command = [_IP_BINARY.cmd, 'addr', 'flush', 'dev', dev] Line 266: _execCmd(command) Line 267: Line 268: Line 269: def linkAdd(link=None, name=None, type=None, linkArgs=None): I would like suggest: linkAdd(dev, type, link=None, linkArgs=None): I don't think we need match the order of ip command. In our API, we just the use first argument to represent the entity we want to operate, and others the arguments for the operation. That makes sense to me. Line 270: command = [_IP_BINARY.cmd, 'link', 'add'] Line 271: if link: Line 272: command += ['link', link] Line 273: if name: -- To view, visit http://gerrit.ovirt.org/18585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9283ae90f84fa930e401b3ecd2e18935ba9ca2f5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Added required ipwrapper functions for iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Added required ipwrapper functions for iproute2 configurator .. Patch Set 1: Code-Review-1 (1 comment) File lib/vdsm/ipwrapper.py Line 265: command = [_IP_BINARY.cmd, 'addr', 'flush', 'dev', dev] Line 266: _execCmd(command) Line 267: Line 268: Line 269: def linkAdd(link=None, name=None, type=None, linkArgs=None): name and type is not optional, right? Line 270: command = [_IP_BINARY.cmd, 'link', 'add'] Line 271: if link: Line 272: command += ['link', link] Line 273: if name: -- To view, visit http://gerrit.ovirt.org/18585 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9283ae90f84fa930e401b3ecd2e18935ba9ca2f5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Unified network persistence [1/3] - Save running config
Mark Wu has posted comments on this change.
Change subject: Unified network persistence [1/3] - Save running config
..
Patch Set 18: Code-Review-1
(5 comments)
Besides the comments inline, I would like to suggest implement the saving of
running config in configurator's commit() function. For setupNetwork, we call
commit() in the end of the call if all networks are setup successfully. for
addNetwork and delNetwork, we only call commit() when it completes
successfully and it's a direct API call, not called from setupNetworks. We can
use the argument configurator to tell if it's called from setupNetworks.
With this proposed change, it's easier to implement the rollback from running
config.
File vdsm/configNetwork.py
Line 164: attrs.update(dict(zip(spec.args, args)))
Line 165: try:
Line 166: ret = func(*args, **kwargs)
Line 167: except:
Line 168: raise
it looks the try .. except statement is useless here. The exception will be
handled as before, so we don't need consider it in the wrapper function.
Line 169: if not saveRunningConf:
Line 170: return ret
Line 171:
Line 172: runningConfig = RunningConfig()
Line 175: bondAttrs = {'nics': attrs.pop('nics')}
Line 176: bondingOptions = attrs.pop('bondingOptions', None)
Line 177: if bondingOptions:
Line 178: bondAttrs['bondingOptions'] = bondingOptions
Line 179: runningConfig.setBonding(bonding, bondAttrs)
I would like to suggest hide setBonding behind running.setNetwork. We could
let running.setNetwork accept the all arguments including those for bonding,
and make setNetwork implement the logic above.
Line 180: else:
Line 181: nics = attrs.pop('nics', None)
Line 182: if nics:
Line 183: attrs['nic'] = nics[0]
Line 183: attrs['nic'] = nics[0]
Line 184:
Line 185: netName = attrs.pop('network')
Line 186: options = attrs.pop('options', None)
Line 187:
how is 'options' included in attrs? And the keyword arguments included in
options are already included in kwargs. So I don't understand the code here.
Line 188: # Clean netAttrs from fields that should not be serialized
Line 189: netAttrs = dict((key, value) for key, value in
attrs.iteritems() if
Line 190: value is not None and
Line 191: key not in ('configurator', '_netinfo',
'force',
Line 188: # Clean netAttrs from fields that should not be serialized
Line 189: netAttrs = dict((key, value) for key, value in
attrs.iteritems() if
Line 190: value is not None and
Line 191: key not in ('configurator', '_netinfo',
'force',
Line 192: 'implicitBonding'))
probably this code should go to unifiedpersistence.py too.
Line 193:
Line 194: if options:
Line 195: netAttrs.update((key, value) for key, value in
options.iteritems()
Line 196: if value is not None)
Line 389: 'still exists' % network)
Line 390: elif config.get('vars', 'persistence') == 'unified':
Line 391: runningConfig = RunningConfig()
Line 392: runningConfig.removeNetwork(network)
Line 393: if bonding:
it could happen that the bonding still exists after the 'delNetwork' call
because it's also used by other network. So we need use
netinfo.NetInfo().ifaceUsers(bonding) to check that.
Line 394: runningConfig.removeBonding(bonding)
Line 395:
Line 396:
Line 397: def clientSeen(timeout):
--
To view, visit http://gerrit.ovirt.org/16699
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7137a96f84abd2c5e532c6c37737e36ef17567a9
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Unified network persistence [3/3] - Restore network configur...
Mark Wu has posted comments on this change.
Change subject: Unified network persistence [3/3] - Restore network
configuration
..
Patch Set 3: Code-Review-1
(4 comments)
File vdsm/vdsm-restore-net-config
Line 42: """
Line 43: nets = {}
Line 44: bonds = {}
Line 45: for netFile in os.listdir(netinfo.NET_CONF_PERS_DIR):
Line 46: nets[os.path.basename(netFile)] = json.load(open(
nets[netFile] is enough, because os.listdir return basename.
Line 47: netinfo.NET_CONF_PERS_DIR + netFile))
Line 48:
Line 49: for bondFile in os.listdir(netinfo.BOND_CONF_PERS_DIR):
Line 50: bonds[os.path.basename(bondFile)] = json.load(open(
Line 47: netinfo.NET_CONF_PERS_DIR + netFile))
Line 48:
Line 49: for bondFile in os.listdir(netinfo.BOND_CONF_PERS_DIR):
Line 50: bonds[os.path.basename(bondFile)] = json.load(open(
Line 51: netinfo.BOND_CONF_PERS_DIR + bondFile))
you could use os.path.join, it can handle the delimiting character '/'
Line 52:
Line 53: logging.debug("Calling setupNetworks with networks: %s, bondings:
%s" %
Line 54: (nets, bonds))
Line 55: setupNetworks(nets, bonds)
Line 51: netinfo.BOND_CONF_PERS_DIR + bondFile))
Line 52:
Line 53: logging.debug("Calling setupNetworks with networks: %s, bondings:
%s" %
Line 54: (nets, bonds))
Line 55: setupNetworks(nets, bonds)
proabably we need the option connectivityCheck=False here. because vdsm doesn't
start at this time and we can't check the network connectivity on the vdsm
host. Without this options, the setupNetworks will fail because no response
from client.
Line 56:
Line 57:
Line 58: def main():
Line 59: try:
Line 53: logging.debug("Calling setupNetworks with networks: %s, bondings:
%s" %
Line 54: (nets, bonds))
Line 55: setupNetworks(nets, bonds)
Line 56:
Line 57:
As we discussed, for the case of rolling back fro persist when vdsm is
running, we still need delete the running networks which are not in persist
configuration.
Line 58: def main():
Line 59: try:
Line 60: logging.config.fileConfig("/etc/vdsm/svdsm.logger.conf")
Line 61: except:
--
To view, visit http://gerrit.ovirt.org/17010
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73462b160ecfbaa7efe71eed905a3bbd69ee6c23
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Allow configurators to be used as context managers
Mark Wu has posted comments on this change. Change subject: Allow configurators to be used as context managers .. Patch Set 3: Code-Review+1 (1 comment) File vdsm/netconf/ifcfg.py Line 50: self.configApplier = ConfigWriter() Line 51: Line 52: def rollback(self): Line 53: self.configApplier.restoreBackups() Line 54: self.configApplier = None are you going to make long living configurator across api verb? otherwise, we could a simple 'pass' is enough for ifcfg's begin and commit. Line 55: Line 56: def commit(self): Line 57: self.configApplier = None Line 58: -- To view, visit http://gerrit.ovirt.org/18412 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c07aa23d637ad3ae8a8b5ce455e67b163338ca8 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix the 'MTU' option in networkTests
Mark Wu has posted comments on this change. Change subject: Fix the 'MTU' option in networkTests .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/18409 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic09a5dd6e8f760eda5dece19bc7e0da2e49611e0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 16: it can pass all network functional tests. -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix the 'MTU' option in networkTests
Mark Wu has uploaded a new change for review.
Change subject: Fix the 'MTU' option in networkTests
..
Fix the 'MTU' option in networkTests
The 'mtu' option should be in lower case. It doesn't fail with ifcfg
configurator, because the 'MTU=1234' passes through to the ifcfg file
as an extra option.
Change-Id: Ic09a5dd6e8f760eda5dece19bc7e0da2e49611e0
Signed-off-by: Mark Wu
---
M tests/functional/networkTests.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/18409/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index ef76ec4..ddd7cbc 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -353,7 +353,7 @@
status, msg = self.vdsm_net.addNetwork(NETWORK_NAME, vlan=VLAN_ID,
bond=BONDING_NAME,
nics=nics,
- opts={'MTU': MTU,
+ opts={'mtu': MTU,
'bridged': bridged})
vlan_name = '%s.%s' % (BONDING_NAME, VLAN_ID)
--
To view, visit http://gerrit.ovirt.org/18409
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic09a5dd6e8f760eda5dece19bc7e0da2e49611e0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Unified network persistence [3/3] - Restore network configur...
Mark Wu has posted comments on this change.
Change subject: Unified network persistence [3/3] - Restore network
configuration
..
Patch Set 2:
(1 comment)
File vdsm/vdsm-restore-net-config
Line 44: nets[os.path.basename(netFile)] = json.load(open(netFile))
Line 45:
Line 46: for bondFile in os.listdir(netinfo.BOND_CONF_PERS_DIR):
Line 47: bonds[os.path.basename(bondFile)] = json.load(open(bondFile))
Line 48: setupNetworks(nets, bonds)
we also need delete the existing network before restore networks with
setupNetworks, otherwise the new added network will not be deleted.
Line 49:
Line 50:
Line 51: def main():
Line 52: if config.get('vars', 'persistence') == 'unified':
--
To view, visit http://gerrit.ovirt.org/17010
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73462b160ecfbaa7efe71eed905a3bbd69ee6c23
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Don't down bonds and nics when adding vlan or resizing
Mark Wu has posted comments on this change. Change subject: Don't down bonds and nics when adding vlan or resizing .. Patch Set 13: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/17491 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id3075a2e65f06416c840c6a98689b77555b22e5d Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Don't down bonds and nics when adding vlan or resizing
Mark Wu has posted comments on this change.
Change subject: Don't down bonds and nics when adding vlan or resizing
..
Patch Set 12: Code-Review+1
(1 comment)
File tests/functional/networkTests.py
Line 660: createBondedNetAndCheck(3,
Line 661: {'nics': nics[1:],
Line 662: 'options': 'mode=3
miimon=250'})
Line 663:
Line 664: # Add a network chaning bond options
s/chaning/changing/
Line 665: with self.assertRaises(OperStateChangedError):
Line 666: with nonChangingOperstate(BONDING_NAME):
Line 667: createBondedNetAndCheck(4,
Line 668: {'nics': nics[1:],
--
To view, visit http://gerrit.ovirt.org/17491
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3075a2e65f06416c840c6a98689b77555b22e5d
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: Add tuneBlockDevIo interface
Mark Wu has posted comments on this change.
Change subject: vdsm: Add tuneBlockDevIo interface
..
Patch Set 10: Code-Review-1
(6 comments)
File client/vdsClient.py
Line 1745: return status['status']['code'], status['status']['message']
Line 1746:
Line 1747: def tuneBlockDevIo(self, args):
Line 1748: params = {}
Line 1749: if len(args) > 2:
if you check the number of args, it should fail early. Otherwise, you could
just use the args without check.
Line 1750: params = self._eqSplit(args[2:])
Line 1751: params['vmId'] = args[0]
Line 1752: params['dev'] = args[1]
Line 1753: return self.ExecAndExit(self.s.tuneBlockDevIo(params))
File vdsm/vm.py
Line 4289: if msg is None:
Line 4290: error = errCode[key]
Line 4291: else:
Line 4292: error = {'status': {'code': errCode[key]
Line 4293: ['status']['code'], 'message': msg}}
i think the code of build customized error status belongs to define.py itself.
It should be resolved in a separate patch.
Line 4294: return error
Line 4295:
Line 4296: def _getBalloonInfo(self):
Line 4297: for dev in self.conf['devices']:
Line 4337: for device in self.conf['devices']:
Line 4338: if device.get('device') == 'disk' and device.get('name'):
Line 4339: devName = device['name']
Line 4340: result = (device.get('specParams', {}).get('ioTune',
{}))
Line 4341: results[devName] = result
I think we could only fill the tune info for the device tuned before. We should
not extend the length of vm's running stats for useless info. Does it make
sense?
Line 4342: return results
Line 4343:
Line 4344: def _checkIoTuneInvalidParams(self, params):
Line 4345: validKeys = set(('total_bytes_sec', 'read_bytes_sec',
Line 4344: def _checkIoTuneInvalidParams(self, params):
Line 4345: validKeys = set(('total_bytes_sec', 'read_bytes_sec',
Line 4346: 'write_bytes_sec', 'total_iops_sec',
Line 4347: 'write_iops_sec', 'read_iops_sec'))
Line 4348: params_set = set(params)
paramsSet, we should use consistent variable naming style.
Line 4349:
Line 4350: invalidParams = params_set -
validKeys.intersection(params_set)
Line 4351: invalidParamNames = ','.join(invalidParams)
Line 4352: return invalidParamNames
Line 4346: 'write_bytes_sec', 'total_iops_sec',
Line 4347: 'write_iops_sec', 'read_iops_sec'))
Line 4348: params_set = set(params)
Line 4349:
Line 4350: invalidParams = params_set -
validKeys.intersection(params_set)
you could just use params_set - validKeys
Line 4351: invalidParamNames = ','.join(invalidParams)
Line 4352: return invalidParamNames
Line 4353:
Line 4354: def _checkIoTuneCategories(self, params):
Line 4347: 'write_iops_sec', 'read_iops_sec'))
Line 4348: params_set = set(params)
Line 4349:
Line 4350: invalidParams = params_set -
validKeys.intersection(params_set)
Line 4351: invalidParamNames = ','.join(invalidParams)
', ' is better.
Line 4352: return invalidParamNames
Line 4353:
Line 4354: def _checkIoTuneCategories(self, params):
Line 4355: categories = ("bytes", "iops")
--
To view, visit http://gerrit.ovirt.org/14394
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb33510a081d221af0f69d4dd2d55adf0b79efd2
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mei Liu
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Doron Fediuck
Gerrit-Reviewer: Eli Mesika
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Mei Liu
Gerrit-Reviewer: oVirt Jenkins CI Server
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: setupNetworks add net bond breaking and resizing.
Mark Wu has posted comments on this change. Change subject: tests: setupNetworks add net bond breaking and resizing. .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/17634 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e031599ac4457cc24cb9d142110e1781325912f Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: enable multiple gateways for iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: netconf: enable multiple gateways for iproute2 configurator
..
Patch Set 4:
(1 comment)
File vdsm/sourceRoute.py
Line 150: # its source
Line 151: rules += [rule for rule in allRules if rule.source == network]
Line 152:
Line 153: return rules
Line 154:
Yes, it makes sense. I will fix it in next patch. Thanks!
Line 155: def remove(self):
Line 156: logging.info("Removing gateway - device: %s" % self.device)
Line 157:
Line 158: rules = self._getRules(self.device)
--
To view, visit http://gerrit.ovirt.org/16276
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I76e1225caffdb2de3073041e541c7c978eefb396
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: enable multiple gateways for iproute2 configurator
Mark Wu has posted comments on this change. Change subject: netconf: enable multiple gateways for iproute2 configurator .. Patch Set 4: Code-Review-1 (1 comment) it misses the interface tracking when removing interface. Then the the dynamic source route can't be removed. File vdsm/netconf/iproute2.py Line 87: Line 88: def _ifaceDownAndCleanup(self, iface, _netinfo): Line 89: """Returns True iff the iface is to be removed.""" Line 90: self.configApplier.ifdown(iface) Line 91: self._removeSourceRoute(iface) I think it has the same flow as dynamic source route with ifcfg. Line 92: if iface.master is None: Line 93: self.configApplier.removeIpConfig(iface) Line 94: return not _netinfo.ifaceUsers(iface.name) Line 95: -- To view, visit http://gerrit.ovirt.org/16276 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76e1225caffdb2de3073041e541c7c978eefb396 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change. Change subject: Add iproute2 configurator .. Patch Set 8: (7 comments) Thanks a lot for the review! File lib/vdsm/ipwrapper.py Line 249: command += rule Line 250: _execCmd(command) Line 251: Line 252: Line 253: def addrAdd(addr): Done Line 254: command = [_IP_BINARY.cmd, 'addr', 'add'] + addr Line 255: _execCmd(command) Line 256: Line 257: Line 254: command = [_IP_BINARY.cmd, 'addr', 'add'] + addr Line 255: _execCmd(command) Line 256: Line 257: Line 258: def addrFlush(addr): Done Line 259: command = [_IP_BINARY.cmd, 'addr', 'flush'] + addr Line 260: _execCmd(command) Line 261: Line 262: Line 259: command = [_IP_BINARY.cmd, 'addr', 'flush'] + addr Line 260: _execCmd(command) Line 261: Line 262: Line 263: def linkAdd(link): Done Line 264: command = [_IP_BINARY.cmd, 'link', 'add'] + link Line 265: _execCmd(command) Line 266: Line 267: Line 264: command = [_IP_BINARY.cmd, 'link', 'add'] + link Line 265: _execCmd(command) Line 266: Line 267: Line 268: def linkSet(link): Done Line 269: command = [_IP_BINARY.cmd, 'link', 'set'] + link Line 270: _execCmd(command) Line 271: Line 272: Line 269: command = [_IP_BINARY.cmd, 'link', 'set'] + link Line 270: _execCmd(command) Line 271: Line 272: Line 273: def linkDel(link): Done Line 274: command = [_IP_BINARY.cmd, 'link', 'del'] + link File vdsm/netconf/iproute2.py Line 56: slave.configure(**opts) Line 57: Line 58: self.configApplier.setIfaceConfigAndUp(bond) Line 59: Line 60: def editBonding(self, bond, _netinfo): thanks! I also noticed this problem and planned to fix it after your patch is merged. Line 61: self.configApplier.ifdown(bond) Line 62: for nic in _netinfo.getNicsForBonding(bond.name): Line 63: slave = Nic(nic, self, _netinfo=_netinfo) Line 64: self.configApplier.removeBondSlave(bond, slave) Line 181: try: Line 182: ipwrapper.linkAdd(['link', vlan.device.name, 'name', vlan.name, Line 183:'type', 'vlan', 'id', vlan.tag]) Line 184: except ipwrapper.IPRoute2Error as e: Line 185: if 'File exists' in e.message[0]: if for some reason, the vlan device is still left on system after the network over it is remove. Then we can't add network over it again. That could make more robust. I agree with that it only helps for the case caused by the defect of our code. But I think it's harmless also because it doesn't swallow the exception of the real issue. Line 186: pass Line 187: else: Line 188: raise Line 189: -- To view, visit http://gerrit.ovirt.org/15301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add dhcp support for iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: netconf: Add dhcp support for iproute2 configurator
..
Patch Set 5:
(3 comments)
Thanks for the good comments!
File lib/vdsm/dhclient.py
Line 1: # Copyright 2013 Red Hat, Inc.
Correct!
Line 2: #
Line 3: # This program is free software; you can redistribute it and/or modify
Line 4: # it under the terms of the GNU General Public License as published by
Line 5: # the Free Software Foundation; either version 2 of the License, or
Line 25: from vdsm.utils import execCmd
Line 26: from vdsm.utils import rmFile
Line 27:
Line 28:
Line 29: class DhcpClient():
Done
Line 30: PID_FILE = '/var/run/dhclient-%s.pid'
Line 31: LEASE_DIR = '/var/lib/dhclient/'
Line 32: LEASE_FILE = LEASE_DIR + 'dhclient-%s.lease'
Line 33: DHCLIENT = CommandPath('dhclient', '/sbin/dhclient')
Line 35: def __init__(self, iface):
Line 36: self.iface = iface
Line 37: self.pidFile = self.PID_FILE % self.iface
Line 38: if not os.path.exists(self.LEASE_DIR):
Line 39: execCmd(['mkdir', self.LEASE_DIR])
Done
Line 40: self.leaseFile = (self.LEASE_FILE % self.iface)
Line 41:
Line 42: def _dhclient(self):
Line 43: rc, out, err = execCmd([self.DHCLIENT.cmd, '-1', '-pf',
--
To view, visit http://gerrit.ovirt.org/15492
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea88e8693e47fa51edb89c33344332c88c5c964d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: Add iproute2 configurator
..
Patch Set 7:
(2 comments)
File lib/vdsm/netinfo.py
Line 46: PROC_NET_VLAN = '/proc/net/vlan/'
Line 47: NET_PATH = '/sys/class/net'
Line 48: BONDING_MASTERS = '/sys/class/net/bonding_masters'
Line 49: BONDING_SLAVES = '/sys/class/net/%s/bonding/slaves'
Line 50: BONDING_OPTION = '/sys/class/net/{bond}/bonding/{option}'
need rebase on http://gerrit.ovirt.org/#/c/17491/, which defines BONDING_OPTIONS
Line 51:
Line 52: LIBVIRT_NET_PREFIX = 'vdsm-'
Line 53: DUMMY_BRIDGE = ';vdsmdummy;'
Line 54: DEFAULT_MTU = '1500'
File vdsm/netconf/iproute2.py
Line 58:
Line 59: self.configApplier.setIfaceConfigAndUp(bond)
Line 60:
Line 61: def editBonding(self, bond, _netinfo):
Line 62: self.configApplier.ifdown(bond)
The 'ifdown/ifup' issue will be fixed in a following up patch based on:
http://gerrit.ovirt.org/#/c/17491/ Don't down bonds and nics when adding vlan
or resizing
Line 63: for nic in _netinfo.getNicsForBonding(bond.name):
Line 64: slave = Nic(nic, self, _netinfo=_netinfo)
Line 65: self.configApplier.removeBondSlave(bond, slave)
Line 66: slave.remove()
--
To view, visit http://gerrit.ovirt.org/15301
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Add iproute2 configurator
Mark Wu has posted comments on this change.
Change subject: Add iproute2 configurator
..
Patch Set 6:
(2 comments)
File lib/vdsm/netinfo.py
Line 46: PROC_NET_VLAN = '/proc/net/vlan/'
Line 47: NET_PATH = '/sys/class/net'
Line 48: BONDING_MASTERS = '/sys/class/net/bonding_masters'
Line 49: BONDING_SLAVES = '/sys/class/net/%s/bonding/slaves'
Line 50: BONDING_OPTION = '/sys/class/net/{bond}/bonding/{option}'
need rebase on http://gerrit.ovirt.org/#/c/17491/, which defines BONDING_OPTIONS
Line 51:
Line 52: LIBVIRT_NET_PREFIX = 'vdsm-'
Line 53: DUMMY_BRIDGE = ';vdsmdummy;'
Line 54: DEFAULT_MTU = '1500'
File vdsm/netconf/iproute2.py
Line 57: slave.configure(**opts)
Line 58:
Line 59: self.configApplier.setIfaceConfigAndUp(bond)
Line 60:
Line 61: def editBonding(self, bond, _netinfo):
The 'ifdown/ifup' issue will be fixed in a following up patch based on:
http://gerrit.ovirt.org/#/c/17491/
Don't down bonds and nics when adding vlan or resizing
Line 62: self.configApplier.ifdown(bond)
Line 63: for nic in _netinfo.getNicsForBonding(bond.name):
Line 64: slave = Nic(nic, self, _netinfo=_netinfo)
Line 65: self.configApplier.removeBondSlave(bond, slave)
--
To view, visit http://gerrit.ovirt.org/15301
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I762cfa30f78c5a46507b86f53e98bcf79dfc5844
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Add config option for network configurator
Mark Wu has uploaded a new change for review.
Change subject: netconf: Add config option for network configurator
..
netconf: Add config option for network configurator
Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1
Signed-off-by: Mark Wu
---
M lib/vdsm/config.py.in
M vdsm/configNetwork.py
2 files changed, 28 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/10/18210/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index bdbd91a..446c20f 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -42,6 +42,9 @@
'Comma-separated list of fnmatch-patterns for dummy hosts nics to '
'be shown to vdsm.'),
+('configurator', 'ifcfg',
+'Whether to use "ifcfg" or "iproute2" to configure networks.'),
+
('nic_model', 'rtl8139,pv',
'NIC model is rtl8139, ne2k_pci pv or any other valid device '
'recognized by kvm/qemu if a coma separated list given then a '
diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 155d22a..b87d8d7 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -23,6 +23,7 @@
import time
import logging
+from vdsm.config import config
from vdsm import constants
from vdsm import utils
from storage.misc import execCmd
@@ -31,7 +32,6 @@
from vdsm import define
from vdsm import netinfo
from netconf.ifcfg import ConfigWriter
-from netconf.ifcfg import Ifcfg
from netmodels import Bond
from netmodels import Bridge
from netmodels import IPv4
@@ -40,6 +40,25 @@
from netmodels import Vlan
CONNECTIVITY_TIMEOUT_DEFAULT = 4
+
+
+def _getConfigurator():
+
+@utils.memoized
+def getConfiguratorClass():
+configurator = config.get('vars', 'configurator')
+if configurator == 'iproute2':
+from netconf.iproute2 import Iproute2
+return Iproute2
+else:
+if configurator != 'ifcfg':
+logging.warn('Invalid config for network configruator: %s. '
+ 'Use ifcfg instead.', configurator)
+from netconf.ifcfg import Ifcfg
+return Ifcfg
+
+configuratorClass = getConfiguratorClass()
+return configuratorClass()
def objectivizeNetwork(bridge=None, vlan=None, bonding=None,
@@ -72,7 +91,7 @@
:returns: the top object of the hierarchy.
"""
if configurator is None:
-configurator = Ifcfg()
+configurator = _getConfigurator()
if _netinfo is None:
_netinfo = netinfo.NetInfo()
if bondingOptions and not bonding:
@@ -191,7 +210,7 @@
mtu, bridged, options)
if configurator is None:
-configurator = Ifcfg()
+configurator = _getConfigurator()
bootproto = options.pop('bootproto', None)
@@ -297,7 +316,7 @@
_netinfo = netinfo.NetInfo()
if configurator is None:
-configurator = Ifcfg()
+configurator = _getConfigurator()
if network not in _netinfo.networks:
logging.info("Network %r: doesn't exist in libvirt database", network)
@@ -347,7 +366,7 @@
def editNetwork(oldBridge, newBridge, vlan=None, bonding=None, nics=None,
**options):
-configurator = Ifcfg()
+configurator = _getConfigurator()
try:
delNetwork(oldBridge, configurator=configurator, **options)
addNetwork(newBridge, vlan=vlan, bonding=bonding, nics=nics,
@@ -480,7 +499,7 @@
"""
logger = logging.getLogger("setupNetworks")
_netinfo = netinfo.NetInfo()
-configurator = Ifcfg()
+configurator = _getConfigurator()
networksAdded = set()
logger.debug("Setting up network according to configuration: "
--
To view, visit http://gerrit.ovirt.org/18210
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I426a38f229131ce6d32568387be7a809b09ae0c1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: vdsm: Add tuneBlockDevIo interface
Mark Wu has posted comments on this change.
Change subject: vdsm: Add tuneBlockDevIo interface
..
Patch Set 9: Code-Review-1
(7 comments)
File vdsm/vm.py
Line 4341: if device.get('device') == 'disk' and device.get('name'):
Line 4342: devName = device['name']
Line 4343: result = default.copy()
Line 4344: result.update(device.get('specParams',
{}).get('ioTune', {}))
Line 4345: results[devName] = result
I think we don't need return an empty info if it's not set before.
Line 4346: return results
Line 4347:
Line 4348: def _validateIoTuneParams(self, params, bytesSetParams,
iopsSetParams):
Line 4349: errReport = partial(self._reportError, key='tuneBlkDevIoErr',
Line 4356: invalidParamNames = ''
Line 4357: while True:
Line 4358: invalidParamNames += str(invalidParams.pop()) +
','
Line 4359: except KeyError:
Line 4360: invalidParamNames = invalidParamNames[:-1]
using
invalidParamNames = ','.join(invalidParams)
to replace lines 4355~4360
Line 4361: return errReport(msg='parameter %s name(s) are
invalid' %
Line 4362: invalidParamNames)
Line 4363:
Line 4364: categories = ("bytes", "iops")
Line 4393:iopsSetParams)
Line 4394: if errorInfo:
Line 4395: return errorInfo
Line 4396:
Line 4397: set_bytes = True if len(bytesSetParams) > 0 else False
set_bytes = len(bytesSetParams) > 0
Line 4398: set_iops = True if len(iopsSetParams) > 0 else False
Line 4399:
Line 4400: try:
Line 4401: #Set I/O tuning parameters.
Line 4394: if errorInfo:
Line 4395: return errorInfo
Line 4396:
Line 4397: set_bytes = True if len(bytesSetParams) > 0 else False
Line 4398: set_iops = True if len(iopsSetParams) > 0 else False
same problem as above
Line 4399:
Line 4400: try:
Line 4401: #Set I/O tuning parameters.
Line 4402: #Bytes and iops values are independent categories.
Line 4402: #Bytes and iops values are independent categories.
Line 4403: #Setting one value leads to reseting the other two in
the same
Line 4404: #category to unlimited.
Line 4405: #A non-zero total value cannot be used with non-zero
read or
Line 4406: #write value.
using triple quota for multiple line comments is better.
Line 4407: self._dom.setBlockIoTune(dev, params,
Line 4408:
libvirt.VIR_DOMAIN_AFFECT_CURRENT)
Line 4409: except Exception, e:
Line 4410: return errReport(msg=e.message)
Line 4405: #A non-zero total value cannot be used with non-zero
read or
Line 4406: #write value.
Line 4407: self._dom.setBlockIoTune(dev, params,
Line 4408:
libvirt.VIR_DOMAIN_AFFECT_CURRENT)
Line 4409: except Exception, e:
except Exception as e:
Line 4410: return errReport(msg=e.message)
Line 4411: else:
Line 4412: for device in self.conf['devices']:
Line 4413: if device.get('name') == dev or device.get('path')
== dev:
File vdsm_api/vdsmapi-schema.json
Line 5322: # @read_iops_sec: Read I/O operations limit per second.
Line 5323: #
Line 5324: # @write_iops_sec: Write I/O operations limit per second.
Line 5325: #
Line 5326: # Since: 4.10.0
I believe we don't have this feature in 4.10.0
Line 5327: ##
Line 5328: {'type': 'VmBlkDevIoTuneStats',
Line 5329: 'data': {'total_bytes_sec': 'uint', 'read_bytes_sec': 'uint',
Line 5330: 'write_bytes_sec': 'uint', 'total_iops_sec': 'uint',
--
To view, visit http://gerrit.ovirt.org/14394
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb33510a081d221af0f69d4dd2d55adf0b79efd2
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mei Liu
Gerrit-Reviewer: Adam Litke
Gerrit-Reviewer: Doron Fediuck
Gerrit-Reviewer: Eli Mesika
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Mei Liu
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Introducing hidden_vlans configurable.
Mark Wu has posted comments on this change. Change subject: Introducing hidden_vlans configurable. .. Patch Set 4: how about using a unified config, like hidden_interfaces? Then we can add a unified filter to all devices. -- To view, visit http://gerrit.ovirt.org/18082 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4340936fb998c5ab9ee46e16a16ae15461176f2 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amador Pahim Gerrit-Reviewer: Amador Pahim Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Don't down bonds and nics when adding vlan or resizing
Mark Wu has posted comments on this change.
Change subject: Don't down bonds and nics when adding vlan or resizing
..
Patch Set 10:
(5 comments)
File tests/functional/networkTests.py
Line 52: def changed(dev, changes):
Line 53: status = operstate(dev)
Line 54: while not done:
Line 55: newState = operstate(dev)
Line 56: if status != newState:
Agree.
Line 57: changes.append(newState)
Line 58: status = newState
Line 59:
Line 60: try:
Line 64: args=(device, changes))
Line 65: monitoring_t.start()
Line 66: yield
Line 67: finally:
Line 68: time.sleep(3) # So that the last action in yield gets to
kernel
ok. got it. thanks
Line 69: done = True
Line 70: if changes:
Line 71: raise ValueError('%s operstate changed: %r' % (device,
changes))
Line 72:
Line 65: monitoring_t.start()
Line 66: yield
Line 67: finally:
Line 68: time.sleep(3) # So that the last action in yield gets to
kernel
Line 69: done = True
yes. my concern is just that the scheduler can't guarantee the monitor thread
start to run before the text code exit. but it's almost the same problem as I
raised in line 56. So it works well in practice, I have no problem with it.
Line 70: if changes:
Line 71: raise ValueError('%s operstate changed: %r' % (device,
changes))
Line 72:
Line 73:
File vdsm/netconf/ifcfg.py
Line 116: ifdown(slave.name) # nics must be down to join a bond
Line 117: self.configApplier.addNic(slave)
Line 118: ifup(slave.name)
Line 119:
Line 120: if not isIfcfgControlled or not areOptionsApplied:
what kind of changes do you want to make to bond by 'ifdown/ifup'?
Line 121: ifdown(bond.name)
Line 122: ifup(bond.name)
Line 123:
Line 124: def configureNic(self, nic, **opts):
File vdsm/netmodels.py
Line 193:
Line 194: def areOptionsApplied(self):
Line 195: activeOpts = netinfo.bondOpts(self.name)
Line 196: confOpts = (option.split('=', 1) for option in
self.options.split(' '))
Line 197: return all(value in activeOpts[name] for name, value in
confOpts)
I prefer:
BONDING_OPT = '/sys/class/net/%s/bonding/%s'
if keys:
paths = [BONDING_OPT % (bonding, key) for key in keys]
else:
paths = iglob(BONDING_OPTS % (bonding, *))
for path in paths:
with open(path) as optFile:
opts[os.path.basename(path)] = \
optFile.read().rstrip().split(' ')
...
Line 198:
Line 199: def remove(self):
Line 200: logging.debug('Removing bond %r', self)
Line 201: self.configurator.removeBond(self)
--
To view, visit http://gerrit.ovirt.org/17491
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3075a2e65f06416c840c6a98689b77555b22e5d
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netmodels: Improve getIpConfig using namedtuple
Mark Wu has posted comments on this change. Change subject: netmodels: Improve getIpConfig using namedtuple .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/18167 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53eb3c4b00f33285a1b299ca0adc6611eb99a989 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Don't down bonds and nics when adding vlan or resizing
Mark Wu has posted comments on this change.
Change subject: Don't down bonds and nics when adding vlan or resizing
..
Patch Set 10: Code-Review-1
(7 comments)
File tests/functional/networkTests.py
Line 51: def nonChangingOperstate(device):
Line 52: def changed(dev, changes):
Line 53: status = operstate(dev)
Line 54: while not done:
Line 55: newState = operstate(dev)
if the device is removed, then the operstate will raise an exception, and
therefore the monitor thread will exit. the text code can't detect this case
because of 'changes' is not changed.
Line 56: if status != newState:
Line 57: changes.append(newState)
Line 58: status = newState
Line 59:
Line 52: def changed(dev, changes):
Line 53: status = operstate(dev)
Line 54: while not done:
Line 55: newState = operstate(dev)
Line 56: if status != newState:
is it possible a race happen here? the vdsm network configuration code run
ifdown/ifup between two times of checks in monitor thread. but I can't
provide a better solution. inotify doesn't work expected with sysfs file.
Line 57: changes.append(newState)
Line 58: status = newState
Line 59:
Line 60: try:
Line 64: args=(device, changes))
Line 65: monitoring_t.start()
Line 66: yield
Line 67: finally:
Line 68: time.sleep(3) # So that the last action in yield gets to
kernel
I don't understand what's purpose of sleep?
Line 69: done = True
Line 70: if changes:
Line 71: raise ValueError('%s operstate changed: %r' % (device,
changes))
Line 72:
Line 65: monitoring_t.start()
Line 66: yield
Line 67: finally:
Line 68: time.sleep(3) # So that the last action in yield gets to
kernel
Line 69: done = True
run motinoring_t.join()?
Line 70: if changes:
Line 71: raise ValueError('%s operstate changed: %r' % (device,
changes))
Line 72:
Line 73:
File vdsm/netconf/ifcfg.py
Line 105: # to bond.
Line 106: isIfcfgControlled = os.path.isfile(netinfo.NET_CONF_PREF +
bond.name)
Line 107: areOptionsApplied = bond.areOptionsApplied()
Line 108: if not isIfcfgControlled or not areOptionsApplied:
Line 109: bridgeName = _netinfo.getBridgedNetworkForIface(bond.name)
if bonding device is not controlled, it should not be possible to get a vdsm
network over it, right?
Line 110: if bridgeName:
Line 111: bond.master = Bridge(bridgeName, self, port=bond)
Line 112: self.configApplier.addBonding(bond)
Line 113:
Line 116: ifdown(slave.name) # nics must be down to join a bond
Line 117: self.configApplier.addNic(slave)
Line 118: ifup(slave.name)
Line 119:
Line 120: if not isIfcfgControlled or not areOptionsApplied:
if the bonding options doesn't change, you just add a cfg file for the
un-controlled bond device. In this case, any change need to be applied to the
live bond device? So do you think we could skip the ifdown/ifup for 'not
isIfcfgControlled'?
Line 121: ifdown(bond.name)
Line 122: ifup(bond.name)
Line 123:
Line 124: def configureNic(self, nic, **opts):
File vdsm/netmodels.py
Line 193:
Line 194: def areOptionsApplied(self):
Line 195: activeOpts = netinfo.bondOpts(self.name)
Line 196: confOpts = (option.split('=', 1) for option in
self.options.split(' '))
Line 197: return all(value in activeOpts[name] for name, value in
confOpts)
you fetch all bonding options to get activeOpts, but actually we only need
check the confOpts. so we could pass the keys of confOpts to netinfo.bondOpts
and make it only return the dict containing those keys. I thought I added
this comment to previous patch, but I can't find it. :-(
Line 198:
Line 199: def remove(self):
Line 200: logging.debug('Removing bond %r', self)
Line 201: self.configurator.removeBond(self)
--
To view, visit http://gerrit.ovirt.org/17491
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3075a2e65f06416c840c6a98689b77555b22e5d
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
__
Change in vdsm[master]: mom: add mom balloon functional tests for running vms
Mark Wu has posted comments on this change. Change subject: mom: add mom balloon functional tests for running vms .. Patch Set 11: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/13156 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I922568233dc769d83e2fdffe1c24439d13d03d7e Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mei Liu Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Doron Fediuck Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Mei Liu Gerrit-Reviewer: Zhou Zheng Sheng Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netmodels: Improve getIpConfig using namedtuple
Mark Wu has posted comments on this change. Change subject: netmodels: Improve getIpConfig using namedtuple .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/18167 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53eb3c4b00f33285a1b299ca0adc6611eb99a989 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netconf: Simplify addSourceRoute arguments
Mark Wu has posted comments on this change. Change subject: netconf: Simplify addSourceRoute arguments .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/18168 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1bf06f865abbed8cf1fdcca909cbf78c6e5f66f4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: netmodels: Improve getIpConfig using namedtuple
Mark Wu has uploaded a new change for review.
Change subject: netmodels: Improve getIpConfig using namedtuple
..
netmodels: Improve getIpConfig using namedtuple
Change-Id: I53eb3c4b00f33285a1b299ca0adc6611eb99a989
Signed-off-by: Mark Wu
---
M vdsm/netconf/__init__.py
M vdsm/netconf/ifcfg.py
M vdsm/netmodels.py
M vdsm/sourceRoute.py
4 files changed, 17 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/18167/1
diff --git a/vdsm/netconf/__init__.py b/vdsm/netconf/__init__.py
index 97a311b..4936fda 100644
--- a/vdsm/netconf/__init__.py
+++ b/vdsm/netconf/__init__.py
@@ -85,8 +85,7 @@
DynamicSourceRoute.addInterfaceTracking(netEnt)
def _removeSourceRoute(self, netEnt):
-_, _, _, bootproto, _, _ = netEnt.getIpConfig()
-if bootproto != 'dhcp' and netEnt.master is None:
+if netEnt.ipConfig.bootproto != 'dhcp' and netEnt.master is None:
logging.debug("Removing source route for device %s" % netEnt.name)
StaticSourceRoute(netEnt.name, self).remove()
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index b3db008..eb47255 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -60,7 +60,7 @@
self._libvirtAdded = set()
def configureBridge(self, bridge, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = bridge.getIpConfig()
+ipaddr, netmask, gateway, bootproto, async, _ = bridge.ipConfig
self.configApplier.addBridge(bridge, **opts)
ifdown(bridge.name)
if bridge.port:
@@ -69,14 +69,14 @@
ifup(bridge.name, async)
def configureVlan(self, vlan, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = vlan.getIpConfig()
+ipaddr, netmask, gateway, bootproto, async, _ = vlan.ipConfig
self.configApplier.addVlan(vlan, **opts)
vlan.device.configure(**opts)
self._addSourceRoute(vlan, ipaddr, netmask, gateway, bootproto)
ifup(vlan.name, async)
def configureBond(self, bond, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = bond.getIpConfig()
+ipaddr, netmask, gateway, bootproto, async, _ = bond.ipConfig
self.configApplier.addBonding(bond, **opts)
if not netinfo.isVlanned(bond.name):
for slave in bond.slaves:
@@ -96,7 +96,7 @@
self.configureBond(bond)
def configureNic(self, nic, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = nic.getIpConfig()
+ipaddr, netmask, gateway, bootproto, async, _ = nic.ipConfig
self.configApplier.addNic(nic, **opts)
self._addSourceRoute(nic, ipaddr, netmask, gateway, bootproto)
if nic.bond is None:
@@ -490,7 +490,7 @@
def addBridge(self, bridge, **opts):
""" Create ifcfg-* file with proper fields for bridge """
ipaddr, netmask, gateway, bootproto, _, defaultRoute = \
-bridge.getIpConfig()
+bridge.ipConfig
conf = 'TYPE=Bridge\nDELAY=%s\n' % bridge.forwardDelay
self._createConfFile(conf, bridge.name, ipaddr, netmask, gateway,
bootproto, bridge.mtu,
@@ -499,7 +499,7 @@
def addVlan(self, vlan, **opts):
""" Create ifcfg-* file with proper fields for VLAN """
ipaddr, netmask, gateway, bootproto, _, defaultRoute = \
-vlan.getIpConfig()
+vlan.ipConfig
conf = 'VLAN=yes\n'
if vlan.bridge:
conf += 'BRIDGE=%s\n' % pipes.quote(vlan.bridge.name)
@@ -543,7 +543,7 @@
@staticmethod
def _getIfaceConfValues(iface, _netinfo):
ipaddr, netmask, gateway, bootproto, _, defaultRoute = \
-iface.getIpConfig()
+iface.ipConfig
defaultRoute = ConfigWriter._toIfcfgFormat(defaultRoute)
mtu = iface.mtu
if _netinfo.ifaceUsers(iface.name):
diff --git a/vdsm/netmodels.py b/vdsm/netmodels.py
index fffba2a..95e87f1 100644
--- a/vdsm/netmodels.py
+++ b/vdsm/netmodels.py
@@ -16,6 +16,7 @@
#
# Refer to the README and COPYING files for full details of the license
#
+from collections import namedtuple
from contextlib import contextmanager
import logging
import os
@@ -29,6 +30,9 @@
class NetDevice(object):
+_ipConfig = namedtuple('ipConfig', 'ipaddr netmask gateway bootproto \
+async defaultRoute')
+
def __init__(self, name, configurator, ipconfig=None, mtu=None):
self.name = name
self.ip = ipconfig
@@ -42,14 +46,16 @@
def configure(self, **opts):
raise NotImplementedError
-def getIpConfig(self):
+@property
+def ipConfig(self):
try:
ipaddr, netmask, gateway, bootproto, async, defaultRoute = \
self.ip.getConfig()
except AttributeError:
ipaddr = netmask = gateway = bootproto = async =
Change in vdsm[master]: netconf: Simplify addSourceRoute arguments
Mark Wu has uploaded a new change for review.
Change subject: netconf: Simplify addSourceRoute arguments
..
netconf: Simplify addSourceRoute arguments
Change-Id: I1bf06f865abbed8cf1fdcca909cbf78c6e5f66f4
Signed-off-by: Mark Wu
---
M vdsm/netconf/__init__.py
M vdsm/netconf/ifcfg.py
2 files changed, 10 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/68/18168/1
diff --git a/vdsm/netconf/__init__.py b/vdsm/netconf/__init__.py
index 4936fda..5513448 100644
--- a/vdsm/netconf/__init__.py
+++ b/vdsm/netconf/__init__.py
@@ -75,7 +75,8 @@
def removeLibvirtNetwork(self, network):
self.configApplier.removeLibvirtNetwork(network)
-def _addSourceRoute(self, netEnt, ipaddr, netmask, gateway, bootproto):
+def _addSourceRoute(self, netEnt):
+ipaddr, netmask, gateway, bootproto, _, _ = netEnt.ipConfig
# bootproto is None for both static and no bootproto
if bootproto != 'dhcp' and netEnt.master is None:
logging.debug("Adding source route %s, %s, %s, %s" %
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index eb47255..41bbc1e 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -60,31 +60,28 @@
self._libvirtAdded = set()
def configureBridge(self, bridge, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = bridge.ipConfig
self.configApplier.addBridge(bridge, **opts)
ifdown(bridge.name)
if bridge.port:
bridge.port.configure(**opts)
-self._addSourceRoute(bridge, ipaddr, netmask, gateway, bootproto)
-ifup(bridge.name, async)
+self._addSourceRoute(bridge)
+ifup(bridge.name, bridge.ipConfig.async)
def configureVlan(self, vlan, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = vlan.ipConfig
self.configApplier.addVlan(vlan, **opts)
vlan.device.configure(**opts)
-self._addSourceRoute(vlan, ipaddr, netmask, gateway, bootproto)
-ifup(vlan.name, async)
+self._addSourceRoute(vlan)
+ifup(vlan.name, vlan.ipConfig.async)
def configureBond(self, bond, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = bond.ipConfig
self.configApplier.addBonding(bond, **opts)
if not netinfo.isVlanned(bond.name):
for slave in bond.slaves:
ifdown(slave.name)
for slave in bond.slaves:
slave.configure(**opts)
-self._addSourceRoute(bond, ipaddr, netmask, gateway, bootproto)
-ifup(bond.name, async)
+self._addSourceRoute(bond)
+ifup(bond.name, bond.ipConfig.async)
def editBonding(self, bond, _netinfo):
ifdown(bond.name)
@@ -96,13 +93,12 @@
self.configureBond(bond)
def configureNic(self, nic, **opts):
-ipaddr, netmask, gateway, bootproto, async, _ = nic.ipConfig
self.configApplier.addNic(nic, **opts)
-self._addSourceRoute(nic, ipaddr, netmask, gateway, bootproto)
+self._addSourceRoute(nic)
if nic.bond is None:
if not netinfo.isVlanned(nic.name):
ifdown(nic.name)
-ifup(nic.name, async)
+ifup(nic.name, nic.ipConfig.async)
def removeBridge(self, bridge):
DynamicSourceRoute.addInterfaceTracking(bridge)
--
To view, visit http://gerrit.ovirt.org/18168
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1bf06f865abbed8cf1fdcca909cbf78c6e5f66f4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: mom: add mom balloon functional tests for running vms
Mark Wu has posted comments on this change.
Change subject: mom: add mom balloon functional tests for running vms
..
Patch Set 10:
(3 comments)
File tests/functional/momTests.py
Line 58:
Line 59: def assertOK(self, result):
Line 60: # code == 0 means OK
Line 61: self.assertEquals(
Line 62: result['status']['code'], 0, str(result))
you could use utils.SUCCESS instead. And I believe this function should belong
to utils too.
Line 63:
Line 64: @testValidation.ValidateRunningAsRoot
Line 65: @skipNoMOM
Line 66: def testKSM(self):
Line 108: r = self.s.setBalloonTarget(
Line 109: stats['vmId'],
Line 110: initial)
Line 111: self.assertOK(r)
Line 112: return candidateStats
what we need keep is only the vmId of the old vm. we don't need other stats info
Line 113:
Line 114: def _setPolicy(self, policy):
Line 115: curpath = os.path.dirname(__file__)
Line 116: file_name = os.path.join(curpath, policy)
Line 150:
Line 151: if not vmsOldStats:
Line 152: raise SkipTest('No VM can be candidate of ballooning
operation.')
Line 153:
Line 154: # Set policy to triger the balloon operation.
s/triger/trigger/
Line 155: self._setPolicy(policy)
Line 156: # Wait for the policy taking effect.
Line 157: time.sleep(22)
Line 158:
--
To view, visit http://gerrit.ovirt.org/13156
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I922568233dc769d83e2fdffe1c24439d13d03d7e
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mei Liu
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Doron Fediuck
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Mei Liu
Gerrit-Reviewer: Zhou Zheng Sheng
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: mom: add mom balloon functional tests for running vms
Mark Wu has posted comments on this change.
Change subject: mom: add mom balloon functional tests for running vms
..
Patch Set 10: Code-Review-1
(4 comments)
File tests/functional/momTests.py
Line 89: except KeyError:
Line 90: return False
Line 91:
Line 92: def _prepare(self, balloonRatio):
Line 93: # Get vms' tatistics before the operation.
s/tatistics/statistics/
Line 94: r = self.s.getAllVmStats()
Line 95: self.assertOK(r)
Line 96:
Line 97: # Filter all vms' statistics to get balloon operation
candidates.
Line 103: # 0.95*max for grow operation.
Line 104: for stats in candidateStats:
Line 105: initial = int(stats['balloonInfo']['balloon_max']) * \
Line 106: balloonRatio.initial
Line 107: if int(stats['balloonInfo']['balloon_cur']) != initial:
it looks you can use filter to get the vms to operate too.
Line 108: r = self.s.setBalloonTarget(
Line 109: stats['vmId'],
Line 110: initial)
Line 111: self.assertOK(r)
Line 116: file_name = os.path.join(curpath, policy)
Line 117: try:
Line 118: with open(file_name, 'r') as f:
Line 119: testPolicyStr = f.read()
Line 120: except IOError, (error, message):
you should use:
except IOError as e:
if e.errno == errno.ENOENT:
...
Line 121: if error == errno.ENOENT:
Line 122: raise SkipTest('The policy file %s is missing.' %
Line 123:file_name)
Line 124: else:
Line 131: # Check the new balloon_cur in the proper range.
Line 132: for vmOldStats in vmsOldStats:
Line 133: r = self.s.getVmStats(vmOldStats['vmId'])
Line 134: # Vm doesn't exist.
Line 135: if r['status']['code'] == 1:
why not using errCode ['noVM']['status']['code'] to replace the magic code?
Line 136: continue
Line 137: else:
Line 138: self.assertOK(r)
Line 139: vmNewStats = r['statsList'][0]
--
To view, visit http://gerrit.ovirt.org/13156
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I922568233dc769d83e2fdffe1c24439d13d03d7e
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mei Liu
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Doron Fediuck
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Martin Sivák
Gerrit-Reviewer: Mei Liu
Gerrit-Reviewer: Zhou Zheng Sheng
Gerrit-Reviewer: oVirt Jenkins CI Server
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: setupNetworks add net bond breaking and resizing.
Mark Wu has posted comments on this change. Change subject: tests: setupNetworks add net bond breaking and resizing. .. Patch Set 4: Code-Review-1 all the three new added test cases have the code of create network over bond device. they just use different nics. So the code is duplicate. testAddDelBondedNetwork also have this code. So my suggestion here is extract the duplicate into the simple test, and construct complicated test based on the existing simple test. Then we can just focus on the test logic. -- To view, visit http://gerrit.ovirt.org/17634 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e031599ac4457cc24cb9d142110e1781325912f Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
