Mailing List Archive

# HG changeset patch
# User emellor@leeni.uk.xensource.com
# 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@xensource.com>

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@lists.xensource.com
http://lists.xensource.com/xen-changelog