Change in vdsm[master]: vdsm-tool: unify ad-hoc execCmd implementations

fsimonce at redhat.com fsimonce at redhat.com
Mon Apr 15 18:19:52 UTC 2013


Federico Simoncelli has posted comments on this change.

Change subject: vdsm-tool: unify ad-hoc execCmd implementations
......................................................................


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

(5 inline comments)

Marking -1 for visibility.

....................................................
File lib/vdsm/tool/load_needed_modules.py.in
Line 20: from vdsm.tool import expose
Line 21: from vdsm.utils import execCmd
Line 22: 
Line 23: 
Line 24: EX_MODPROBE = '@MODPROBE_PATH@'
Since we're at it you could start using CommandPath, we might even get rid of automake for these files.
Line 25: 
Line 26: 
Line 27: def _exec_command(argv):
Line 28:     """


Line 36: def _enable_bond_dev():
Line 37:     REQUIRED = set(['bond0', 'bond1', 'bond2', 'bond3', 'bond4'])
Line 38:     MASTER_FILE = '/sys/class/net/bonding_masters'
Line 39: 
Line 40:     # @ENGINENAME@ currently assumes that all bonding devices pre-exist
Feel free to remove this macro for the greater benefit of not having to generate this file.
Line 41:     existing = set(file(MASTER_FILE).read().split())
Line 42:     for bond in REQUIRED - existing:
Line 43:         with open(MASTER_FILE, 'w') as f:
Line 44:             f.write('+%s\n' % bond)


....................................................
File lib/vdsm/tool/validate_ovirt_certs.py.in
Line 29: except ImportError:
Line 30:     pass
Line 31: 
Line 32: 
Line 33: EX_OPENSSL = '@OPENSSL_PATH@'
You could use CommandPath (try not to generate this file).
Line 34: 
Line 35: PATH_CERT = '/etc/pki/vdsm/certs/'
Line 36: PATH_CACERT = PATH_CERT + 'cacert.pem'
Line 37: PATH_VDSMCERT = PATH_CERT + 'vdsmcert.pem'


....................................................
File vdsm-tool/vdsm-tool.in
Line 1: #!/bin/sh
Line 2: 
Line 3: TOOL="@VDSMDIR@/vdsm-tool.py"
Just install this in the pythonpath (e.g. __init__.py) and let's get rid of this.
For example this file could be a python script (instead of a bash script) that just contains:

 import sys
 from vdsm import tool

 tool.main(sys.argv)

This would even spare you from embedding the "vdsm-tool" name in the next file.
Line 4: [ -e "$TOOL" ] || TOOL="$TOOL"c
Line 5: 
Line 6: [ -n "$PYTHONPATH" ] && PYTHON_FORMATTED_PATH=":$PYTHONPATH"


....................................................
File vdsm-tool/vdsm-tool.py
Line 108:         _usage_command(cmd_name, cmd_docs)
Line 109: 
Line 110: 
Line 111: def usage_and_exit(exit_code):
Line 112:     print "Usage: vdsm-tool [options] <action> [arguments]\n"
Check vdsm-tool.in to see how to avoid this change.
Line 113: 
Line 114:     print "Valid options:"
Line 115:     print "  -h, --help"
Line 116: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ca64ec5dbfebe5c5090ec8fac46785a538b79d3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list