[PATCH v5 0/8] qdisk local attach

classic Classic list List threaded Threaded
35 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 0/8] qdisk local attach

Stefano Stabellini-3
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 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 (8):
      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: 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"
 
 tools/examples/xl.conf                          |    3 +
 tools/hotplug/Linux/init.d/sysconfig.xencommons |    3 +
 tools/hotplug/Linux/init.d/xencommons           |    3 +
 tools/libxl/libxl.c                             |  232 +----------------
 tools/libxl/libxl.h                             |    7 -
 tools/libxl/libxl_bootloader.c                  |   11 +-
 tools/libxl/libxl_create.c                      |    3 +
 tools/libxl/libxl_device.c                      |   17 +-
 tools/libxl/libxl_internal.c                    |  319 +++++++++++++++++++++++
 tools/libxl/libxl_internal.h                    |   25 ++-
 tools/libxl/libxl_linux.c                       |   45 ++++
 tools/libxl/libxl_netbsd.c                      |    6 +
 tools/libxl/libxl_pci.c                         |    2 +-
 tools/libxl/libxl_types.idl                     |    1 +
 tools/libxl/xl.c                                |    3 +
 tools/libxl/xl.h                                |    1 +
 tools/libxl/xl_cmdimpl.c                        |    2 +
 17 files changed, 435 insertions(+), 248 deletions(-)

Cheers,

Stefano

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 1/8] libxl: make libxl_device_disk_local_attach/detach internal functions

Stefano Stabellini-3
Changes in v5:

- remove _hidden from the implementation.

Signed-off-by: Stefano Stabellini <[hidden email]>
Acked-by: Ian Campbell <[hidden email]>
---
 tools/libxl/libxl.c            |   73 ----------------------------------------
 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(+), 82 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9984d46..1357efc 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1664,79 +1664,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.
-     */
-
-    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 d59f0ee..79d5679 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -619,13 +619,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 2774062..977c6d3 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -386,7 +386,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
         goto out_close;
     }
 
-    diskpath = libxl_device_disk_local_attach(ctx, disk);
+    diskpath = libxl__device_disk_local_attach(gc, disk);
     if (!diskpath) {
         goto out_close;
     }
@@ -453,7 +453,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
     rc = 0;
 out_close:
     if (diskpath) {
-        libxl_device_disk_local_detach(ctx, disk);
+        libxl__device_disk_local_detach(gc, disk);
         free(diskpath);
     }
     if (fifo_fd > -1)
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 34ea15c..ddd6a37 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1003,6 +1003,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


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 2/8] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

Stefano Stabellini-3
In reply to this post by Stefano Stabellini-3
Introduce a new libxl_device_disk** parameter to
libxl__device_disk_local_attach, the parameter is allocated on the gc by
libxl__device_disk_local_attach. It is going to fill it 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 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 <[hidden email]>
---
 tools/libxl/libxl_bootloader.c |   10 +++++-----
 tools/libxl/libxl_internal.c   |   20 ++++++++++++++------
 tools/libxl/libxl_internal.h   |    3 ++-
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 977c6d3..83cac78 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -330,6 +330,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
     char *fifo = NULL;
     char *diskpath = NULL;
     char **args = NULL;
+    libxl_device_disk *tmpdisk = NULL;
 
     char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
     char *tempdir;
@@ -386,8 +387,9 @@ int libxl_run_bootloader(libxl_ctx *ctx,
         goto out_close;
     }
 
-    diskpath = libxl__device_disk_local_attach(gc, disk);
+    diskpath = libxl__device_disk_local_attach(gc, disk, &tmpdisk);
     if (!diskpath) {
+        rc = ERROR_FAIL;
         goto out_close;
     }
 
@@ -452,10 +454,8 @@ int libxl_run_bootloader(libxl_ctx *ctx,
 
     rc = 0;
 out_close:
-    if (diskpath) {
-        libxl__device_disk_local_detach(gc, disk);
-        free(diskpath);
-    }
+    if (tmpdisk)
+        libxl__device_disk_local_detach(gc, tmpdisk);
     if (fifo_fd > -1)
         close(fifo_fd);
     if (bootloader_fd > -1)
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index fc771ff..55dc55c 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -323,12 +323,21 @@ 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 **new_disk)
 {
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL;
-    char *ret = NULL;
     int rc;
+    libxl_device_disk *disk = libxl__zalloc(gc, sizeof(libxl_device_disk));
+
+    memcpy(disk, in_disk, sizeof(libxl_device_disk));
+    if (in_disk->pdev_path != NULL)
+        disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
+    if (in_disk->script != NULL)
+        disk->script = libxl__strdup(gc, in_disk->script);
+    disk->vdev = NULL;
 
     rc = libxl__device_disk_setdefault(gc, disk);
     if (rc) goto out;
@@ -368,7 +377,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
                 break;
             }
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
-                       disk->pdev_path);
+                       in_disk->pdev_path);
             dev = disk->pdev_path;
             break;
         default:
@@ -378,9 +387,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
     }
 
  out:
-    if (dev != NULL)
-        ret = strdup(dev);
-    return ret;
+    *new_disk = disk;
+    return dev;
 }
 
 int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ddd6a37..965d13b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1008,7 +1008,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);
 
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 3/8] libxl: add a transaction parameter to libxl__device_generic_add

Stefano Stabellini-3
In reply to this post by Stefano Stabellini-3
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 <[hidden email]>
Acked-by: Ian Jackson <[hidden email]>
---
 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 1357efc..525f0d6 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1401,7 +1401,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));
 
@@ -1802,7 +1802,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));
 
@@ -2080,7 +2080,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;
@@ -2151,7 +2151,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;
@@ -2284,7 +2284,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 965d13b..2c8f06a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -683,8 +683,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 3856bd9..335c645 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -102,7 +102,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


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 4/8] libxl: introduce libxl__device_disk_add

Stefano Stabellini-3
In reply to this post by Stefano Stabellini-3
Introduce libxl__device_disk_add that takes an additional
xs_transaction_t paramter.
Implement libxl_device_disk_add using libxl__device_disk_add.
Move libxl__device_from_disk to libxl_internal.c.
No functional change.

Changes in v5:
- rename libxl__device_generic_add_t to libxl__device_generic_add.

Signed-off-by: Stefano Stabellini <[hidden email]>
---
 tools/libxl/libxl.c          |  151 +----------------------------------------
 tools/libxl/libxl_internal.c |  157 ++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |    6 ++
 3 files changed, 164 insertions(+), 150 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 525f0d6..941dbb9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1258,159 +1258,10 @@ 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,
-                                   libxl_device_disk *disk,
-                                   libxl__device *device)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-    int devid;
-
-    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
-    if (devid==-1) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->vdev);
-        return ERROR_INVAL;
-    }
-
-    device->backend_domid = disk->backend_domid;
-    device->backend_devid = devid;
-
-    switch (disk->backend) {
-        case LIBXL_DISK_BACKEND_PHY:
-            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
-            break;
-        case LIBXL_DISK_BACKEND_TAP:
-            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
-            break;
-        case LIBXL_DISK_BACKEND_QDISK:
-            device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
-            break;
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
-                       disk->backend);
-            return ERROR_INVAL;
-    }
-
-    device->domid = domid;
-    device->devid = devid;
-    device->kind  = LIBXL__DEVICE_KIND_VBD;
-
-    return 0;
-}
-
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
 {
     GC_INIT(ctx);
-    flexarray_t *front;
-    flexarray_t *back;
-    char *dev;
-    libxl__device device;
-    int major, minor, rc;
-
-    rc = libxl__device_disk_setdefault(gc, disk);
-    if (rc) goto out;
-
-    front = flexarray_make(16, 1);
-    if (!front) {
-        rc = ERROR_NOMEM;
-        goto out;
-    }
-    back = flexarray_make(16, 1);
-    if (!back) {
-        rc = ERROR_NOMEM;
-        goto out_free;
-    }
-
-    if (disk->script) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
-                   " not yet supported, sorry");
-        rc = ERROR_INVAL;
-        goto out_free;
-    }
-
-    rc = libxl__device_from_disk(gc, domid, disk, &device);
-    if (rc != 0) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
-               " virtual disk identifier %s", disk->vdev);
-        goto out_free;
-    }
-
-    switch (disk->backend) {
-        case LIBXL_DISK_BACKEND_PHY:
-            dev = disk->pdev_path;
-    do_backend_phy:
-            libxl__device_physdisk_major_minor(dev, &major, &minor);
-            flexarray_append(back, "physical-device");
-            flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
-
-            flexarray_append(back, "params");
-            flexarray_append(back, dev);
-
-            assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
-            break;
-        case LIBXL_DISK_BACKEND_TAP:
-            dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
-            if (!dev) {
-                rc = ERROR_FAIL;
-                goto out_free;
-            }
-            flexarray_append(back, "tapdisk-params");
-            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
-                libxl__device_disk_string_of_format(disk->format),
-                disk->pdev_path));
-
-            /* now create a phy device to export the device to the guest */
-            goto do_backend_phy;
-        case LIBXL_DISK_BACKEND_QDISK:
-            flexarray_append(back, "params");
-            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
-                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
-            assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
-            break;
-        default:
-            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
-            rc = ERROR_INVAL;
-            goto out_free;
-    }
-
-    flexarray_append(back, "frontend-id");
-    flexarray_append(back, libxl__sprintf(gc, "%d", domid));
-    flexarray_append(back, "online");
-    flexarray_append(back, "1");
-    flexarray_append(back, "removable");
-    flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
-    flexarray_append(back, "bootable");
-    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
-    flexarray_append(back, "state");
-    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
-    flexarray_append(back, "dev");
-    flexarray_append(back, disk->vdev);
-    flexarray_append(back, "type");
-    flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
-    flexarray_append(back, "mode");
-    flexarray_append(back, disk->readwrite ? "w" : "r");
-    flexarray_append(back, "device-type");
-    flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
-
-    flexarray_append(front, "backend-id");
-    flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
-    flexarray_append(front, "state");
-    flexarray_append(front, libxl__sprintf(gc, "%d", 1));
-    flexarray_append(front, "virtual-device");
-    flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
-    flexarray_append(front, "device-type");
-    flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
-
-    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;
-
-out_free:
-    flexarray_free(back);
-    flexarray_free(front);
-out:
+    int rc = libxl__device_disk_add(gc, domid, XBT_NULL, disk);
     GC_FREE;
     return rc;
 }
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 55dc55c..1bf5d73 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -323,6 +323,163 @@ out:
     return rc;
 }
 
+int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+                                   libxl_device_disk *disk,
+                                   libxl__device *device)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int devid;
+
+    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
+    if (devid==-1) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+               " virtual disk identifier %s", disk->vdev);
+        return ERROR_INVAL;
+    }
+
+    device->backend_domid = disk->backend_domid;
+    device->backend_devid = devid;
+
+    switch (disk->backend) {
+        case LIBXL_DISK_BACKEND_PHY:
+            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+            break;
+        case LIBXL_DISK_BACKEND_TAP:
+            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
+            break;
+        case LIBXL_DISK_BACKEND_QDISK:
+            device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
+            break;
+        default:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
+                       disk->backend);
+            return ERROR_INVAL;
+    }
+
+    device->domid = domid;
+    device->devid = devid;
+    device->kind  = LIBXL__DEVICE_KIND_VBD;
+
+    return 0;
+}
+
+int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+        xs_transaction_t t, libxl_device_disk *disk)
+{
+    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;
+
+    front = flexarray_make(16, 1);
+    if (!front) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+    back = flexarray_make(16, 1);
+    if (!back) {
+        rc = ERROR_NOMEM;
+        goto out_free;
+    }
+
+    if (disk->script) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
+                   " not yet supported, sorry");
+        rc = ERROR_INVAL;
+        goto out_free;
+    }
+
+    rc = libxl__device_from_disk(gc, domid, disk, &device);
+    if (rc != 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
+               " virtual disk identifier %s", disk->vdev);
+        goto out_free;
+    }
+
+    switch (disk->backend) {
+        case LIBXL_DISK_BACKEND_PHY:
+            dev = disk->pdev_path;
+    do_backend_phy:
+            libxl__device_physdisk_major_minor(dev, &major, &minor);
+            flexarray_append(back, "physical-device");
+            flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
+
+            flexarray_append(back, "params");
+            flexarray_append(back, dev);
+
+            assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
+            break;
+        case LIBXL_DISK_BACKEND_TAP:
+            dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
+            if (!dev) {
+                rc = ERROR_FAIL;
+                goto out_free;
+            }
+            flexarray_append(back, "tapdisk-params");
+            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
+                libxl__device_disk_string_of_format(disk->format),
+                disk->pdev_path));
+
+            /* now create a phy device to export the device to the guest */
+            goto do_backend_phy;
+        case LIBXL_DISK_BACKEND_QDISK:
+            flexarray_append(back, "params");
+            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
+                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
+            assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
+            break;
+        default:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
+            rc = ERROR_INVAL;
+            goto out_free;
+    }
+
+    flexarray_append(back, "frontend-id");
+    flexarray_append(back, libxl__sprintf(gc, "%d", domid));
+    flexarray_append(back, "online");
+    flexarray_append(back, "1");
+    flexarray_append(back, "removable");
+    flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
+    flexarray_append(back, "bootable");
+    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+    flexarray_append(back, "state");
+    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
+    flexarray_append(back, "dev");
+    flexarray_append(back, disk->vdev);
+    flexarray_append(back, "type");
+    flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
+    flexarray_append(back, "mode");
+    flexarray_append(back, disk->readwrite ? "w" : "r");
+    flexarray_append(back, "device-type");
+    flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
+
+    flexarray_append(front, "backend-id");
+    flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
+    flexarray_append(front, "state");
+    flexarray_append(front, libxl__sprintf(gc, "%d", 1));
+    flexarray_append(front, "virtual-device");
+    flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
+    flexarray_append(front, "device-type");
+    flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
+
+    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));
+
+    rc = 0;
+
+out_free:
+    flexarray_free(back);
+    flexarray_free(front);
+out:
+    return rc;
+}
+
 char * libxl__device_disk_local_attach(libxl__gc *gc,
         const libxl_device_disk *in_disk,
         libxl_device_disk **new_disk)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2c8f06a..096c96d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1003,6 +1003,12 @@ _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);
+_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
  * a device.
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 5/8] xl/libxl: add a blkdev_start parameter

Stefano Stabellini-3
In reply to this post by Stefano Stabellini-3
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 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 <[hidden email]>
---
 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 ++
 9 files changed, 19 insertions(+), 3 deletions(-)

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 83cac78..123b06e 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -387,7 +387,8 @@ int libxl_run_bootloader(libxl_ctx *ctx,
         goto out_close;
     }
 
-    diskpath = libxl__device_disk_local_attach(gc, disk, &tmpdisk);
+    diskpath = libxl__device_disk_local_attach(gc, disk, &tmpdisk,
+            info->blkdev_start);
     if (!diskpath) {
         rc = ERROR_FAIL;
         goto out_close;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 7ab2f72..f2fcdf0 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 = strdup("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 1bf5d73..bd5ebb3 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -482,7 +482,8 @@ out:
 
 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)
 {
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 096c96d..0074ec4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1015,7 +1015,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 68599cb..7131696 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 a6ffd25..4596c4f 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -35,6 +35,7 @@
 xentoollog_logger_stdiostream *logger;
 int dryrun_only;
 int autoballoon = 1;
+char *blkdev_start;
 char *lockfile;
 char *default_vifscript = NULL;
 char *default_bridge = NULL;
@@ -91,6 +92,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 f0d0ec8..3fbe14d 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -112,6 +112,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 28f5cab..7338562 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -595,6 +595,8 @@ static void parse_config_data(const char *configfile_filename_report,
     }
 
     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


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 6/8] libxl: introduce libxl__alloc_vdev

Stefano Stabellini-3
In reply to this post by Stefano Stabellini-3
Introduce libxl__alloc_vdev: find a spare virtual block device in the
domain passed as argument.

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 <[hidden email]>
---
 tools/libxl/libxl_internal.c |   31 ++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |    4 +++
 tools/libxl/libxl_linux.c    |   45 ++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_netbsd.c   |    6 +++++
 4 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index bd5ebb3..7a1e017 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -480,6 +480,37 @@ 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 *vdev = NULL;
+    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 {
+        vdev = GCSPRINTF("d%dp0", disk);
+        devid = libxl__device_disk_dev_number(vdev, NULL, 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 (disk <= (1 << 20));
+    return NULL;
+}
+
 char * libxl__device_disk_local_attach(libxl__gc *gc,
         const libxl_device_disk *in_disk,
         libxl_device_disk **new_disk,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0074ec4..a451c79 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -77,6 +77,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
@@ -763,6 +765,8 @@ _hidden int libxl__ev_devstate_wait(libxl__gc *gc, libxl__ev_devstate *ds,
  */
 _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..cb596a9 100644
--- a/tools/libxl/libxl_linux.c
+++ b/tools/libxl/libxl_linux.c
@@ -25,3 +25,48 @@ 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))
+
+/* same as in Linux */
+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)
+{
+    int minor;
+    int offset;
+    int nr_parts;
+    char *ptr = NULL;
+    char *ret = libxl__zalloc(gc, 32);
+
+    if (!VDEV_IS_EXTENDED(devid)) {
+        minor = devid & 0xff;
+        nr_parts = 16;
+    } else {
+        minor = BLKIF_MINOR_EXT(devid);
+        nr_parts = 256;
+    }
+    offset = minor / nr_parts;
+
+ /* arbitrary upper bound */
+ if (offset > 676)
+ return NULL;
+
+    strcpy(ret, "xvd");
+    ptr = encode_disk_name(ret + 3, offset);
+    if (minor % nr_parts == 0)
+        *ptr = 0;
+    else
+        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


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 7/8] xl/libxl: implement QDISK libxl_device_disk_local_attach

Stefano Stabellini-3
In reply to this post by Stefano Stabellini-3
- 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.

Chanegs 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 <[hidden email]>
---
 tools/hotplug/Linux/init.d/sysconfig.xencommons |    3 +
 tools/hotplug/Linux/init.d/xencommons           |    3 +
 tools/libxl/libxl_internal.c                    |   58 ++++++++++++++++++----
 3 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons b/tools/hotplug/Linux/init.d/sysconfig.xencommons
index 6543204..0f3b9ad 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 and log file
+#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 7a1e017..e180498 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -519,6 +519,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL;
     int rc;
+    xs_transaction_t t = XBT_NULL;
     libxl_device_disk *disk = libxl__zalloc(gc, sizeof(libxl_device_disk));
 
     memcpy(disk, in_disk, sizeof(libxl_device_disk));
@@ -561,13 +562,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;
+                    }
+                    rc = xs_transaction_end(ctx->xsh, t, 0);
+                } while (rc == 0 && errno == EAGAIN);
+                t = XBT_NULL;
+                if (rc == 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\n",
-                       in_disk->pdev_path);
-            dev = disk->pdev_path;
+                       dev);
             break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
@@ -576,18 +599,31 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
     }
 
  out:
+    if (t != XBT_NULL)
+        xs_transaction_end(ctx->xsh, t, 1);
     *new_disk = disk;
     return dev;
 }
 
 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


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH v5 8/8] libxl__device_disk_local_attach: wait for state "connected"

Stefano Stabellini-3
In reply to this post by Stefano Stabellini-3
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 <[hidden email]>

Changes in v5:
- unify error paths.

Changes in v4:
- improve exit paths.

Signed-off-by: Stefano Stabellini <[hidden email]>
---
 tools/libxl/libxl_internal.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index e180498..05faff5 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -517,8 +517,9 @@ 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;
     int rc;
+    libxl__device device;
     xs_transaction_t t = XBT_NULL;
     libxl_device_disk *disk = libxl__zalloc(gc, sizeof(libxl_device_disk));
 
@@ -598,11 +599,24 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             break;
     }
 
+    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;
+    }
+
+    *new_disk = disk;
+    return dev;
  out:
     if (t != XBT_NULL)
         xs_transaction_end(ctx->xsh, t, 1);
-    *new_disk = disk;
-    return dev;
+    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


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 2/8] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

Ian Campbell-10
In reply to this post by Stefano Stabellini-3
On Fri, 2012-05-04 at 12:13 +0100, Stefano Stabellini wrote:

> Introduce a new libxl_device_disk** parameter to
> libxl__device_disk_local_attach, the parameter is allocated on the gc by
> libxl__device_disk_local_attach. It is going to fill it 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 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 <[hidden email]>
> ---
>  tools/libxl/libxl_bootloader.c |   10 +++++-----
>  tools/libxl/libxl_internal.c   |   20 ++++++++++++++------
>  tools/libxl/libxl_internal.h   |    3 ++-
>  3 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index 977c6d3..83cac78 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -330,6 +330,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>      char *fifo = NULL;
>      char *diskpath = NULL;
>      char **args = NULL;
> +    libxl_device_disk *tmpdisk = NULL;

Do you need to keep tmpdisk valid outside the scope of this function?
You don't seem to in this patch (I didn't look over the next patches
yet).

If not then you could just use "libxl_device_disk tmpdisk" and have
libxl__device_disk_local_attach update in place instead allocating and
passing the pointer back.

>      char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
>      char *tempdir;
> @@ -386,8 +387,9 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>          goto out_close;
>      }
>  
> -    diskpath = libxl__device_disk_local_attach(gc, disk);
> +    diskpath = libxl__device_disk_local_attach(gc, disk, &tmpdisk);
>      if (!diskpath) {
> +        rc = ERROR_FAIL;
>          goto out_close;
>      }
>  
> @@ -452,10 +454,8 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>  
>      rc = 0;
>  out_close:
> -    if (diskpath) {
> -        libxl__device_disk_local_detach(gc, disk);
> -        free(diskpath);
> -    }
> +    if (tmpdisk)
> +        libxl__device_disk_local_detach(gc, tmpdisk);
>      if (fifo_fd > -1)
>          close(fifo_fd);
>      if (bootloader_fd > -1)
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index fc771ff..55dc55c 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -323,12 +323,21 @@ 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 **new_disk)
>  {
>      libxl_ctx *ctx = gc->owner;
>      char *dev = NULL;
> -    char *ret = NULL;
>      int rc;
> +    libxl_device_disk *disk = libxl__zalloc(gc, sizeof(libxl_device_disk));
> +
> +    memcpy(disk, in_disk, sizeof(libxl_device_disk));
> +    if (in_disk->pdev_path != NULL)
> +        disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
> +    if (in_disk->script != NULL)
> +        disk->script = libxl__strdup(gc, in_disk->script);
> +    disk->vdev = NULL;
>  
>      rc = libxl__device_disk_setdefault(gc, disk);
>      if (rc) goto out;
> @@ -368,7 +377,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
>                  break;
>              }
>              LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
> -                       disk->pdev_path);
> +                       in_disk->pdev_path);
>              dev = disk->pdev_path;
>              break;
>          default:
> @@ -378,9 +387,8 @@ char * libxl__device_disk_local_attach(libxl__gc *gc, libxl_device_disk *disk)
>      }
>  
>   out:
> -    if (dev != NULL)
> -        ret = strdup(dev);
> -    return ret;
> +    *new_disk = disk;
> +    return dev;
>  }
>  
>  int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk)
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index ddd6a37..965d13b 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1008,7 +1008,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);
>  



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 3/8] libxl: add a transaction parameter to libxl__device_generic_add

Ian Campbell-10
In reply to this post by Stefano Stabellini-3
On Fri, 2012-05-04 at 12:13 +0100, Stefano Stabellini wrote:

> 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 <[hidden email]>
> Acked-by: Ian Jackson <[hidden email]>

Acked-by: Ian Campbell <[hidden email]>

> ---
>  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 1357efc..525f0d6 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1401,7 +1401,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));
>  
> @@ -1802,7 +1802,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));
>  
> @@ -2080,7 +2080,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;
> @@ -2151,7 +2151,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;
> @@ -2284,7 +2284,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 965d13b..2c8f06a 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -683,8 +683,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 3856bd9..335c645 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -102,7 +102,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));
>  



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 4/8] libxl: introduce libxl__device_disk_add

Ian Campbell-10
In reply to this post by Stefano Stabellini-3
On Fri, 2012-05-04 at 12:13 +0100, Stefano Stabellini wrote:

> Introduce libxl__device_disk_add that takes an additional
> xs_transaction_t paramter.
> Implement libxl_device_disk_add using libxl__device_disk_add.
> Move libxl__device_from_disk to libxl_internal.c.
> No functional change.
>
> Changes in v5:
> - rename libxl__device_generic_add_t to libxl__device_generic_add.
>
> Signed-off-by: Stefano Stabellini <[hidden email]>

Acked-by: Ian Campbell <[hidden email]>

> ---
>  tools/libxl/libxl.c          |  151 +----------------------------------------
>  tools/libxl/libxl_internal.c |  157 ++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h |    6 ++
>  3 files changed, 164 insertions(+), 150 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 525f0d6..941dbb9 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1258,159 +1258,10 @@ 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,
> -                                   libxl_device_disk *disk,
> -                                   libxl__device *device)
> -{
> -    libxl_ctx *ctx = libxl__gc_owner(gc);
> -    int devid;
> -
> -    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
> -    if (devid==-1) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
> -               " virtual disk identifier %s", disk->vdev);
> -        return ERROR_INVAL;
> -    }
> -
> -    device->backend_domid = disk->backend_domid;
> -    device->backend_devid = devid;
> -
> -    switch (disk->backend) {
> -        case LIBXL_DISK_BACKEND_PHY:
> -            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
> -            break;
> -        case LIBXL_DISK_BACKEND_TAP:
> -            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
> -            break;
> -        case LIBXL_DISK_BACKEND_QDISK:
> -            device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
> -            break;
> -        default:
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
> -                       disk->backend);
> -            return ERROR_INVAL;
> -    }
> -
> -    device->domid = domid;
> -    device->devid = devid;
> -    device->kind  = LIBXL__DEVICE_KIND_VBD;
> -
> -    return 0;
> -}
> -
>  int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
>  {
>      GC_INIT(ctx);
> -    flexarray_t *front;
> -    flexarray_t *back;
> -    char *dev;
> -    libxl__device device;
> -    int major, minor, rc;
> -
> -    rc = libxl__device_disk_setdefault(gc, disk);
> -    if (rc) goto out;
> -
> -    front = flexarray_make(16, 1);
> -    if (!front) {
> -        rc = ERROR_NOMEM;
> -        goto out;
> -    }
> -    back = flexarray_make(16, 1);
> -    if (!back) {
> -        rc = ERROR_NOMEM;
> -        goto out_free;
> -    }
> -
> -    if (disk->script) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
> -                   " not yet supported, sorry");
> -        rc = ERROR_INVAL;
> -        goto out_free;
> -    }
> -
> -    rc = libxl__device_from_disk(gc, domid, disk, &device);
> -    if (rc != 0) {
> -        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
> -               " virtual disk identifier %s", disk->vdev);
> -        goto out_free;
> -    }
> -
> -    switch (disk->backend) {
> -        case LIBXL_DISK_BACKEND_PHY:
> -            dev = disk->pdev_path;
> -    do_backend_phy:
> -            libxl__device_physdisk_major_minor(dev, &major, &minor);
> -            flexarray_append(back, "physical-device");
> -            flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
> -
> -            flexarray_append(back, "params");
> -            flexarray_append(back, dev);
> -
> -            assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
> -            break;
> -        case LIBXL_DISK_BACKEND_TAP:
> -            dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
> -            if (!dev) {
> -                rc = ERROR_FAIL;
> -                goto out_free;
> -            }
> -            flexarray_append(back, "tapdisk-params");
> -            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> -                libxl__device_disk_string_of_format(disk->format),
> -                disk->pdev_path));
> -
> -            /* now create a phy device to export the device to the guest */
> -            goto do_backend_phy;
> -        case LIBXL_DISK_BACKEND_QDISK:
> -            flexarray_append(back, "params");
> -            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> -                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
> -            assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
> -            break;
> -        default:
> -            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
> -            rc = ERROR_INVAL;
> -            goto out_free;
> -    }
> -
> -    flexarray_append(back, "frontend-id");
> -    flexarray_append(back, libxl__sprintf(gc, "%d", domid));
> -    flexarray_append(back, "online");
> -    flexarray_append(back, "1");
> -    flexarray_append(back, "removable");
> -    flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
> -    flexarray_append(back, "bootable");
> -    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> -    flexarray_append(back, "state");
> -    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> -    flexarray_append(back, "dev");
> -    flexarray_append(back, disk->vdev);
> -    flexarray_append(back, "type");
> -    flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
> -    flexarray_append(back, "mode");
> -    flexarray_append(back, disk->readwrite ? "w" : "r");
> -    flexarray_append(back, "device-type");
> -    flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> -
> -    flexarray_append(front, "backend-id");
> -    flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
> -    flexarray_append(front, "state");
> -    flexarray_append(front, libxl__sprintf(gc, "%d", 1));
> -    flexarray_append(front, "virtual-device");
> -    flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
> -    flexarray_append(front, "device-type");
> -    flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
> -
> -    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;
> -
> -out_free:
> -    flexarray_free(back);
> -    flexarray_free(front);
> -out:
> +    int rc = libxl__device_disk_add(gc, domid, XBT_NULL, disk);
>      GC_FREE;
>      return rc;
>  }
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 55dc55c..1bf5d73 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -323,6 +323,163 @@ out:
>      return rc;
>  }
>  
> +int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
> +                                   libxl_device_disk *disk,
> +                                   libxl__device *device)
> +{
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    int devid;
> +
> +    devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
> +    if (devid==-1) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
> +               " virtual disk identifier %s", disk->vdev);
> +        return ERROR_INVAL;
> +    }
> +
> +    device->backend_domid = disk->backend_domid;
> +    device->backend_devid = devid;
> +
> +    switch (disk->backend) {
> +        case LIBXL_DISK_BACKEND_PHY:
> +            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
> +            break;
> +        case LIBXL_DISK_BACKEND_TAP:
> +            device->backend_kind = LIBXL__DEVICE_KIND_VBD;
> +            break;
> +        case LIBXL_DISK_BACKEND_QDISK:
> +            device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
> +            break;
> +        default:
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
> +                       disk->backend);
> +            return ERROR_INVAL;
> +    }
> +
> +    device->domid = domid;
> +    device->devid = devid;
> +    device->kind  = LIBXL__DEVICE_KIND_VBD;
> +
> +    return 0;
> +}
> +
> +int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
> +        xs_transaction_t t, libxl_device_disk *disk)
> +{
> +    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;
> +
> +    front = flexarray_make(16, 1);
> +    if (!front) {
> +        rc = ERROR_NOMEM;
> +        goto out;
> +    }
> +    back = flexarray_make(16, 1);
> +    if (!back) {
> +        rc = ERROR_NOMEM;
> +        goto out_free;
> +    }
> +
> +    if (disk->script) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
> +                   " not yet supported, sorry");
> +        rc = ERROR_INVAL;
> +        goto out_free;
> +    }
> +
> +    rc = libxl__device_from_disk(gc, domid, disk, &device);
> +    if (rc != 0) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
> +               " virtual disk identifier %s", disk->vdev);
> +        goto out_free;
> +    }
> +
> +    switch (disk->backend) {
> +        case LIBXL_DISK_BACKEND_PHY:
> +            dev = disk->pdev_path;
> +    do_backend_phy:
> +            libxl__device_physdisk_major_minor(dev, &major, &minor);
> +            flexarray_append(back, "physical-device");
> +            flexarray_append(back, libxl__sprintf(gc, "%x:%x", major, minor));
> +
> +            flexarray_append(back, "params");
> +            flexarray_append(back, dev);
> +
> +            assert(device.backend_kind == LIBXL__DEVICE_KIND_VBD);
> +            break;
> +        case LIBXL_DISK_BACKEND_TAP:
> +            dev = libxl__blktap_devpath(gc, disk->pdev_path, disk->format);
> +            if (!dev) {
> +                rc = ERROR_FAIL;
> +                goto out_free;
> +            }
> +            flexarray_append(back, "tapdisk-params");
> +            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> +                libxl__device_disk_string_of_format(disk->format),
> +                disk->pdev_path));
> +
> +            /* now create a phy device to export the device to the guest */
> +            goto do_backend_phy;
> +        case LIBXL_DISK_BACKEND_QDISK:
> +            flexarray_append(back, "params");
> +            flexarray_append(back, libxl__sprintf(gc, "%s:%s",
> +                          libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
> +            assert(device.backend_kind == LIBXL__DEVICE_KIND_QDISK);
> +            break;
> +        default:
> +            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);
> +            rc = ERROR_INVAL;
> +            goto out_free;
> +    }
> +
> +    flexarray_append(back, "frontend-id");
> +    flexarray_append(back, libxl__sprintf(gc, "%d", domid));
> +    flexarray_append(back, "online");
> +    flexarray_append(back, "1");
> +    flexarray_append(back, "removable");
> +    flexarray_append(back, libxl__sprintf(gc, "%d", (disk->removable) ? 1 : 0));
> +    flexarray_append(back, "bootable");
> +    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> +    flexarray_append(back, "state");
> +    flexarray_append(back, libxl__sprintf(gc, "%d", 1));
> +    flexarray_append(back, "dev");
> +    flexarray_append(back, disk->vdev);
> +    flexarray_append(back, "type");
> +    flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
> +    flexarray_append(back, "mode");
> +    flexarray_append(back, disk->readwrite ? "w" : "r");
> +    flexarray_append(back, "device-type");
> +    flexarray_append(back, disk->is_cdrom ? "cdrom" : "disk");
> +
> +    flexarray_append(front, "backend-id");
> +    flexarray_append(front, libxl__sprintf(gc, "%d", disk->backend_domid));
> +    flexarray_append(front, "state");
> +    flexarray_append(front, libxl__sprintf(gc, "%d", 1));
> +    flexarray_append(front, "virtual-device");
> +    flexarray_append(front, libxl__sprintf(gc, "%d", device.devid));
> +    flexarray_append(front, "device-type");
> +    flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
> +
> +    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));
> +
> +    rc = 0;
> +
> +out_free:
> +    flexarray_free(back);
> +    flexarray_free(front);
> +out:
> +    return rc;
> +}
> +
>  char * libxl__device_disk_local_attach(libxl__gc *gc,
>          const libxl_device_disk *in_disk,
>          libxl_device_disk **new_disk)
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 2c8f06a..096c96d 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1003,6 +1003,12 @@ _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);
> +_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
>   * a device.



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 5/8] xl/libxl: add a blkdev_start parameter

Ian Campbell-10
In reply to this post by Stefano Stabellini-3
On Fri, 2012-05-04 at 12:13 +0100, Stefano Stabellini wrote:

> 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 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 <[hidden email]>
> ---
>  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 ++
>  9 files changed, 19 insertions(+), 3 deletions(-)
>
> 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"

Need a patch to docs/man/xl.conf.pod.5 as well. If you add that then:
Acked-by: Ian Campbell <[hidden email]>


I suppose this is necessarily Linux specific and we'll need a patch from
the BSD folks?

> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index 83cac78..123b06e 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -387,7 +387,8 @@ int libxl_run_bootloader(libxl_ctx *ctx,
>          goto out_close;
>      }
>  
> -    diskpath = libxl__device_disk_local_attach(gc, disk, &tmpdisk);
> +    diskpath = libxl__device_disk_local_attach(gc, disk, &tmpdisk,
> +            info->blkdev_start);
>      if (!diskpath) {
>          rc = ERROR_FAIL;
>          goto out_close;
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 7ab2f72..f2fcdf0 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 = strdup("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 1bf5d73..bd5ebb3 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -482,7 +482,8 @@ out:
>  
>  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)
>  {
>      libxl_ctx *ctx = gc->owner;
>      char *dev = NULL;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 096c96d..0074ec4 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1015,7 +1015,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 68599cb..7131696 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 a6ffd25..4596c4f 100644
> --- a/tools/libxl/xl.c
> +++ b/tools/libxl/xl.c
> @@ -35,6 +35,7 @@
>  xentoollog_logger_stdiostream *logger;
>  int dryrun_only;
>  int autoballoon = 1;
> +char *blkdev_start;
>  char *lockfile;
>  char *default_vifscript = NULL;
>  char *default_bridge = NULL;
> @@ -91,6 +92,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 f0d0ec8..3fbe14d 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -112,6 +112,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 28f5cab..7338562 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -595,6 +595,8 @@ static void parse_config_data(const char *configfile_filename_report,
>      }
>  
>      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))



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 6/8] libxl: introduce libxl__alloc_vdev

Ian Campbell-10
In reply to this post by Stefano Stabellini-3
On Fri, 2012-05-04 at 12:13 +0100, Stefano Stabellini wrote:

> Introduce libxl__alloc_vdev: find a spare virtual block device in the
> domain passed as argument.
>
> 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 <[hidden email]>
> ---
>  tools/libxl/libxl_internal.c |   31 ++++++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h |    4 +++
>  tools/libxl/libxl_linux.c    |   45 ++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_netbsd.c   |    6 +++++
>  4 files changed, 86 insertions(+), 0 deletions(-)
>
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index bd5ebb3..7a1e017 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -480,6 +480,37 @@ 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 *vdev = NULL;
> +    char *dompath = libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID);
> +
> +    libxl__device_disk_dev_number(blkdev_start, &disk, &part);

If you specify the default blkdev_start in xl as d0 instead of xvda
doesn't at least this bit magically become portable to BSD etc too?

Or couldn't it actually be an int and save you parsing at all?

> +    if (part != 0) {
> +        LOG(ERROR, "blkdev_start is invalid");
> +        return NULL;
> +    }
> +
> +    do {
> +        vdev = GCSPRINTF("d%dp0", disk);
> +        devid = libxl__device_disk_dev_number(vdev, NULL, NULL);

libxl__device_disk_dev_number does the right thing with "d<N>" (i.e.
without the hardcoded "p0"), I think.

I'd be tempted to inline the GCSPRINTF in the arg and do away with vdev
since you don't use it again.

> +        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 (disk <= (1 << 20));

That's a whole lotta disks ;-)

> +    return NULL;
> +}
> +
>  char * libxl__device_disk_local_attach(libxl__gc *gc,
>          const libxl_device_disk *in_disk,
>          libxl_device_disk **new_disk,
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 0074ec4..a451c79 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -77,6 +77,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
> @@ -763,6 +765,8 @@ _hidden int libxl__ev_devstate_wait(libxl__gc *gc, libxl__ev_devstate *ds,
>   */
>  _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..cb596a9 100644
> --- a/tools/libxl/libxl_linux.c
> +++ b/tools/libxl/libxl_linux.c
> @@ -25,3 +25,48 @@ 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))
> +
> +/* same as in Linux */
> +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)
> +{
> +    int minor;
> +    int offset;
> +    int nr_parts;
> +    char *ptr = NULL;
> +    char *ret = libxl__zalloc(gc, 32);
> +
> +    if (!VDEV_IS_EXTENDED(devid)) {
> +        minor = devid & 0xff;
> +        nr_parts = 16;
> +    } else {
> +        minor = BLKIF_MINOR_EXT(devid);
> +        nr_parts = 256;
> +    }
> +    offset = minor / nr_parts;
> +
> + /* arbitrary upper bound */
> + if (offset > 676)
> + return NULL;
> +
> +    strcpy(ret, "xvd");
> +    ptr = encode_disk_name(ret + 3, offset);
> +    if (minor % nr_parts == 0)
> +        *ptr = 0;
> +    else
> +        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

Aha, here's where we need a BSD contribution. CC someone?

> @@ -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;
> +}



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 7/8] xl/libxl: implement QDISK libxl_device_disk_local_attach

Ian Campbell-10
In reply to this post by Stefano Stabellini-3
On Fri, 2012-05-04 at 12:13 +0100, Stefano Stabellini wrote:

> - 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.
>
> Chanegs 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 <[hidden email]>
> ---
>  tools/hotplug/Linux/init.d/sysconfig.xencommons |    3 +
>  tools/hotplug/Linux/init.d/xencommons           |    3 +
>  tools/libxl/libxl_internal.c                    |   58 ++++++++++++++++++----
>  3 files changed, 53 insertions(+), 11 deletions(-)
>
> diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons b/tools/hotplug/Linux/init.d/sysconfig.xencommons
> index 6543204..0f3b9ad 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 and log file

What does "and log file" mean?

Apart from that:
Acked-by:Ian Campbell <[hidden email]>

> +#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 7a1e017..e180498 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -519,6 +519,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
>      libxl_ctx *ctx = gc->owner;
>      char *dev = NULL;
>      int rc;
> +    xs_transaction_t t = XBT_NULL;
>      libxl_device_disk *disk = libxl__zalloc(gc, sizeof(libxl_device_disk));
>  
>      memcpy(disk, in_disk, sizeof(libxl_device_disk));
> @@ -561,13 +562,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;
> +                    }
> +                    rc = xs_transaction_end(ctx->xsh, t, 0);
> +                } while (rc == 0 && errno == EAGAIN);
> +                t = XBT_NULL;
> +                if (rc == 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\n",
> -                       in_disk->pdev_path);
> -            dev = disk->pdev_path;
> +                       dev);
>              break;
>          default:
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend "
> @@ -576,18 +599,31 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
>      }
>  
>   out:
> +    if (t != XBT_NULL)
> +        xs_transaction_end(ctx->xsh, t, 1);
>      *new_disk = disk;
>      return dev;
>  }
>  
>  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;
>  }



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 8/8] libxl__device_disk_local_attach: wait for state "connected"

Ian Campbell-10
In reply to this post by Stefano Stabellini-3
On Fri, 2012-05-04 at 12:13 +0100, Stefano Stabellini wrote:

> 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 <[hidden email]>
>
> Changes in v5:
> - unify error paths.
>
> Changes in v4:
> - improve exit paths.
>
> Signed-off-by: Stefano Stabellini <[hidden email]>
> ---
>  tools/libxl/libxl_internal.c |   20 +++++++++++++++++---
>  1 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index e180498..05faff5 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -517,8 +517,9 @@ 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;
>      int rc;
> +    libxl__device device;
>      xs_transaction_t t = XBT_NULL;
>      libxl_device_disk *disk = libxl__zalloc(gc, sizeof(libxl_device_disk));
>  
> @@ -598,11 +599,24 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
>              break;
>      }
>  
> +    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;
> +    }
> +
> +    *new_disk = disk;
> +    return dev;
>   out:
>      if (t != XBT_NULL)
>          xs_transaction_end(ctx->xsh, t, 1);

There's no way to reach the preceding "return dev" with the transaction
still open? Previously we would have fallen through and done it.

> -    *new_disk = disk;
> -    return dev;
> +    else
> +        libxl__device_disk_local_detach(gc, disk);
> +    return NULL;
>  }
>  
>  int libxl__device_disk_local_detach(libxl__gc *gc, libxl_device_disk *disk)



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 2/8] libxl: libxl__device_disk_local_attach return a new libxl_device_disk

Ian Jackson-2
In reply to this post by Stefano Stabellini-3
Stefano Stabellini writes ("[PATCH v5 2/8] 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 on the gc by
> libxl__device_disk_local_attach. It is going to fill it with
> informations about the new locally attached disk.  The new
> libxl_device_disk should be passed to libxl_device_disk_local_detach
> afterwards.
...
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index fc771ff..55dc55c 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
...
>              LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "locally attaching qdisk %s\n",
> -                       disk->pdev_path);
> +                       in_disk->pdev_path);

I think in_disk->pdev_path can be NULL here ?

Also log messages should not contain "\n"; the logging functions add
it.

Ian.

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 5/8] xl/libxl: add a blkdev_start parameter

Ian Jackson-2
In reply to this post by Stefano Stabellini-3
Stefano Stabellini writes ("[PATCH v5 5/8] xl/libxl: add a blkdev_start parameter"):
> +    if (b_info->blkdev_start == NULL)
> +        b_info->blkdev_start = strdup("xvda");

This should be   libxl__strdup(0, "xvda")
to get the correct error behaviour.

Aside from that,

Acked-by: Ian Jackson <[hidden email]>

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 6/8] libxl: introduce libxl__alloc_vdev

Ian Jackson-2
In reply to this post by Stefano Stabellini-3
Stefano Stabellini writes ("[PATCH v5 6/8] libxl: introduce libxl__alloc_vdev"):
> Introduce libxl__alloc_vdev: find a spare virtual block device in the
> domain passed as argument.
...

> +    do {
> +        vdev = GCSPRINTF("d%dp0", disk);
> +        devid = libxl__device_disk_dev_number(vdev, NULL, 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 (disk <= (1 << 20));

This should be "<", as disk==(1<<20) is too big.
And the magic number 20 should not be repeated.

But it turns out that you don't need to do this check here at all
because libxl__device_disk_dev_number will check this, correctly.

All you need to do is actually pay attention to its error return.

> +    return NULL;

All the NULL error exits should log an error surely.

> +/* same as in Linux */
> +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;
> +}

This still makes it difficult to see that the buffer cannot be
overrun.  I mentioned this in response to a previous posting of this
code and IIRC you were going to do something about it, if only a
comment explaining what the maximum buffer length is and why it's
safe.

> +    strcpy(ret, "xvd");
> +    ptr = encode_disk_name(ret + 3, offset);
> +    if (minor % nr_parts == 0)
> +        *ptr = 0;
> +    else
> +        snprintf(ptr, ret + 32 - ptr,
> +                "%d", minor & (nr_parts - 1));

You do not handle snprintf buffer truncation sensibly here.  As it
happens I think this overflow cannot happen but this deserves a
comment at the very least, to explain the lack of error handling.

Ian.

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v5 7/8] xl/libxl: implement QDISK libxl_device_disk_local_attach

Ian Jackson-2
In reply to this post by Stefano Stabellini-3
Stefano Stabellini writes ("[PATCH v5 7/8] xl/libxl: implement QDISK libxl_device_disk_local_attach"):
> - Spawn a QEMU instance at boot time to handle disk local attach
> requests.
...
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 7a1e017..e180498 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
...
> +                    rc = xs_transaction_end(ctx->xsh, t, 0);
> +                } while (rc == 0 && errno == EAGAIN);
> +                t = XBT_NULL;
> +                if (rc == 0) {
> +                    LOGE(ERROR, "xenstore transaction failed");
> +                    goto out;

Maybe I'm being excessively picky, but I think "rc" should be used
only for a variable which contains libxl error codes.  I had to do a
double-take when I saw
  if (rc == 0) {
     error handling;
since rc==0 is of course the success case.

Ian.

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
12