[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] Check when using physical devices that the device is not already in use. Using



# HG changeset patch
# User emellor@xxxxxxxxxxxxxxxxxxxxxx
# Node ID 8c3ed250366cb79392d3eee2b3f1439a828a1ddb
# Parent  701ec436d5adcabafb5338e478dd4d9793e5a320
Check when using physical devices that the device is not already in use.  Using
a device from two guests, or from dom 0 and a guest, can destroy the filesystem,
and this check prevents that.  This should ensure that the xm tests
11_block_attach_shared_dom0 and 12_block_attach_shared_domU should pass.
Closes bug #331.

Devices may be shared only if all uses are read-only.  If anyone has read-write
access to a device, then no sharing is allowed.

The mode in the config file may now have an exclamation mark appended, to
indicate that devices may be shared (i.e. that this new check should be
bypassed).  This supports some network block devices, but is clearly dangerous
and should only be used when you know what you are doing.

The mode specified in the config file is now written explicitly to the store,
To implement this, the blkback driver has been changed to create two entries
in /sys describing each device in use.  This means that we can determine which
devices are in use without crawling through the store.

The physical-device node has been changed to give the major and minor of the
device, separated by a colon.  This means that we do not need to pack these
numbers into one, removing the restriction to 8 bit minors that we had in place
before.

The mode specified in the config file is now written explicitly to the store,
rather than using the presence or absence of a read-only node.  This supports
the write-sharing override above.

If the device is in use, a new hotplug status of "busy" is written to the
store, and a message is written to a new hotplug-error node.  Xend uses these
things for diagnosis.

The block scripts do not need to handle online or offline events, so this
support has been removed.  These scripts can be called twice, depending upon
the hotplug config, so they now identify the second run and do not attempt to
set up the device again.

The device directories in the store are cleaned out before new details are
written there.  This prevents stale information from messing up block device
hotplugging.  This will be one cause of "blkback: changing physical device
not supported" messages.

Signed-off-by: Ewan Mellor <ewan@xxxxxxxxxxxxx>

diff -r 701ec436d5ad -r 8c3ed250366c 
linux-2.6-xen-sparse/drivers/xen/blkback/common.h
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/common.h Fri Nov 25 17:12:12 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/common.h Fri Nov 25 20:58:07 2005
@@ -78,8 +78,8 @@
        } while (0)
 
 /* Create a vbd. */
-int vbd_create(blkif_t *blkif, blkif_vdev_t vdevice, u32 pdevice,
-              int readonly);
+int vbd_create(blkif_t *blkif, blkif_vdev_t vdevice, unsigned major,
+              unsigned minor, int readonly);
 void vbd_free(struct vbd *vbd);
 
 unsigned long vbd_size(struct vbd *vbd);
diff -r 701ec436d5ad -r 8c3ed250366c 
linux-2.6-xen-sparse/drivers/xen/blkback/vbd.c
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/vbd.c    Fri Nov 25 17:12:12 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/vbd.c    Fri Nov 25 20:58:07 2005
@@ -9,10 +9,6 @@
 #include "common.h"
 #include <asm-xen/xenbus.h>
 
-static inline dev_t vbd_map_devnum(u32 cookie)
-{
-       return MKDEV(BLKIF_MAJOR(cookie), BLKIF_MINOR(cookie));
-}
 #define vbd_sz(_v)   ((_v)->bdev->bd_part ?                            \
        (_v)->bdev->bd_part->nr_sects : (_v)->bdev->bd_disk->capacity)
 #define bdev_put(_b) blkdev_put(_b)
@@ -32,8 +28,8 @@
        return bdev_hardsect_size(vbd->bdev);
 }
 
-int vbd_create(blkif_t *blkif, blkif_vdev_t handle,
-              u32 pdevice, int readonly)
+int vbd_create(blkif_t *blkif, blkif_vdev_t handle, unsigned major,
+              unsigned minor, int readonly)
 {
        struct vbd *vbd;
 
@@ -42,10 +38,10 @@
        vbd->readonly = readonly;
        vbd->type     = 0;
 
-       vbd->pdevice  = pdevice;
+       vbd->pdevice  = MKDEV(major, minor);
 
        vbd->bdev = open_by_devnum(
-               vbd_map_devnum(vbd->pdevice),
+               vbd->pdevice,
                vbd->readonly ? FMODE_READ : FMODE_WRITE);
        if (IS_ERR(vbd->bdev)) {
                DPRINTK("vbd_creat: device %08x doesn't exist.\n",
diff -r 701ec436d5ad -r 8c3ed250366c 
linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c Fri Nov 25 17:12:12 2005
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c Fri Nov 25 20:58:07 2005
@@ -37,8 +37,9 @@
        blkif_t *blkif;
        struct xenbus_watch backend_watch;
 
-       long int pdev;
-       long int readonly;
+       unsigned major;
+       unsigned minor;
+       char *mode;
 };
 
 
@@ -49,6 +50,25 @@
                            unsigned int);
 
 
+static ssize_t show_physical_device(struct device *_dev, char *buf)
+{
+       struct xenbus_device *dev = to_xenbus_device(_dev);
+       struct backend_info *be = dev->data;
+       return sprintf(buf, "%x:%x\n", be->major, be->minor);
+}
+DEVICE_ATTR(physical_device, S_IRUSR | S_IRGRP | S_IROTH,
+           show_physical_device, NULL);
+
+
+static ssize_t show_mode(struct device *_dev, char *buf)
+{
+       struct xenbus_device *dev = to_xenbus_device(_dev);
+       struct backend_info *be = dev->data;
+       return sprintf(buf, "%s\n", be->mode);
+}
+DEVICE_ATTR(mode, S_IRUSR | S_IRGRP | S_IROTH, show_mode, NULL);
+
+
 static int blkback_remove(struct xenbus_device *dev)
 {
        struct backend_info *be = dev->data;
@@ -64,6 +84,10 @@
                blkif_put(be->blkif);
                be->blkif = NULL;
        }
+
+       device_remove_file(&dev->dev, &dev_attr_physical_device);
+       device_remove_file(&dev->dev, &dev_attr_mode);
+
        kfree(be);
        dev->data = NULL;
        return 0;
@@ -73,7 +97,7 @@
 /**
  * Entry point to this code when a new device is created.  Allocate the basic
  * structures, and watch the store waiting for the hotplug scripts to tell us
- * the device's physical-device.  Switch to InitWait.
+ * the device's physical major and minor numbers.  Switch to InitWait.
  */
 static int blkback_probe(struct xenbus_device *dev,
                         const struct xenbus_device_id *id)
@@ -119,64 +143,71 @@
 
 /**
  * Callback received when the hotplug scripts have placed the physical-device
- * node.  Read it and the read-only node, and create a vbd.  If the frontend
- * is ready, connect.
+ * node.  Read it and the mode node, and create a vbd.  If the frontend is
+ * ready, connect.
  */
 static void backend_changed(struct xenbus_watch *watch,
                            const char **vec, unsigned int len)
 {
        int err;
-       char *p;
-       long pdev;
+       unsigned major;
+       unsigned minor;
        struct backend_info *be
                = container_of(watch, struct backend_info, backend_watch);
        struct xenbus_device *dev = be->dev;
 
        DPRINTK("");
 
-       err = xenbus_scanf(NULL, dev->nodename,
-                          "physical-device", "%li", &pdev);
+       err = xenbus_scanf(NULL, dev->nodename, "physical-device", "%x:%x",
+                          &major, &minor);
        if (XENBUS_EXIST_ERR(err)) {
                /* Since this watch will fire once immediately after it is
                   registered, we expect this.  Ignore it, and wait for the
                   hotplug scripts. */
                return;
        }
-       if (err != 1) {
+       if (err != 2) {
                xenbus_dev_fatal(dev, err, "reading physical-device");
                return;
        }
-       if (be->pdev && be->pdev != pdev) {
+
+       if (be->major && be->minor &&
+           (be->major != major || be->minor != minor)) {
                printk(KERN_WARNING
-                      "blkback: changing physical-device (from %ld to %ld) "
-                      "not supported.\n", be->pdev, pdev);
-               return;
-       }
-
-       /* If there's a read-only node, we're read only. */
-       p = xenbus_read(NULL, dev->nodename, "read-only", NULL);
-       if (!IS_ERR(p)) {
-               be->readonly = 1;
-               kfree(p);
-       }
-
-       if (be->pdev == 0L) {
+                      "blkback: changing physical device (from %x:%x to "
+                      "%x:%x) not supported.\n", be->major, be->minor,
+                      major, minor);
+               return;
+       }
+
+       be->mode = xenbus_read(NULL, dev->nodename, "mode", NULL);
+       if (IS_ERR(be->mode)) {
+               err = PTR_ERR(be->mode);
+               be->mode = NULL;
+               xenbus_dev_fatal(dev, err, "reading mode");
+               return;
+       }
+
+       if (be->major == 0 && be->minor == 0) {
                /* Front end dir is a number, which is used as the handle. */
 
-               long handle;
-
-               p = strrchr(dev->otherend, '/') + 1;
-               handle = simple_strtoul(p, NULL, 0);
-
-               be->pdev = pdev;
-
-               err = vbd_create(be->blkif, handle, be->pdev, be->readonly);
+               char *p = strrchr(dev->otherend, '/') + 1;
+               long handle = simple_strtoul(p, NULL, 0);
+
+               be->major = major;
+               be->minor = minor;
+
+               err = vbd_create(be->blkif, handle, major, minor,
+                                (NULL == strchr(be->mode, 'w')));
                if (err) {
-                       be->pdev = 0L;
-                       xenbus_dev_fatal(dev, err,
-                                        "creating vbd structure");
+                       be->major = 0;
+                       be->minor = 0;
+                       xenbus_dev_fatal(dev, err, "creating vbd structure");
                        return;
                }
+
+               device_create_file(&dev->dev, &dev_attr_physical_device);
+               device_create_file(&dev->dev, &dev_attr_mode);
 
                maybe_connect(be);
        }
@@ -230,7 +261,8 @@
 
 static void maybe_connect(struct backend_info *be)
 {
-       if (be->pdev != 0L && be->blkif->status == CONNECTED)
+       if ((be->major != 0 || be->minor != 0) &&
+           be->blkif->status == CONNECTED)
                connect(be);
 }
 
diff -r 701ec436d5ad -r 8c3ed250366c tools/examples/block
--- a/tools/examples/block      Fri Nov 25 17:12:12 2005
+++ b/tools/examples/block      Fri Nov 25 20:58:07 2005
@@ -2,12 +2,6 @@
 
 dir=$(dirname "$0")
 . "$dir/block-common.sh"
-
-case "$command" in
-    online | offline)
-        exit 0
-        ;;
-esac
 
 expand_dev() {
   local dev
@@ -22,21 +16,122 @@
   echo -n $dev
 }
 
-t=$(xenstore_read_default "$XENBUS_PATH"/type "MISSING")
+
+##
+# check_sharing device device_major_minor writable
+#
+# Check whether the device requested is already in use.  To use the device in
+# read-only mode, it may be in use in read-only mode, but may not be in use in
+# read-write anywhere at all.  To use the device in read-write mode, it must
+# not be in use anywhere at all.
+#
+check_sharing()
+{
+
+  local dev="$1"
+  local devmm="$2"
+  local writable="$3"
+  local file
+
+  if [ "$writable" ]
+  then
+    toskip="^$"
+  else
+    toskip="^[^ ]* [^ ]* [^ ]* ro "
+  fi
+
+  for file in $(cat /proc/mounts | grep -v "$toskip" | cut -f 1 -d ' ')
+  do
+    if [ -e "$file" ]
+    then
+      local d=$(device_major_minor "$file")
+
+      if [ "$d" == "$devmm" ]
+      then
+        if [ "$writable" ]
+        then
+          m1=""
+          m2=""
+        else
+          m1="read-write "
+          m2="read-only "
+        fi
+
+        ebusy \
+"Device $dev is mounted ${m1}in the privileged domain,
+and so cannot be mounted ${m2}by a guest."
+      fi
+    fi
+  done
+
+  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 [ "$writable" ]
+        then
+          ebusy \
+"Device $dev is already mounted in a guest domain, and so
+cannot be mounted read-write now."
+        else
+          local m=$(cat "${file/physical_device/mode}")
+
+          if expr index "$m" 'w' >/dev/null
+          then
+            ebusy \
+"Device $dev is already mounted read-write in a guest domain,
+and so cannot be mounted read-only again."
+          fi
+        fi
+      fi
+    fi
+  done
+}
+
+
+check_device_sharing()
+{
+  local dev="$1"
+  local devmm=$(device_major_minor "$dev")
+  local mode=$(xenstore_read "$XENBUS_PATH/mode")
+
+  if ! expr index "$mode" 'w' >/dev/null
+  then
+    # No w implies read-only use; sharing with writers must be prevented.
+    check_sharing "$dev" "$devmm" ""
+  elif ! expr index "$mode" '!' >/dev/null
+  then
+    # No exclamation mark implies all sharing must be prevented.
+    check_sharing "$dev" "$devmm" 1
+  fi
+}
+
+
+t=$(xenstore_read_default "$XENBUS_PATH/type" "MISSING")
 
 case "$command" in 
   add)
-    p=$(xenstore_read "$XENBUS_PATH"/params)
+    phys=$(xenstore_read_default "$XENBUS_PATH/physical-device" "MISSING")
+    if [ "$phys" != "MISSING" ]
+    then
+      # Depending upon the hotplug configuration, it is possible for this
+      # script to be called twice, so just bail.
+      exit 0
+    fi
+    p=$(xenstore_read "$XENBUS_PATH/params")
     case $t in 
       phy)
         dev=$(expand_dev $p)
+        check_device_sharing "$dev"
        write_dev "$dev"
        exit 0
        ;;
 
       file)
        for dev in /dev/loop* ; do
-         echo "dev is $dev, p is $p"
          if losetup $dev $p; then
            write_dev "$dev"
             exit 0
@@ -54,7 +149,7 @@
        ;;
 
       file)
-        node=$(xenstore_read "$XENBUS_PATH"/node)
+        node=$(xenstore_read "$XENBUS_PATH/node")
        losetup -d $node
        exit 0
        ;;
diff -r 701ec436d5ad -r 8c3ed250366c tools/examples/block-common.sh
--- a/tools/examples/block-common.sh    Fri Nov 25 17:12:12 2005
+++ b/tools/examples/block-common.sh    Fri Nov 25 20:58:07 2005
@@ -21,9 +21,7 @@
 
 findCommand "$@"
 
-if [ "$command" != "online" ]  &&
-   [ "$command" != "offline" ] &&
-   [ "$command" != "add" ]     &&
+if [ "$command" != "add" ] &&
    [ "$command" != "remove" ]
 then
   log err "Invalid command: $command"
@@ -34,28 +32,42 @@
 XENBUS_PATH="${XENBUS_PATH:?}"
 
 
+ebusy()
+{
+  xenstore_write "$XENBUS_PATH/hotplug-status" busy
+  xenstore_write "$XENBUS_PATH/hotplug-error" "$*"
+  log err "$@"
+  exit 1
+}
+
+
 ##
-# Write physical-device = 0xMMmm and node = device to the store, where MM
-# and mm are the major and minor numbers of device.
+# Print the given device's major and minor numbers, written in hex and
+# separated by a colon.
+device_major_minor()
+{
+  stat -L -c %t:%T "$1"
+}
+
+
+##
+# Write physical-device = MM,mm to the store, where MM and mm are the major 
+# and minor numbers of device respectively.
 #
 # @param device The device from which major and minor numbers are read, which
 #               will be written into the store.
 #
 write_dev() {
-  local major
-  local minor
-  local pdev
+  local mm
   
-  major=$(stat -L -c %t "$1")
-  minor=$(stat -L -c %T "$1")
+  mm=$(device_major_minor "$1")
  
-  if [ -z $major  -o -z $minor ]; then
+  if [ -z $mm ]
+  then
     fatal "Backend device does not exist"
   fi
  
-  pdev=$(printf "0x%02x%02x" "0x$major" "0x$minor")
-  xenstore_write "$XENBUS_PATH"/physical-device "$pdev" \
-                 "$XENBUS_PATH"/node "$1"
+  xenstore_write "$XENBUS_PATH/physical-device" "$mm"
 
   success
 }
diff -r 701ec436d5ad -r 8c3ed250366c 
tools/python/xen/xend/server/DevController.py
--- a/tools/python/xen/xend/server/DevController.py     Fri Nov 25 17:12:12 2005
+++ b/tools/python/xen/xend/server/DevController.py     Fri Nov 25 20:58:07 2005
@@ -27,13 +27,16 @@
 
 DEVICE_CREATE_TIMEOUT = 5
 HOTPLUG_STATUS_NODE = "hotplug-status"
+HOTPLUG_ERROR_NODE  = "hotplug-error"
 HOTPLUG_STATUS_ERROR = "error"
+HOTPLUG_STATUS_BUSY  = "busy"
 
 Connected = 1
 Died      = 2
 Error     = 3
 Missing   = 4
 Timeout   = 5
+Busy      = 6
 
 xenbusState = {
     'Unknown'      : 0,
@@ -99,7 +102,8 @@
                 log.debug('DevController: writing %s to %s.', str(back),
                           backpath)
 
-                t.remove2(backpath, HOTPLUG_STATUS_NODE)
+                t.remove(frontpath)
+                t.remove(backpath)
 
                 t.write2(frontpath, front)
                 t.write2(backpath,  back)
@@ -125,23 +129,37 @@
         if status == Timeout:
             self.destroyDevice(devid)
             raise VmError("Device %s (%s) could not be connected. "
-                          "Hotplug scripts not working" %
+                          "Hotplug scripts not working." %
                           (devid, self.deviceClass))
 
         elif status == Error:
             self.destroyDevice(devid)
             raise VmError("Device %s (%s) could not be connected. "
-                          "Backend device not found" %
+                          "Backend device not found." %
                           (devid, self.deviceClass))
 
         elif status == Missing:
             raise VmError("Device %s (%s) could not be connected. "
-                          "Device not found" % (devid, self.deviceClass))
+                          "Device not found." % (devid, self.deviceClass))
 
         elif status == Died:
             self.destroyDevice(devid)
             raise VmError("Device %s (%s) could not be connected. "
-                          "Device has died" % (devid, self.deviceClass))
+                          "Device has died." % (devid, self.deviceClass))
+
+        elif status == Busy:
+            err = None
+            frontpath = self.frontendPath(devid)
+            backpath = xstransact.Read(frontpath, "backend")
+            if backpath:
+                err = xstransact.Read(backpath, HOTPLUG_ERROR_NODE)
+            if not err:
+                err = "Busy."
+                
+            self.destroyDevice(devid)
+            raise VmError("Device %s (%s) could not be connected.\n%s" %
+                          (devid, self.deviceClass, err))
+
 
 
     def reconfigureDevice(self, devid, config):
@@ -384,6 +402,8 @@
         if status is not None:
             if status == HOTPLUG_STATUS_ERROR:
                 result['status'] = Error
+            elif status == HOTPLUG_STATUS_BUSY:
+                result['status'] = Busy
             else:
                 result['status'] = Connected
         else:
diff -r 701ec436d5ad -r 8c3ed250366c tools/python/xen/xend/server/blkif.py
--- a/tools/python/xen/xend/server/blkif.py     Fri Nov 25 17:12:12 2005
+++ b/tools/python/xen/xend/server/blkif.py     Fri Nov 25 20:58:07 2005
@@ -48,13 +48,11 @@
         devid = blkif.blkdev_name_to_number(dev)
 
         (typ, params) = string.split(sxp.child_value(config, 'uname'), ':', 1)
-        back = { 'dev' : dev,
-                 'type' : typ,
-                 'params' : params
+        back = { 'dev'    : dev,
+                 'type'   : typ,
+                 'params' : params,
+                 'mode'   : sxp.child_value(config, 'mode', 'r')
                  }
-
-        if 'r' == sxp.child_value(config, 'mode', 'r'):
-            back['read-only'] = ""  # existence indicates read-only
 
         front = { 'virtual-device' : "%i" % devid }
 
@@ -66,18 +64,16 @@
 
         result = DevController.configuration(self, devid)
 
-        (dev, typ, params, ro) = self.readBackend(devid,
-                                                  'dev', 'type', 'params',
-                                                  'read-only')
+        (dev, typ, params, mode) = self.readBackend(devid,
+                                                    'dev', 'type', 'params',
+                                                    'mode')
 
         if dev:
             result.append(['dev', dev])
         if typ and params:
             result.append(['uname', typ + ":" + params])
-        if ro:
-            result.append(['mode', 'r'])
-        else:
-            result.append(['mode', 'w'])
+        if mode:
+            result.append(['mode', mode])
 
         return result
 

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.