Change in vdsm[master]: tests : setupNetworks resizeBond, remove param validation.
Giuseppe Vallarelli has posted comments on this change.
Change subject: tests : setupNetworks resizeBond, remove param validation.
..
Patch Set 3:
(2 comments)
File tests/functional/networkTests.py
Line 1116:
Line 1117: self.vdsm_net.bondExists(BONDING_NAME, nics)
Line 1118:
Line 1119: # Reduce bond size
Line 1120: reqmode = '3'
Done
Line 1121: bondings[BONDING_NAME]['nics'] = nics[:2]
Line 1122: bondings[BONDING_NAME]['options'] = 'mode=%s' %
reqmode
Line 1123: status, msg = self.vdsm_net.setupNetworks({},
bondings, {})
Line 1124:
Line 1125: self.assertEquals(status, SUCCESS, msg)
Line 1126:
Line 1127: self.vdsm_net.bondExists(BONDING_NAME, nics[:2])
Line 1128:
self.assertEquals(self.vdsm_net.getBondMode(BONDING_NAME),
Line 1129: ['broadcast', reqmode])
... and networking people know bonding modes by heart :-)
Line 1130:
Line 1131: bondings = {BONDING_NAME: dict(remove=True)}
Line 1132: status, msg = self.vdsm_net.setupNetworks({},
bondings, {})
Line 1133:
--
To view, visit http://gerrit.ovirt.org/17922
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6471a50152314e53e86e9f8e2e1359059fff17f0
Gerrit-PatchSet: 3
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: 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 resizeBond, remove param validation.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests : setupNetworks resizeBond, remove param validation. .. Patch Set 4: Verified+1 Addressed comments in the code review -- To view, visit http://gerrit.ovirt.org/17922 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6471a50152314e53e86e9f8e2e1359059fff17f0 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: 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 : setupNetworks resizeBond, remove param validation.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests : setupNetworks resizeBond, remove param validation. .. Patch Set 3: Verified+1 Added the missing assertion (bond mode) to the test. -- To view, visit http://gerrit.ovirt.org/17922 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6471a50152314e53e86e9f8e2e1359059fff17f0 Gerrit-PatchSet: 3 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: 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]: Functional Tests [1/3]: Added module for dummy nics
Giuseppe Vallarelli has posted comments on this change. Change subject: Functional Tests [1/3]: Added module for dummy nics .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.ovirt.org/18039 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c7ed4110cd8d45a3fb7a9cc2cd2dc07004e96b9 Gerrit-PatchSet: 6 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: 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]: Functional Tests [1/3]: Added module for dummy nics
Giuseppe Vallarelli has posted comments on this change. Change subject: Functional Tests [1/3]: Added module for dummy nics .. Patch Set 6: Code-Review+1 http://jenkins.ovirt.org/view/vdsm/job/vdsm_functional_tests/146/consoleFull Build green for this patch except for the delNetworkWithMTU fixed already and available in master. -- To view, visit http://gerrit.ovirt.org/18039 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c7ed4110cd8d45a3fb7a9cc2cd2dc07004e96b9 Gerrit-PatchSet: 6 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: 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]: test : delNetwork with bond accumulation.
Giuseppe Vallarelli has posted comments on this change. Change subject: test : delNetwork with bond accumulation. .. Patch Set 2: Green except for testDelNetworkMTU whose fix has not been merged yet http://jenkins.ovirt.org/view/vdsm/job/vdsm_functional_tests/143/console -- To view, visit http://gerrit.ovirt.org/17944 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12ebeeaff8949e6020dcfcaebaa94f3641b622d5 Gerrit-PatchSet: 2 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: 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
Giuseppe Vallarelli has posted comments on this change. Change subject: fix testDelNetworkWithMTU .. Patch Set 2: Code-Review+1 http://jenkins.ovirt.org/view/vdsm/job/vdsm_functional_tests/142/console Green -- 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]: test : delNetwork with bond accumulation.
Giuseppe Vallarelli has posted comments on this change. Change subject: test : delNetwork with bond accumulation. .. Patch Set 2: Verified+1 Cherrypicked on top of master. -- To view, visit http://gerrit.ovirt.org/17944 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12ebeeaff8949e6020dcfcaebaa94f3641b622d5 Gerrit-PatchSet: 2 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: 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]: Unified network persistence [1/3] - Save running config
Giuseppe Vallarelli has posted comments on this change. Change subject: Unified network persistence [1/3] - Save running config .. Patch Set 20: Code-Review+1 -- 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: No ___ 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
Giuseppe Vallarelli has posted comments on this change. Change subject: Unified network persistence [1/3] - Save running config .. Patch Set 19: Code-Review-1 (5 comments) A few hints see comments. File lib/vdsm/netinfo.py Line 40: from ipwrapper import linkShowDev Line 41: Line 42: NET_CONF_DIR = '/etc/sysconfig/network-scripts/' Line 43: # ifcfg persistence directories Line 44: NET_CONF_BACK_DIR = constants.P_VDSM_LIB + 'netconfback/' I would change it to NET_CONF_PERS_DIR instead of having this comment. Line 45: NET_LOGICALNET_CONF_BACK_DIR = NET_CONF_BACK_DIR + 'logicalnetworks/' Line 46: Line 47: NET_CONF_PREF = NET_CONF_DIR + 'ifcfg-' Line 48: PROC_NET_VLAN = '/proc/net/vlan/' File lib/vdsm/unifiedpersistence.py Line 33: class Config(object): Line 34: def __init__(self, savePath): Line 35: self.networksPath = savePath + 'nets/' Line 36: self.bondingsPath = savePath + 'bonds/' Line 37: self.networks = self._getNetworks() You can replace it with self._getConfigs(self.networksPath) and get rid of _getNetworks Line 38: self.bonds = self._getBondings() Line 39: Line 40: def __eq__(self, other): Line 41: return (dict.__eq__(self.networks, other.networks) and Line 34: def __init__(self, savePath): Line 35: self.networksPath = savePath + 'nets/' Line 36: self.bondingsPath = savePath + 'bonds/' Line 37: self.networks = self._getNetworks() Line 38: self.bonds = self._getBondings() You can replace it with self._getConfigs(self.bondingsPath) and get rid of _getBondings. Line 39: Line 40: def __eq__(self, other): Line 41: return (dict.__eq__(self.networks, other.networks) and Line 42: dict.__eq__(self.bonds, other.bonds)) Line 37: self.networks = self._getNetworks() Line 38: self.bonds = self._getBondings() Line 39: Line 40: def __eq__(self, other): Line 41: return (dict.__eq__(self.networks, other.networks) and what about using self? Line 42: dict.__eq__(self.bonds, other.bonds)) Line 43: Line 44: def __repr(self): Line 45: return '%s(%s, %s)' % (self.__class__.__name__, self.networks, Line 50: Line 51: def _bondingPath(self, bonding): Line 52: return self.bondingsPath + bonding Line 53: Line 54: def _getConfig(self, path): _getConfig and _getConfigs? I find the implementation okay I don't like that they have a very similar name I would change in _getConfig and _getNetworkEnts but I don't want to have the discriminant being a single 's'. Line 55: try: Line 56: with open(path, 'r') as configurationFile: Line 57: return json.load(configurationFile) Line 58: except IOError as ioe: -- 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: 19 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]: tests: adding missing cleanup decoration
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: adding missing cleanup decoration .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.ovirt.org/18606 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0774f901f654022080d39c486a1dd6d6d7f021fb Gerrit-PatchSet: 2 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: 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: adding missing cleanup decoration
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: adding missing cleanup decoration .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/18606 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0774f901f654022080d39c486a1dd6d6d7f021fb Gerrit-PatchSet: 1 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: 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: adding missing cleanup decoration
Giuseppe Vallarelli has uploaded a new change for review.
Change subject: tests: adding missing cleanup decoration
..
tests: adding missing cleanup decoration
Some tests need to cleanup the env in case of failure.
This patch adds the missed cleanupNet decoration to
some tests.
Change-Id: I0774f901f654022080d39c486a1dd6d6d7f021fb
Signed-off-by: Giuseppe Vallarelli
---
M tests/functional/networkTests.py
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/18606/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 6a64259..aa70392 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -457,6 +457,7 @@
opts={'bridged': bridged})
self.assertEqual(status, neterrors.ERR_BAD_BRIDGE, msg)
+@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -478,6 +479,7 @@
{}, {})
self.assertEqual(status, SUCCESS, msg)
+@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -783,6 +785,7 @@
self.assertFalse(self.vdsm_net.networkExists(
netNameVlanBridged, bridged=True))
+@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -829,6 +832,7 @@
bondings, {})
self.assertEquals(status, SUCCESS, msg)
+@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
@@ -866,6 +870,7 @@
{}, {})
self.assertEquals(status, SUCCESS, msg)
+@cleanupNet
@permutations([[True], [False]])
@RequireDummyMod
@ValidateRunningAsRoot
--
To view, visit http://gerrit.ovirt.org/18606
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0774f901f654022080d39c486a1dd6d6d7f021fb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: setupNetworks with invalid params.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks with invalid params. .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.ovirt.org/17435 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic77b3a9c0d84e38e8b9060191b138c3e09104dc6 Gerrit-PatchSet: 8 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: 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: setupNetworks with invalid params.
Giuseppe Vallarelli has posted comments on this change.
Change subject: tests: setupNetworks with invalid params.
..
Patch Set 7:
(1 comment)
File tests/functional/networkTests.py
Line 687: attrs = dict(vlan=VLAN_ID, bridged=bridged)
Line 688: status, msg = self.vdsm_net.setupNetworks({NETWORK_NAME:
attrs},
Line 689: {}, {})
Line 690:
Line 691: self.assertTrue(status != SUCCESS, msg)
just realized that I should have used assertNotEqual instead.
--
To view, visit http://gerrit.ovirt.org/17435
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic77b3a9c0d84e38e8b9060191b138c3e09104dc6
Gerrit-PatchSet: 7
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: 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 compatibility bond and nic.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks compatibility bond and nic. .. Patch Set 7: Verified+1 Added cleanupNet decorations for completeness. -- To view, visit http://gerrit.ovirt.org/17621 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If4a497d82723937dbecd14d42fc2c176c4c05bd2 Gerrit-PatchSet: 7 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: 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: setupNetworks compatibility bond and nic.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks compatibility bond and nic. .. Patch Set 6: Verified+1 Rebase on top of master. Would be cool if we can merge something guys :-D -- To view, visit http://gerrit.ovirt.org/17621 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If4a497d82723937dbecd14d42fc2c176c4c05bd2 Gerrit-PatchSet: 6 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: 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: setupNetworks with invalid params.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks with invalid params. .. Patch Set 7: Verified+1 I believe we can merge this small patchset :) -- To view, visit http://gerrit.ovirt.org/17435 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic77b3a9c0d84e38e8b9060191b138c3e09104dc6 Gerrit-PatchSet: 7 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: 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: setupNetworks with invalid params.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks with invalid params. .. Patch Set 7: cherrypicked on top of master -- To view, visit http://gerrit.ovirt.org/17435 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic77b3a9c0d84e38e8b9060191b138c3e09104dc6 Gerrit-PatchSet: 7 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: 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: setupNetworks convertVlanNetBridgeness, Mtus
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks convertVlanNetBridgeness, Mtus .. Patch Set 4: Rebased at the moment a test is failing testSetupNetworksMtus... Used to work before. -- To view, visit http://gerrit.ovirt.org/17866 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib8723b066228729807f7ab46c9fabad3ebff128b 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: 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]: Unified network persistence [3/3] - Restore network configur...
Giuseppe Vallarelli has posted comments on this change.
Change subject: Unified network persistence [3/3] - Restore network
configuration
..
Patch Set 4: Code-Review-1
(2 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(
I think there is no need of os.path.basename here.
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 46: nets[os.path.basename(netFile)] = 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(
same as previously said.
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))
--
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: 4
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]: Unified network persistence [1/3] - Save running config
Giuseppe Vallarelli has posted comments on this change. Change subject: Unified network persistence [1/3] - Save running config .. Patch Set 15: I made some comments on patchset 13 which have been completely ignored, but I might be wrong. -- 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: 15 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]: Allow configurators to be used as context managers
Giuseppe Vallarelli has posted comments on this change. Change subject: Allow configurators to be used as context managers .. Patch Set 3: Code-Review+1 -- 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: No ___ 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
Giuseppe Vallarelli has posted comments on this change. Change subject: Allow configurators to be used as context managers .. Patch Set 3: http://jenkins.ovirt.org/view/vdsm/job/vdsm_functional_tests/42/console -- 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: 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
Giuseppe Vallarelli has posted comments on this change. Change subject: Fix the 'MTU' option in networkTests .. Patch Set 1: Code-Review+1 Ehy Mark thanks -- 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-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
Giuseppe Vallarelli has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 2: Code-Review+1 Thanks, looks cool -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli 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]: xmlrpcTests: refactoring
Giuseppe Vallarelli has posted comments on this change. Change subject: xmlrpcTests: refactoring .. Patch Set 16: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/17893 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I683c9e056137e7a189815bf6be2eb79ee80994cf Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Vinzenz Feenstra 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]: Get rid of mutables(lists) as default parameters
Giuseppe Vallarelli has posted comments on this change. Change subject: Get rid of mutables(lists) as default parameters .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/18356 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I911485cf30d587f5b6b8bf44cf3552c1517abfd5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Vinzenz Feenstra Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: xmlrpcTests: refactoring
Giuseppe Vallarelli has posted comments on this change. Change subject: xmlrpcTests: refactoring .. Patch Set 12: (1 comment) File tests/functional/virtTests.py Line 166: Line 167: def assertVmUp(self, vmid): Line 168: status, msg, result = self.vdsm.getVmStats(vmid) Line 169: self.assertEqual(status, SUCCESS, msg) Line 170: self.assertTrue(result['status'] in self.UPSTATES) I see okay Line 171: Line 172: def assertGuestUp(self, vmid, shortcut=0): Line 173: status, msg, result = self.vdsm.getVmStats(vmid) Line 174: self.assertEqual(status, SUCCESS, msg) -- To view, visit http://gerrit.ovirt.org/17893 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I683c9e056137e7a189815bf6be2eb79ee80994cf Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Vinzenz Feenstra 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]: xmlrpcTests: refactoring
Giuseppe Vallarelli has posted comments on this change.
Change subject: xmlrpcTests: refactoring
..
Patch Set 12:
(8 comments)
File tests/functional/virtTests.py
Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20:
Line 21: import os
For the import statements the you should for each group follow an order
(alphabetical)
the identified groups are: stdlib - third party libs - application specific
stuff.
The order for application specific stuff is for proximity so in this specific
case first vdsm and then testrunner being closer to the virtTests.
Have a look to for a comprehensive descriptions:
http://www.python.org/dev/peps/pep-0008/#imports
Line 22: import tempfile
Line 23: import math
Line 24: from functools import partial, wraps
Line 25:
Line 33: from vdsm.utils import CommandPath
Line 34:
Line 35: from utils import VdsProxy, SUCCESS
Line 36:
Line 37: _VARTMP = '/var/tmp'
I think that there's no need of prefixing with _
Line 38:
Line 39: if not config.getboolean('vars', 'xmlrpc_enable'):
Line 40: raise SkipTest("XML-RPC Bindings are disabled")
Line 41:
Line 47: _exportfs = CommandPath("exportfs", "/usr/sbin/exportfs")
Line 48: _initramfsPath = None
Line 49:
Line 50:
Line 51: def teardown():
I think that teardownModule communicates better that the teardown is at module
level. It's subjective I'll leave the decision of change to you.
Line 52: if _initramfsPath is not None:
Line 53: os.unlink(_initramfsPath)
Line 54:
Line 55:
Line 53: os.unlink(_initramfsPath)
Line 54:
Line 55:
Line 56: def readableBy(filePath, user):
Line 57: rc, out, err = execCmd([EXT_SUDO, '-u', user, 'head', '-c', '0',
filePath])
when you're not going to use the variables, in python you are allowed to not
assign any name with the _ . In this case you can change in rc, _, _ = ...
Line 58: return rc == 0
Line 59:
Line 60:
Line 61: def skipNoKVM(method):
Line 57: rc, out, err = execCmd([EXT_SUDO, '-u', user, 'head', '-c', '0',
filePath])
Line 58: return rc == 0
Line 59:
Line 60:
Line 61: def skipNoKVM(method):
I don't like much using negations in names or when composing boolean values.
I would change it in requireKVM and reverse the condition in the implementation:
if kvmEnabled:
do stuff
else:
raise SkipTest.
Line 62: @wraps(method)
Line 63: def wrapped(self, *args, **kwargs):
Line 64: status, msg, result = self.vdsm.getVdsCapabilities()
Line 65: self.assertEqual(status, SUCCESS, msg)
Line 68: return method(self, *args, **kwargs)
Line 69: return wrapped
Line 70:
Line 71:
Line 72: class RunningVm(object):
What happens in case there's an assertion failure? Is the Vm turned down
anyway? In the networkTest we have defined a cleanupNet to prevent a dirty
environment when a following test is executed.
Line 73: KERNEL_ARGS_DISTRO = {
Line 74: 'fedora': 'rd.break=cmdline rd.shell rd.skipfsck',
Line 75: 'rhel': 'rd.break=cmdline rd.shell rd.skipfsck'}
Line 76:
Line 146:
Line 147:
Line 148: @expandPermutations
Line 149: class XMLRPCTest(TestCaseBase):
Line 150: UPSTATES = frozenset(('Up', 'Powering up', 'Running'))
nice
Line 151:
Line 152: def setUp(self):
Line 153: self.vdsm = VdsProxy()
Line 154:
Line 166:
Line 167: def assertVmUp(self, vmid):
Line 168: status, msg, result = self.vdsm.getVmStats(vmid)
Line 169: self.assertEqual(status, SUCCESS, msg)
Line 170: self.assertTrue(result['status'] in self.UPSTATES)
Why don't we assert that the status is Up like the method suggests?
Line 171:
Line 172: def assertGuestUp(self, vmid, shortcut=0):
Line 173: status, msg, result = self.vdsm.getVmStats(vmid)
Line 174: self.assertEqual(status, SUCCESS, msg)
--
To view, visit http://gerrit.ovirt.org/17893
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I683c9e056137e7a189815bf6be2eb79ee80994cf
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Mark Wu
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Petr Šebek
Gerrit-Reviewer: Vinzenz Feenstra
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]: xmlrpcTests: refactoring
Giuseppe Vallarelli has posted comments on this change. Change subject: xmlrpcTests: refactoring .. Patch Set 13: Comments are on patchset 12. -- To view, visit http://gerrit.ovirt.org/17893 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I683c9e056137e7a189815bf6be2eb79ee80994cf Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Vinzenz Feenstra 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]: xmlrpcTests: refactoring
Giuseppe Vallarelli has posted comments on this change. Change subject: xmlrpcTests: refactoring .. Patch Set 13: Code-Review-1 Martin looks cool already :) I've put some comments here and there and some questions. Thanks Giuseppe -- To view, visit http://gerrit.ovirt.org/17893 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I683c9e056137e7a189815bf6be2eb79ee80994cf Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Vinzenz Feenstra 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]: Drop single use inheritance
Giuseppe Vallarelli has posted comments on this change. Change subject: Drop single use inheritance .. Patch Set 1: Code-Review+1 Thanks, looks cool. -- To view, visit http://gerrit.ovirt.org/18351 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If781ab20110874e71ba16b60d1d5511a54914979 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Vinzenz Feenstra 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
Giuseppe Vallarelli 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 Would be cool to merge this asap, I need to use to get the bond options in a different functional test patch. Thanks Toni -- 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]: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists
Giuseppe Vallarelli has posted comments on this change. Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists .. Patch Set 3: Code-Review-1 (4 comments) File lib/vdsm/ipwrapper.py Line 105: def __iter__(self): Line 106: for word in str(self).split(): Line 107: yield word Line 108: Line 109: def __eq__(self, other): I see that you define the same couple of methods __eq__ and __ne__ for two objects Route and Rule. You can create a class decorator to avoid this duplication here's an example: In [9]: def equals(cls): def __eq__(self, other): return self.__dict__ == other.__dict__ cls.__eq__ = __eq__ return cls In [10]: @equals class Pippo(object): def __init__(self, a, b): self.a = a self.b = b : In [11]: c = Pippo(1, 2) In [12]: d = Pippo(3, 4) In [13]: c == d Out[13]: False In [14]: e = Pippo(1, 2) In [15]: c == e Out[15]: True Line 110: return self.__dict__ == other.__dict__ Line 111: Line 112: def __ne__(self, other): Line 113: return not self.__eq__(other) File tests/functional/networkTests.py Line 410: @RequireDummyMod Line 411: @ValidateRunningAsRoot Line 412: def testRuleExists(self): Line 413: with dummyIf(1) as nics: Line 414: nic = nics[0] I anticipate Toni by saying nic, = nics Line 415: dummy.setIP(nic, IP_ADDRESS, IP_CIDR) Line 416: dummy.setLinkUp(nic) Line 417: Line 418: rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE), Line 428: @RequireDummyMod Line 429: @ValidateRunningAsRoot Line 430: def testRouteExists(self): Line 431: with dummyIf(1) as nics: Line 432: nic = nics[0] Same as previously stated. Line 433: dummy.setIP(nic, IP_ADDRESS, IP_CIDR) Line 434: dummy.setLinkUp(nic) Line 435: Line 436: routes = [Route(network='0.0.0.0/0', ipaddr=IP_GATEWAY, File tests/functional/utils.py Line 79: raise Line 80: return wrapper Line 81: Line 82: Line 83: def saveRules(): Can you use ipwrapper.ruleList also in cleanupRules as you did in restoreRules? When I've read saveRules() with ipwrapper.ruleList() in the implementation I've been a little surprised. Line 84: return ipwrapper.ruleList() Line 85: Line 86: Line 87: def restoreRules(base): -- To view, visit http://gerrit.ovirt.org/18040 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf7c28e85f53f51879750308f3a9f93fc635cf33 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli 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]: Functional Tests [1/3]: Added module for dummy nics
Giuseppe Vallarelli has posted comments on this change. Change subject: Functional Tests [1/3]: Added module for dummy nics .. Patch Set 4: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/18039 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c7ed4110cd8d45a3fb7a9cc2cd2dc07004e96b9 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli 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: Reboot capability for VM
Giuseppe Vallarelli has posted comments on this change. Change subject: vdsm: Reboot capability for VM .. Patch Set 17: Code-Review-1 (1 comment) File vdsm/vm.py Line 1619: return m Line 1620: Line 1621: Line 1622: class VmPowerDown(utils.CallbackChain): Line 1623: def __init__(self, vm, info): Right now in the client code you're only using create and the __init__ internally in the create classmethod - create is taking place of the constructor that's why you shouldn't have externally 2 different constructors. Does it make sense to have: powerDown = VmPowerDown(...) and then call on it powerDown.start() ? Because right now that's a possibility. Hope this helps. Line 1624: super(VmPowerDown, self).__init__(checkSuccess=info['checkSuccess']) Line 1625: self.vm = vm Line 1626: self.info = info Line 1627: -- To view, visit http://gerrit.ovirt.org/15829 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12737e363a80679ffb1db55f14eaee158312d7da Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Betak Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Martin Betak Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra 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]: modify mom functional test to use VdsProxy
Giuseppe Vallarelli has posted comments on this change. Change subject: modify mom functional test to use VdsProxy .. Patch Set 1: -Code-Review Related to my previous comment perhaps is too early now to think about splitting up, let see how it evolves with the usage of this module and in case of troubles we will do some changes. -- To view, visit http://gerrit.ovirt.org/18271 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I874d527e3919c1beff328183eb12f332f2399b95 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mei Liu Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Mei Liu 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]: modify mom functional test to use VdsProxy
Giuseppe Vallarelli has posted comments on this change. Change subject: modify mom functional test to use VdsProxy .. Patch Set 1: Code-Review-1 Hi Mei looks good already only a minor comment. What about creating a different VdsProxy object ? As you can see the current one contains methods specialized to test the networking part. I'm in favour of breaking it down in smaller objects which act as wrapper only to the relevant part of the API under test. -- To view, visit http://gerrit.ovirt.org/18271 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I874d527e3919c1beff328183eb12f332f2399b95 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mei Liu Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Mark Wu Gerrit-Reviewer: Mei Liu 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: Reboot capability for VM
Giuseppe Vallarelli has posted comments on this change. Change subject: vdsm: Reboot capability for VM .. Patch Set 17: (1 comment) File vdsm/vm.py Line 1619: return m Line 1620: Line 1621: Line 1622: class VmPowerDown(utils.CallbackChain): Line 1623: def __init__(self, vm, info): At this point having both the __init__ and create doesn't have much sense I suggest to remove this one and expose only create. Line 1624: super(VmPowerDown, self).__init__(checkSuccess=info['checkSuccess']) Line 1625: self.vm = vm Line 1626: self.info = info Line 1627: -- To view, visit http://gerrit.ovirt.org/15829 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12737e363a80679ffb1db55f14eaee158312d7da Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Betak Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Martin Betak Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Omer Frenkel Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra 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]: Functional Tests [1/3]: Added module for dummy nics
Giuseppe Vallarelli has posted comments on this change.
Change subject: Functional Tests [1/3]: Added module for dummy nics
..
Patch Set 3: Code-Review-1
(7 comments)
See the different comments (they're really minor), is pretty much good already.
File tests/functional/Makefile.am
Line 20:
Line 21: vdsmfunctestsdir = ${vdsmtestsdir}/functional
Line 22:
Line 23: dist_vdsmfunctests_PYTHON = \
Line 24:dummy.py \
I like the idea of this external module.
Line 25:momTests.py \
Line 26:networkTests.py \
Line 27:sosPluginTests.py \
Line 28:supervdsmFuncTests.py \
File tests/functional/dummy.py
Line 16: #
Line 17: # Refer to the README and COPYING files for full details of the license
Line 18: #
Line 19: import random
Line 20: from contextlib import contextmanager
nitpick - can you reverse the order of the statements (before it was first
contextlib and then random).
Line 21:
Line 22: from nose.plugins.skip import SkipTest
Line 23: from vdsm.ipwrapper import linkAdd, linkDel, addrAdd, linkSet,
IPRoute2Error
Line 24:
Line 19: import random
Line 20: from contextlib import contextmanager
Line 21:
Line 22: from nose.plugins.skip import SkipTest
Line 23: from vdsm.ipwrapper import linkAdd, linkDel, addrAdd, linkSet,
IPRoute2Error
nitpick - add blank line between nose stuff and vdsm (the usual standard lib -
third party - application specific stuff)
Line 24:
Line 25:
Line 26: def create():
Line 27: """
Line 60: except IPRoute2Error:
Line 61: raise SkipTest('Failed to set device ip')
Line 62:
Line 63:
Line 64: def setLinkUp(dummyName):
I still see setLinkUp and setLinkDown instead of simply setLinkState.
Line 65: _setLinkState(dummyName, 'up')
Line 66:
Line 67:
Line 68: def setLinkDown(dummyName):
Line 61: raise SkipTest('Failed to set device ip')
Line 62:
Line 63:
Line 64: def setLinkUp(dummyName):
Line 65: _setLinkState(dummyName, 'up')
In one of his patches Toni defined a constant for the link state
http://gerrit.ovirt.org/#/c/17491/10/lib/vdsm/netinfo.py
Due to the simple string usage in many different places.
Line 66:
Line 67:
Line 68: def setLinkDown(dummyName):
Line 69: _setLinkState(dummyName, 'down')
File tests/functional/networkTests.py
Line 22: expandPermutations, permutations)
Line 23: from testValidation import RequireDummyMod, ValidateRunningAsRoot
Line 24:
Line 25: from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy
Line 26: from dummy import dummyIf
nitpick import order first from dummy and then from utils.
Line 27:
Line 28:
Line 29: NETWORK_NAME = 'test-network'
Line 30: VLAN_ID = '27'
File tests/functional/utils.py
Line 30: SUCCESS = 0
Line 31: Qos = namedtuple('Qos', 'inbound outbound')
Line 32:
Line 33:
Line 34: ip = utils.CommandPath('ip',
Is it still used? I guess no, if so you can drop it, thanks.
Line 35:'/sbin/ip', # EL6
Line 36:'/usr/sbin/ip', # Fedora
Line 37:)
Line 38:
--
To view, visit http://gerrit.ovirt.org/18039
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c7ed4110cd8d45a3fb7a9cc2cd2dc07004e96b9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Giuseppe Vallarelli
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.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks add net bond breaking and resizing. .. Patch Set 5: I changed the wording of the commit msg, I gave a try to refactor common code across the three tests but I wasn't fully satisfied of the result it only meant to wrap some code in external functions and move the asserts around, which frankly I dislike. -- 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]: tests: setupNetworks add net bond breaking and resizing.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks add net bond breaking and resizing. .. Patch Set 5: Verified+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]: macspoof hooks: new hook script to enable macspoof filtering...
Giuseppe Vallarelli has posted comments on this change.
Change subject: macspoof hooks: new hook script to enable macspoof filtering
per vnic.
..
Patch Set 8:
(2 comments)
File vdsm_hooks/macspoof/macspoof_vnic.py
Line 13:
Line 14: def removeMacSpoofingFilter(interface):
Line 15: for filterElement in interface.getElementsByTagName('filterref'):
Line 16: if isMacSpoofingFilter(filterElement):
Line 17: removeFilter(interface, filterElement)
Done
Line 18:
Line 19:
Line 20: def isMacSpoofingFilter(filterElement):
Line 21: """
Line 37: def main():
Line 38:
Line 39: if hooking.tobool(os.environ.get('ifacemacspoof')):
Line 40: domxml = hooking.read_domxml()
Line 41: interface = domxml.getElementsByTagName('interface')[0]
Done
Line 42: removeMacSpoofingFilter(interface)
Line 43: hooking.write_domxml(domxml)
Line 44:
Line 45:
--
To view, visit http://gerrit.ovirt.org/17678
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb3e7bec7ce400dddc6f4e649b7d21dfe5010631
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Federico Simoncelli
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Livnat Peer
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]: macspoof hooks: new hook script to enable macspoof filtering...
Giuseppe Vallarelli has posted comments on this change. Change subject: macspoof hooks: new hook script to enable macspoof filtering per vnic. .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.ovirt.org/17678 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb3e7bec7ce400dddc6f4e649b7d21dfe5010631 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli 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]: refactoring: replacing ifcfg dependency to storage with one ...
Giuseppe Vallarelli has posted comments on this change. Change subject: refactoring: replacing ifcfg dependency to storage with one from lib. .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/18217 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43b0552e30351c6ae8c3765ab863d1abad750f45 Gerrit-PatchSet: 1 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: 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]: refactoring: replacing ifcfg dependency to storage with one ...
Giuseppe Vallarelli has uploaded a new change for review.
Change subject: refactoring: replacing ifcfg dependency to storage with one
from lib.
..
refactoring: replacing ifcfg dependency to storage with one from lib.
Ifcfg is using execCmd from storage.misc module, this patch replaces it
with its equivalent from utils module.
Change-Id: I43b0552e30351c6ae8c3765ab863d1abad750f45
Signed-off-by: Giuseppe Vallarelli
---
M vdsm/netconf/ifcfg.py
1 file changed, 6 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/18217/1
diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index 41bbc1e..8fafbd0 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -33,7 +33,6 @@
from neterrors import ConfigNetworkError
from netmodels import Nic, Bridge
from sourceRoute import DynamicSourceRoute
-from storage.misc import execCmd
from vdsm import constants
from vdsm import netinfo
from vdsm import utils
@@ -104,7 +103,7 @@
DynamicSourceRoute.addInterfaceTracking(bridge)
ifdown(bridge.name)
self._removeSourceRoute(bridge)
-execCmd([constants.EXT_BRCTL, 'delbr', bridge.name])
+utils.execCmd([constants.EXT_BRCTL, 'delbr', bridge.name])
self.configApplier.removeBridge(bridge.name)
if bridge.port:
bridge.port.remove()
@@ -191,7 +190,7 @@
mounts = open('/proc/mounts').read()
if ' /config ext3' in mounts and ' %s ext3' % filename in mounts:
-execCmd([constants.EXT_UMOUNT, '-n', filename])
+utils.execCmd([constants.EXT_UMOUNT, '-n', filename])
utils.rmFile(filename)
logging.debug("Removed file %s", filename)
@@ -348,8 +347,8 @@
def _persistentBackup(cls, filename):
""" Persistently backup ifcfg-* config files """
if os.path.exists('/usr/libexec/ovirt-functions'):
-execCmd([constants.EXT_SH, '/usr/libexec/ovirt-functions',
-'unmount_config', filename])
+utils.execCmd([constants.EXT_SH, '/usr/libexec/ovirt-functions',
+ 'unmount_config', filename])
logging.debug("unmounted %s using ovirt", filename)
(dummy, basename) = os.path.split(filename)
@@ -657,14 +656,14 @@
def ifdown(iface):
"Bring down an interface"
-rc, out, err = execCmd([constants.EXT_IFDOWN, iface], raw=True)
+rc, out, err = utils.execCmd([constants.EXT_IFDOWN, iface], raw=True)
return rc
def ifup(iface, async=False):
"Bring up an interface"
def _ifup(netIf):
-rc, out, err = execCmd([constants.EXT_IFUP, netIf], raw=False)
+rc, out, err = utils.execCmd([constants.EXT_IFUP, netIf], raw=False)
if rc != 0:
# In /etc/sysconfig/network-scripts/ifup* the last line usually
--
To view, visit http://gerrit.ovirt.org/18217
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I43b0552e30351c6ae8c3765ab863d1abad750f45
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli
___
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
Giuseppe Vallarelli has posted comments on this change. Change subject: netconf: Add dhcp support for iproute2 configurator .. Patch Set 5: Code-Review+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: 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: 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
Giuseppe Vallarelli has posted comments on this change. Change subject: netconf: Add config option for network configurator .. Patch Set 1: Code-Review+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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mark Wu Gerrit-Reviewer: Giuseppe Vallarelli 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]: [3/3] Use fixed getVlanDevice() and getVlanID().
Giuseppe Vallarelli has posted comments on this change. Change subject: [3/3] Use fixed getVlanDevice() and getVlanID(). .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/18182 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9af01e25e1257c18e6b17c4c3c5cb74dc535947a Gerrit-PatchSet: 2 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: Giuseppe Vallarelli 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]: [2/3] Add "vlanid" to VLAN information
Giuseppe Vallarelli has posted comments on this change. Change subject: [2/3] Add "vlanid" to VLAN information .. Patch Set 2: Verified+1 Code-Review+1 I've verified the patch series (3) by running the functional tests and all tests are green. -- To view, visit http://gerrit.ovirt.org/18181 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c83a7639bd4219771df0c7b3dd41b8f9806e9f1 Gerrit-PatchSet: 2 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: Giuseppe Vallarelli 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]: [1/3] Fix getVlanID() and getVlanDevice().
Giuseppe Vallarelli has posted comments on this change. Change subject: [1/3] Fix getVlanID() and getVlanDevice(). .. Patch Set 2: Verified+1 Except for Toni's comment related to code arrangement. I've verified the patch series (3) by running the functional tests and all tests are green. -- To view, visit http://gerrit.ovirt.org/18180 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieab0f44cf4c4f6f16f81435cb732ccacf57b0767 Gerrit-PatchSet: 2 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: Giuseppe Vallarelli 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]: [3/3] Use fixed getVlanDevice() and getVlanID().
Giuseppe Vallarelli has posted comments on this change. Change subject: [3/3] Use fixed getVlanDevice() and getVlanID(). .. Patch Set 2: Verified+1 I've verified the patch series (3) by running the functional tests and all tests are green. -- To view, visit http://gerrit.ovirt.org/18182 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9af01e25e1257c18e6b17c4c3c5cb74dc535947a Gerrit-PatchSet: 2 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: Giuseppe Vallarelli 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]: Unified network persistence [3/3] - Restore network configur...
Giuseppe Vallarelli 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 40: """
Line 41: nets = {}
Line 42: bonds = {}
Line 43: for netFile in os.listdir(netinfo.NET_CONF_PERS_DIR):
Line 44: nets[os.path.basename(netFile)] = json.load(open(netFile))
Thanks for the clarification json should be okay (no <> thanks ;-))
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)
--
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]: Unified network persistence [3/3] - Restore network configur...
Giuseppe Vallarelli 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 27: from vdsm import netinfo Line 28: from configNetwork import setupNetworks Line 29: Line 30: Line 31: def ifcfg_restoration(): Have a look to the comments made to the first patch series, perhaps it will be more clear. Line 32: configWriter = ifcfg.ConfigWriter() Line 33: configWriter.restorePersistentBackup() Line 34: Line 35: -- 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]: Add vlanid to vlans information
Giuseppe Vallarelli has posted comments on this change. Change subject: Add vlanid to vlans information .. Patch Set 4: Except for the additions needed as pointed by Toni the code looks okay to me. -- To view, visit http://gerrit.ovirt.org/18148 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If94dfd94c99272f6cb7dbc3d57cca78b05712119 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: 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
Giuseppe Vallarelli has posted comments on this change. Change subject: netmodels: Improve getIpConfig using namedtuple .. Patch Set 2: Code-Review+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]: Introducing hidden_vlans configurable.
Giuseppe Vallarelli has posted comments on this change. Change subject: Introducing hidden_vlans configurable. .. Patch Set 3: Thanks for the patch Amador. -- 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: 3 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: Giuseppe Vallarelli 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]: Introducing hidden_vlans configurable.
Giuseppe Vallarelli has posted comments on this change. Change subject: Introducing hidden_vlans configurable. .. Patch Set 3: Verified+1 Code-Review+1 Verified creating a couple of vlans: ip link add link em2 name em2.10-awesome type vlan id 10 ip link add link em2 name em2.amazing type vlan id 100 Updating vdsm.conf in etc with: hidden_vlans=em2.*-* vdsClient 0 getVdsCaps shows only em2.amazing. -- 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: 3 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: Giuseppe Vallarelli 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
Giuseppe Vallarelli has posted comments on this change.
Change subject: netmodels: Improve getIpConfig using namedtuple
..
Patch Set 1: Code-Review+1
(1 comment)
Thanks, looks good only a minor comment.
File vdsm/netmodels.py
Line 29: import neterrors as ne
Line 30:
Line 31:
Line 32: class NetDevice(object):
Line 33: _ipConfig = namedtuple('ipConfig', 'ipaddr netmask gateway
bootproto \
Here you can use ['ipConfig', 'ipaddr'...] instead of the string to avoid the \
Line 34: async defaultRoute')
Line 35:
Line 36: def __init__(self, name, configurator, ipconfig=None, mtu=None):
Line 37: self.name = name
--
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: 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: Simplify addSourceRoute arguments
Giuseppe Vallarelli has posted comments on this change. Change subject: netconf: Simplify addSourceRoute arguments .. Patch Set 1: Code-Review+1 Thanks ;-) -- 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: 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]: Introducing hidden_vlans configurable
Giuseppe Vallarelli has posted comments on this change.
Change subject: Introducing hidden_vlans configurable
..
Patch Set 2: Code-Review-1
(1 comment)
File lib/vdsm/netinfo.py
Line 140:
Line 141:
Line 142: def vlans():
Line 143: hidden_vlans = config.get('vars', 'hidden_vlans').split(',')
Line 144: return [b.split('/')[-1] for b in iglob('/sys/class/net/*.*')
Instead of b.split('/')[-1] can you use os.path.basename(b) ?
Line 145: if not _match_name(b.split('/')[-1], hidden_vlans)]
Line 146:
Line 147:
Line 148: def bridges():
--
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: 2
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: Giuseppe Vallarelli
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]: macspoof hooks: new hook script to enable macspoof filtering...
Giuseppe Vallarelli has posted comments on this change. Change subject: macspoof hooks: new hook script to enable macspoof filtering per vnic. .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.ovirt.org/17678 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb3e7bec7ce400dddc6f4e649b7d21dfe5010631 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli 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]: Unified network persistence [3/3] - Restore network configur...
Giuseppe Vallarelli has posted comments on this change.
Change subject: Unified network persistence [3/3] - Restore network
configuration
..
Patch Set 2: Code-Review-1
(2 comments)
File vdsm/vdsm-restore-net-config
Line 27: from vdsm import netinfo
Line 28: from configNetwork import setupNetworks
Line 29:
Line 30:
Line 31: def ifcfg_restoration():
Of course I don't agree on having this behaviour in a different script.
We got now if config.get('vars', 'persistence') == 'unified' 6..
Including the ones in the first patch of the series.
Line 32: configWriter = ifcfg.ConfigWriter()
Line 33: configWriter.restorePersistentBackup()
Line 34:
Line 35:
Line 40: """
Line 41: nets = {}
Line 42: bonds = {}
Line 43: for netFile in os.listdir(netinfo.NET_CONF_PERS_DIR):
Line 44: nets[os.path.basename(netFile)] = json.load(open(netFile))
By looking at the usage of the json module we only use json for convenience
there are no real requirements for the json format, so at this point why don't
we use the pickle module which serve for this exact purpose.
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)
--
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]: Unified network persistence [1/3] - Save running config
Giuseppe Vallarelli has posted comments on this change.
Change subject: Unified network persistence [1/3] - Save running config
..
Patch Set 13: Code-Review-1
(4 comments)
See comments.
File vdsm/configNetwork.py
Line 157: dirPath = os.path.dirname(path)
Line 158: if not os.path.exists(dirPath):
Line 159: os.makedirs(dirPath)
Line 160: with open(path, 'w') as configurationFile:
Line 161: json.dump(config, configurationFile)
Why do we use json to dump the config ?
Line 162:
Line 163:
Line 164: def _removeRunningConfig(path):
Line 165: try:
Line 168: if ose.errno != errno.ENOENT:
Line 169: raise
Line 170:
Line 171:
Line 172: def _addRunningConfig(func):
Are we going to have unit tests for this ? If so I suggest to break it down by
separating its concerns.
Line 173: spec = inspect.getargspec(func)
Line 174:
Line 175: @wraps(func)
Line 176: def wrapped(*args, **kwargs):
Line 174:
Line 175: @wraps(func)
Line 176: def wrapped(*args, **kwargs):
Line 177: saveRunningConf = config.get('vars', 'persistence') ==
'unified'
Line 178: if saveRunningConf:
I've counted in this patch 5 ifs asking the same thing if it's unified
persistence of not.
This is not abstracting complexity but embracing it and spread it out in
different places.
What about creating 2 objects with same interface one for the available
persistence model and the other one for the unified and move all the
persistence details over there, the choice of which persistence model to
provide should be done only in one place both objects will have the same
interface. So it will not matter the details of how they do the persistence but
that they are able to persist the configuration. TL;DR Duck Typing - it should
not matter what type of persistence is but what you can do with it.
Is it possible to do that? If not what are the limitations? My feeling is that
we are adding complexity to already complex code which is going to bite us.
Line 179: attrs = kwargs.copy()
Line 180: attrs.update(dict(zip(spec.args, args)))
Line 181: try:
Line 182: ret = func(*args, **kwargs)
File vdsm/netconf/ifcfg.py
Line 232: os.chown(backup, vdsm_uid, 0)
Line 233: logging.debug("Persistently backed up %s "
Line 234: "(until next 'set safe config')", backup)
Line 235:
Line 236: def _networkBackup(self, network):
My choice would be to provide the configurator by composition one of the object
able to persist the configuration.
Line 237: self._atomicNetworkBackup(network)
Line 238: if config.get('vars', 'persistence') != 'unified':
Line 239: self._persistentNetworkBackup(network)
Line 240:
--
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: 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: 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
Giuseppe Vallarelli has posted comments on this change.
Change subject: Don't down bonds and nics when adding vlan or resizing
..
Patch Set 9: Code-Review-1
(5 comments)
Very minor refinements.
File lib/vdsm/netinfo.py
Line 218: return open(BONDING_SLAVES % bonding).readline().split()
Line 219:
Line 220:
Line 221: def bondOpts(bonding):
Line 222: """ Returns a dictionary of bond mode name and a values
iterable."""
bond mode and which values ? Since you added the docstring better be complete :)
Thanks I appreciate.
Line 223: opts = {}
Line 224: for path in iglob(BONDING_OPTS % bonding):
Line 225: with open(path) as optFile:
Line 226: opts[os.path.basename(path)] =
optFile.read().rstrip().split(' ')
Line 278: try:
Line 279: # nics() filters out OS devices (bonds, vlans, bridges)
Line 280: # operstat() filters out down/disabled nics
Line 281: # virtio is a valid device, but doesn't support speed
Line 282: if dev in nics() and operstate(dev) == OPERSTATE_UP and \
Nitpick: instead of \ use parenthesis for a longer explanation have a look at
the link below:
http://www.python.org/dev/peps/pep-0008/#maximum-line-length
Line 283: not isvirtio(dev):
Line 284: # the device may have been disabled/downed after checking
Line 285: # so we validate the return value as sysfs may return
Line 286: # special values to indicate the device is down/disabled
File tests/functional/networkTests.py
Line 47: restoreNetConfig()
Line 48:
Line 49:
Line 50: @contextmanager
Line 51: def nonChangingOperstate(device):
why in networkTest module and not utils?
what about constantOperstate instead of nonChaningOperstate?
Line 52: def changed(dev, changes):
Line 53: status = operstate(dev)
Line 54: while not done:
Line 55: newState = operstate(dev)
File vdsm/netmodels.py
Line 80: super(Nic, self).__init__(name, configurator, ipconfig,
Line 81: mtu=max(mtu, netinfo.getMtu(name)))
Line 82:
Line 83: def configure(self, **opts):
Line 84: if not self.vlan or \
Parenthesis instead of \ :
http://www.python.org/dev/peps/pep-0008/#maximum-line-length
Line 85: netinfo.operstate(self.name) != netinfo.OPERSTATE_UP:
Line 86: self.configurator.configureNic(self, **opts)
Line 87:
Line 88: def remove(self):
Line 180: def __repr__(self):
Line 181: return 'Bond(%s: %r)' % (self.name, self.slaves)
Line 182:
Line 183: def configure(self, **opts):
Line 184: # When the bond is up and we are not changing the
configuration that
At this point I think you can turn the comment into a docstring.
Line 185: # is already applied in any way, we can skip the configuring.
Line 186: if not(self.vlan and
Line 187:self.name in netinfo.bondings() and
Line 188:netinfo.operstate(self.name) == netinfo.OPERSTATE_UP
and
--
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: 9
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]: macspoof hooks: new hook script to enable macspoof filtering...
Giuseppe Vallarelli has posted comments on this change. Change subject: macspoof hooks: new hook script to enable macspoof filtering per vnic. .. Patch Set 7: Hi Assaf thanks for feedback. My understanding is that the hook scripts provided are self contained and not extended, they serve for a single purpose. There has been a similar concern regarding the openstack hooks for the common openstack constants, the adopted solution was to copy over the common file in the different hooks folder. Now I think that a good idea is to have only a single script copied over the 2 different hooks folder with the work done by make. So instead of 2 files one file copied the 2 different locations. I'll provide another patch. -- To view, visit http://gerrit.ovirt.org/17678 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb3e7bec7ce400dddc6f4e649b7d21dfe5010631 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli 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]: tests: setupNetworks add net bond breaking and resizing.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks add net bond breaking and resizing. .. Patch Set 4: Hi Mark I know that there's a little bit of duplication, however extracting the common setup code would mean only add another level of indirection (considering that we use a vdsproxy object) and would make the test less readable. I prefer to have tests where it easy to identify those 3 steps: *input preparation *code exercise *related asserts on result -- 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
Change in vdsm[master]: tests: setupNetworks convertVlanNetBridgeness, Mtus
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks convertVlanNetBridgeness, Mtus .. Patch Set 3: Verified+1 Rebased. -- To view, visit http://gerrit.ovirt.org/17866 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib8723b066228729807f7ab46c9fabad3ebff128b Gerrit-PatchSet: 3 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: 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]: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists
Giuseppe Vallarelli has posted comments on this change. Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists .. Patch Set 1: (1 comment) Added a comment for cleaning up added rules/routes. File tests/functional/networkTests.py Line 419: srcDevice=nic)] Line 420: for rule in rules: Line 421: self.assertFalse(ruleExists(rule)) Line 422: ruleAdd(rule) Line 423: self.assertTrue(ruleExists(rule)) if an assertion fails how do we cleanup the added rule? Line 424: ruleDel(rule) Line 425: self.assertFalse(ruleExists(rule)) Line 426: Line 427: @RequireDummyMod -- To view, visit http://gerrit.ovirt.org/18040 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf7c28e85f53f51879750308f3a9f93fc635cf33 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli 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]: Functional Tests [3/3]: Added static source routing function...
Giuseppe Vallarelli has posted comments on this change. Change subject: Functional Tests [3/3]: Added static source routing functional test .. Patch Set 1: (1 comment) File tests/functional/networkTests.py Line 28: Line 29: from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \ Line 30: ruleExists, Route, Rule Line 31: Line 32: from vdsm.netinfo import prefix2netmask we're usually hiding netinfo and providing all netinfo services from utils. Line 33: Line 34: Line 35: NETWORK_NAME = 'test-network' Line 36: VLAN_ID = '27' -- To view, visit http://gerrit.ovirt.org/18041 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4188e6ee5daea12fbd35cb8e1c87782e9605c7cd Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli 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]: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists
Giuseppe Vallarelli has posted comments on this change. Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists .. Patch Set 1: Code-Review-1 (4 comments) It's pretty good very minor comments. File lib/vdsm/ipwrapper.py Line 246: Line 247: Line 248: def _validate(validator, entry): Line 249: try: Line 250: validator.fromText(entry) I don't understand this choice of having fromText raising an exception and using the exception as a signal of meaning entry valid or invalid. fromText should return already this boolean instead of having clients of this method wrapping it in try..except. Line 251: except: Line 252: return False Line 253: else: Line 254: return True File tests/functional/networkTests.py Line 21: from testrunner import (VdsmTestCase as TestCaseBase, Line 22: expandPermutations, permutations) Line 23: from testValidation import RequireDummyMod, ValidateRunningAsRoot Line 24: Line 25: import dummyUtils I found the Utils word a bit overused, why not simply dummy we already have an utils module in tests/functional Line 26: from dummyUtils import dummyIf Line 27: from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy Line 28: Line 29: from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \ Line 25: import dummyUtils Line 26: from dummyUtils import dummyIf Line 27: from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy Line 28: Line 29: from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \ Don't use \ use parenthesis instead (...) Line 30: ruleExists, Route, Rule Line 31: Line 32: Line 33: NETWORK_NAME = 'test-network' Line 410: @ValidateRunningAsRoot Line 411: def testRuleExists(self): Line 412: with dummyIf(1) as nics: Line 413: nic = nics[0] Line 414: dummyUtils.setIP(nic, IP_ADDRESS + '/' + IP_CIDR) you're creating IP_ADDRESS '/' + IP_CIDR twice across the two tests can we do only once? You can set up vars shared by more tests in the setUp. Line 415: dummyUtils.setLinkUp(nic) Line 416: Line 417: rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE), Line 418: Rule(destination=IP_NETWORK_AND_CIDR, table=IP_TABLE, -- To view, visit http://gerrit.ovirt.org/18040 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf7c28e85f53f51879750308f3a9f93fc635cf33 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Giuseppe Vallarelli 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]: Functional Tests [1/3]: Added module for dummy nics
Giuseppe Vallarelli has posted comments on this change.
Change subject: Functional Tests [1/3]: Added module for dummy nics
..
Patch Set 1: Code-Review-1
(2 comments)
Look at the comments.
File tests/functional/Makefile.am
Line 20:
Line 21: vdsmfunctestsdir = ${vdsmtestsdir}/functional
Line 22:
Line 23: dist_vdsmfunctests_PYTHON = \
Line 24: dummyUtils.py \
Alignment?
Line 25:momTests.py \
Line 26:networkTests.py \
Line 27:sosPluginTests.py \
Line 28:supervdsmFuncTests.py \
File tests/functional/dummyUtils.py
Line 75: def setLinkDown(dummyName):
Line 76: _setLinkState(dummyName, 'down')
Line 77:
Line 78:
Line 79: def _setLinkState(dummyName, state):
You can create 2 class constants STATE_UP, STATE_DOWN and provide them to
setLinkState. Within setLinkState you can use an assert to verify that the link
state provided if one of the allowed ones (the first thing to do).
assert state in (STATE_UP, STATE_DOWN).
when you invoke the function you will use something like:
setLinkState('dummy0', STATE_UP)
These 2 functions setLinkUp and setLinkUp don't do any useful work if not
providing the state type.
Line 80: activateDevice = [ip.cmd, 'link', 'set', 'dev', dummyName, state]
Line 81: rc, _, _ = utils.execCmd(activateDevice, sudo=True)
Line 82: if rc == SUCCESS:
Line 83: return
--
To view, visit http://gerrit.ovirt.org/18039
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c7ed4110cd8d45a3fb7a9cc2cd2dc07004e96b9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Assaf Muller
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Assaf Muller
Gerrit-Reviewer: Giuseppe Vallarelli
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]: macspoof hooks: new hook script to enable macspoof filtering...
Giuseppe Vallarelli has posted comments on this change. Change subject: macspoof hooks: new hook script to enable macspoof filtering per vnic. .. Patch Set 7: Verified+1 Verified as before and also by adding a vnic with ifacemacspoof property set to true eliminates the filter element (in that case the before_nic_hotplug kicks in. -- To view, visit http://gerrit.ovirt.org/17678 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb3e7bec7ce400dddc6f4e649b7d21dfe5010631 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli 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]: Don't down bonds and nics when adding vlan or resizing
Giuseppe Vallarelli has posted comments on this change. Change subject: Don't down bonds and nics when adding vlan or resizing .. Patch Set 8: 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: 8 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]: macspoof hooks: new hook script to enable macspoof filtering...
Giuseppe Vallarelli has posted comments on this change. Change subject: macspoof hooks: new hook script to enable macspoof filtering per vnic. .. Patch Set 6: -Verified Dan good feedback I think I should provide the same script for before_nic_hotplug. -- To view, visit http://gerrit.ovirt.org/17678 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb3e7bec7ce400dddc6f4e649b7d21dfe5010631 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Assaf Muller Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Federico Simoncelli 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]: vdsm: Reboot capability for VM
Giuseppe Vallarelli has posted comments on this change.
Change subject: vdsm: Reboot capability for VM
..
Patch Set 15:
(6 comments)
Generally it looks good, I appreciate your good will of adding docstrings you
might think to do some strategic changes, I've suggested quite a few.
File lib/vdsm/utils.py
Line 916:
Line 917: Callback = namedtuple('Callback', ('func', 'timeout', 'args',
'kwargs'))
Line 918:
Line 919:
Line 920: class CallbackChain(object):
I've seen that you use this class as a base one. Before using inheritance I try
to have more than one class usually 3 or more simply because you see in
different cases what it's really needed to factor out in a base class. You can
find an example of this principle in the netmodels module with the NetDevice
base class and related children.
For now I would go for a single class than in case we have something similar we
might think of a base class, what do you think?
Line 921: """
Line 922: Encapsulates the pattern of calling multiple alternative functions
Line 923: (each with a given timeout) to achieve some action.
Line 924:
File vdsm/API.py
Line 586: return v.setTicket(password, ttl, existingConnAction, params)
Line 587:
Line 588: def shutdown(self, delay=None, message=None, reboot=False,
force=False):
Line 589: """
Line 590: Shut a VM down politely.
This sentence doesn't hold anymore. It's a little more complicated also I've
never seen code that is polite :)
Line 591:
Line 592: :param message: message to be shown to guest user before
shutting down
Line 593: his machine.
Line 594: :param delay: grace period (seconds) to let guest user close
his
Line 593: his machine.
Line 594: :param delay: grace period (seconds) to let guest user close
his
Line 595: applications.
Line 596: :param reboot: True if reboot is desired
Line 597:False for shutdown
The only way to understand how shutdown should be used is by reading the
docstring, what I'm saying is that the API doesn't convey the sementic.
Now the original sin here is that we have the same method to do shutdown and
reboot even if they have common implementation. I guess we cannot break in 2
methods.
The alternative is to have another distinct parameter one for reboot and
another one for the shutdown - now the challenge might be to find a meaningful
name having method name called 'shutdown'.
Having reboot that means reboot for True and False for shutdown is a huge pity
for me.
Line 598: :param force: True if shutdown/reboot desired by any means
necessary
Line 599: (forceful reboot/shutdown if all graceful
methods fail)
Line 600: """
Line 601: try:
File vdsm/vm.py
Line 1651: self.vm._guestEventTime = time.time()
Line 1652: self.vm._guestEvent = self.info['guestEvent']
Line 1653:
Line 1654: def _initCallbacks(self):
Line 1655: sys_timeout = config.getint('vars', 'sys_shutdown_timeout')
The convention for variable names is lowerCamelCase, no more underscores. We
only use them for some NICE_CONSTANTS :)
Line 1656: agent_timeout = int(self.timeout) + sys_timeout
Line 1657:
Line 1658: # first try agent
Line 1659: if self.vm.guestAgent and self.vm.guestAgent.isResponsive():
Line 1655: sys_timeout = config.getint('vars', 'sys_shutdown_timeout')
Line 1656: agent_timeout = int(self.timeout) + sys_timeout
Line 1657:
Line 1658: # first try agent
Line 1659: if self.vm.guestAgent and self.vm.guestAgent.isResponsive():
According to the availability of the guestAgent you're building by wiring
different callables 2 different types of VmPowerDown.
What I suggest is to create a builder function which creates the 2 different
powerdowns by providing at the constructor level all the needed callables
and you move over there this choice. Right now you're hardcoding in this class
the specific callables when they can be passed over.
Line 1660: self._addCallback(self.info['guestAgent'], agent_timeout,
Line 1661: self.timeout, self.message)
Line 1662: else:
Line 1663: self._addCallback(self.info['qemuAgent'], agent_timeout)
Line 2408: finally:
Line 2409: if not guestCpuLocked:
Line 2410: self._guestCpuLock.release()
Line 2411:
Line 2412: def shutdown(self, timeout, message, reboot=False, force=False):
A small consideration I think we should put vm.py on a diet otherwise it will
continue to grow and with that our sufferings.
Line 2413: if reboot:
Line 2414: #
Change in vdsm[master]: tests : setupNetworks resizeBond, remove param validation.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests : setupNetworks resizeBond, remove param validation. .. Patch Set 2: Verified+1 Cherrypicked on top of the updated dependency: Waiting for http://gerrit.ovirt.org/#/c/17491/ in order to add another assertion with the bond mode. -- To view, visit http://gerrit.ovirt.org/17922 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6471a50152314e53e86e9f8e2e1359059fff17f0 Gerrit-PatchSet: 2 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: 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]: test : delNetwork with bond accumulation.
Giuseppe Vallarelli has posted comments on this change. Change subject: test : delNetwork with bond accumulation. .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/17944 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12ebeeaff8949e6020dcfcaebaa94f3641b622d5 Gerrit-PatchSet: 1 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-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: test : delNetwork with bond accumulation.
Giuseppe Vallarelli has uploaded a new change for review.
Change subject: test : delNetwork with bond accumulation.
..
test : delNetwork with bond accumulation.
Added functional test covering behaviour of delNetwork
when creating and deleting a bond.
Change-Id: I12ebeeaff8949e6020dcfcaebaa94f3641b622d5
Signed-off-by: Giuseppe Vallarelli
---
M tests/functional/networkTests.py
1 file changed, 19 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/44/17944/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 9e35120..2bc1b30 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -393,3 +393,22 @@
nics=nics,
opts={'bridged': bridged})
self.assertEqual(status, neterrors.ERR_BAD_BRIDGE, msg)
+
+@cleanupNet
+@RequireDummyMod
+@ValidateRunningAsRoot
+def testDelNetworkBondAccumulation(self):
+with dummyIf(1) as nics:
+for bigBond in ('bond555', 'bond666', 'bond777'):
+status, msg = self.vdsm_net.addNetwork(NETWORK_NAME, VLAN_ID,
+ bigBond, nics)
+
+self.assertEqual(status, SUCCESS, msg)
+
+self.assertTrue(self.vdsm_net.bondExists(bigBond, nics))
+
+status, msg = self.vdsm_net.delNetwork(NETWORK_NAME)
+
+self.assertEqual(status, SUCCESS, msg)
+
+self.assertFalse(self.vdsm_net.bondExists(bigBond, nics))
--
To view, visit http://gerrit.ovirt.org/17944
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I12ebeeaff8949e6020dcfcaebaa94f3641b622d5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Vallarelli
___
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.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks add net bond breaking and resizing. .. Patch Set 3: Verified+1 Cherrypicked on top of the updated depedency. -- 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: 3 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: 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: setupNetworks compatibility bond and nic.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks compatibility bond and nic. .. Patch Set 5: Verified+1 Simply a rebase on top of master. -- To view, visit http://gerrit.ovirt.org/17621 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If4a497d82723937dbecd14d42fc2c176c4c05bd2 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: 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: setupNetworks with invalid params.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks with invalid params. .. Patch Set 6: Verified+1 Cherrypicked on top of the updated dependency. -- To view, visit http://gerrit.ovirt.org/17435 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic77b3a9c0d84e38e8b9060191b138c3e09104dc6 Gerrit-PatchSet: 6 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: 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: setupNetworks add one or more vlans.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks add one or more vlans. .. Patch Set 4: This can be merged. -- To view, visit http://gerrit.ovirt.org/17432 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62fa6de1faa7d4e886c45361a001b0009fd92502 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: 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: bondHwAddress, safeNetworkConfig, volatileConfig
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: bondHwAddress, safeNetworkConfig, volatileConfig .. Patch Set 3: Patchset 3 cherrypicking on top of the updated dependency. This patch depends on the fix submitted here http://gerrit.ovirt.org/#/c/17941/ -- 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: 3 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: 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: adding missing cleanupNet and refined testQosNetwork.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: adding missing cleanupNet and refined testQosNetwork. .. Patch Set 1: (1 comment) File tests/functional/networkTests.py Line 340: self.assertEqual(qos['qosOutbound'], qosOutbound) Line 341: Line 342: status, msg = self.vdsm_net.delNetwork(NETWORK_NAME) Line 343: Line 344: self.assertEqual(status, SUCCESS, msg) Currently we're not doing that in many other different places. I will postpone it to a follow up a patch. Line 345: Line 346: @cleanupNet Line 347: @permutations([[True], [False]]) Line 348: @RequireDummyMod -- To view, visit http://gerrit.ovirt.org/17941 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2fa9619a38acdfcc19f889329d375f868a3e921e Gerrit-PatchSet: 1 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: 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: adding missing cleanupNet and refined testQosNetwork.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: adding missing cleanupNet and refined testQosNetwork. .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.ovirt.org/17941 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2fa9619a38acdfcc19f889329d375f868a3e921e Gerrit-PatchSet: 1 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: 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: adding missing cleanupNet and refined testQosNetwork.
Giuseppe Vallarelli has uploaded a new change for review. Change subject: tests: adding missing cleanupNet and refined testQosNetwork. .. tests: adding missing cleanupNet and refined testQosNetwork. Some additional cleanupNet were necessary on some tests which might, on possible failure, make the env dirty. Added the deletion of network created in the testQosNetwork. Change-Id: I2fa9619a38acdfcc19f889329d375f868a3e921e Signed-off-by: Giuseppe Vallarelli --- M tests/functional/networkTests.py 1 file changed, 7 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/41/17941/1 diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 9e35120..61b88a8 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -320,6 +320,7 @@ nics=nics) self.assertEqual(status, SUCCESS, msg) +@cleanupNet @RequireDummyMod @ValidateRunningAsRoot def testQosNetwork(self): @@ -338,6 +339,11 @@ self.assertEqual(qos['qosInbound'], qosInbound) self.assertEqual(qos['qosOutbound'], qosOutbound) +status, msg = self.vdsm_net.delNetwork(NETWORK_NAME) + +self.assertEqual(status, SUCCESS, msg) + +@cleanupNet @permutations([[True], [False]]) @RequireDummyMod @ValidateRunningAsRoot @@ -360,6 +366,7 @@ status, msg = self.vdsm_net.delNetwork(NETWORK_NAME) self.assertEqual(status, SUCCESS, msg) +@cleanupNet @permutations([[True], [False]]) @RequireDummyMod @ValidateRunningAsRoot -- To view, visit http://gerrit.ovirt.org/17941 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2fa9619a38acdfcc19f889329d375f868a3e921e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tests: setupNetworks convertVlanNetBridgeness, Mtus
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks convertVlanNetBridgeness, Mtus .. Patch Set 2: Verified+1 Fixed the 'stylistic' issue no intermediate lists are now present. -- To view, visit http://gerrit.ovirt.org/17866 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib8723b066228729807f7ab46c9fabad3ebff128b Gerrit-PatchSet: 2 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: 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]: Refactored xmlrpcTests to allow for more complex tests using...
Giuseppe Vallarelli has posted comments on this change. Change subject: Refactored xmlrpcTests to allow for more complex tests using running VM .. Patch Set 1: -Verified Code-Review-1 -- To view, visit http://gerrit.ovirt.org/17893 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I683c9e056137e7a189815bf6be2eb79ee80994cf Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Vinzenz Feenstra 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]: Refactored xmlrpcTests to allow for more complex tests using...
Giuseppe Vallarelli has posted comments on this change.
Change subject: Refactored xmlrpcTests to allow for more complex tests using
running VM
..
Patch Set 1: Verified-1
(4 comments)
Hi Martin good work, a few fixes are needed.
Cheers, Giuseppe
File tests/functional/xmlrpcTests.py
Line 73: return method(self, *args, **kwargs)
Line 74: return wrapped
Line 75:
Line 76:
Line 77: class runningVm:
UpperCamelCase for a ClassName. We use new-style classes you should inherit
from object so: RunningVm(object).
Line 78: kernelArgsDistro = {
Line 79: 'fedora': 'rd.break=cmdline rd.shell rd.skipfsck',
Line 80: 'rhel': 'rd.break=cmdline rd.shell rd.skipfsck'}
Line 81:
Line 74: return wrapped
Line 75:
Line 76:
Line 77: class runningVm:
Line 78: kernelArgsDistro = {
http://www.python.org/dev/peps/pep-0008/#constants KERNEL_ARGS_DISTRO
Read the pep8 document. We have slightly different conventions for var names
methods, functions: lowerCamelCase that can be prefixed with _ meaning private
but all the rest in that document applies.
Line 79: 'fedora': 'rd.break=cmdline rd.shell rd.skipfsck',
Line 80: 'rhel': 'rd.break=cmdline rd.shell rd.skipfsck'}
Line 81:
Line 82: def __init__(self, testbase, vmParams={}, timeout=65,
distro='fedora'):
Line 104: self.testbase = testbase
Line 105: self.timeout = timeout
Line 106:
Line 107: def __enter__(self):
Line 108: tb = self.testbase
self.testbase.assert... If it's over 80 chars you can break the line at the
open parenthesis.
Line 109: tb.assertVdsOK(self.vds.create(self.template))
Line 110: tb.retryAssert(partial(tb.assertVMAndGuestUp, self.vmid),
Line 111:timeout=self.timeout)
Line 112: return self
Line 184: 'error code: %s, message: %s' %
(vdsResult['status']['code'],
Line 185:
vdsResult['status']['message']))
Line 186:
Line 187: @skipNoKVM
Line 188: def testStartEmptyVM(self):
I'm not a big fan of this style of testing usually I prefer something in the
lines of:
1. test setup - you prepare the test fixture.
2. exercise the code under test by passing the test fixture.
3. apply assertions on the result code under test.
Have a look here: http://c2.com/cgi/wiki?ArrangeActAssert
I find tests written in this style much more readable.
Line 189: customization = {'vmId':
'----',
Line 190: 'memSize': '100', 'display': 'vnc',
Line 191: 'vmName': 'vdsm_testEmptyVM'}
Line 192:
--
To view, visit http://gerrit.ovirt.org/17893
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I683c9e056137e7a189815bf6be2eb79ee80994cf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik
Gerrit-Reviewer: Antoni Segura Puimedon
Gerrit-Reviewer: Giuseppe Vallarelli
Gerrit-Reviewer: Michal Skrivanek
Gerrit-Reviewer: Peter V. Saveliev
Gerrit-Reviewer: Vinzenz Feenstra
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 functional test for hotplugNic() call
Giuseppe Vallarelli has posted comments on this change. Change subject: Added functional test for hotplugNic() call .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.ovirt.org/17867 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2b5b4e3e5cf21e1f2f0ea39cd133ee7038aadeda Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Peter V. Saveliev Gerrit-Reviewer: Petr Šebek Gerrit-Reviewer: Vinzenz Feenstra 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: setupNetworks add one or more vlans.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks add one or more vlans. .. Patch Set 4: Verified+1 Updated patch based on feedback. -- To view, visit http://gerrit.ovirt.org/17432 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62fa6de1faa7d4e886c45361a001b0009fd92502 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: 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: setupNetworks add one or more vlans.
Giuseppe Vallarelli has posted comments on this change.
Change subject: tests: setupNetworks add one or more vlans.
..
Patch Set 3:
(2 comments)
File tests/functional/networkTests.py
Line 327: @ValidateRunningAsRoot
Line 328: def testSetupNetworksAddVlan(self, bridged):
Line 329: with dummyIf(1) as nics:
Line 330: with self.vdsm_net.pinger():
Line 331: nic = nics[0]
Done
Line 332: attrs = dict(vlan=VLAN_ID, nic=nic, bridged=bridged)
Line 333: status, msg =
self.vdsm_net.setupNetworks({NETWORK_NAME:
Line 334:attrs},
{}, {})
Line 335:
Line 361: with self.vdsm_net.pinger():
Line 362: status, msg = self.vdsm_net.setupNetworks(networks,
{}, {})
Line 363: self.assertEqual(status, SUCCESS, msg)
Line 364:
Line 365: networks = dict((vlan_net, {'remove': True})
Done
Line 366: for vlan_net, _ in NET_VLANS)
Line 367: for vlan_net, tag in NET_VLANS:
Line 368:
self.assertTrue(self.vdsm_net.networkExists(vlan_net,
Line 369:
bridged))
--
To view, visit http://gerrit.ovirt.org/17432
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I62fa6de1faa7d4e886c45361a001b0009fd92502
Gerrit-PatchSet: 3
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: 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 resizeBond, remove param validation.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests : setupNetworks resizeBond, remove param validation. .. Patch Set 1: -Verified Waiting for http://gerrit.ovirt.org/#/c/17491/ in order to add another assertion with the bond mode. -- To view, visit http://gerrit.ovirt.org/17922 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6471a50152314e53e86e9f8e2e1359059fff17f0 Gerrit-PatchSet: 1 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: 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: setupNetworks compatibility bond and nic.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks compatibility bond and nic. .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.ovirt.org/17621 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If4a497d82723937dbecd14d42fc2c176c4c05bd2 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: 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: setupNetworks compatibility bond and nic.
Giuseppe Vallarelli has posted comments on this change.
Change subject: tests: setupNetworks compatibility bond and nic.
..
Patch Set 3:
(1 comment)
File tests/functional/networkTests.py
Line 304:
Line 305: # Try to add additional VLANed bridged network,
should fail
Line 306: netNameVlanBridged = NETWORK_NAME + '-5'
Line 307: networks['vlan'] = '200'
Line 308: networks['bridged'] = 'True'
Sure, np.
Line 309: status, msg =
self.vdsm_net.setupNetworks({netNameVlanBridged:
Line 310:networks},
{}, {})
Line 311: self.assertTrue(status != SUCCESS, msg)
Line 312:
--
To view, visit http://gerrit.ovirt.org/17621
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4a497d82723937dbecd14d42fc2c176c4c05bd2
Gerrit-PatchSet: 3
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: 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.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks add net bond breaking and resizing. .. Patch Set 2: Verified+1 Slicing served! -- 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: 2 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: 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: setupNetworks compatibility bond and nic.
Giuseppe Vallarelli has posted comments on this change. Change subject: tests: setupNetworks compatibility bond and nic. .. Patch Set 3: Verified+1 New patch according to the review's feedback. -- To view, visit http://gerrit.ovirt.org/17621 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If4a497d82723937dbecd14d42fc2c176c4c05bd2 Gerrit-PatchSet: 3 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: 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: setupNetworks compatibility bond and nic.
Giuseppe Vallarelli has posted comments on this change.
Change subject: tests: setupNetworks compatibility bond and nic.
..
Patch Set 2:
(1 comment)
File tests/functional/networkTests.py
Line 304:
Line 305: # Try to add additional VLANed bridged network,
should fail
Line 306: netNameVlanBridged = NETWORK_NAME + '-5'
Line 307: networks['vlan'] = '200'
Line 308: networks['bridged'] = 'False'
Done
Line 309: status, msg =
self.vdsm_net.setupNetworks({netNameVlanBridged:
Line 310:networks},
{}, {})
Line 311: self.assertTrue(status != SUCCESS, msg)
Line 312:
--
To view, visit http://gerrit.ovirt.org/17621
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If4a497d82723937dbecd14d42fc2c176c4c05bd2
Gerrit-PatchSet: 2
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: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
