Change in vdsm[master]: netinfo, tool: Add a vdsm-tool to determine bonding defaults...

danken at redhat.com danken at redhat.com
Fri Sep 26 16:41:26 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: netinfo, tool: Add a vdsm-tool to determine bonding defaults offline
......................................................................


Patch Set 2: Code-Review-1

(8 comments)

http://gerrit.ovirt.org/#/c/33403/2//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2014-09-26 15:16:23 +0200
Line 4: Commit:     Ondřej Svoboda <osvoboda at redhat.com>
Line 5: CommitDate: 2014-09-26 17:44:28 +0200
Line 6: 
Line 7: netinfo, tool: Add a vdsm-tool to determine bonding defaults offline
this patch would be much simpler if it did not depend on the complex NM-related patch. PLEASE, let us have a simple utility first, and worry about NM later.
Line 8: 
Line 9: Change-Id: I89b907ba80f23f417d5e481db9350247445ab772


http://gerrit.ovirt.org/#/c/33403/2/init/vdsmd_init_common.sh.in
File init/vdsmd_init_common.sh.in:

Line 100: }
Line 101: 
Line 102: 
Line 103: task_dump_bonding_defaults(){
Line 104:     "${VDSM_TOOL}" dump_bonding_defaults
vdsm-tool prefer hyphens to underscores in command names. let's conform.
Line 105: }
Line 106: 
Line 107: 
Line 108: task_tune_system(){


http://gerrit.ovirt.org/#/c/33403/2/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:

Line 87: P_VDSM_LOG = '@VDSMLOGDIR@'
Line 88: P_VDSM_NODE_ID = '/etc/vdsm/vdsm.id'
Line 89: 
Line 90: P_VDSM_EXEC = '@LIBEXECDIR@'
Line 91: P_VDSM_BONDING_DEFAULTS = P_VDSM_LIB + 'bonding-defaults.json'
contants.py is a junk yard. this new constant is very specific and it fits well into netinfo.
Line 92: 
Line 93: #
Line 94: # Configuration file definitions
Line 95: #


http://gerrit.ovirt.org/#/c/33403/2/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:

Line 406:     """
Line 407:     Return default options per mode, in a dictionary of dictionaries. All keys
Line 408:     are numeric modes stored as strings for coherence with 'mode' option value.
Line 409:     """
Line 410:     return json.load(open(constants.P_VDSM_BONDING_DEFAULTS).read())
use

 with open() as f:
   ...

to make sure the file is closed immediately when execution flows out of the block.
Line 411: 
Line 412: 
Line 413: @memoized
Line 414: def _getDefaultBondingOptions(mode=None):


http://gerrit.ovirt.org/#/c/33403/2/lib/vdsm/tool/dump_bonding_defaults.py
File lib/vdsm/tool/dump_bonding_defaults.py:

Line 31: REFERENCE_BOND = ';vdsmrefbond;'
Line 32: 
Line 33: 
Line 34: @contextmanager
Line 35: def _avoidNetworkManager(interface):
let's cut our losses and ignore NetworkManager completely, at least for now.
Line 36:     """Make a network interface unmanaged by NetworkManager.
Line 37: 
Line 38:     Until NM supports placing its 'unmanaged-devices' directive in
Line 39:     /etc/NetworkManager/conf.d/00-server.conf we are left with a hack.


Line 92: 
Line 93:     return opts
Line 94: 
Line 95: 
Line 96: @expose('dump_bonding_defaults')
use dump-bonding-defaults with hyphens, please.
Line 97: def main(*args):
Line 98:     """dump_bonding_defaults
Line 99: 
Line 100:     Read bonding option defaults and print them out as a dictionary (per mode)


Line 96: @expose('dump_bonding_defaults')
Line 97: def main(*args):
Line 98:     """dump_bonding_defaults
Line 99: 
Line 100:     Read bonding option defaults and print them out as a dictionary (per mode)
more precise text:

 Read bonding option defaults (per mode) and dump them to
 BONDING_DEFAULTS in json format.
Line 101:     in JSON format.
Line 102:     """
Line 103: 
Line 104:     if len(args) > 1:


http://gerrit.ovirt.org/#/c/33403/2/tests/netinfoTests.py
File tests/netinfoTests.py:

Line 301:         self.assertIn('valid2', dhcp4)
Line 302:         self.assertNotIn('expired', dhcp4)
Line 303:         self.assertNotIn('expired2', dhcp4)
Line 304: 
Line 305: # TODO: move to toolTests.py?
yes.
Line 306: #    @ValidateRunningAsRoot
Line 307: #    @RequireBondingMod
Line 308: #    def testGetBondingOptions(self):
Line 309: #        INTERVAL = '12345'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I89b907ba80f23f417d5e481db9350247445ab772
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list