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