[PATCH] RFC: libxl: Change hotplug script interface to use physical-device-path

[Ian Jackson <ian.jackson@eu.citrix.com>]

* Change block-common.sh on Linux to write physical-device-path with
  the path of the device node, rather than physical-device with its
  major:minor numbers.

* Have the libxl Linux hotplug script scheduler convert this, by
  reading physical-device-path and writing physical-device.  This
  happens just when we have decided that there is no more script to
  run.

* When libxl itself writes the device to xenstore, have it write
  physical-device-path.  On Linux this will be converted to
  physical-device by the new code discussed above.

* Add a note about libxl__device_physdisk_major_minor's poor error
  handling.

* Add an xxx about the fact that the sharing check in
  tools/hotplug/Linux/block needs adjusting.  Note that WITHOUT THIS
  OTHER FIX THE CHANGE TO BLOCK-COMMON.SH IS BROKEN.  This is one
  reason this patch is RFC.

* Untested.  That is the other reason this patch is RFC.
  (I have checked that it does compile.)

I wasn't able to find a document describing the hotplug script
interface (!) so this patch does not contain docs hunks.

CC: George Dunlap <george.dunlap@citrix.com>
CC: Roger Pau Monne <roger.pau@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tools/hotplug/Linux/block-common.sh |   19 +++-------
 tools/libxl/libxl.c                 |    5 +--
 tools/libxl/libxl_internal.h        |    1 +
 tools/libxl/libxl_linux.c           |   70 ++++++++++++++++++++++++++++++++++-
 4 files changed, 75 insertions(+), 20 deletions(-)

diff --git a/tools/hotplug/Linux/block-common.sh b/tools/hotplug/Linux/block-common.sh
index ee95009..560f189 100644
--- a/tools/hotplug/Linux/block-common.sh
+++ b/tools/hotplug/Linux/block-common.sh
@@ -50,23 +50,14 @@ device_major_minor()
 
 
 ##
-# Write physical-device = MM,mm to the store, where MM and mm are the major 
-# and minor numbers of device respectively.
+# Write physical-device-path = "$1"
 #
-# @param device The device from which major and minor numbers are read, which
-#               will be written into the store.
+# @param device The device which will be written into the store.
 #
 write_dev() {
-  local mm
-  
-  mm=$(device_major_minor "$1")
- 
-  if [ -z $mm ]
-  then
-    fatal "Backend device does not exist"
-  fi
- 
-  xenstore_write "$XENBUS_PATH/physical-device" "$mm"
+  xxx check_sharing in block needs fixing too
+
+  xenstore_write "$XENBUS_PATH/physical-device-path" "$1"
 
   success
 }
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4cdc169..0bcfada 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2450,10 +2450,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
                  */
                 if (!disk->script &&
                     disk->backend_domid == LIBXL_TOOLSTACK_DOMID) {
-                    int major, minor;
-                    if (!libxl__device_physdisk_major_minor(dev, &major, &minor))
-                        flexarray_append_pair(back, "physical-device",
-                                              GCSPRINTF("%x:%x", major, minor));
+                    flexarray_append_pair(back, "physical-device-path", dev);
                 }
 
                 assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 005fe53..ada5f40 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1154,6 +1154,7 @@ _hidden char *libxl__device_disk_string_of_format(libxl_disk_format format);
 _hidden int libxl__device_disk_set_backend(libxl__gc*, libxl_device_disk*);
 
 _hidden int libxl__device_physdisk_major_minor(const char *physpath, int *major, int *minor);
+    /* xxx libxl__device_physdisk_major_minor has irregular error handling */
 _hidden int libxl__device_disk_dev_number(const char *virtpath,
                                           int *pdisk, int *ppartition);
 
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index be4afc6..aa7b42d 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -233,6 +233,65 @@ error:
     return rc;
 }
 
+static int disk_copy_block_device(libxl__gc *gc, libxl__device *dev,
+                                  libxl__device_action action)
+{
+    int rc;
+    xs_transaction_t t = 0;
+    const char *be_path = libxl__device_frontend_path(gc, dev);
+    const char *majmin_path = GCSPRINTF("%s/physical-device", be_path);
+    const char *path_path = GCSPRINTF("%s/physical-device-path", be_path);
+    int major, minor;
+
+    if (action != LIBXL__DEVICE_ACTION_ADD)
+        return 0;
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        const char *majmin;
+        rc = libxl__xs_read_checked(gc,t, majmin_path, &majmin);
+        if (rc) goto out;
+        if (majmin) {
+            /* Old-style Linux-only hotplug script wrote physical-device */
+            rc = 0;
+            goto out;
+        }
+
+        const char *bdev;
+        rc = libxl__xs_read_checked(gc,t, path_path, &bdev);
+        if (rc) goto out;
+        if (!bdev) {
+            LOGE(ERROR, "nothing wrote physical device to %s", path_path);
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        if (!libxl__device_physdisk_major_minor(bdev, &major, &minor)) {
+            LOG(ERROR, "libxl__device_physdisk_major_minor failed (on %s)",
+                bdev);
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        majmin = GCSPRINTF("%x:%x", major, minor);
+        rc = libxl__xs_write_checked(gc,t, majmin_path, majmin);
+        if (rc) goto out;
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc<0) goto out;
+    }
+
+    return rc;
+
+ out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+
 int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    char ***args, char ***env,
                                    libxl__device_action action,
@@ -245,9 +304,16 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
         if (num_exec != 0) {
             LOG(DEBUG, "num_exec %d, not running hotplug scripts", num_exec);
             rc = 0;
-            goto out;
+        } else {
+            rc = libxl__hotplug_disk(gc, dev, args, env, action);
+        }
+        if (!rc) {
+            /* No more hotplug scripts to run.  But, the hotplug
+             * scripts write physical-device-path but we blkback wants
+             * physical-device (major:minor).  So now we adjust
+             * that. */
+            rc = disk_copy_block_device(gc, dev, action);
         }
-        rc = libxl__hotplug_disk(gc, dev, args, env, action);
         break;
     case LIBXL__DEVICE_KIND_VIF:
         /*
-- 
1.7.10.4


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