Fix the block-sharing check for physical devices by using a lock to serialise

the check, reading from the store rather than /sys, and checking whether the
VM for apparently-conflicting uses is actually the same (i.e. this is a
migration or a reboot in progress).

This fixes recent failures in 12_block_attach_shared_domU and recloses bug #331.

This fix includes particularly skanky path slicing inside xenbus_probe, which
it would be nice to remove very soon.

The loopback device check still suffers from the problem that filenames over
64 characters long are truncated.

Signed-off-by: Ewan Mellor <ewan@xensource.com>

linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c

@@ -59,6 +59,8 @@ extern struct semaphore xenwatch_mutex;
 
 #define streq(a, b) (strcmp((a), (b)) == 0)
 
+static char *kasprintf(const char *fmt, ...);
+
 static struct notifier_block *xenstore_chain;
 
 /* If something in array of ids matches this device, return it. */
@@ -226,8 +228,11 @@ static int xenbus_hotplug_backend(struct device *dev, char **envp,
 				  int num_envp, char *buffer, int buffer_size)
 {
 	struct xenbus_device *xdev;
+	struct xenbus_driver *drv = NULL;
 	int i = 0;
 	int length = 0;
+	char *basepath_end;
+	char *frontend_id;
 
 	DPRINTK("");
 
@@ -238,6 +243,9 @@ static int xenbus_hotplug_backend(struct device *dev, char **envp,
 	if (xdev == NULL)
 		return -ENODEV;
 
+	if (dev->driver)
+		drv = to_xenbus_driver(dev->driver);
+
 	/* stuff we want to pass to /sbin/hotplug */
 	add_hotplug_env_var(envp, num_envp, &i,
 			    buffer, buffer_size, &length,
@@ -247,6 +255,25 @@ static int xenbus_hotplug_backend(struct device *dev, char **envp,
 			    buffer, buffer_size, &length,
 			    "XENBUS_PATH=%s", xdev->nodename);
 
+	add_hotplug_env_var(envp, num_envp, &i,
+			    buffer, buffer_size, &length,
+			    "XENBUS_BASE_PATH=%s", xdev->nodename);
+
+	basepath_end = strrchr(envp[i - 1], '/');
+	length -= strlen(basepath_end);
+	*basepath_end = '\0';
+	basepath_end = strrchr(envp[i - 1], '/');
+	length -= strlen(basepath_end);
+	*basepath_end = '\0';
+
+	basepath_end++;
+	frontend_id = kmalloc(strlen(basepath_end) + 1, GFP_KERNEL);
+	strcpy(frontend_id, basepath_end);
+	add_hotplug_env_var(envp, num_envp, &i,
+			    buffer, buffer_size, &length,
+			    "XENBUS_FRONTEND_ID=%s", frontend_id);
+	kfree(frontend_id);
+
 	/* terminate, set to next free slot, shrink available space */
 	envp[i] = NULL;
 	envp = &envp[i];
@@ -254,9 +281,9 @@ static int xenbus_hotplug_backend(struct device *dev, char **envp,
 	buffer = &buffer[length];
 	buffer_size -= length;
 
-	if (dev->driver && to_xenbus_driver(dev->driver)->hotplug)
-		return to_xenbus_driver(dev->driver)->hotplug
-			(xdev, envp, num_envp, buffer, buffer_size);
+	if (drv && drv->hotplug)
+		return drv->hotplug(xdev, envp, num_envp, buffer,
+				    buffer_size);
 
 	return 0;
 }

tools/examples/block

@@ -89,39 +89,54 @@ check_sharing()
     fi
   done
 
-##
-## XXX SMH: the below causes live migration on localhost to fail sometimes
-## since the source domain may still appear to be using a local device. 
-## For now simply comment it out - a proper fix will come in due course. 
+  for dom in $(xenstore-list "$XENBUS_BASE_PATH")
+  do
+    for dev in $(xenstore-list "$XENBUS_BASE_PATH/$dom")
+    do
+      d=$(xenstore_read_default \
+            "$XENBUS_BASE_PATH/$dom/$dev/physical-device" "")
 
-#   for file in /sys/devices/xen-backend/*/physical_device
-#   do
-#     if [ -e "$file" ] # Cope with no devices, i.e. the * above did not expand.
-#     then
-#       local d=$(cat "$file")
-#       if [ "$d" == "$devmm" ]
-#       then
-#         if [ "$mode" == 'w' ]
-#         then
-#           echo 'guest'
-#           return
-#         else
-#           local m=$(cat "${file/physical_device/mode}")
+      if [ "$d" == "$devmm" ]
+      then
+        if [ "$mode" == 'w' ]
+        then
+          if ! same_vm $dom
+          then
+            echo 'guest'
+            return
+          fi
+        else
+          local m=$(xenstore_read "$XENBUS_BASE_PATH/$dom/$dev/mode")
+          m=$(canonicalise_mode "$m")
 
-#           if expr index "$m" 'w' >/dev/null
-#           then
-#             echo 'guest'
-#             return
-#           fi
-#         fi
-#       fi
-#     fi
-#   done
+          if [ "$m" == 'w' ]
+          then
+            if ! same_vm $dom
+            then
+              echo 'guest'
+              return
+            fi
+          fi
+        fi
+      fi
+    done
+  done
 
   echo 'ok'
 }
 
 
+same_vm()
+{
+  local thisdom="$XENBUS_FRONTEND_ID"
+  local otherdom="$1"
+  local thisvm=$(xenstore-read "/local/domain/$thisdom/vm")
+  local othervm=$(xenstore-read "/local/domain/otherdom/vm")
+
+  return [ "$thisvm" == "$othervm" ]
+}
+
+
 ##
 # check_device_sharing dev mode
 #
@@ -200,6 +215,7 @@ do_ebusy()
     m2='read-only '
   fi
 
+  release_lock "block"
   ebusy \
 "${prefix}${m1}in ${dom}domain,
 and so cannot be mounted ${m2}${when}."
@@ -224,8 +240,10 @@ case "$command" in
     case $t in 
       phy)
         dev=$(expand_dev $p)
+        claim_lock "block"
         check_device_sharing "$dev" "$mode"
 	write_dev "$dev"
+        release_lock "block"
 	exit 0
 	;;
 
@@ -235,68 +253,62 @@ case "$command" in
         file=$(readlink -f "$p")
         mode=$(canonicalise_mode "$mode")
 
+        claim_lock "block"
+
         if [ "$mode" == 'w' ] && ! stat "$file" -c %A | grep -q w
         then
+          release_lock "block"
           ebusy \
 "File $file is read-only, and so I will not
 mount it read-write in a guest domain."
         fi
 
+        loopdev=''
+        for dev in /dev/loop*
+        do
+          if [ ! -b "$dev" ]
+          then
+            continue
+          fi
 
-	while true
-        do 
-          loopdev=''
-          for dev in /dev/loop*
-          do
-            if [ ! -b "$dev" ]
+          f=$(losetup "$dev" 2>/dev/null) || f='()'
+          f=$(echo "$f" | sed -e 's/.*(\(.*\)).*/\1/g')
+
+          if [ "$f" ]
+          then
+            # $dev is in use.  Check sharing.
+            if [ "$mode" == '!' ]
             then
               continue
             fi
 
-            f=$(losetup "$dev" 2>/dev/null) || f='()'
-            f=$(echo "$f" | sed -e 's/.*(\(.*\)).*/\1/g')
+            f=$(readlink -f "$f")
 
-            log err "$file $f $dev"
-
-            if [ "$f" ]
+            if [ "$f" == "$file" ]
             then
-              # $dev is in use.  Check sharing.
-              if [ "$mode" == '!' ]
-              then
-                continue
-              fi
-
-              f=$(readlink -f "$f")
-
-              if [ "$f" == "$file" ]
-              then
-                check_file_sharing "$file" "$dev" "$mode"
-              fi
-            else
-              # $dev is not in use, so we'll remember it for use later; we want
-              # to finish the sharing check first.
-              
-              if [ "$loopdev" == '' ]
-              then
-                loopdev="$dev"
-              fi
+              check_file_sharing "$file" "$dev" "$mode"
             fi
-          done
-
-          if [ "$loopdev" == '' ]
-          then
-            fatal 'Failed to find an unused loop device'
-          fi
-          if losetup "$loopdev" "$file"
-          then
-	    log err "mapped $file using $loopdev"
-            xenstore_write "$XENBUS_PATH/node" "$loopdev"
-            write_dev "$loopdev"
-            exit 0
           else
-            log err "losetup $loopdev $file failed, retry"
+            # $dev is not in use, so we'll remember it for use later; we want
+            # to finish the sharing check first.
+
+            if [ "$loopdev" == '' ]
+            then
+              loopdev="$dev"
+            fi
           fi
-	done
+        done
+
+        if [ "$loopdev" == '' ]
+        then
+          fatal 'Failed to find an unused loop device'
+        fi
+
+        do_or_die losetup "$loopdev" "$file"
+        xenstore_write "$XENBUS_PATH/node" "$loopdev"
+        write_dev "$loopdev"
+        release_lock "block"
+        exit 0
 	;;
     esac
     ;;