[PATCH v7 0/11] qdisk local attach

[PATCH v7 0/11] qdisk local attach

[Stefano Stabellini]

Hi all,
this patch implements local_attach support for QDISK: that means that
you can use qcow2 as disk format for your PV guests disks and use
pygrub to retrieve the kernel and initrd in dom0.

The idea is that we start a QEMU instance at boot time to listen to
local_attach requests. When libxl_device_disk_local_attach is called on
a QDISK images, libxl sets up a backend/frontend pair in dom0 for the disk
so that blkfront in dom0 will create a new xvd device for it.
Then pygrub can be pointed at this device to retrieve kernel and
initrd.


Changes in v7:
- rename tmpdisk to localdisk;
- add a comment in libxl__bootloader_state about localdisk;
- keep libxl__device_from_disk in libxl.c;
- implement libxl__device_disk_add in libxl.c;
- remove the upper bound in libxl__devid_to_localdev and document why.


Changes in v6:
- rebase on "nstore: rename public xenstore headers";
- new patch: "libxl_string_to_backend: add qdisk";
- new patch: "main_blockdetach: destroy the disk on successful removal";
- split "libxl: introduce libxl__device_disk_add" in two patches;
- return error in case pdev_path is NULL;
- libxl__device_disk_local_attach update the new disk, the caller
allocates it;
- remove \n from logs;
- document blkdev_start;
- use libxl__strdup to allocate blkdev_start; 
- more comments in libxl__devid_to_localdev;
- inline GCSPRINTF;
- introduce ret for xs_* error codes.


Changes in v5:
- remove _hidden from the libxl_device_disk_local_attach/detach
  implementation;
- libxl__device_disk_local_attach: rename disk to in_disk;
- libxl__device_disk_local_attach: rename tmpdisk to disk;
- libxl__device_disk_local_attach: copy disk to new_disk only on success;
- libxl__device_disk_local_attach: remove check on libxl__zalloc success.
- rename libxl__device_generic_add_t to libxl__device_generic_add;
- change the type of the blkdev_start parameter to
  libxl__device_disk_local_attach to const char *;
- remove domid paramter to libxl__alloc_vdev (assume
  LIBXL_TOOLSTACK_DOMID);
- remove scaling limit from libxl__alloc_vdev;
- libxl__device_disk_local_attach: replace LIBXL__LOG with LOG;
- libxl__device_disk_local_attach: unify error paths.


Changes in v4:
- split the first patch into 2 patches: the first is a simple move, the
  second one adds a new parameter;
- libxl__device_generic_add: do not end the transaction is the caller
  provided it;
- libxl__device_generic_add: fix success exit path;
- pass blkdev_start in libxl_domain_build_info;
- rename libxl__devid_to_vdev to libxl__devid_to_localdev;
- introduce upper bound for encode_disk_name;
- improve error handling and exit paths in libxl__alloc_vdev and
  libxl__device_disk_local_attach.


Changes in v3:
- libxl__device_disk_local_attach: wait for the backend to be
"connected" before returning.


Changes in v2:
- make libxl_device_disk_local_(de)attach internal functions;
- allocate the new disk in libxl_device_disk_local_attatch on the GC;
- remove libxl__device_generic_add_t, add a transaction parameter to
libxl__device_generic_add instead;
- add a new patch to introduce a blkdev_start option to xl.conf;
- reimplement libxl__alloc_vdev checking for the frontend path and
starting from blkdev_start;
- introduce a Linux specific libxl__devid_to_vdev function based on last
Jan's patch to blkfront.


Stefano Stabellini (11):
      libxl: make libxl_device_disk_local_attach/detach internal functions
      libxl: libxl__device_disk_local_attach return a new libxl_device_disk
      libxl: add a transaction parameter to libxl__device_generic_add
      libxl: export libxl__device_from_disk
      libxl: introduce libxl__device_disk_add
      xl/libxl: add a blkdev_start parameter
      libxl: introduce libxl__alloc_vdev
      xl/libxl: implement QDISK libxl_device_disk_local_attach
      libxl__device_disk_local_attach: wait for state "connected"
      libxl_string_to_backend: add qdisk
      main_blockdetach: destroy the disk on successful removal

 docs/man/xl.conf.pod.5                          |    6 +
 tools/examples/xl.conf                          |    3 +
 tools/hotplug/Linux/init.d/sysconfig.xencommons |    3 +
 tools/hotplug/Linux/init.d/xencommons           |    3 +
 tools/libxl/libxl.c                             |  103 +++------------
 tools/libxl/libxl.h                             |    7 -
 tools/libxl/libxl_bootloader.c                  |    7 +-
 tools/libxl/libxl_create.c                      |    3 +
 tools/libxl/libxl_device.c                      |   17 ++-
 tools/libxl/libxl_internal.c                    |  166 +++++++++++++++++++++++
 tools/libxl/libxl_internal.h                    |   32 ++++-
 tools/libxl/libxl_linux.c                       |   52 +++++++
 tools/libxl/libxl_netbsd.c                      |    6 +
 tools/libxl/libxl_pci.c                         |    2 +-
 tools/libxl/libxl_types.idl                     |    1 +
 tools/libxl/libxl_utils.c                       |    2 +
 tools/libxl/xl.c                                |    3 +
 tools/libxl/xl.h                                |    1 +
 tools/libxl/xl_cmdimpl.c                        |    5 +-
 19 files changed, 317 insertions(+), 105 deletions(-)


Cheers,

Stefano

[PATCH v7 01/11] libxl: make libxl_device_disk_local_attach/detach internal functions

[Stefano Stabellini]

Changes in v5:

- remove _hidden from the implementation.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl.c            |   77 ----------------------------------------
 tools/libxl/libxl.h            |    7 ----
 tools/libxl/libxl_bootloader.c |    4 +-
 tools/libxl/libxl_internal.c   |   72 +++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h   |    9 +++++
 5 files changed, 83 insertions(+), 86 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index f24d021..9ec0130 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1735,83 +1735,6 @@ out:
     return ret;
 }
 
-char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk)
-{
-    GC_INIT(ctx);
-    char *dev = NULL;
-    char *ret = NULL;
-    int rc;
-
-    rc = libxl__device_disk_setdefault(gc, disk);
-    if (rc) goto out;
-
-    switch (disk->backend) {
-        case LIBXL_DISK_BACKEND_PHY:
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk %s",
-                       disk->pdev_path);
-            dev = disk->pdev_path;
-            break;
-        case LIBXL_DISK_BACKEND_TAP:
-            switch (disk->format) {
-            case LIBXL_DISK_FORMAT_RAW:
-                /* optimise away the early tapdisk attach in this case */
-                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching"
-                           " tap disk %s directly (ie without using blktap)",
-                           disk->pdev_path);
-                dev = disk->pdev_path;
-                break;
-            case LIBXL_DISK_FORMAT_VHD:
-                dev = libxl__blktap_devpath(gc, disk->pdev_path,
-                                            disk->format);
-                break;
-            case LIBXL_DISK_FORMAT_QCOW:
-            case LIBXL_DISK_FORMAT_QCOW2:
-                abort(); /* prevented by libxl__device_disk_set_backend */
-            default:
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
-                           "unrecognized disk format: %d", disk->format);
-                break;
-            }
-            break;
-        case LIBXL_DISK_BACKEND_QDISK:
-            if (disk->format != LIBXL_DISK_FORMAT_RAW) {
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally"
-                           " attach a qdisk image if the format is not raw");
-                break;
-            }
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
-                       disk->pdev_path);
-            dev = disk->pdev_path;
-            break;
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
-                "type: %d", disk->backend);
-            break;
-    }
-
- out:
-    if (dev != NULL)
-        ret = strdup(dev);
-    GC_FREE;
-    return ret;
-}
-
-int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk)
-{
-    /* Nothing to do for PHYSTYPE_PHY. */
-
-    /*
-     * For other device types assume that the blktap2 process is
-     * needed by the soon to be started domain and do nothing.
-     */
-    /*
-     * FIXME
-     * This appears to leak the disk in failure paths
-     */
-
-    return 0;
-}
-
 /******************************************************************************/
 
 int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 316d290..21e6510 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -686,13 +686,6 @@ int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
  */
 int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk);
 
-/*
- * Make a disk available in this (the control) domain. Returns path to
- * a device.
- */
-char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk);
-int libxl_device_disk_local_detach(libxl_ctx *ctx, libxl_device_disk *disk);
-
 /* Network Interfaces */
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic);
 int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index ca8409c..60863e4 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -228,7 +228,7 @@ static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl)
     if (bl->outputdir) libxl__remove_directory(gc, bl->outputdir);
 
     if (bl->diskpath) {
-        libxl_device_disk_local_detach(CTX, bl->disk);
+        libxl__device_disk_local_detach(gc, bl->disk);
         free(bl->diskpath);
         bl->diskpath = 0;
     }
@@ -330,7 +330,7 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
         goto out;
     }
 
-    bl->diskpath = libxl_device_disk_local_attach(CTX, bl->disk);
+    bl->diskpath = libxl__device_disk_local_attach(gc, bl->disk);
     if (!bl->diskpath) {
         rc = ERROR_FAIL;
         goto out;
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index b89aef7..fc771ff 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -323,6 +323,78 @@ out:
     return rc;
 }
 
+char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
+{
+    libxl_ctx *ctx = gc->owner;
+    char *dev = NULL;
+    char *ret = NULL;
+    int rc;
+
+    rc = libxl__device_disk_setdefault(gc, disk);
+    if (rc) goto out;
+
+    switch (disk->backend) {
+        case LIBXL_DISK_BACKEND_PHY:
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching PHY disk %s",
+                       disk->pdev_path);
+            dev = disk->pdev_path;
+            break;
+        case LIBXL_DISK_BACKEND_TAP:
+            switch (disk->format) {
+            case LIBXL_DISK_FORMAT_RAW:
+                /* optimise away the early tapdisk attach in this case */
+                LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching"
+                           " tap disk %s directly (ie without using blktap)",
+                           disk->pdev_path);
+                dev = disk->pdev_path;
+                break;
+            case LIBXL_DISK_FORMAT_VHD:
+                dev = libxl__blktap_devpath(gc, disk->pdev_path,
+                                            disk->format);
+                break;
+            case LIBXL_DISK_FORMAT_QCOW:
+            case LIBXL_DISK_FORMAT_QCOW2:
+                abort(); /* prevented by libxl__device_disk_set_backend */
+            default:
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                           "unrecognized disk format: %d", disk->format);
+                break;
+            }
+            break;
+        case LIBXL_DISK_BACKEND_QDISK:
+            if (disk->format != LIBXL_DISK_FORMAT_RAW) {
+                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally"
+                           " attach a qdisk image if the format is not raw");
+                break;
+            }
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
+                       disk->pdev_path);
+            dev = disk->pdev_path;
+            break;
+        default:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
+                "type: %d", disk->backend);
+            break;
+    }
+
+ out:
+    if (dev != NULL)
+        ret = strdup(dev);
+    return ret;
+}
+
+int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk)
+{
+    /* Nothing to do for PHYSTYPE_PHY. */
+
+    /*
+     * For other device types assume that the blktap2 process is
+     * needed by the soon to be started domain and do nothing.
+     */
+
+    return 0;
+}
+
 libxl_device_model_version libxl__device_model_version_running(libxl__gc *gc,
                                                                uint32_t domid)
 {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 49d01a8..343752b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1236,6 +1236,15 @@ _hidden char *libxl__blktap_devpath(libxl__gc *gc,
  */
 _hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
 
+/*
+ * Make a disk available in this (the control) domain. Returns path to
+ * a device.
+ */
+_hidden char * libxl__device_disk_local_attach(libxl__gc *gc,
+        libxl_device_disk *disk);
+_hidden int libxl__device_disk_local_detach(libxl__gc *gc,
+        libxl_device_disk *disk);
+
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);
 
 struct libxl__xen_console_reader {
-- 
1.7.2.5

[PATCH v7 02/11] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

[Stefano Stabellini]

Introduce a new libxl_device_disk* parameter to
libxl__device_disk_local_attach, the parameter is allocated by the
caller. libxl__device_disk_local_attach is going to fill the new disk
with informations about the new locally attached disk.  The new
libxl_device_disk should be passed to libxl_device_disk_local_detach
afterwards.

Changes in v7:
- rename tmpdisk to localdisk;
- add a comment in libxl__bootloader_state about localdisk.

Changes in v6:
- return error in case pdev_path is NULL;
- libxl__device_disk_local_attach update the new disk, the caller
allocates it;
- remove \n from logs.

Changes in v5:
- rename disk to in_disk;
- rename tmpdisk to disk;
- copy disk to new_disk only on success;
- remove check on libxl__zalloc success.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl_bootloader.c |    6 ++++--
 tools/libxl/libxl_internal.c   |   17 ++++++++++++++---
 tools/libxl/libxl_internal.h   |   10 +++++++++-
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 60863e4..87dfed7 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -228,7 +228,9 @@ static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl)
     if (bl->outputdir) libxl__remove_directory(gc, bl->outputdir);
 
     if (bl->diskpath) {
-        libxl__device_disk_local_detach(gc, bl->disk);
+        libxl__device_disk_local_detach(gc, &bl->localdisk);
+        free(bl->localdisk.pdev_path);
+        free(bl->localdisk.script);
         free(bl->diskpath);
         bl->diskpath = 0;
     }
@@ -330,7 +332,7 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
         goto out;
     }
 
-    bl->diskpath = libxl__device_disk_local_attach(gc, bl->disk);
+    bl->diskpath = libxl__device_disk_local_attach(gc, bl->disk, &bl->localdisk);
     if (!bl->diskpath) {
         rc = ERROR_FAIL;
         goto out;
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index fc771ff..5e5083b 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -323,13 +323,24 @@ out:
     return rc;
 }
 
-char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
+char * libxl__device_disk_local_attach(libxl__gc *gc,
+        const libxl_device_disk *in_disk,
+        libxl_device_disk *disk)
 {
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL;
     char *ret = NULL;
     int rc;
 
+    if (in_disk->pdev_path == NULL)
+        return NULL;
+
+    memcpy(disk, in_disk, sizeof(libxl_device_disk));
+    disk->pdev_path = strdup(in_disk->pdev_path);
+    if (in_disk->script != NULL)
+        disk->script = strdup(in_disk->script);
+    disk->vdev = NULL;
+
     rc = libxl__device_disk_setdefault(gc, disk);
     if (rc) goto out;
 
@@ -367,8 +378,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
                            " attach a qdisk image if the format is not raw");
                 break;
             }
-            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
-                       disk->pdev_path);
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s",
+                       in_disk->pdev_path);
             dev = disk->pdev_path;
             break;
         default:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 343752b..bdd3303 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1241,7 +1241,8 @@ _hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
  * a device.
  */
 _hidden char * libxl__device_disk_local_attach(libxl__gc *gc,
-        libxl_device_disk *disk);
+        const libxl_device_disk *in_disk,
+        libxl_device_disk *new_disk);
 _hidden int libxl__device_disk_local_detach(libxl__gc *gc,
         libxl_device_disk *disk);
 
@@ -1775,6 +1776,13 @@ struct libxl__bootloader_state {
     libxl__bootloader_console_callback *console_available;
     libxl_domain_build_info *info; /* u.pv.{kernel,ramdisk,cmdline} updated */
     libxl_device_disk *disk;
+    /* Should be zeroed by caller on entry.  Will be filled in by
+     * bootloader machinery; represents the local attachment of the
+     * disk for the benefit of the bootloader.  Must be detached by
+     * the caller using libxl__device_disk_local_detach, but only
+     * after the domain's kernel and initramfs have been loaded into
+     * memory and the file references disposed of. */
+    libxl_device_disk localdisk;
     uint32_t domid;
     /* private to libxl__run_bootloader */
     char *outputpath, *outputdir, *logfile;
-- 
1.7.2.5

[PATCH v7 03/11] libxl: add a transaction parameter to libxl__device_generic_add

[Stefano Stabellini]

Add a xs_transaction_t parameter to libxl__device_generic_add, if it is
XBT_NULL, allocate a proper one.

Update all the callers.

This patch contains a large number of unchecked xenstore operations, we
might want to fix this in the future.

Changes in v4:
- libxl__device_generic_add: do not end the transaction is the caller
provided it;
- libxl__device_generic_add: fix success exit path.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c          |   10 +++++-----
 tools/libxl/libxl_device.c   |   17 +++++++++++------
 tools/libxl/libxl_internal.h |    4 ++--
 tools/libxl/libxl_pci.c      |    2 +-
 4 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9ec0130..7b82470 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1472,7 +1472,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
     flexarray_append(front, "device-type");
     flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
 
-    libxl__device_generic_add(gc, &device,
+    libxl__device_generic_add(gc, XBT_NULL, &device,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
@@ -1873,7 +1873,7 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic)
     flexarray_append(front, "mac");
     flexarray_append(front, libxl__sprintf(gc,
                                     LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
-    libxl__device_generic_add(gc, &device,
+    libxl__device_generic_add(gc, XBT_NULL, &device,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
@@ -2166,7 +2166,7 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
         flexarray_append(front, LIBXL_XENCONSOLE_PROTOCOL);
     }
 
-    libxl__device_generic_add(gc, &device,
+    libxl__device_generic_add(gc, XBT_NULL, &device,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
     rc = 0;
@@ -2237,7 +2237,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb)
     flexarray_append(front, "state");
     flexarray_append(front, libxl__sprintf(gc, "%d", 1));
 
-    libxl__device_generic_add(gc, &device,
+    libxl__device_generic_add(gc, XBT_NULL, &device,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
     rc = 0;
@@ -2370,7 +2370,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb)
                           libxl__sprintf(gc, "%d", vfb->backend_domid));
     flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", 1));
 
-    libxl__device_generic_add(gc, &device,
+    libxl__device_generic_add(gc, XBT_NULL, &device,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
     rc = 0;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index c7e057d..88cdc7d 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -58,14 +58,14 @@ int libxl__parse_backend_path(libxl__gc *gc,
     return libxl__device_kind_from_string(strkind, &dev->backend_kind);
 }
 
-int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
-                             char **bents, char **fents)
+int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
+        libxl__device *device, char **bents, char **fents)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *frontend_path, *backend_path;
-    xs_transaction_t t;
     struct xs_permissions frontend_perms[2];
     struct xs_permissions backend_perms[2];
+    int create_transaction = t == XBT_NULL;
 
     frontend_path = libxl__device_frontend_path(gc, device);
     backend_path = libxl__device_backend_path(gc, device);
@@ -81,7 +81,8 @@ int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
     backend_perms[1].perms = XS_PERM_READ;
 
 retry_transaction:
-    t = xs_transaction_start(ctx->xsh);
+    if (create_transaction)
+        t = xs_transaction_start(ctx->xsh);
     /* FIXME: read frontend_path and check state before removing stuff */
 
     if (fents) {
@@ -100,13 +101,17 @@ retry_transaction:
         libxl__xs_writev(gc, t, backend_path, bents);
     }
 
+    if (!create_transaction)
+        return 0;
+
     if (!xs_transaction_end(ctx->xsh, t, 0)) {
         if (errno == EAGAIN)
             goto retry_transaction;
-        else
+        else {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs transaction failed");
+            return ERROR_FAIL;
+        }
     }
-
     return 0;
 }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index bdd3303..f3833bb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -784,8 +784,8 @@ _hidden int libxl__device_console_add(libxl__gc *gc, uint32_t domid,
                                       libxl__device_console *console,
                                       libxl__domain_build_state *state);
 
-_hidden int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
-                             char **bents, char **fents);
+_hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
+        libxl__device *device, char **bents, char **fents);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index e980995..92764fc 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -103,7 +103,7 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid,
     flexarray_append_pair(front, "backend-id", libxl__sprintf(gc, "%d", 0));
     flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", 1));
 
-    libxl__device_generic_add(gc, &device,
+    libxl__device_generic_add(gc, XBT_NULL, &device,
                               libxl__xs_kvs_of_flexarray(gc, back, back->count),
                               libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
-- 
1.7.2.5

[PATCH v7 04/11] libxl: export libxl__device_from_disk

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl.c          |    2 +-
 tools/libxl/libxl_internal.h |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 7b82470..37f36d7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1327,7 +1327,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
     return rc;
 }
 
-static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
                                    libxl_device_disk *disk,
                                    libxl__device *device)
 {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index f3833bb..d7ed657 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1236,6 +1236,10 @@ _hidden char *libxl__blktap_devpath(libxl__gc *gc,
  */
 _hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
 
+_hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+                                   libxl_device_disk *disk,
+                                   libxl__device *device);
+
 /*
  * Make a disk available in this (the control) domain. Returns path to
  * a device.
-- 
1.7.2.5

[PATCH v7 05/11] libxl: introduce libxl__device_disk_add

[Stefano Stabellini]

Introduce libxl__device_disk_add that takes an additional
xs_transaction_t paramter.
Implement libxl_device_disk_add using libxl__device_disk_add.


Changes in v7:

- implement libxl__device_disk_add in libxl.c.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl.c          |   16 ++++++++++++----
 tools/libxl/libxl_internal.h |    2 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 37f36d7..b80b845 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1367,14 +1367,15 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
     return 0;
 }
 
-int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
+int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+        xs_transaction_t t, libxl_device_disk *disk)
 {
-    GC_INIT(ctx);
     flexarray_t *front;
     flexarray_t *back;
     char *dev;
     libxl__device device;
     int major, minor, rc;
+    libxl_ctx *ctx = gc->owner;
 
     rc = libxl__device_disk_setdefault(gc, disk);
     if (rc) goto out;
@@ -1421,7 +1422,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
             dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
             if (!dev) {
                 LOG(ERROR, "failed to get blktap devpath for %p\n",
-                    disk->pdev_path);
+                        disk->pdev_path);
                 rc = ERROR_FAIL;
                 goto out_free;
             }
@@ -1472,7 +1473,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
     flexarray_append(front, "device-type");
     flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
 
-    libxl__device_generic_add(gc, XBT_NULL, &device,
+    libxl__device_generic_add(gc, t, &device,
                              libxl__xs_kvs_of_flexarray(gc, back, back->count),
                              libxl__xs_kvs_of_flexarray(gc, front, front->count));
 
@@ -1482,6 +1483,13 @@ out_free:
     flexarray_free(back);
     flexarray_free(front);
 out:
+    return rc;
+}
+
+int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
+{
+    GC_INIT(ctx);
+    int rc = libxl__device_disk_add(gc, domid, XBT_NULL, disk);
     GC_FREE;
     return rc;
 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d7ed657..b06053d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1239,6 +1239,8 @@ _hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
 _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
                                    libxl_device_disk *disk,
                                    libxl__device *device);
+_hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+        xs_transaction_t t, libxl_device_disk *disk);
 
 /*
  * Make a disk available in this (the control) domain. Returns path to
-- 
1.7.2.5

[PATCH v7 06/11] xl/libxl: add a blkdev_start parameter

[Stefano Stabellini]

Introduce a blkdev_start in xl.conf and a corresponding string in
libxl_domain_build_info.

Add a blkdev_start parameter to libxl__device_disk_local_attach: it is
going to be used in a following patch.

blkdev_start specifies the first block device to be used for temporary
block device allocations by the toolstack.

Changes in v6:
- document blkdev_start;
- use libxl__strdup to allocate blkdev_start.

Changes in v5:
- change the type of the blkdev_start parameter to
libxl__device_disk_local_attach to const char *.

Changes in v4:
- pass blkdev_start in libxl_domain_build_info.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 docs/man/xl.conf.pod.5         |    6 ++++++
 tools/examples/xl.conf         |    3 +++
 tools/libxl/libxl_bootloader.c |    3 ++-
 tools/libxl/libxl_create.c     |    3 +++
 tools/libxl/libxl_internal.c   |    3 ++-
 tools/libxl/libxl_internal.h   |    3 ++-
 tools/libxl/libxl_types.idl    |    1 +
 tools/libxl/xl.c               |    3 +++
 tools/libxl/xl.h               |    1 +
 tools/libxl/xl_cmdimpl.c       |    2 ++
 10 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 8bd45ea..149430c 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -84,6 +84,12 @@ previous C<xm> toolstack this can be configured to use the old C<SXP>
 
 Default: C<json>
 
+=item B<blkdev_start="NAME">
+
+Configures the name of the first block device to be used for temporary
+block device allocations by the toolstack.
+The default choice is "xvda".
+
 =back
 
 =head1 SEE ALSO
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 56d3b3b..ebf057c 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -12,3 +12,6 @@
 
 # default output format used by "xl list -l"
 #output_format="json"
+
+# first block device to be used for temporary VM disk mounts
+#blkdev_start="xvda"
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 87dfed7..272614b 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -332,7 +332,8 @@ void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
         goto out;
     }
 
-    bl->diskpath = libxl__device_disk_local_attach(gc, bl->disk, &bl->localdisk);
+    bl->diskpath = libxl__device_disk_local_attach(gc, bl->disk, &bl->localdisk,
+            info->blkdev_start);
     if (!bl->diskpath) {
         rc = ERROR_FAIL;
         goto out;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 14721eb..9fb4b81 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -107,6 +107,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         }
     }
 
+    if (b_info->blkdev_start == NULL)
+        b_info->blkdev_start = libxl__strdup(0, "xvda");
+
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
         if (!b_info->u.hvm.bios)
             switch (b_info->device_model_version) {
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 5e5083b..9970103 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -325,7 +325,8 @@ out:
 
 char * libxl__device_disk_local_attach(libxl__gc *gc,
         const libxl_device_disk *in_disk,
-        libxl_device_disk *disk)
+        libxl_device_disk *disk,
+        const char *blkdev_start)
 {
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b06053d..12c0bc4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1248,7 +1248,8 @@ _hidden int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
  */
 _hidden char * libxl__device_disk_local_attach(libxl__gc *gc,
         const libxl_device_disk *in_disk,
-        libxl_device_disk *new_disk);
+        libxl_device_disk *new_disk,
+        const char *blkdev_start);
 _hidden int libxl__device_disk_local_detach(libxl__gc *gc,
         libxl_device_disk *disk);
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index a21bd85..2dc2b68 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -253,6 +253,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("localtime",       libxl_defbool),
     ("disable_migrate", libxl_defbool),
     ("cpuid",           libxl_cpuid_policy_list),
+    ("blkdev_start",    string),
     
     ("device_model_version", libxl_device_model_version),
     ("device_model_stubdomain", libxl_defbool),
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 750a61c..827f3f8 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -38,6 +38,7 @@ xentoollog_logger_stdiostream *logger;
 int dryrun_only;
 int force_execution;
 int autoballoon = 1;
+char *blkdev_start;
 char *lockfile;
 char *default_vifscript = NULL;
 char *default_bridge = NULL;
@@ -94,6 +95,8 @@ static void parse_global_config(const char *configfile,
             fprintf(stderr, "invalid default output format \"%s\"\n", buf);
         }
     }
+    if (!xlu_cfg_get_string (config, "blkdev_start", &buf, 0))
+        blkdev_start = strdup(buf);
     xlu_cfg_destroy(config);
 }
 
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index b7eacaa..74a8a1a 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -117,6 +117,7 @@ extern int dryrun_only;
 extern char *lockfile;
 extern char *default_vifscript;
 extern char *default_bridge;
+extern char *blkdev_start;
 
 enum output_format {
     OUTPUT_FORMAT_JSON,
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3c55a69..ce9237b 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -626,6 +626,8 @@ static void parse_config_data(const char *config_source,
     }
 
     libxl_domain_build_info_init_type(b_info, c_info->type);
+    if (blkdev_start)
+        b_info->blkdev_start = strdup(blkdev_start);
 
     /* the following is the actual config parsing with overriding values in the structures */
     if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
-- 
1.7.2.5

[PATCH v7 07/11] libxl: introduce libxl__alloc_vdev

[Stefano Stabellini]

Introduce libxl__alloc_vdev: find a spare virtual block device in the
domain passed as argument.


Changes in v7:
- remove the upper bound and document why.

Changes in v6:
- more comments in libxl__devid_to_localdev;
- inline GCSPRINTF.

Changes in v5:
- remove domid paramter to libxl__alloc_vdev (assume
  LIBXL_TOOLSTACK_DOMID);
- remove scaling limit from libxl__alloc_vdev.

Changes in v4:
- rename libxl__devid_to_vdev to libxl__devid_to_localdev;
- introduce upper bound for encode_disk_name;
- better error handling;

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxl/libxl_internal.c |   32 +++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |    4 +++
 tools/libxl/libxl_linux.c    |   52 ++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_netbsd.c   |    6 +++++
 4 files changed, 94 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 9970103..3961701 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -323,6 +323,38 @@ out:
     return rc;
 }
 
+/* libxl__alloc_vdev only works on the local domain, that is the domain
+ * where the toolstack is running */
+static char * libxl__alloc_vdev(libxl__gc *gc, const char *blkdev_start,
+        xs_transaction_t t)
+{
+    int devid = 0, disk = 0, part = 0;
+    char *dompath = libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID);
+
+    libxl__device_disk_dev_number(blkdev_start, &disk, &part);
+    if (part != 0) {
+        LOG(ERROR, "blkdev_start is invalid");
+        return NULL;
+    }
+
+    do {
+        devid = libxl__device_disk_dev_number(GCSPRINTF("d%dp0", disk),
+                NULL, NULL);
+        if (devid < 0)
+            return NULL;
+        if (libxl__xs_read(gc, t,
+                    libxl__sprintf(gc, "%s/device/vbd/%d/backend",
+                        dompath, devid)) == NULL) {
+            if (errno == ENOENT)
+                return libxl__devid_to_localdev(gc, devid);
+            else
+                return NULL;
+        }
+        disk++;
+    } while (1);
+    return NULL;
+}
+
 char * libxl__device_disk_local_attach(libxl__gc *gc,
         const libxl_device_disk *in_disk,
         libxl_device_disk *disk,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 12c0bc4..4d2ca0e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -78,6 +78,8 @@
 #define LIBXL_PV_EXTRA_MEMORY 1024
 #define LIBXL_HVM_EXTRA_MEMORY 2048
 #define LIBXL_MIN_DOM0_MEM (128*1024)
+/* use 0 as the domid of the toolstack domain for now */
+#define LIBXL_TOOLSTACK_DOMID 0
 #define QEMU_SIGNATURE "DeviceModelRecord0002"
 #define STUBDOM_CONSOLE_LOGGING 0
 #define STUBDOM_CONSOLE_SAVE 1
@@ -907,6 +909,8 @@ static inline void libxl__domaindeathcheck_stop(libxl__gc *gc,
 _hidden int libxl__try_phy_backend(mode_t st_mode);
 
 
+_hidden char *libxl__devid_to_localdev(libxl__gc *gc, int devid);
+
 /* from libxl_pci */
 
 _hidden int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int starting);
diff --git a/tools/libxl/libxl_linux.c b/tools/libxl/libxl_linux.c
index 925248b..0169b2f 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -25,3 +25,55 @@ int libxl__try_phy_backend(mode_t st_mode)
 
     return 1;
 }
+
+#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;
+}
diff --git a/tools/libxl/libxl_netbsd.c b/tools/libxl/libxl_netbsd.c
index 9e0ed6d..dbf5f71 100644
--- a/tools/libxl/libxl_netbsd.c
+++ b/tools/libxl/libxl_netbsd.c
@@ -24,3 +24,9 @@ int libxl__try_phy_backend(mode_t st_mode)
 
     return 0;
 }
+
+char *libxl__devid_to_localdev(libxl__gc *gc, int devid)
+{
+    /* TODO */
+    return NULL;
+}
-- 
1.7.2.5

[PATCH v7 08/11] xl/libxl: implement QDISK libxl_device_disk_local_attach

[Stefano Stabellini]

- Spawn a QEMU instance at boot time to handle disk local attach
requests.

- Implement libxl_device_disk_local_attach for QDISKs in terms of a
regular disk attach with the frontend and backend running in the same
domain.

Changes in v6:
- introduce xs_ret for xs_* error codes.

Changes in v5:
- replace LIBXL__LOG with LOG.

Changes on v4:
- improve error handling and exit paths in libxl__device_disk_local_attach.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by:Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/hotplug/Linux/init.d/sysconfig.xencommons |    3 +
 tools/hotplug/Linux/init.d/xencommons           |    3 +
 tools/libxl/libxl_internal.c                    |   60 ++++++++++++++++++-----
 3 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons b/tools/hotplug/Linux/init.d/sysconfig.xencommons
index 6543204..38ea85a 100644
--- a/tools/hotplug/Linux/init.d/sysconfig.xencommons
+++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons
@@ -12,3 +12,6 @@
 
 # Running xenbackendd in debug mode
 #XENBACKENDD_DEBUG=[yes|on|1]
+
+# qemu path
+#QEMU_XEN=/usr/lib/xen/bin/qemu-system-i386
diff --git a/tools/hotplug/Linux/init.d/xencommons b/tools/hotplug/Linux/init.d/xencommons
index 2f81ea2..9dda6e2 100644
--- a/tools/hotplug/Linux/init.d/xencommons
+++ b/tools/hotplug/Linux/init.d/xencommons
@@ -104,6 +104,9 @@ do_start () {
 	xenconsoled --pid-file=$XENCONSOLED_PIDFILE $XENCONSOLED_ARGS
 	test -z "$XENBACKENDD_DEBUG" || XENBACKENDD_ARGS="-d"
 	test "`uname`" != "NetBSD" || xenbackendd $XENBACKENDD_ARGS
+	echo Starting QEMU as disk backend for dom0
+	test -z "$QEMU_XEN" && QEMU_XEN=/usr/lib/xen/bin/qemu-system-i386
+	$QEMU_XEN -xen-domid 0 -xen-attach -name dom0 -nographic -M xenpv -daemonize -monitor /dev/null
 }
 do_stop () {
         echo Stopping xenconsoled
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 3961701..9bbb75d 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -363,7 +363,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL;
     char *ret = NULL;
-    int rc;
+    int rc, xs_ret;
+    xs_transaction_t t = XBT_NULL;
 
     if (in_disk->pdev_path == NULL)
         return NULL;
@@ -407,13 +408,35 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             break;
         case LIBXL_DISK_BACKEND_QDISK:
             if (disk->format != LIBXL_DISK_FORMAT_RAW) {
-                LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot locally"
-                           " attach a qdisk image if the format is not raw");
-                break;
+                do {
+                    t = xs_transaction_start(ctx->xsh);
+                    if (t == XBT_NULL) {
+                        LOG(ERROR, "failed to start a xenstore transaction");
+                        goto out;
+                    }
+                    disk->vdev = libxl__alloc_vdev(gc, blkdev_start, t);
+                    if (disk->vdev == NULL) {
+                        LOG(ERROR, "libxl__alloc_vdev failed");
+                        goto out;
+                    }
+                    if (libxl__device_disk_add(gc, LIBXL_TOOLSTACK_DOMID,
+                                t, disk)) {
+                        LOG(ERROR, "libxl_device_disk_add failed");
+                        goto out;
+                    }
+                    xs_ret = xs_transaction_end(ctx->xsh, t, 0);
+                } while (xs_ret == 0 && errno == EAGAIN);
+                t = XBT_NULL;
+                if (xs_ret == 0) {
+                    LOGE(ERROR, "xenstore transaction failed");
+                    goto out;
+                }
+                dev = GCSPRINTF("/dev/%s", disk->vdev);
+            } else {
+                dev = disk->pdev_path;
             }
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s",
-                       in_disk->pdev_path);
-            dev = disk->pdev_path;
+                       dev);
             break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
@@ -422,6 +445,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
     }
 
  out:
+    if (t != XBT_NULL)
+        xs_transaction_end(ctx->xsh, t, 1);
     if (dev != NULL)
         ret = strdup(dev);
     return ret;
@@ -429,12 +454,23 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
 
 int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk)
 {
-    /* Nothing to do for PHYSTYPE_PHY. */
-
-    /*
-     * For other device types assume that the blktap2 process is
-     * needed by the soon to be started domain and do nothing.
-     */
+    switch (disk->backend) {
+        case LIBXL_DISK_BACKEND_QDISK:
+            if (disk->vdev != NULL) {
+                libxl_device_disk_remove(gc->owner, LIBXL_TOOLSTACK_DOMID,
+                        disk, 0);
+                return libxl_device_disk_destroy(gc->owner,
+                        LIBXL_TOOLSTACK_DOMID, disk);
+            }
+            break;
+        default:
+            /*
+             * Nothing to do for PHYSTYPE_PHY.
+             * For other device types assume that the blktap2 process is
+             * needed by the soon to be started domain and do nothing.
+             */
+            break;
+    }
 
     return 0;
 }
-- 
1.7.2.5

[PATCH v7 09/11] libxl__device_disk_local_attach: wait for state "connected"

[Stefano Stabellini]

In order to make sure that the block device is available and ready to be
used, wait for state "connected" in the backend before returning.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Changes in v5:
- unify error paths.

Changes in v4:
- improve exit paths.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_internal.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 9bbb75d..edb6029 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -361,9 +361,10 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
         const char *blkdev_start)
 {
     libxl_ctx *ctx = gc->owner;
-    char *dev = NULL;
+    char *dev = NULL, *be_path = NULL;
     char *ret = NULL;
     int rc, xs_ret;
+    libxl__device device;
     xs_transaction_t t = XBT_NULL;
 
     if (in_disk->pdev_path == NULL)
@@ -444,12 +445,25 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             break;
     }
 
- out:
-    if (t != XBT_NULL)
-        xs_transaction_end(ctx->xsh, t, 1);
+    if (disk->vdev != NULL) {
+        rc = libxl__device_from_disk(gc, LIBXL_TOOLSTACK_DOMID, disk, &device);
+        if (rc < 0)
+            goto out;
+        be_path = libxl__device_backend_path(gc, &device);
+        rc = libxl__wait_for_backend(gc, be_path, "4");
+        if (rc < 0)
+            goto out;
+    }
     if (dev != NULL)
         ret = strdup(dev);
     return ret;
+
+ out:
+    if (t != XBT_NULL)
+        xs_transaction_end(ctx->xsh, t, 1);
+    else
+        libxl__device_disk_local_detach(gc, disk);
+    return NULL;
 }
 
 int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk)
-- 
1.7.2.5

[PATCH v7 10/11] libxl_string_to_backend: add qdisk

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/libxl_utils.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 73b00b3..4cf403d 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -240,6 +240,8 @@ int libxl_string_to_backend(libxl_ctx *ctx, char *s, libxl_disk_backend *backend
         *backend = LIBXL_DISK_BACKEND_PHY;
     } else if (!strcmp(s, "file")) {
         *backend = LIBXL_DISK_BACKEND_TAP;
+    } else if (!strcmp(s, "qdisk")) {
+        *backend = LIBXL_DISK_BACKEND_QDISK;
     } else if (!strcmp(s, "tap")) {
         p = strchr(s, ':');
         if (!p) {
-- 
1.7.2.5

[PATCH v7 11/11] main_blockdetach: destroy the disk on successful removal

[Stefano Stabellini]

main_blockdetach needs to call libxl_device_disk_destroy so that all the
disk related entries are properly removed from xenstore.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/libxl/xl_cmdimpl.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ce9237b..1167ed0 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5396,7 +5396,8 @@ int main_blockdetach(int argc, char **argv)
     }
     if (libxl_device_disk_remove(ctx, domid, &disk, 0)) {
         fprintf(stderr, "libxl_device_disk_remove failed.\n");
-    }
+    } else
+        libxl_device_disk_destroy(ctx, domid, &disk);
     return 0;
 }
 
-- 
1.7.2.5

Re: [PATCH v7 01/11] libxl: make libxl_device_disk_local_attach/detach internal functions

[Ian Jackson]

Stefano Stabellini writes ("[PATCH v7 01/11] libxl: make libxl_device_disk_local_attach/detach internal functions"):
> Changes in v5:

Didn't we agree that this unnecessary code motion would go away ?

Ian.

Re: [PATCH v7 02/11] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

[Ian Jackson]

Stefano Stabellini writes ("[PATCH v7 02/11] libxl: libxl__device_disk_local_attach return a new libxl_device_disk"):
> Introduce a new libxl_device_disk* parameter to
> libxl__device_disk_local_attach, the parameter is allocated by the
> caller. libxl__device_disk_local_attach is going to fill the new disk
> with informations about the new locally attached disk.  The new
> libxl_device_disk should be passed to libxl_device_disk_local_detach
> afterwards.

Thanks.

So comparing the comment:

> +    /* Should be zeroed by caller on entry.  Will be filled in by
> +     * bootloader machinery; represents the local attachment of the
> +     * disk for the benefit of the bootloader.  Must be detached by
> +     * the caller using libxl__device_disk_local_detach, but only
> +     * after the domain's kernel and initramfs have been loaded into
> +     * memory and the file references disposed of. */
> +    libxl_device_disk localdisk;

with the code in the caller: on entry it is indeed zeroed by the GCNEW
of the whole domain_create_state.  However on exit:

>      if (bl->diskpath) {
> -        libxl__device_disk_local_detach(gc, bl->disk);
> +        libxl__device_disk_local_detach(gc, &bl->localdisk);
> +        free(bl->localdisk.pdev_path);
> +        free(bl->localdisk.script);
>          free(bl->diskpath);

This does not correspond exactly to the comment.  Not only do we call
detach but apparently we need to free some things too.

It's not clear to me why these things haven't come from a suitable
gc.  But if they can't come from a gc then the comment needs to
mention that they need to be freed.

> -char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
> +char * libxl__device_disk_local_attach(libxl__gc *gc,
> +        const libxl_device_disk *in_disk,
> +        libxl_device_disk *disk)
>  {
>      libxl_ctx *ctx = gc->owner;
>      char *dev = NULL;
>      char *ret = NULL;
>      int rc;
>  
> +    if (in_disk->pdev_path == NULL)
> +        return NULL;
> +
> +    memcpy(disk, in_disk, sizeof(libxl_device_disk));
> +    disk->pdev_path = strdup(in_disk->pdev_path);
> +    if (in_disk->script != NULL)
> +        disk->script = strdup(in_disk->script);
> +    disk->vdev = NULL;

Re my comment about the gc, do we call this function anywhere else ?
Could it take the ao for the benefit of its gc ?  Would that violate
some rules about the usual contents of a libxl_device_disk ?

> @@ -367,8 +378,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
>                             " attach a qdisk image if the format is not raw");
>                  break;
>              }
> -            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
> -                       disk->pdev_path);
> +            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s",
> +                       in_disk->pdev_path);

A very minor comment, but: you might want to change this to LOG while
you're at it.

Thanks,
Ian.

Re: [PATCH v7 04/11] libxl: export libxl__device_from_disk

[Ian Jackson]

Stefano Stabellini writes ("[PATCH v7 04/11] libxl: export libxl__device_from_disk"):
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

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

Re: [PATCH v7 05/11] libxl: introduce libxl__device_disk_add

[Ian Jackson]

Stefano Stabellini writes ("[PATCH v7 05/11] libxl: introduce libxl__device_disk_add"):
> Introduce libxl__device_disk_add that takes an additional
> xs_transaction_t paramter.
> Implement libxl_device_disk_add using libxl__device_disk_add.
...
> @@ -1421,7 +1422,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *dis
>              dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
>              if (!dev) {
>                  LOG(ERROR, "failed to get blktap devpath for %p\n",
> -                    disk->pdev_path);
> +                        disk->pdev_path);

Unrelated and arguably erroneous whitespace change ?

Otherwise,

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

Re: [PATCH v7 07/11] libxl: introduce libxl__alloc_vdev

[Ian Jackson]

Stefano Stabellini writes ("[PATCH v7 07/11] libxl: introduce libxl__alloc_vdev"):
> Introduce libxl__alloc_vdev: find a spare virtual block device in the
> domain passed as argument.
...
> Changes in v7:
> - remove the upper bound and document why.

Great, thanks.

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

Re: [PATCH v7 02/11] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

[Stefano Stabellini]

On Tue, 22 May 2012, Ian Jackson wrote:
> Stefano Stabellini writes ("[PATCH v7 02/11] libxl: libxl__device_disk_local_attach return a new libxl_device_disk"):
> > Introduce a new libxl_device_disk* parameter to
> > libxl__device_disk_local_attach, the parameter is allocated by the
> > caller. libxl__device_disk_local_attach is going to fill the new disk
> > with informations about the new locally attached disk.  The new
> > libxl_device_disk should be passed to libxl_device_disk_local_detach
> > afterwards.
> 
> Thanks.
> 
> So comparing the comment:
> 
> > +    /* Should be zeroed by caller on entry.  Will be filled in by
> > +     * bootloader machinery; represents the local attachment of the
> > +     * disk for the benefit of the bootloader.  Must be detached by
> > +     * the caller using libxl__device_disk_local_detach, but only
> > +     * after the domain's kernel and initramfs have been loaded into
> > +     * memory and the file references disposed of. */
> > +    libxl_device_disk localdisk;
> 
> with the code in the caller: on entry it is indeed zeroed by the GCNEW
> of the whole domain_create_state.  However on exit:
> 
> >      if (bl->diskpath) {
> > -        libxl__device_disk_local_detach(gc, bl->disk);
> > +        libxl__device_disk_local_detach(gc, &bl->localdisk);
> > +        free(bl->localdisk.pdev_path);
> > +        free(bl->localdisk.script);
> >          free(bl->diskpath);
> 
> This does not correspond exactly to the comment.  Not only do we call
> detach but apparently we need to free some things too.
> 
> It's not clear to me why these things haven't come from a suitable
> gc.  But if they can't come from a gc then the comment needs to
> mention that they need to be freed.
> 
> > -char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
> > +char * libxl__device_disk_local_attach(libxl__gc *gc,
> > +        const libxl_device_disk *in_disk,
> > +        libxl_device_disk *disk)
> >  {
> >      libxl_ctx *ctx = gc->owner;
> >      char *dev = NULL;
> >      char *ret = NULL;
> >      int rc;
> >  
> > +    if (in_disk->pdev_path == NULL)
> > +        return NULL;
> > +
> > +    memcpy(disk, in_disk, sizeof(libxl_device_disk));
> > +    disk->pdev_path = strdup(in_disk->pdev_path);
> > +    if (in_disk->script != NULL)
> > +        disk->script = strdup(in_disk->script);
> > +    disk->vdev = NULL;
> 
> Re my comment about the gc, do we call this function anywhere else ?
> Could it take the ao for the benefit of its gc ?  Would that violate
> some rules about the usual contents of a libxl_device_disk ?

I am not sure I want to get into the whole AO thing at this point.

Am I supposed to just change the libxl__gc *gc argument into libxl__ao
*ao and then call STATE_AO_GC on function entry?
Then remove the free(bl->localdisk.script) and
free(bl->localdisk.pdev_path)?
Anything else is required?