Change in vdsm[master]: Handling signals interrupts, using pidofproc and prevent loc...

fsimonce at redhat.com fsimonce at redhat.com
Tue Aug 13 09:07:56 UTC 2013


Federico Simoncelli has posted comments on this change.

Change subject: Handling signals interrupts, using pidofproc and prevent lock over systemd
......................................................................


Patch Set 6: Code-Review-1

(4 comments)

....................................................
File vdsm/vdsmd.init.in
Line 107: }
Line 108: 
Line 109: acquire_lock() {
Line 110:     # todo: add additional flock on free fd
Line 111:     if [ -f "$LOCK_FILE" ]; then
raceful
Line 112:         return 1
Line 113:     fi
Line 114:     touch "$LOCK_FILE"
Line 115:     trap cleanup SIGINT SIGHUP SIGTERM


Line 111:     if [ -f "$LOCK_FILE" ]; then
Line 112:         return 1
Line 113:     fi
Line 114:     touch "$LOCK_FILE"
Line 115:     trap cleanup SIGINT SIGHUP SIGTERM
raceful
Line 116: }
Line 117: 
Line 118: release_lock() {
Line 119:     "@RM_PATH@" -f "$LOCK_FILE" > /dev/null 2>&1


Line 115:     trap cleanup SIGINT SIGHUP SIGTERM
Line 116: }
Line 117: 
Line 118: release_lock() {
Line 119:     "@RM_PATH@" -f "$LOCK_FILE" > /dev/null 2>&1
Not sure if this matter as this function might be removed, but a shorter form is:

 "@RM_PATH@" -f "$LOCK_FILE" &> /dev/null
Line 120: }
Line 121: 
Line 122: start_needed_srv() {
Line 123:     local srv


Line 318:     release_lock
Line 319:     exit 0
Line 320: }
Line 321: 
Line 322: locked_start() {
locked_start is raceful in multiple places. You need to use flock (man 1 flock). It should be as easy as:

 (
     flock -n 9 || exit 1
     start
 ) 9> "$LOCK_FILE"

I think you can get rid of several lines of code here (locked_start, acquire_lock, release_lock, cleanup, ...)
Line 323:     if ! acquire_lock; then
Line 324:         log_failure_msg "cannot start VDS daemon when another instance runs"
Line 325:         return 1
Line 326:     else


-- 
To view, visit http://gerrit.ovirt.org/17926
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b58672957840c9dc5d13dce6af8de1717a9cb2d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Elad Ben Aharon <eladba1990 at gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Ohad Basan <obasan at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list