Change in vdsm[master]: Move load_needed_modules to vdsm-tool
danken at redhat.com
danken at redhat.com
Fri Aug 31 09:32:05 UTC 2012
Dan Kenigsberg has posted comments on this change.
Change subject: Move load_needed_modules to vdsm-tool
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm-tool/load_needed_modules.py.in
Line 27:
Line 28: EX_MODPROBE = '@MODPROBE_PATH@'
Line 29:
Line 30: BOND = ['bond0', 'bond1', 'bond2', 'bond3', 'bond4']
Line 31: DEVS = '/sys/class/net/bonding_masters'
this is better kept private to a function, and could use a better name.
MASTERS_FILE
maybe?
Line 32:
Line 33:
Line 34: def _exec_command(argv):
Line 35: """
Line 44: stderr=subprocess.PIPE)
Line 45: out, err = p.communicate()
Line 46: rc = p.returncode
Line 47: except:
Line 48: logging.error(traceback.format_exc())
converting exceptions into return value is not very pythonic.
why don't you
from vdsm.utils import execCmd
?
Line 49:
Line 50: return (rc, out, err)
Line 51:
Line 52:
Line 54: devs = []
Line 55:
Line 56: if os.path.exists(DEVS):
Line 57: with open(DEVS, 'r') as f:
Line 58: devs = f.read().strip('\n').split(' ')
please consider an implementation like this:
existing = set(file(MASTERS).read().split())
with open(MASTERS, 'w') as f:
for bond in REQUIRED - existing:
f.write('+%s\n' % bond)
Line 59:
Line 60: # RHEV-M currently assumes that all bonding devices pre-exist
Line 61: for bond in BOND:
Line 62: if bond not in devs:
Line 70: Load needed modules
Line 71: """
Line 72:
Line 73: for mod in ['tun', 'bonding', '8021q']:
Line 74: _exec_command([EX_MODPROBE, '%s' % mod])
why bother with
'%s' % mod == mod
?
--
To view, visit http://gerrit.ovirt.org/7487
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdc8f9d07a7150c4a846228811d6c5dcb17fa358
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list