Adam Litke has posted comments on this change.
Change subject: [WIP] Move vdsmd.init to vdsm-tool
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(17 inline comments)
Need to convert more shell commands to python equivalents.
....................................................
File vdsm-tool/vdsmd.py
Line 49: EX_AWK = constants.EXT_AWK
In python, do not call out to awk, sed, grep, chmod, chown, tail, etc. These functions
all have easy python equivalents that are more efficient and which integrate better into
the program. I will identify the ones I see in this file and suggest some
alternatives...
Line 138: _exec_command([EX_SED, '-i', '--copy', '/%s/d' %
BY_VDSM, file])
In this one you would want something like:
newfile = []
with open(file) as f:
for line in f.readlines():
if line.endswith(BY_VDSM):
continue
newfile.append(line)
with open(file, "w") as f:
f.writelines(newfile)
Line 142: rc = _exec_command([EX_GREP, '-q', '^\s*%s\s*=' % key,
file])[2]
Here you would read the file with:
with open(file) as f:
str = f.read()
Then search for your config entry:
if re.search('^\s*%s\s*=' % key, str) is None:
... do the f.write below
Line 149: if _exec_command([EX_GREP, '-q', '%s' % BY_VDSM, file])[2]
== 0:
Use the read and re example above here too.
Line 156: out = _exec_command([GET_CONF_ITEM, file, section, item, default])[0]
The above is a simple python script. Now that your program is python, you can just copy
the code from that script into this function.
Line 187: p1 = subprocess.Popen(['/bin/netstat', '-ntl'],
stdout=subprocess.PIPE)
Calling netstat is fine, but you should capture its output into a string and use re to
parse that string instead of calling grep.
Line 202: _exec_command([EX_CHOWN, 'vdsm:kvm', path])
See python's os.chown()
http://docs.python.org/library/os.html
Line 232:
All of these mk_*_path functions are pretty similar. Please collapse them into a single
function:
def _mk_vdsm_path(path, mode, user, group, restorecon=False):
...
See os.chmod, os.chown
http://docs.python.org/library/grp.html
http://docs.python.org/library/pwd.html
Is is ok to call restorecon
Line 241: stdin=p2.stdout, stdout=subprocess.PIPE)
read the file.
use re.search to get a match object
get the last match by using index -1 into the match object
You can use a grouped regular expression to easily extract the configuration value.
See
http://docs.python.org/library/re.html#match-objects
Line 306: stdout=subprocess.PIPE, env=environ)
You know how to eliminate grep now.
Line 324: stdout=subprocess.PIPE)
Awk just splits a string by whitespace. In python do this instead:
# read the output of df into str
# Get the last line
lastline = str.splitlines()[-1]
# Then get the 4th field
lastline.split()[3]
That's it!
Line 402: '"1:libvirt 3:event 3:json 1:util 1:qemu"')
We can make this more efficient that the original shell script by setting all of the conf
items at the same time. Do do this, you would pass a dictionary of config params to set
into _set_vdsm_conf() and it should add all of them in one shot.
Line 412: ssl == 'ture'):
ssl == 'true'
Line 425: pass
You are silently ignoring lots of potential errors. You probably mean to ignore only the
error if the file does not exist.
Line 430: llogr])[0]
Read the file into an array like one of my previous examples. Then only keep the lines
that are not between '#vdsm and # end vdsm'. Finally, write out the file again.
Line 477: os.rename('/etc/sysctl.conf.vdsm', sysconf)
You should be able to replicate this file editing...
Line 513: p2.stdout.close()
No grep or tail please.
--
To view, visit
http://gerrit.ovirt.org/4738
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I52db67207532ac7ded51cdd8c5dc388e85629104
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Wenyi Gao <wenyi(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh(a)us.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>