[Xen-devel] [PATCH] tools/hotplug: fix locking

[Zhigang Wang <zhigang.x.wang@xxxxxxxxxx>]

# HG changeset patch
# User Zhigang Wang <zhigang.x.wang@xxxxxxxxxx>
# Date 1339421383 14400
# Node ID eb72d7090b5956490b2f26c858cd9d896feca309
# Parent  32034d1914a607d7b6f1f060352b4cac973600f8
tools/hotplug: fix locking

The current locking implementation would allow two processes get the lock
simultaneously:

++ echo 16741: /etc/xen/scripts/block
++ cut -d : -f 1
+ local pid=16741
+ '[' -n 16741 -a 16741 '!=' unknown -a '!' -f /proc/16741/status ']'
+ '[' 5 -gt 5 ']'
+ sleep 0
+ retries=6
+ '[' 6 -lt 100 ']'
+ mkdir /var/run/xen-hotplug/block
++ _lock_owner /var/run/xen-hotplug/block
++ cat /var/run/xen-hotplug/block/owner
+ local 'new_owner=16741: /etc/xen/scripts/block'
+ '[' '16741: /etc/xen/scripts/block' '!=' '16741: /etc/xen/scripts/block' ']'
++ echo 16741: /etc/xen/scripts/block
++ cut -d : -f 1
+ local pid=16741
+ '[' -n 16741 -a 16741 '!=' unknown -a '!' -f /proc/16741/status ']'
+ '[' 6 -gt 5 ']'
+ sleep 1
+ do_or_die losetup -r /dev/loop27 
/OVS/Repositories/0004fb00000300000aac184a9bbab7a9/VirtualDisks/0004fb00001200002b0ef651033c8381.img
+ losetup -r /dev/loop27 
/OVS/Repositories/0004fb00000300000aac184a9bbab7a9/VirtualDisks/0004fb00001200002b0ef651033c8381.img
+ do_or_die losetup -r /dev/loop27 
/OVS/Repositories/0004fb00000300000aac184a9bbab7a9/VirtualDisks/0004fb0000120000143173990458f2a7.img
+ losetup -r /dev/loop27 
/OVS/Repositories/0004fb00000300000aac184a9bbab7a9/VirtualDisks/0004fb0000120000143173990458f2a7.img
+ xenstore_write backend/vbd/33/51920/node /dev/loop27
+ _xenstore_write backend/vbd/33/51920/node /dev/loop27
+ log debug 'Writing backend/vbd/33/51920/node' '/dev/loop27 to xenstore.'
+ local level=debug
+ shift
+ logger -p daemon.debug -- /etc/xen/scripts/block: 'Writing 
backend/vbd/33/51920/node' '/dev/loop27 to xenstore.'
ioctl: LOOP_SET_FD: Device or resource busy
+ xenstore-write backend/vbd/33/51920/node /dev/loop27
+ fatal losetup -r /dev/loop27 
'/OVS/Repositories/0004fb00000300000aac184a9bbab7a9/VirtualDisks/0004fb0000120000143173990458f2a7.img
 failed'

This patch removes the ownner support and fixed this issue per my test.

Kouya: would you please help to confirm this fix is correct?

Anyone has encountered this issue please help to test.

Signed-off-by: Zhigang Wang <zhigang.x.wang@xxxxxxxxxx>
Cc: Kouya Shimura <kouya@xxxxxxxxxxxxxx>

diff -r 32034d1914a6 -r eb72d7090b59 tools/hotplug/Linux/locking.sh
--- a/tools/hotplug/Linux/locking.sh    Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/hotplug/Linux/locking.sh    Mon Jun 11 09:29:43 2012 -0400
@@ -48,32 +48,14 @@ sigerr() {
 _claim_lock()
 {
   local lockdir="$1"
-  local owner=$(_lock_owner "$lockdir")
   local retries=0
 
   while [ $retries -lt $LOCK_RETRIES ]
   do
-    mkdir "$lockdir" 2>/dev/null && trap "_release_lock $lockdir; sigerr" ERR 
&&
-      _update_lock_info "$lockdir" && return
-
-    local new_owner=$(_lock_owner "$lockdir")
-    if [ "$new_owner" != "$owner" ]
-    then
-      owner="$new_owner"
-      retries=0
-    else
-      local pid=$(echo $owner | cut -d : -f 1)
-      if [ -n "$pid" -a "$pid" != "unknown" -a ! -f "/proc/$pid/status" ]
-      then
-        _release_lock $lockdir
-      fi
-    fi
-
+    mkdir "$lockdir" 2>/dev/null && trap "_release_lock $lockdir; sigerr" ERR 
&& return
     if [ $retries -gt $LOCK_SPINNING_RETRIES ]
     then
       sleep $LOCK_SLEEPTIME
-    else
-      sleep 0
     fi
     retries=$(($retries + 1))
   done
@@ -91,20 +73,7 @@ _release_lock()
 _steal_lock()
 {
   local lockdir="$1"
-  local owner=$(cat "$lockdir/owner" 2>/dev/null || echo "unknown")
-  log err "Forced to steal lock on $lockdir from $owner!"
+  log err "Forced to steal lock on $lockdir!"
   _release_lock "$lockdir"
   _claim_lock "$lockdir"
 }
-
-
-_lock_owner()
-{
-  cat "$1/owner" 2>/dev/null || echo "unknown"
-}
-
-
-_update_lock_info()
-{
-  echo "$$: $0" >"$1/owner"
-}

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel