xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Roger Pau Monne <roger.pau@citrix.com>
To: xen-devel@lists.xen.org
Cc: Ian Jackson <ian.jackson@eu.citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: [PATCH v8 05/15] libxl: refactor disk addition to take a helper
Date: Wed, 4 Jul 2012 12:59:26 +0100	[thread overview]
Message-ID: <1341403176-50715-6-git-send-email-roger.pau@citrix.com> (raw)
In-Reply-To: <1341403176-50715-1-git-send-email-roger.pau@citrix.com>

Change libxl__device_disk_add to no longer take a xs transaction and
instead pass a helper for the local attach case that's used to get the
free vdev.

This function contains some non-functional changes due to an
indentation change.

Changes since v7:

 * Fixed the use of the transaction inside device_disk_add (which in
   v7 was fixed in the next patch).

Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
---
 tools/libxl/libxl.c          |  254 +++++++++++++++++++++++-------------------
 tools/libxl/libxl_internal.h |    2 +-
 2 files changed, 138 insertions(+), 118 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 7e6d5c9..4d832bb 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1733,115 +1733,146 @@ int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
     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;
+/* Specific function called directly only by local disk attach,
+ * all other users should instead use the regular
+ * libxl__device_disk_add wrapper
+ */
+static int device_disk_add(libxl__gc *gc, uint32_t domid,
+                           libxl_device_disk *disk,
+                           char *(*fn)(libxl__gc *, const char *,
+                                              xs_transaction_t),
+                           const char *blkdev_start)
+{
+    flexarray_t *front = NULL;
+    flexarray_t *back = NULL;
     char *dev;
     libxl__device device;
     int major, minor, rc;
     libxl_ctx *ctx = gc->owner;
+    xs_transaction_t t = XBT_NULL;
 
-    rc = libxl__device_disk_setdefault(gc, disk);
-    if (rc) goto out;
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        if (fn) {
+            assert(blkdev_start);
+            disk->vdev = fn(gc, blkdev_start, t);
+            if (disk->vdev == NULL) {
+                LOG(ERROR, "libxl__alloc_vdev failed");
+                rc = ERROR_FAIL;
+                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;
-    }
+        rc = libxl__device_disk_setdefault(gc, disk);
+        if (rc) goto out;
 
-    if (disk->script) {
-        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
-                   " not yet supported, sorry");
-        rc = ERROR_INVAL;
-        goto out_free;
-    }
+        if (front)
+            flexarray_free(front);
+        front = flexarray_make(16, 1);
+        if (!front) {
+            rc = ERROR_NOMEM;
+            goto out;
+        }
+        if (back)
+            flexarray_free(back);
+        back = flexarray_make(16, 1);
+        if (!back) {
+            rc = ERROR_NOMEM;
+            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;
-    }
+        if (disk->script) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "External block scripts"
+                       " not yet supported, sorry");
+            rc = ERROR_INVAL;
+            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));
+        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;
+        }
 
-            flexarray_append(back, "params");
-            flexarray_append(back, dev);
+        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));
 
-            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) {
-                LOG(ERROR, "failed to get blktap devpath for %p\n",
-                    disk->pdev_path);
-                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));
+                flexarray_append(back, "params");
+                flexarray_append(back, dev);
 
-            /* 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;
-    }
+                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) {
+                    LOG(ERROR, "failed to get blktap devpath for %p\n",
+                        disk->pdev_path);
+                    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(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");
 
-    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));
 
-    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 = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out_free;
+    }
 
     rc = 0;
 
@@ -1849,13 +1880,20 @@ out_free:
     flexarray_free(back);
     flexarray_free(front);
 out:
+    libxl__xs_transaction_abort(gc, &t);
     return rc;
 }
 
+int libxl__device_disk_add(libxl__gc *gc, uint32_t domid,
+                           libxl_device_disk *disk)
+{
+    return device_disk_add(gc, domid, disk, NULL, NULL);
+}
+
 int libxl_device_disk_add(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk)
 {
     GC_INIT(ctx);
-    int rc = libxl__device_disk_add(gc, domid, XBT_NULL, disk);
+    int rc = libxl__device_disk_add(gc, domid, disk);
     GC_FREE;
     return rc;
 }
@@ -2113,7 +2151,7 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
     libxl_ctx *ctx = gc->owner;
     char *dev = NULL, *be_path = NULL;
     char *ret = NULL;
-    int rc, xs_ret;
+    int rc;
     libxl__device device;
     xs_transaction_t t = XBT_NULL;
 
@@ -2159,27 +2197,9 @@ char * libxl__device_disk_local_attach(libxl__gc *gc,
             break;
         case LIBXL_DISK_BACKEND_QDISK:
             if (disk->format != LIBXL_DISK_FORMAT_RAW) {
-                do {
-                    t = xs_transaction_start(ctx->xsh);
-                    if (t == XBT_NULL) {
-                        LOG(ERROR, "failed to start a xenstore transaction");
-                        goto out;
-                    }
-                    disk->vdev = libxl__alloc_vdev(gc, blkdev_start, t);
-                    if (disk->vdev == NULL) {
-                        LOG(ERROR, "libxl__alloc_vdev failed");
-                        goto out;
-                    }
-                    if (libxl__device_disk_add(gc, LIBXL_TOOLSTACK_DOMID,
-                                t, disk)) {
-                        LOG(ERROR, "libxl_device_disk_add failed");
-                        goto out;
-                    }
-                    xs_ret = xs_transaction_end(ctx->xsh, t, 0);
-                } while (xs_ret == 0 && errno == EAGAIN);
-                t = XBT_NULL;
-                if (xs_ret == 0) {
-                    LOGE(ERROR, "xenstore transaction failed");
+                if (device_disk_add(gc, LIBXL_TOOLSTACK_DOMID, disk,
+                                    libxl__alloc_vdev, blkdev_start)) {
+                    LOG(ERROR, "libxl_device_disk_add failed");
                     goto out;
                 }
                 dev = GCSPRINTF("/dev/%s", disk->vdev);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7a75809..1befdc5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1291,7 +1291,7 @@ _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);
+                                   libxl_device_disk *disk);
 
 /*
  * Make a disk available in this (the control) domain. Returns path to
-- 
1.7.7.5 (Apple Git-26)

  parent reply	other threads:[~2012-07-04 11:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-04 11:59 [PATCH v8 00/15] execute hotplug scripts from libxl Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 01/15] libxl: change ao_device_remove to ao_device Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 02/15] libxl: move device model creation prototypes Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 03/15] libxl: convert libxl_domain_destroy to an async op Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 04/15] libxl: move bootloader data strucutres and prototypes Roger Pau Monne
2012-07-04 11:59 ` Roger Pau Monne [this message]
2012-07-04 11:59 ` [PATCH v8 06/15] libxl: convert libxl__device_disk_local_attach to an async op Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 07/15] libxl: rename vifs to nics Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 08/15] libxl: convert libxl_device_disk_add to an async op Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 09/15] libxl: convert libxl_device_nic_add to an async operation Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 10/15] libxl: add option to choose who executes hotplug scripts Roger Pau Monne
2012-07-06 10:28   ` Ian Campbell
2012-07-10  9:20     ` Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 11/15] libxl: rename _IOEMU nic type to VIF_IOEMU Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 12/15] libxl: set correct nic type depending on the guest Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 13/15] libxl: use libxl__xs_path_cleanup on device_destroy Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 14/15] libxl: call hotplug scripts for disk devices from libxl Roger Pau Monne
2012-07-04 11:59 ` [PATCH v8 15/15] libxl: call hotplug scripts for nic " Roger Pau Monne
2012-07-08 18:32 ` [PATCH v8 00/15] execute hotplug scripts " Ian Campbell
2012-07-08 18:41   ` Ian Campbell
2012-07-10 11:31   ` Roger Pau Monne
2012-07-10 15:55     ` Ian Campbell
2012-07-10 16:07       ` Roger Pau Monne
2012-07-10 16:57         ` Ian Campbell
2012-07-10 17:23           ` Roger Pau Monne
2012-07-10 17:30             ` Ian Campbell
2012-07-10 17:39               ` Roger Pau Monne
2012-07-10 17:40             ` Ian Campbell
2012-07-11  9:08               ` Roger Pau Monne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1341403176-50715-6-git-send-email-roger.pau@citrix.com \
    --to=roger.pau@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).