[Xen-changelog] Fix the block-sharing check for physical devices by using a lock to serialise

[Xen patchbot -unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User emellor@xxxxxxxxxxxxxxxxxxxxxx
# Node ID 485871ff1d393fd2ef0281d97032a2af056fb885
# Parent  df011cae33e988efc3b6d784ce33b4521553edbe
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@xxxxxxxxxxxxx>

diff -r df011cae33e9 -r 485871ff1d39 
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
--- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c    Fri Dec  2 
15:39:13 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c    Fri Dec  2 
15:48:44 2005
@@ -59,6 +59,8 @@
 
 #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 @@
                                  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("");
 
@@ -237,6 +242,9 @@
        xdev = to_xenbus_device(dev);
        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,
@@ -246,6 +254,25 @@
        add_hotplug_env_var(envp, num_envp, &i,
                            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;
@@ -254,9 +281,9 @@
        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;
 }
diff -r df011cae33e9 -r 485871ff1d39 tools/examples/block
--- a/tools/examples/block      Fri Dec  2 15:39:13 2005
+++ b/tools/examples/block      Fri Dec  2 15:48:44 2005
@@ -89,36 +89,51 @@
     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 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 expr index "$m" 'w' >/dev/null
-#           then
-#             echo 'guest'
-#             return
-#           fi
-#         fi
-#       fi
-#     fi
-#   done
+  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" "")
+
+      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 [ "$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" ]
 }
 
 
@@ -200,6 +215,7 @@
     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 $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 @@
         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
 
-
-       while true
-        do 
-          loopdev=''
-          for dev in /dev/loop*
-          do
-            if [ ! -b "$dev" ]
+        loopdev=''
+        for dev in /dev/loop*
+        do
+          if [ ! -b "$dev" ]
+          then
+            continue
+          fi
+
+          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')
-
-            log err "$file $f $dev"
-
-            if [ "$f" ]
+            f=$(readlink -f "$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'
+          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
           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"
-          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
     ;;

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog