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@linux.vnet.ibm.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Ewoud Kohl van Wijngaarden ewoud@kohlvanwijngaarden.nl 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 Gerrit-Reviewer: Wenyi Gao wenyi@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com