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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com