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