Change in vdsm[master]: vdsmd.init.in: extract common init tasks

Alon Bar-Lev alonbl at redhat.com
Sun Jul 14 13:54:14 UTC 2013


Alon Bar-Lev has posted comments on this change.

Change subject: vdsmd.init.in: extract common init tasks
......................................................................


Patch Set 10: (9 inline comments)

Thanks!!!

Now that there is no modular functionality there is no reason for these two patches, it is perfectly OK to squash.

I think it is much simpler now.

....................................................
File vdsm/Makefile.am
Line 61: 	ovirt_functions.sh
Line 62: 
Line 63: nodist_vdsmexec_SCRIPTS = \
Line 64: 	vdsm-gencerts.sh \
Line 65: 	vdsmd_init_common.sh
please add $(NULL) while you at it.
Line 66: 
Line 67: nodist_vdsm_PYTHON = \
Line 68: 	dsaversion.py \
Line 69: 	dumpStorageTable.py


....................................................
File vdsm/vdsmd_init_common.sh.in
Line 142:     local pathToTest="$1"
Line 143:     df -P "${pathToTest}" | awk '{print $4}'| tail -1
Line 144: }
Line 145: 
Line 146: test_space() {
what do you think of a little work using conventions?

task_ prefix for tasks

 task_test_space()

_ prefix for internal, and prefix with namespace:

 _task_test_space_free_space()

then you can remove the ## comments...
Line 147:     local MIN_SPACE_KB=10000
Line 148: 
Line 149:     if [ "$(free_space "@VDSMLOGDIR@")" -lt "${MIN_SPACE_KB}" ]; then
Line 150:         echo "${prog}: low log space"


Line 145: 
Line 146: test_space() {
Line 147:     local MIN_SPACE_KB=10000
Line 148: 
Line 149:     if [ "$(free_space "@VDSMLOGDIR@")" -lt "${MIN_SPACE_KB}" ]; then
anyway... if free_space is used only once and is a one liner...

 test_space() {
     local MIN_SPACE_KB=10000
     if [ "$(df -P "@VDSMLOGDIR@" | awk '{print $4}'| tail -l -1)" -lt ...
Line 150:         echo "${prog}: low log space"
Line 151:         return 1
Line 152:     fi
Line 153:     return 0


Line 157: 
Line 158: test_lo() {
Line 159:     if ! LC_ALL=C "@IP_PATH@" link show lo | "@GREP_PATH@" -q UP; then
Line 160:         echo "VDSMD: lo interface is down, can't run !"
Line 161:         echo "VDSMD: lo interface is down, can't run !" > /dev/kmsg
please use logger
Line 162:         return 1
Line 163:     fi
Line 164:     return 0
Line 165: }


Line 172: 
Line 173: 
Line 174: #### post-stop tasks ####
Line 175: run_final_hooks(){
Line 176:     python "@VDSMDIR@/hooks.pyc" after_vdsm_stop
python is the most important to detect :)
Line 177: }
Line 178: #### post-stop tasks end ####
Line 179: 
Line 180: 


Line 241:         tasks="run_init_hooks gencerts libvirt_reconfigure \
Line 242:                syslog_available nwfilter dummybr load_needed_modules \
Line 243:                tune_system mkdirs test_space test_lo \
Line 244:                test_conflicting_conf"
Line 245:         run_tasks "$tasks"
just run_tasks and pass parameter?
Line 246:         ;;
Line 247:     --post-stop)
Line 248:         tasks="run_final_hooks"
Line 249:         run_tasks "$tasks"


Line 253:         start_needed_srv "$@"
Line 254:         ;;
Line 255:     --shutdown-conflicting-srv)
Line 256:         shift
Line 257:         shutdown_conflicting_srv "$@"
I would have split the --start-needed-srv and --shutdown-conflicting-srv into their own script.
Line 258:         ;;
Line 259:     --)
Line 260:         usage
Line 261:         exit 1


Line 255:     --shutdown-conflicting-srv)
Line 256:         shift
Line 257:         shutdown_conflicting_srv "$@"
Line 258:         ;;
Line 259:     --)
--help|*) ?
Line 260:         usage
Line 261:         exit 1
Line 262:         ;;


....................................................
File vdsm/vdsmd.init.in
Line 26: PIDFILE="@VDSMRUNDIR@/vdsmd.pid"
Line 27: RESPAWNPIDFILE="@VDSMRUNDIR@/respawn.pid"
Line 28: NEEDED_SERVICES="iscsid multipathd ntpd wdmd sanlock network libvirtd
Line 29:                  supervdsmd"
Line 30: CONFLICTING_SERVICES="libvirt-guests ksmtuned"
shouldn't the above be in separate patch?
Line 31: VDSM_TOOL="@BINDIR@/vdsm-tool"
Line 32: VDSMD_INIT_COMMON="@LIBEXECDIR@/vdsmd_init_common.sh"
Line 33: RETVAL=0
Line 34: 


-- 
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: 10
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