[PATCH v3 0/8] libxl: add support for FreeBSD block hotplug scripts

[PATCH v3 0/8] libxl: add support for FreeBSD block hotplug scripts

[Roger Pau Monne]

This series enables using hotplug scripts with the FreeBSD blkback
implementation. Since FreeBSD blkback can use both block devices and regular
RAW files as disks, the physical-device-path xenstore backend node is used 
in order to store the absolute path to the virtual disk image (either a 
block or a regular file). This series is based on top of George Dunlap's HVM 
support for hotplug scripts (v3).

The current blkback implementation in FreeBSD HEAD doesn't yet know about
hotplug scripts, I plan to commit the FreeBSD side once this series is
reviewed.

This series can be found in my git repo at:

git://xenbits.xen.org/people/royger/xen.git freebsd_hotplug_v3

And the FreeBSD side is at:

git://xenbits.xen.org/people/royger/freebsd.git hotplug_scripts

Thanks, Roger.


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

[PATCH v3 1/8] blkif: document how FreeBSD uses the physical-device backend node

[Roger Pau Monne]

FreeBSD blkback uses the physical-device-path xenstore node in order to
fetch the path to the underlying backing storage (either a block device or
raw image). This node is set by the hotplug scripts. Also clarify the usage
of the physical-device node.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>
---
Changes since v2:
 - Use physical-device-path instead of physical-device in order to store the
   absolute path to the backing disk image.
---
 xen/include/public/io/blkif.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
index 99f0326..ce0dbfe 100644
--- a/xen/include/public/io/blkif.h
+++ b/xen/include/public/io/blkif.h
@@ -89,15 +89,23 @@
  *      Values:         string
  *
  *      A free formatted string providing sufficient information for the
- *      backend driver to open the backing device.  (e.g. the path to the
- *      file or block device representing the backing store.)
+ *      hotplug script to attach the device and provide a suitable
+ *      handler (ie: a block device) for blkback to use.
  *
  * physical-device
  *      Values:         "MAJOR:MINOR"
+ *      Notes: 11
  *
  *      MAJOR and MINOR are the major number and minor number of the
  *      backing device respectively.
  *
+ * physical-device-path
+ *      Values:         path string
+ *
+ *      A string that contains the absolute path to the disk image. On
+ *      NetBSD and Linux this is always a block device, while on FreeBSD
+ *      it can be either a block device or a regular file.
+ *
  * type
  *      Values:         "file", "phy", "tap"
  *
@@ -391,6 +399,7 @@
  *     than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST.
  *(10) The discard-secure property may be present and will be set to 1 if the
  *     backing device supports secure discard.
+ *(11) Only used by Linux and NetBSD.
  */
 
 /*
-- 
2.6.4 (Apple Git-63)


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

Re: [PATCH v3 1/8] blkif: document how FreeBSD uses the physical-device backend node

[George Dunlap]

On Thu, Mar 31, 2016 at 4:09 PM, Roger Pau Monne <roger.pau@citrix.com> wrote:
> FreeBSD blkback uses the physical-device-path xenstore node in order to
> fetch the path to the underlying backing storage (either a block device or
> raw image). This node is set by the hotplug scripts. Also clarify the usage
> of the physical-device node.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Looks good to me:

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Tim Deegan <tim@xen.org>
> ---
> Changes since v2:
>  - Use physical-device-path instead of physical-device in order to store the
>    absolute path to the backing disk image.
> ---
>  xen/include/public/io/blkif.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> index 99f0326..ce0dbfe 100644
> --- a/xen/include/public/io/blkif.h
> +++ b/xen/include/public/io/blkif.h
> @@ -89,15 +89,23 @@
>   *      Values:         string
>   *
>   *      A free formatted string providing sufficient information for the
> - *      backend driver to open the backing device.  (e.g. the path to the
> - *      file or block device representing the backing store.)
> + *      hotplug script to attach the device and provide a suitable
> + *      handler (ie: a block device) for blkback to use.
>   *
>   * physical-device
>   *      Values:         "MAJOR:MINOR"
> + *      Notes: 11
>   *
>   *      MAJOR and MINOR are the major number and minor number of the
>   *      backing device respectively.
>   *
> + * physical-device-path
> + *      Values:         path string
> + *
> + *      A string that contains the absolute path to the disk image. On
> + *      NetBSD and Linux this is always a block device, while on FreeBSD
> + *      it can be either a block device or a regular file.
> + *
>   * type
>   *      Values:         "file", "phy", "tap"
>   *
> @@ -391,6 +399,7 @@
>   *     than RING_SIZE * BLKIF_MAX_SEGMENTS_PER_REQUEST.
>   *(10) The discard-secure property may be present and will be set to 1 if the
>   *     backing device supports secure discard.
> + *(11) Only used by Linux and NetBSD.
>   */
>
>  /*
> --
> 2.6.4 (Apple Git-63)
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: [PATCH v3 1/8] blkif: document how FreeBSD uses the physical-device backend node

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3 1/8] blkif: document how FreeBSD uses the physical-device backend node"):
> FreeBSD blkback uses the physical-device-path xenstore node in order to
> fetch the path to the underlying backing storage (either a block device or
> raw image). This node is set by the hotplug scripts. Also clarify the usage
> of the physical-device node.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

[PATCH v3 2/8] hotplug/FreeBSD: add block hotplug script

[Roger Pau Monne]

This is the default hotplug script for block devices. Its only job is to
copy the "params" blkback xenstore node to "physical-device".

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/hotplug/FreeBSD/Makefile |  2 +-
 tools/hotplug/FreeBSD/block    | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 tools/hotplug/FreeBSD/block

diff --git a/tools/hotplug/FreeBSD/Makefile b/tools/hotplug/FreeBSD/Makefile
index 10fce4f..bd7a86f 100644
--- a/tools/hotplug/FreeBSD/Makefile
+++ b/tools/hotplug/FreeBSD/Makefile
@@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 # Xen script dir and scripts to go there.
-XEN_SCRIPTS = vif-bridge
+XEN_SCRIPTS = vif-bridge block
 
 XEN_SCRIPT_DATA =
 
diff --git a/tools/hotplug/FreeBSD/block b/tools/hotplug/FreeBSD/block
new file mode 100644
index 0000000..a31cca4
--- /dev/null
+++ b/tools/hotplug/FreeBSD/block
@@ -0,0 +1,25 @@
+#!/bin/sh -e
+#
+# FreeBSD hotplug script for attaching disks
+#
+# Parameters:
+#	$1: xenstore backend path of the vbd
+#	$2: action, either "add" or "remove"
+#
+# Environment variables:
+#	None
+#
+
+path=$1
+action=$2
+params=$(xenstore-read "$path/params")
+
+case $action in
+add)
+	xenstore-write $path/physical-device-path $params
+	exit 0
+	;;
+*)
+	exit 0
+	;;
+esac
-- 
2.6.4 (Apple Git-63)


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

[PATCH v3 3/8] libxl: refactor the FreeBSD hotplug script code

[Roger Pau Monne]

This factors out the nic hotplug specific code from the common code path in
order to make it easier to add support for disk hotplug scripts. It
shouldn't include any functional change.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v2:
 - Split the refactoring code out of the hotplug disk addition change.
---
 tools/libxl/libxl_freebsd.c | 76 +++++++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 31 deletions(-)

diff --git a/tools/libxl/libxl_freebsd.c b/tools/libxl/libxl_freebsd.c
index 47c3391..54db3d5 100644
--- a/tools/libxl/libxl_freebsd.c
+++ b/tools/libxl/libxl_freebsd.c
@@ -59,14 +59,36 @@ static int libxl__hotplug_env_nic(libxl__gc *gc, libxl__device *dev, char ***env
     return 0;
 }
 
-static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev, char ***args,
-                              libxl__device_action action)
+static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev,
+                              char ***args, char ***env,
+                              libxl__device_action action,
+                              int num_exec)
 {
+    libxl_nic_type nictype;
     char *be_path = libxl__device_backend_path(gc, dev);
     char *script;
-    int nr = 0, rc = 0, arraysize = 4;
+    int nr = 0, rc;
 
-    assert(dev->backend_kind == LIBXL__DEVICE_KIND_VIF);
+    rc = libxl__nic_type(gc, dev, &nictype);
+    if (rc) {
+        LOG(ERROR, "error when fetching nic type");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    /*
+     * For PV domains only one pass is needed (because there's no emulated
+     * interface). For HVM domains two passes are needed in order to add
+     * both the PV and the tap interfaces to the bridge.
+     */
+    if (nictype == LIBXL_NIC_TYPE_VIF && num_exec != 0) {
+        rc = 0;
+        goto out;
+    }
+
+    rc = libxl__hotplug_env_nic(gc, dev, env, num_exec);
+    if (rc)
+        goto out;
 
     script = libxl__xs_read(gc, XBT_NULL,
                             GCSPRINTF("%s/%s", be_path, "script"));
@@ -76,13 +98,14 @@ static int libxl__hotplug_nic(libxl__gc *gc, libxl__device *dev, char ***args,
         goto out;
     }
 
+    const int arraysize = 4;
     GCNEW_ARRAY(*args, arraysize);
     (*args)[nr++] = script;
     (*args)[nr++] = be_path;
-    (*args)[nr++] = GCSPRINTF("%s", action == LIBXL__DEVICE_ACTION_ADD ?
-                                    "add" : "remove");
+    (*args)[nr++] = (char *) libxl__device_action_to_string(action);
     (*args)[nr++] = NULL;
     assert(nr == arraysize);
+    rc = 1;
 
 out:
     return rc;
@@ -93,36 +116,27 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    libxl__device_action action,
                                    int num_exec)
 {
-    libxl_nic_type nictype;
     int rc;
 
-    if (dev->backend_kind != LIBXL__DEVICE_KIND_VIF || num_exec == 2)
-        return 0;
-
-    rc = libxl__nic_type(gc, dev, &nictype);
-    if (rc) {
-        LOG(ERROR, "error when fetching nic type");
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
-    /*
-     * For PV domains only one pass is needed (because there's no emulated
-     * interface). For HVM domains two passes are needed in order to add
-     * both the PV and the tap interfaces to the bridge.
-     */
-    if (nictype == LIBXL_NIC_TYPE_VIF && num_exec != 0) {
+    switch (dev->backend_kind) {
+    case LIBXL__DEVICE_KIND_VIF:
+        /*
+         * If domain has a stubdom we don't have to execute hotplug scripts
+         * for emulated interfaces
+         */
+        if ((num_exec > 1) ||
+            (libxl_get_stubdom_id(CTX, dev->domid) && num_exec)) {
+            rc = 0;
+            goto out;
+        }
+        rc = libxl__hotplug_nic(gc, dev, args, env, action, num_exec);
+        break;
+    default:
+        /* No need to execute any hotplug scripts */
         rc = 0;
-        goto out;
+        break;
     }
 
-    rc = libxl__hotplug_env_nic(gc, dev, env, num_exec);
-    if (rc)
-        goto out;
-
-    rc = libxl__hotplug_nic(gc, dev, args, action);
-    if (!rc) rc = 1;
-
 out:
     return rc;
 }
-- 
2.6.4 (Apple Git-63)


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

Re: [PATCH v3 3/8] libxl: refactor the FreeBSD hotplug script code

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3 3/8] libxl: refactor the FreeBSD hotplug script code"):
> This factors out the nic hotplug specific code from the common code path in
> order to make it easier to add support for disk hotplug scripts. It
> shouldn't include any functional change.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

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

[PATCH v3 4/8] libxl: add support for disk hotplug scripts on FreeBSD

[Roger Pau Monne]

Allow FreeBSD to execute hotplug scripts when attaching disk devices.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v2:
 - Split the refactoring in a separate patch.
---
 tools/libxl/libxl_freebsd.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tools/libxl/libxl_freebsd.c b/tools/libxl/libxl_freebsd.c
index 54db3d5..e86e28c 100644
--- a/tools/libxl/libxl_freebsd.c
+++ b/tools/libxl/libxl_freebsd.c
@@ -111,6 +111,35 @@ out:
     return rc;
 }
 
+static int libxl__hotplug_disk(libxl__gc *gc, libxl__device *dev,
+                               char ***args, char ***env,
+                               libxl__device_action action)
+{
+    char *be_path = libxl__device_backend_path(gc, dev);
+    char *script;
+    int nr = 0, rc;
+
+    script = libxl__xs_read(gc, XBT_NULL,
+                            GCSPRINTF("%s/%s", be_path, "script"));
+    if (!script) {
+        LOGEV(ERROR, errno, "unable to read script from %s", be_path);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    const int arraysize = 4;
+    GCNEW_ARRAY(*args, arraysize);
+    (*args)[nr++] = script;
+    (*args)[nr++] = be_path;
+    (*args)[nr++] = (char *) libxl__device_action_to_string(action);
+    (*args)[nr++] = NULL;
+    assert(nr == arraysize);
+    rc = 1;
+
+out:
+    return rc;
+}
+
 int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
                                    char ***args, char ***env,
                                    libxl__device_action action,
@@ -131,6 +160,13 @@ int libxl__get_hotplug_script_info(libxl__gc *gc, libxl__device *dev,
         }
         rc = libxl__hotplug_nic(gc, dev, args, env, action, num_exec);
         break;
+    case LIBXL__DEVICE_KIND_VBD:
+        if (num_exec != 0) {
+            rc = 0;
+            goto out;
+        }
+        rc = libxl__hotplug_disk(gc, dev, args, env, action);
+        break;
     default:
         /* No need to execute any hotplug scripts */
         rc = 0;
-- 
2.6.4 (Apple Git-63)


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

Re: [PATCH v3 4/8] libxl: add support for disk hotplug scripts on FreeBSD

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3 4/8] libxl: add support for disk hotplug scripts on FreeBSD"):
> Allow FreeBSD to execute hotplug scripts when attaching disk devices.

I am going to commit the FreeBSD-only patches without detailed
review, on the basis that you're the expert.

Thanks,
Ian.

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

[PATCH v3 5/8] libxl: properly use vdev vs local device

[Roger Pau Monne]

The current code in libxl assumed that vdev is equal to local device, but
this is only true for Linux systems. In other OSes the local device can use
a nomenclature completely different from the virtual device one.

Move the current libxl__devid_to_localdev Linux implementation out of the
OS-specific file and rename it to libxl__devid_to_vdev, and then make sure
local_device_attach_cb return the local device in the diskpath field.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c          | 12 ++++------
 tools/libxl/libxl_device.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/libxl_linux.c    | 49 +------------------------------------
 4 files changed, 64 insertions(+), 55 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 944f4d1..a2767fa 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2989,7 +2989,7 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void *get_vdev_user,
                     GCSPRINTF("%s/device/vbd/%d/backend",
                         dompath, devid)) == NULL) {
             if (errno == ENOENT)
-                return libxl__devid_to_localdev(gc, devid);
+                return libxl__devid_to_vdev(gc, devid);
             else
                 return NULL;
         }
@@ -3126,7 +3126,7 @@ static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev)
 {
     STATE_AO_GC(aodev->ao);
     libxl__disk_local_state *dls = CONTAINER_OF(aodev, *dls, aodev);
-    char *dev = NULL, *be_path = NULL;
+    char *be_path = NULL;
     int rc;
     libxl__device device;
     libxl_device_disk *disk = &dls->disk;
@@ -3140,9 +3140,6 @@ static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev)
         goto out;
     }
 
-    dev = GCSPRINTF("/dev/%s", disk->vdev);
-    LOG(DEBUG, "locally attaching disk %s", dev);
-
     rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &device);
     if (rc < 0)
         goto out;
@@ -3151,8 +3148,9 @@ static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev)
     if (rc < 0)
         goto out;
 
-    if (dev != NULL)
-        dls->diskpath = libxl__strdup(gc, dev);
+    dls->diskpath = GCSPRINTF("/dev/%s",
+                              libxl__devid_to_localdev(gc, device.devid));
+    LOG(DEBUG, "locally attached disk %s", dls->diskpath);
 
     dls->callback(egc, dls, 0);
     return;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 4ced9b6..9410563 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -436,6 +436,63 @@ int libxl__device_disk_dev_number(const char *virtpath, int *pdisk,
     return -1;
 }
 
+static char *encode_disk_name(char *ptr, unsigned int n)
+{
+    if (n >= 26)
+        ptr = encode_disk_name(ptr, n / 26 - 1);
+    *ptr = 'a' + n % 26;
+    return ptr + 1;
+}
+
+char *libxl__devid_to_vdev(libxl__gc *gc, int devid)
+{
+    unsigned int minor;
+    int offset;
+    int nr_parts;
+    char *ptr = NULL;
+/* Same as in Linux.
+ * encode_disk_name might end up using up to 29 bytes (BUFFER_SIZE - 3)
+ * including the trailing \0.
+ *
+ * The code is safe because 26 raised to the power of 28 (that is the
+ * maximum offset that can be stored in the allocated buffer as a
+ * string) is far greater than UINT_MAX on 64 bits so offset cannot be
+ * big enough to exhaust the available bytes in ret. */
+#define BUFFER_SIZE 32
+    char *ret = libxl__zalloc(gc, BUFFER_SIZE);
+
+#define EXT_SHIFT 28
+#define EXTENDED (1<<EXT_SHIFT)
+#define VDEV_IS_EXTENDED(dev) ((dev)&(EXTENDED))
+#define BLKIF_MINOR_EXT(dev) ((dev)&(~EXTENDED))
+/* the size of the buffer to store the device name is 32 bytes to match the
+ * equivalent buffer in the Linux kernel code */
+
+    if (!VDEV_IS_EXTENDED(devid)) {
+        minor = devid & 0xff;
+        nr_parts = 16;
+    } else {
+        minor = BLKIF_MINOR_EXT(devid);
+        nr_parts = 256;
+    }
+    offset = minor / nr_parts;
+
+    strcpy(ret, "xvd");
+    ptr = encode_disk_name(ret + 3, offset);
+    if (minor % nr_parts == 0)
+        *ptr = 0;
+    else
+        /* overflow cannot happen, thanks to the upper bound */
+        snprintf(ptr, ret + 32 - ptr,
+                "%d", minor & (nr_parts - 1));
+    return ret;
+#undef BUFFER_SIZE
+#undef EXT_SHIFT
+#undef EXTENDED
+#undef VDEV_IS_EXTENDED
+#undef BLKIF_MINOR_EXT
+}
+
 /* Device AO operations */
 
 void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9fde212..6f80917 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1156,6 +1156,7 @@ _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);
 _hidden int libxl__device_disk_dev_number(const char *virtpath,
                                           int *pdisk, int *ppartition);
+_hidden char *libxl__devid_to_vdev(libxl__gc *gc, int devid);
 
 _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                                       libxl__device_console *console,
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index be4afc6..624f028 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -26,56 +26,9 @@ int libxl__try_phy_backend(mode_t st_mode)
     return 0;
 }
 
-#define EXT_SHIFT 28
-#define EXTENDED (1<<EXT_SHIFT)
-#define VDEV_IS_EXTENDED(dev) ((dev)&(EXTENDED))
-#define BLKIF_MINOR_EXT(dev) ((dev)&(~EXTENDED))
-/* the size of the buffer to store the device name is 32 bytes to match the
- * equivalent buffer in the Linux kernel code */
-#define BUFFER_SIZE 32
-
-/* Same as in Linux.
- * encode_disk_name might end up using up to 29 bytes (BUFFER_SIZE - 3)
- * including the trailing \0.
- *
- * The code is safe because 26 raised to the power of 28 (that is the
- * maximum offset that can be stored in the allocated buffer as a
- * string) is far greater than UINT_MAX on 64 bits so offset cannot be
- * big enough to exhaust the available bytes in ret. */
-static char *encode_disk_name(char *ptr, unsigned int n)
-{
-    if (n >= 26)
-        ptr = encode_disk_name(ptr, n / 26 - 1);
-    *ptr = 'a' + n % 26;
-    return ptr + 1;
-}
-
 char *libxl__devid_to_localdev(libxl__gc *gc, int devid)
 {
-    unsigned int minor;
-    int offset;
-    int nr_parts;
-    char *ptr = NULL;
-    char *ret = libxl__zalloc(gc, BUFFER_SIZE);
-
-    if (!VDEV_IS_EXTENDED(devid)) {
-        minor = devid & 0xff;
-        nr_parts = 16;
-    } else {
-        minor = BLKIF_MINOR_EXT(devid);
-        nr_parts = 256;
-    }
-    offset = minor / nr_parts;
-
-    strcpy(ret, "xvd");
-    ptr = encode_disk_name(ret + 3, offset);
-    if (minor % nr_parts == 0)
-        *ptr = 0;
-    else
-        /* overflow cannot happen, thanks to the upper bound */
-        snprintf(ptr, ret + 32 - ptr,
-                "%d", minor & (nr_parts - 1));
-    return ret;
+    return libxl__devid_to_vdev(gc, devid);
 }
 
 /* Hotplug scripts helpers */
-- 
2.6.4 (Apple Git-63)


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

[PATCH v3 6/8] libxl: add a FreeBSD implementation of libxl__devid_to_localdev

[Roger Pau Monne]

This code is extracted from the FreeBSD blkfront implementation.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_freebsd.c | 54 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_freebsd.c b/tools/libxl/libxl_freebsd.c
index e86e28c..c527455 100644
--- a/tools/libxl/libxl_freebsd.c
+++ b/tools/libxl/libxl_freebsd.c
@@ -27,8 +27,58 @@ int libxl__try_phy_backend(mode_t st_mode)
 
 char *libxl__devid_to_localdev(libxl__gc *gc, int devid)
 {
-    /* TODO */
-    return NULL;
+    /* This translation table has been copied from the FreeBSD blkfront code. */
+    const static struct vdev_info {
+        int major;
+        int shift;
+        int base;
+        const char *name;
+    } info[] = {
+        {3,     6,  0,      "ada"}, /* ide0 */
+        {22,    6,  2,      "ada"}, /* ide1 */
+        {33,    6,  4,      "ada"}, /* ide2 */
+        {34,    6,  6,      "ada"}, /* ide3 */
+        {56,    6,  8,      "ada"}, /* ide4 */
+        {57,    6,  10,     "ada"}, /* ide5 */
+        {88,    6,  12,     "ada"}, /* ide6 */
+        {89,    6,  14,     "ada"}, /* ide7 */
+        {90,    6,  16,     "ada"}, /* ide8 */
+        {91,    6,  18,     "ada"}, /* ide9 */
+
+        {8,     4,  0,      "da"},  /* scsi disk0 */
+        {65,    4,  16,     "da"},  /* scsi disk1 */
+        {66,    4,  32,     "da"},  /* scsi disk2 */
+        {67,    4,  48,     "da"},  /* scsi disk3 */
+        {68,    4,  64,     "da"},  /* scsi disk4 */
+        {69,    4,  80,     "da"},  /* scsi disk5 */
+        {70,    4,  96,     "da"},  /* scsi disk6 */
+        {71,    4,  112,    "da"},  /* scsi disk7 */
+        {128,   4,  128,    "da"},  /* scsi disk8 */
+        {129,   4,  144,    "da"},  /* scsi disk9 */
+        {130,   4,  160,    "da"},  /* scsi disk10 */
+        {131,   4,  176,    "da"},  /* scsi disk11 */
+        {132,   4,  192,    "da"},  /* scsi disk12 */
+        {133,   4,  208,    "da"},  /* scsi disk13 */
+        {134,   4,  224,    "da"},  /* scsi disk14 */
+        {135,   4,  240,    "da"},  /* scsi disk15 */
+
+        {202,   4,  0,      "xbd"}, /* xbd */
+
+        {0, 0,  0,  NULL},
+    };
+    int major = devid >> 8;
+    int minor = devid & 0xff;
+    int i;
+
+    if (devid & (1 << 28))
+        return GCSPRINTF("%s%d", "xbd", (devid & ((1 << 28) - 1)) >> 8);
+
+    for (i = 0; info[i].major; i++)
+        if (info[i].major == major)
+            return GCSPRINTF("%s%d", info[i].name,
+                             info[i].base + (minor >> info[i].shift));
+
+    return GCSPRINTF("%s%d", "xbd", minor >> 4);
 }
 
 /* Hotplug scripts caller functions */
-- 
2.6.4 (Apple Git-63)


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

[PATCH v3 7/8] libxl: fix error message in local_device_attach_cb

[Roger Pau Monne]

The fields that are printed might not be set in the case of a failure, which
generates a segmentation fault.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a2767fa..0d2f55c 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3133,10 +3133,7 @@ static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev)
 
     rc = aodev->rc;
     if (rc) {
-        LOGE(ERROR, "unable to %s %s with id %u",
-                    libxl__device_action_to_string(aodev->action),
-                    libxl__device_kind_to_string(aodev->dev->kind),
-                    aodev->dev->devid);
+        LOGE(ERROR, "unable locally attach device: %s", disk->pdev_path);
         goto out;
     }
 
-- 
2.6.4 (Apple Git-63)


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

[PATCH v3 8/8] hotplug/FreeBSD: document disk hotplug interface

[Roger Pau Monne]

Add the FreeBSD disk hotplug interface details to the block-scripts.txt
document.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 docs/misc/block-scripts.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/docs/misc/block-scripts.txt b/docs/misc/block-scripts.txt
index f9a001e..eabab10 100644
--- a/docs/misc/block-scripts.txt
+++ b/docs/misc/block-scripts.txt
@@ -48,6 +48,19 @@ written into xenstore:
  $XENBUS/params    The contents of the 'target' section of the disk specification verbatim.
  $XENBUS/mode      'r' (for readonly) or 'w' (for read-write)
 
+Inputs (FreeBSD)
+--------------
+
+The scripts are always called with the same set of arguments. The first
+parameter is the xenstore backend path of the device, while the second
+argument is the action, which is always either "add" or "remove".
+
+When the script is run, the following nodes shall already have been
+written into xenstore:
+
+ $XENBUS/params    The contents of the 'target' section of the disk specification verbatim.
+ $XENBUS/mode      'r' (for readonly) or 'w' (for read-write)
+
 Inputs (NetBSD)
 ---------------
 
-- 
2.6.4 (Apple Git-63)


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

Re: [PATCH v3 8/8] hotplug/FreeBSD: document disk hotplug interface

[George Dunlap]

On Thu, Mar 31, 2016 at 4:09 PM, Roger Pau Monne <roger.pau@citrix.com> wrote:
> Add the FreeBSD disk hotplug interface details to the block-scripts.txt
> document.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks!

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

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

Re: [PATCH v3 0/8] libxl: add support for FreeBSD block hotplug scripts

[George Dunlap]

On Thu, Mar 31, 2016 at 4:09 PM, Roger Pau Monne <roger.pau@citrix.com> wrote:
> This series enables using hotplug scripts with the FreeBSD blkback
> implementation. Since FreeBSD blkback can use both block devices and regular
> RAW files as disks, the physical-device-path xenstore backend node is used
> in order to store the absolute path to the virtual disk image (either a
> block or a regular file). This series is based on top of George Dunlap's HVM
> support for hotplug scripts (v3).
>
> The current blkback implementation in FreeBSD HEAD doesn't yet know about
> hotplug scripts, I plan to commit the FreeBSD side once this series is
> reviewed.
>
> This series can be found in my git repo at:
>
> git://xenbits.xen.org/people/royger/xen.git freebsd_hotplug_v3

Looks like you've also got a couple of other miscellaneous fixes in
the tree that aren't in the series:

66294f1 Thu Mar 31 14:56:35 2016 +0200    Roger Pau Monne
tools/build: remove usage of sed -i
663275d Thu Feb 25 16:29:04 2016 +0100    Roger Pau Monne    libxl:
fix POLLHUP handling

:-)

(Just pointing that out so nobody else is similarly confused)

 -George

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

Re: [PATCH v3 0/8] libxl: add support for FreeBSD block hotplug scripts

[Roger Pau Monné]

On Thu, 31 Mar 2016, George Dunlap wrote:

> On Thu, Mar 31, 2016 at 4:09 PM, Roger Pau Monne <roger.pau@citrix.com> wrote:
> > This series enables using hotplug scripts with the FreeBSD blkback
> > implementation. Since FreeBSD blkback can use both block devices and regular
> > RAW files as disks, the physical-device-path xenstore backend node is used
> > in order to store the absolute path to the virtual disk image (either a
> > block or a regular file). This series is based on top of George Dunlap's HVM
> > support for hotplug scripts (v3).
> >
> > The current blkback implementation in FreeBSD HEAD doesn't yet know about
> > hotplug scripts, I plan to commit the FreeBSD side once this series is
> > reviewed.
> >
> > This series can be found in my git repo at:
> >
> > git://xenbits.xen.org/people/royger/xen.git freebsd_hotplug_v3
> 
> Looks like you've also got a couple of other miscellaneous fixes in
> the tree that aren't in the series:
> 
> 66294f1 Thu Mar 31 14:56:35 2016 +0200    Roger Pau Monne
> tools/build: remove usage of sed -i
> 663275d Thu Feb 25 16:29:04 2016 +0100    Roger Pau Monne    libxl:
> fix POLLHUP handling
> 
> :-)
> 
> (Just pointing that out so nobody else is similarly confused)

Oh yes, sorry, those are needed in order to build or test on FreeBSD. They 
are all already posted to xen-devel (just not as part of this series 
because it didn't make sense).

Roger.

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

Re: [PATCH v3 0/8] libxl: add support for FreeBSD block hotplug scripts

[Ian Jackson]

Roger Pau Monne writes ("[Xen-devel] [PATCH v3 0/8] libxl: add support for FreeBSD block hotplug scripts"):
> This series enables using hotplug scripts with the FreeBSD blkback
> implementation. Since FreeBSD blkback can use both block devices and regular
> RAW files as disks, the physical-device-path xenstore backend node is used 
> in order to store the absolute path to the virtual disk image (either a 
> block or a regular file). This series is based on top of George Dunlap's HVM 
> support for hotplug scripts (v3).
> 
> The current blkback implementation in FreeBSD HEAD doesn't yet know about
> hotplug scripts, I plan to commit the FreeBSD side once this series is
> reviewed.
> 
> This series can be found in my git repo at:
> 
> git://xenbits.xen.org/people/royger/xen.git freebsd_hotplug_v3

I have queued this for staging.

Thanks,
Ian.

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