Ido Barkan has posted comments on this change. Change subject: net: introducing KernelConfig ......................................................................
Patch Set 4: (4 comments) https://gerrit.ovirt.org/#/c/41973/4/lib/vdsm/netconfpersistence.py File lib/vdsm/netconfpersistence.py: Line 33: # The persistent path is inside of an extra "persistence" dir in order to get Line 34: # oVirt Node to persist the symbolic links that are necessary for the Line 35: # atomic storage of running config into persistent config. Line 36: CONF_PERSIST_DIR = constants.P_VDSM_LIB + 'persistence/netconf/' Line 37: BONDING_MODES = { > Please add a Done Line 38: '0': 'balance-rr', '1': 'active-backup', '2': 'balance-xor', Line 39: '3': 'broadcast', '4': '802.3ad', '5': 'balance-tlb', '6': 'balance-alb' Line 40: } Line 41: Line 428: else: # isolated bridged network Line 429: return [] Line 430: Line 431: Line 432: def unicodize(d): > I'm not sure what is the motivation for this function. private: true. motivation: our persistance serialization is json based. let > d = {'foo':'bar'}. > json.loads(json.dumps(d)) == d > True is always true, but observe the left hand side of the equation: > print json.loads(json.dumps(d)) > {u'foo': u'bar'} Python will agree that: > {u'foo': u'bar'} == {'foo': 'bar'} > True But unfortunatly assertEquals method of the test framework refuses to swallow this. So unicodize is there to please the tests gods. https://gerrit.ovirt.org/#/c/41973/4/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 306: Line 307: def prefix2netmask(prefix): Line 308: if not 0 <= prefix <= 32: Line 309: raise ValueError('%s is not a valid prefix value. It must be between ' Line 310: '0 and 32' % prefix) > unrelated bugfix. It deserves its own backportable patch. Done Line 311: return socket.inet_ntoa( Line 312: struct.pack("!I", int('1' * prefix + '0' * (32 - prefix), 2))) Line 313: Line 314: https://gerrit.ovirt.org/#/c/41973/4/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 330: self.assertIn(bondName, netinfo.bondings) Line 331: self.assertEqual(netinfo.bondings[bondName]['active_slave'], '') Line 332: Line 333: def setupNetworks(self, *args, **kwargs): Line 334: status, msg = self.vdsm_net.setupNetworks(*args, **kwargs) > this local wrapper creates SO much graphical noise. I disagree. IMO it is 'counter-noise' since self.vdsm_net.setupNetworks is an annoying boilerplate. this was a good opportunity to kill it :). Stack issue fixed (nice catch!). I think I will add the calling code line in an unrelated patch. Line 335: self._assert_kernel_config_matches_running_config() Line 336: return status, msg Line 337: Line 338: def _assert_kernel_config_matches_running_config(self): -- To view, visit https://gerrit.ovirt.org/41973 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I79f1eb553a42f1398ad12aa1bc33522f8af30c79 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
