Change in vdsm[master]: vdsmd.init.in: break to pieces
Alon Bar-Lev
alonbl at redhat.com
Sun Jun 30 19:06:38 UTC 2013
Alon Bar-Lev has posted comments on this change.
Change subject: vdsmd.init.in: break to pieces
......................................................................
Patch Set 8: (6 inline comments)
....................................................
File vdsm/init/init_gencerts.sh
Line 22: local rt=0
Line 23: if ! "${LIBEXECDIR}/vdsm-gencerts.sh" --check; then
Line 24: printf $"Configuring a self-signed VDSM host certificate: "
Line 25: "${LIBEXECDIR}/vdsm-gencerts.sh"
Line 26: rt=$?
this can be replaced by || return 1 within the above statement
Line 27: [ "${rt}" -eq 0 ] && echo "OK" || echo "Failed"
Line 28: fi
Line 29: return ${rt}
Line 30: }
Line 23: if ! "${LIBEXECDIR}/vdsm-gencerts.sh" --check; then
Line 24: printf $"Configuring a self-signed VDSM host certificate: "
Line 25: "${LIBEXECDIR}/vdsm-gencerts.sh"
Line 26: rt=$?
Line 27: [ "${rt}" -eq 0 ] && echo "OK" || echo "Failed"
this should go into scheduler, common to all.
Line 28: fi
Line 29: return ${rt}
Line 30: }
Line 31:
....................................................
File vdsm/init/init_shutdown_conflicting_srv.sh
Line 22:
Line 23: shutdown_conflicting_srv() {
Line 24: local srv
Line 25:
Line 26: for srv in "${CONFLICTING_SERVICES}"
if you use:
if xxx;l then
you should use:
for srv in xxx; do
Line 27: do
Line 28: /usr/bin/vdsm-tool service-disable "${srv}"
Line 29: if /usr/bin/vdsm-tool service-status "${srv}" > /dev/null 2>&1;
Line 30: then
Line 24: local srv
Line 25:
Line 26: for srv in "${CONFLICTING_SERVICES}"
Line 27: do
Line 28: /usr/bin/vdsm-tool service-disable "${srv}"
all these absolute paths should be removed.
You are inconsistent with your-self... sometimes you use ${VDSM_TOOL} and sometimes hard code path.
Line 29: if /usr/bin/vdsm-tool service-status "${srv}" > /dev/null 2>&1;
Line 30: then
Line 31: if [ "${srv}" = "libvirt-guests" ]; then
Line 32: /bin/rm -f "${LOCKDIR}/libvirt-guests"
....................................................
File vdsm/init/init_start_needed_srv.sh
Line 33: /usr/bin/vdsm-tool service-start "${srv}"
Line 34: ret_val=$?
Line 35: if [ "${ret_val}" -ne 0 ]
Line 36: then
Line 37: bark "${prog}: Dependent ${srv }failed to start"
don't know what bark is, and where you take prog from. I don't think this should be here as I wrote before. These functions should only log to file, and return success/failure. scheduler should decide what to print.
Line 38: return ${ret_val}
Line 39: fi
Line 40: fi
Line 41: done
....................................................
File vdsm/init/init_test_space.sh
Line 25:
Line 26: test_space() {
Line 27: local MIN_SPACE_KB=10000
Line 28:
Line 29: if [ "$(free_space /var/log/vdsm)" -lt "${MIN_SPACE_KB}" ]; then
tabs or spaces.
Line 30: bark "${prog}: low log space"
Line 31: return 1
Line 32: fi
Line 33: return 0
--
To view, visit http://gerrit.ovirt.org/14826
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c652704000c764c6e6f248605c6a3f4f3af5ace
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list