Change in vdsm[master]: Move vdsmd.init to vdsm-tool
wenyi at linux.vnet.ibm.com
wenyi at linux.vnet.ibm.com
Fri Jun 8 09:27:40 UTC 2012
Wenyi Gao has posted comments on this change.
Change subject: Move vdsmd.init to vdsm-tool
......................................................................
Patch Set 4: (17 inline comments)
Response for partial comments.
....................................................
File vdsm-tool/vdsmd.py
Line 1: #!/usr/bin/python
Yes, it should be removed.
Line 71: VDSM_VER = '4.9.6'
Adam, here the version number only indicates if the configuration is changed. I am not sure if the version needs to keep consistent with the vdsm version. Also, the vdsmd.py will be installed in the directory "/usr/lib/python2.7/site-packets/vdsm/tool". It seems that it can't import dsaversion.py that is in the directory "/usr/share/vdsm"
Line 113: return (out, err, rc)
Yes, you are right. There is something wrong with the code here.
Line 128: print >> sys.stderr, "Execution failed:", e
Hmm, I will modify it to logger error.
Line 130: return rc
Yes, rc is something wrong here. Your suggestion make much sense.
Line 157: skip = False
Sorry for the obvious error.
Line 182: lines.append(line)
Yes, I will combine them into a single one.
Line 211: except:
Good point.
Line 215: def _is_ovirt():
Yes, I copy it form vdsm_reg/deployUtil.py. I will consider to move it to a common place.
Line 219: if os.path.exists('/etc/rhev-hypervisor-release'):
Not defined in constants.py yet.
Line 221: elif not len(glob.glob('/etc/ovirt-node-*-release')) == 0:
Also not defined.
Line 232: return False
Yes. Looks simple.
Line 291: services = ['libvirt-guests']
Yes, good suggestion.
Line 306: This fucntion test if the "/sbin/initctl" can be executed.
Yes, thanks.
Line 306: This fucntion test if the "/sbin/initctl" can be executed.
Yes, thanks too.
Line 313: services = ['iscsid', 'multipathd', 'ntpd', 'wdmd', 'sanlock']
Yes, constant is much better.
Line 343: p = subprocess.Popen([EX_DF, '-P', path], stdout=subprocess.PIPE)
Yes, I will add a docstring.
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <ewoud at kohlvanwijngaarden.nl>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh at us.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj at linux.vnet.ibm.com>
More information about the vdsm-patches
mailing list