xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0 of 7] libxl: refactor tap disk handling
@ 2011-04-07  9:52 Ian Campbell
  2011-04-07  9:52 ` [PATCH 1 of 7] libxl: remove impossible check for backend != DISK_BACKEND_QDISK Ian Campbell
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

I'm not personally convinced that support for blktap2 devices should
be conflated in libxl with the PV block backend support but given it's
there lets at least correct it.

In a blktap2 system there is no "tap" PV backend type. blktap2 exposes
a standard block device and this is passed to a guest using the
standard blkback "vbd" (sometimes called "phy") backend. Drop the
"tap" backend type and associated (libxl internal ) DEVICE_TAP
enumeration value.

Also try and clarify the paths which fallback from blktap2 to qdisk
when the former is not present. Falling through a switch statement is
a neat way of doing this in some cases, but I don't think this is one
of them.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1 of 7] libxl: remove impossible check for backend != DISK_BACKEND_QDISK
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
@ 2011-04-07  9:52 ` Ian Campbell
  2011-04-07  9:52 ` [PATCH 2 of 7] libxl: make fallback from blktap2 to qdisk more explicit Ian Campbell
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302167172 -3600
# Node ID 29af1669de3ab2e7d41078eb33a0dfa9b69f09bb
# Parent  bbb1ffd81018b0b7677c8b526de118d4501f6d39
libxl: remove impossible check for backend != DISK_BACKEND_QDISK

In this case we are already in the DISK_BACKEND_QDISK case of a switch
statement on the same variable.

It is possible that we fell through from the DISK_BACKEND_TAP case
(although I'm about to remove that in a subsequent patch), however in
that case we are explicitly falling back from blktap2 to qdisk so
DEVICE_QDISK is still the right answer.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r bbb1ffd81018 -r 29af1669de3a tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 07 09:56:46 2011 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 07 10:06:12 2011 +0100
@@ -1016,12 +1016,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",
                           libxl__device_disk_string_of_format(disk->format), disk->pdev_path));
-
-            if (libxl__blktap_enabled(&gc) && 
-                 disk->backend != DISK_BACKEND_QDISK)
-                device.backend_kind = DEVICE_TAP;
-            else
-                device.backend_kind = DEVICE_QDISK;
+            device.backend_kind = DEVICE_QDISK;
             break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n", disk->backend);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2 of 7] libxl: make fallback from blktap2 to qdisk more explicit
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
  2011-04-07  9:52 ` [PATCH 1 of 7] libxl: remove impossible check for backend != DISK_BACKEND_QDISK Ian Campbell
@ 2011-04-07  9:52 ` Ian Campbell
  2011-04-07  9:52 ` [PATCH 3 of 7] libxl: convert an empty tap disk into a qdisk Ian Campbell
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302167195 -3600
# Node ID 76d363a113e5ee6111bc9e0eca5a314644ce6295
# Parent  29af1669de3ab2e7d41078eb33a0dfa9b69f09bb
libxl: make fallback from blktap2 to qdisk more explicit.

When blktap2 is not present we fallback to qdisk, instead of falling
through a switch statement instead make this explicit, with a comment,
prior to the switch statement.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 29af1669de3a -r 76d363a113e5 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 07 10:06:12 2011 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 07 10:06:35 2011 +0100
@@ -979,6 +979,10 @@ int libxl_device_disk_add(libxl_ctx *ctx
     device.domid = domid;
     device.kind = DEVICE_VBD;
 
+    /* If blktap is not available then fallback to qdisk */
+    if (disk->backend == DISK_BACKEND_TAP && !libxl__blktap_enabled(&gc))
+        disk->backend = DISK_BACKEND_QDISK;
+
     switch (disk->backend) {
         case DISK_BACKEND_PHY: 
             libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
@@ -991,7 +995,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             device.backend_kind = DEVICE_VBD;
             break;
         case DISK_BACKEND_TAP:
-            if (libxl__blktap_enabled(&gc) && disk->format != DISK_FORMAT_EMPTY) {
+            if (disk->format != DISK_FORMAT_EMPTY) {
                 const char *dev = libxl__blktap_devpath(&gc,
                                                disk->pdev_path, disk->format);
                 if (!dev) {
@@ -1012,7 +1016,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
 
                 break;
             }
-        case DISK_BACKEND_QDISK: 
+            break;
+        case 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));

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3 of 7] libxl: convert an empty tap disk into a qdisk
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
  2011-04-07  9:52 ` [PATCH 1 of 7] libxl: remove impossible check for backend != DISK_BACKEND_QDISK Ian Campbell
  2011-04-07  9:52 ` [PATCH 2 of 7] libxl: make fallback from blktap2 to qdisk more explicit Ian Campbell
@ 2011-04-07  9:52 ` Ian Campbell
  2011-04-07  9:52 ` [PATCH 4 of 7] libxl: only a CDROM type disk can be empty Ian Campbell
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302167196 -3600
# Node ID f5418873be3d097cbb374009898ce9c74255e047
# Parent  76d363a113e5ee6111bc9e0eca5a314644ce6295
libxl: convert an empty tap disk into a qdisk

I'm not sure that empty disks which are is_cdrom are especially valid,
or that a cdrom can ever be handled by tapdisk anyway but try to do
something sane since it seems that xl's parse_disk_config() routine
could potentially generate such a configuration (although whether from
a valid input string or not I'm not sure).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 76d363a113e5 -r f5418873be3d tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 07 10:06:35 2011 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 07 10:06:36 2011 +0100
@@ -983,6 +983,14 @@ int libxl_device_disk_add(libxl_ctx *ctx
     if (disk->backend == DISK_BACKEND_TAP && !libxl__blktap_enabled(&gc))
         disk->backend = DISK_BACKEND_QDISK;
 
+    /*
+     * blktap cannot handle empty disks (aka cdroms). Fall back to
+     * qdisk because qemu-xen creates the disk based on the xenstore
+     * entries.
+     */
+    if (disk->backend == DISK_BACKEND_TAP && disk->format == DISK_FORMAT_EMPTY)
+        disk->backend == DISK_BACKEND_QDISK;
+
     switch (disk->backend) {
         case DISK_BACKEND_PHY: 
             libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
@@ -995,7 +1003,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
             device.backend_kind = DEVICE_VBD;
             break;
         case DISK_BACKEND_TAP:
-            if (disk->format != DISK_FORMAT_EMPTY) {
+            {
                 const char *dev = libxl__blktap_devpath(&gc,
                                                disk->pdev_path, disk->format);
                 if (!dev) {
@@ -1003,8 +1011,8 @@ int libxl_device_disk_add(libxl_ctx *ctx
                     goto out_free;
                 }
                 flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(&gc, "%s:%s", 
-                    libxl__device_disk_string_of_format(disk->format), 
+                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, libxl__strdup(&gc, dev));
@@ -1013,8 +1021,6 @@ int libxl_device_disk_add(libxl_ctx *ctx
                 flexarray_append(back, "physical-device");
                 flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
                 device.backend_kind = DEVICE_VBD;
-
-                break;
             }
             break;
         case DISK_BACKEND_QDISK:

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4 of 7] libxl: only a CDROM type disk can be empty
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
                   ` (2 preceding siblings ...)
  2011-04-07  9:52 ` [PATCH 3 of 7] libxl: convert an empty tap disk into a qdisk Ian Campbell
@ 2011-04-07  9:52 ` Ian Campbell
  2011-04-07  9:52 ` [PATCH 5 of 7] libxl: refactor DISK_BACKEND_PHY handling in libxl_device_disk_add Ian Campbell
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302167211 -3600
# Node ID cbf6ec01faef6502845d8201a717edc9fa6fae52
# Parent  f5418873be3d097cbb374009898ce9c74255e047
libxl: only a CDROM type disk can be empty.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r f5418873be3d -r cbf6ec01faef tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 07 10:06:36 2011 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 07 10:06:51 2011 +0100
@@ -908,8 +908,13 @@ static int validate_virtual_disk(libxl__
     struct stat stat_buf;
     char *delimiter;
 
-    if (disk->format == DISK_FORMAT_EMPTY)
-        return 0;
+    if (disk->format == DISK_FORMAT_EMPTY) {
+        if (disk->is_cdrom)
+            return 0;
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Empty disk %s is not a CDROM device\n",
+                   disk->vdev);
+        return ERROR_INVAL;
+    }
 
     if (disk->format == DISK_FORMAT_RAW) {
         delimiter = strchr(file_name, ':');

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5 of 7] libxl: refactor DISK_BACKEND_PHY handling in libxl_device_disk_add
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
                   ` (3 preceding siblings ...)
  2011-04-07  9:52 ` [PATCH 4 of 7] libxl: only a CDROM type disk can be empty Ian Campbell
@ 2011-04-07  9:52 ` Ian Campbell
  2011-04-07  9:52 ` [PATCH 6 of 7] libxl: handle the tail end of a tap device using the phy backend handling code Ian Campbell
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302167211 -3600
# Node ID ec3b25b5aa8095c5fe070173af888f032f6c2c5e
# Parent  cbf6ec01faef6502845d8201a717edc9fa6fae52
libxl: refactor DISK_BACKEND_PHY handling in libxl_device_disk_add

A step on the path to sharing this code with the tail-end of the
DISK_BACKEND_TAP case.

I made the result of libxl__blktap_devpath non-const to achieve
this. The existing caller calls libxl__strdup on the result but since
the function is an internal one and the result is already garbage
collected I think this is unnecessary and we can just use the
non-const result directly.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r cbf6ec01faef -r ec3b25b5aa80 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 07 10:06:51 2011 +0100
@@ -949,6 +949,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     libxl__gc gc = LIBXL_INIT_GC(ctx);
     flexarray_t *front;
     flexarray_t *back;
+    char *dev;
     char *backend_type;
     int devid;
     libxl__device device;
@@ -997,36 +998,39 @@ int libxl_device_disk_add(libxl_ctx *ctx
         disk->backend == DISK_BACKEND_QDISK;
 
     switch (disk->backend) {
-        case DISK_BACKEND_PHY: 
-            libxl__device_physdisk_major_minor(disk->pdev_path, &major, &minor);
+        case DISK_BACKEND_PHY:
+            dev = disk->pdev_path;
+
+            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, disk->pdev_path);
+            flexarray_append(back, dev);
 
             device.backend_kind = DEVICE_VBD;
             break;
         case DISK_BACKEND_TAP:
-            {
-                const char *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));
-                flexarray_append(back, "params");
-                flexarray_append(back, libxl__strdup(&gc, dev));
-                backend_type = "phy";
-                libxl__device_physdisk_major_minor(dev, &major, &minor);
-                flexarray_append(back, "physical-device");
-                flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
-                device.backend_kind = DEVICE_VBD;
+            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));
+
+            flexarray_append(back, "params");
+            flexarray_append(back, dev);
+
+            backend_type = "phy";
+
+            libxl__device_physdisk_major_minor(dev, &major, &minor);
+            flexarray_append(back, "physical-device");
+            flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
+
+            device.backend_kind = DEVICE_VBD;
             break;
         case DISK_BACKEND_QDISK:
             flexarray_append(back, "params");
@@ -1110,7 +1114,7 @@ int libxl_device_disk_del(libxl_ctx *ctx
 char * libxl_device_disk_local_attach(libxl_ctx *ctx, libxl_device_disk *disk)
 {
     libxl__gc gc = LIBXL_INIT_GC(ctx);
-    const char *dev = NULL;
+    char *dev = NULL;
     char *ret = NULL;
 
     switch (disk->backend) {
diff -r cbf6ec01faef -r ec3b25b5aa80 tools/libxl/libxl_blktap2.c
--- a/tools/libxl/libxl_blktap2.c	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl_blktap2.c	Thu Apr 07 10:06:51 2011 +0100
@@ -24,9 +24,9 @@ int libxl__blktap_enabled(libxl__gc *gc)
     return !tap_ctl_check(&msg);
 }
 
-const char *libxl__blktap_devpath(libxl__gc *gc,
-                                 const char *disk,
-                                 libxl_disk_format format)
+char *libxl__blktap_devpath(libxl__gc *gc,
+                            const char *disk,
+                            libxl_disk_format format)
 {
     const char *type;
     char *params, *devname = NULL;
diff -r cbf6ec01faef -r ec3b25b5aa80 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Apr 07 10:06:51 2011 +0100
@@ -325,9 +325,9 @@ _hidden int libxl__blktap_enabled(libxl_
  *    returns device path xenstore wants to have. returns NULL
  *      if no device corresponds to the disk.
  */
-_hidden const char *libxl__blktap_devpath(libxl__gc *gc,
-                                 const char *disk,
-                                 libxl_disk_format format);
+_hidden char *libxl__blktap_devpath(libxl__gc *gc,
+                                    const char *disk,
+                                    libxl_disk_format format);
 
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);
 
diff -r cbf6ec01faef -r ec3b25b5aa80 tools/libxl/libxl_noblktap2.c
--- a/tools/libxl/libxl_noblktap2.c	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl_noblktap2.c	Thu Apr 07 10:06:51 2011 +0100
@@ -21,9 +21,9 @@ int libxl__blktap_enabled(libxl__gc *gc)
     return 0;
 }
 
-const char *libxl__blktap_devpath(libxl__gc *gc,
-                                 const char *disk,
-                                 libxl_disk_format format)
+char *libxl__blktap_devpath(libxl__gc *gc,
+                            const char *disk,
+                            libxl_disk_format format)
 {
     return NULL;
 }

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 6 of 7] libxl: handle the tail end of a tap device using the phy backend handling code
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
                   ` (4 preceding siblings ...)
  2011-04-07  9:52 ` [PATCH 5 of 7] libxl: refactor DISK_BACKEND_PHY handling in libxl_device_disk_add Ian Campbell
@ 2011-04-07  9:52 ` Ian Campbell
  2011-04-07  9:52 ` [PATCH 7 of 7] libxl: Drop internal DEVICE_TAP backend type Ian Campbell
  2011-04-07 12:31 ` [PATCH 0 of 7] libxl: refactor tap disk handling Christoph Egger
  7 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302167211 -3600
# Node ID 40b0954cb5d9bdd0b943963afb79914ba5cb08c9
# Parent  ec3b25b5aa8095c5fe070173af888f032f6c2c5e
libxl: handle the tail end of a tap device using the phy backend handling code

We are literally creating a phy backend on top of a blktap2 created
device anyway so we might as well reuse the code and make this
explicit.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r ec3b25b5aa80 -r 40b0954cb5d9 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 07 10:06:51 2011 +0100
@@ -1000,7 +1000,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     switch (disk->backend) {
         case 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));
@@ -1021,17 +1021,11 @@ int libxl_device_disk_add(libxl_ctx *ctx
                 libxl__device_disk_string_of_format(disk->format),
                 disk->pdev_path));
 
-            flexarray_append(back, "params");
-            flexarray_append(back, dev);
-
             backend_type = "phy";
 
-            libxl__device_physdisk_major_minor(dev, &major, &minor);
-            flexarray_append(back, "physical-device");
-            flexarray_append(back, libxl__sprintf(&gc, "%x:%x", major, minor));
-
-            device.backend_kind = DEVICE_VBD;
-            break;
+            /* now create a phy device to export the device to the guest */
+            goto do_backend_phy;
+
         case DISK_BACKEND_QDISK:
             flexarray_append(back, "params");
             flexarray_append(back, libxl__sprintf(&gc, "%s:%s",

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 7 of 7] libxl: Drop internal DEVICE_TAP backend type
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
                   ` (5 preceding siblings ...)
  2011-04-07  9:52 ` [PATCH 6 of 7] libxl: handle the tail end of a tap device using the phy backend handling code Ian Campbell
@ 2011-04-07  9:52 ` Ian Campbell
  2011-04-08 15:46   ` Ian Jackson
  2011-04-07 12:31 ` [PATCH 0 of 7] libxl: refactor tap disk handling Christoph Egger
  7 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2011-04-07  9:52 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302167211 -3600
# Node ID 6162ae4cc9a7fae505f6b081a6698ecdd94418c2
# Parent  40b0954cb5d9bdd0b943963afb79914ba5cb08c9
libxl: Drop internal DEVICE_TAP backend type

There is no such thing with blktap2, the backend in that case is PHY.

libxl_device_disk_del was just plain wrong in this regard, fix it to
select the appropriate backend_kind.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 40b0954cb5d9 -r 6162ae4cc9a7 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl.c	Thu Apr 07 10:06:51 2011 +0100
@@ -950,7 +950,6 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_t *front;
     flexarray_t *back;
     char *dev;
-    char *backend_type;
     int devid;
     libxl__device device;
     int major, minor, rc;
@@ -970,7 +969,6 @@ int libxl_device_disk_add(libxl_ctx *ctx
         goto out_free;
     }
 
-    backend_type = libxl__device_disk_string_of_backend(disk->backend);
     devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
     if (devid==-1) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Invalid or unsupported"
@@ -1021,8 +1019,6 @@ int libxl_device_disk_add(libxl_ctx *ctx
                 libxl__device_disk_string_of_format(disk->format),
                 disk->pdev_path));
 
-            backend_type = "phy";
-
             /* now create a phy device to export the device to the guest */
             goto do_backend_phy;
 
@@ -1051,7 +1047,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
     flexarray_append(back, "dev");
     flexarray_append(back, disk->vdev);
     flexarray_append(back, "type");
-    flexarray_append(back, backend_type);
+    flexarray_append(back, libxl__device_disk_string_of_backend(disk->backend));
     flexarray_append(back, "mode");
     flexarray_append(back, disk->readwrite ? "w" : "r");
 
@@ -1093,14 +1089,30 @@ int libxl_device_disk_del(libxl_ctx *ctx
     devid = libxl__device_disk_dev_number(disk->vdev, NULL, NULL);
     device.backend_domid    = disk->backend_domid;
     device.backend_devid    = devid;
-    device.backend_kind     =
-        (disk->backend == DISK_BACKEND_PHY) ? DEVICE_VBD : DEVICE_TAP;
+
+    switch (disk->backend) {
+        case DISK_BACKEND_PHY:
+            device.backend_kind = DEVICE_VBD;
+            break;
+        case DISK_BACKEND_TAP:
+            device.backend_kind = DEVICE_VBD;
+            break;
+        case DISK_BACKEND_QDISK:
+            device.backend_kind = DEVICE_QDISK;
+            break;
+        default:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend type: %d\n",
+                       disk->backend);
+            rc = ERROR_INVAL;
+            goto out_free;
+    }
     device.domid            = domid;
     device.devid            = devid;
     device.kind             = DEVICE_VBD;
     rc = libxl__device_del(&gc, &device, wait);
     xs_rm(ctx->xsh, XBT_NULL, libxl__device_backend_path(&gc, &device));
     xs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(&gc, &device));
+out_free:
     libxl__free_all(&gc);
     return rc;
 }
diff -r 40b0954cb5d9 -r 6162ae4cc9a7 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl_device.c	Thu Apr 07 10:06:51 2011 +0100
@@ -31,7 +31,6 @@
 static const char *string_of_kinds[] = {
     [DEVICE_VIF] = "vif",
     [DEVICE_VBD] = "vbd",
-    [DEVICE_TAP] = "tap",
     [DEVICE_QDISK] = "qdisk",
     [DEVICE_PCI] = "pci",
     [DEVICE_VFB] = "vfb",
@@ -135,7 +134,7 @@ char *libxl__device_disk_string_of_backe
 {
     switch (backend) {
         case DISK_BACKEND_QDISK: return "qdisk";
-        case DISK_BACKEND_TAP: return "tap";
+        case DISK_BACKEND_TAP: return "phy";
         case DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }
diff -r 40b0954cb5d9 -r 6162ae4cc9a7 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu Apr 07 10:06:51 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Thu Apr 07 10:06:51 2011 +0100
@@ -97,7 +97,6 @@ struct libxl__ctx {
 typedef enum {
     DEVICE_VIF = 1,
     DEVICE_VBD,
-    DEVICE_TAP,
     DEVICE_QDISK,
     DEVICE_PCI,
     DEVICE_VFB,

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0 of 7] libxl: refactor tap disk handling
  2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
                   ` (6 preceding siblings ...)
  2011-04-07  9:52 ` [PATCH 7 of 7] libxl: Drop internal DEVICE_TAP backend type Ian Campbell
@ 2011-04-07 12:31 ` Christoph Egger
  2011-04-07 14:12   ` Ian Campbell
  7 siblings, 1 reply; 11+ messages in thread
From: Christoph Egger @ 2011-04-07 12:31 UTC (permalink / raw)
  To: xen-devel

On 04/07/11 11:52, Ian Campbell wrote:
> I'm not personally convinced that support for blktap2 devices should
> be conflated in libxl with the PV block backend support but given it's
> there lets at least correct it.
>
> In a blktap2 system there is no "tap" PV backend type. blktap2 exposes
> a standard block device and this is passed to a guest using the
> standard blkback "vbd" (sometimes called "phy") backend. Drop the
> "tap" backend type and associated (libxl internal ) DEVICE_TAP
> enumeration value.

What happens on a no-blktap2 system?

>
> Also try and clarify the paths which fallback from blktap2 to qdisk
> when the former is not present. Falling through a switch statement is
> a neat way of doing this in some cases, but I don't think this is one
> of them.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0 of 7] libxl: refactor tap disk handling
  2011-04-07 12:31 ` [PATCH 0 of 7] libxl: refactor tap disk handling Christoph Egger
@ 2011-04-07 14:12   ` Ian Campbell
  0 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2011-04-07 14:12 UTC (permalink / raw)
  To: Christoph Egger; +Cc: xen-devel@lists.xensource.com

On Thu, 2011-04-07 at 13:31 +0100, Christoph Egger wrote:
> On 04/07/11 11:52, Ian Campbell wrote:
> > I'm not personally convinced that support for blktap2 devices should
> > be conflated in libxl with the PV block backend support but given it's
> > there lets at least correct it.
> >
> > In a blktap2 system there is no "tap" PV backend type. blktap2 exposes
> > a standard block device and this is passed to a guest using the
> > standard blkback "vbd" (sometimes called "phy") backend. Drop the
> > "tap" backend type and associated (libxl internal ) DEVICE_TAP
> > enumeration value.
> 
> What happens on a no-blktap2 system?

Same as before, libxl__blktap_enabled still returns false and the same
things should happen as a result.

Ian.
> 
> >
> > Also try and clarify the paths which fallback from blktap2 to qdisk
> > when the former is not present. Falling through a switch statement is
> > a neat way of doing this in some cases, but I don't think this is one
> > of them.
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> >
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 7 of 7] libxl: Drop internal DEVICE_TAP backend type
  2011-04-07  9:52 ` [PATCH 7 of 7] libxl: Drop internal DEVICE_TAP backend type Ian Campbell
@ 2011-04-08 15:46   ` Ian Jackson
  0 siblings, 0 replies; 11+ messages in thread
From: Ian Jackson @ 2011-04-08 15:46 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("[Xen-devel] [PATCH 7 of 7] libxl: Drop internal DEVICE_TAP backend type"):
> libxl: Drop internal DEVICE_TAP backend type

Applied all seven, thanks.

Ian.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2011-04-08 15:46 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-07  9:52 [PATCH 0 of 7] libxl: refactor tap disk handling Ian Campbell
2011-04-07  9:52 ` [PATCH 1 of 7] libxl: remove impossible check for backend != DISK_BACKEND_QDISK Ian Campbell
2011-04-07  9:52 ` [PATCH 2 of 7] libxl: make fallback from blktap2 to qdisk more explicit Ian Campbell
2011-04-07  9:52 ` [PATCH 3 of 7] libxl: convert an empty tap disk into a qdisk Ian Campbell
2011-04-07  9:52 ` [PATCH 4 of 7] libxl: only a CDROM type disk can be empty Ian Campbell
2011-04-07  9:52 ` [PATCH 5 of 7] libxl: refactor DISK_BACKEND_PHY handling in libxl_device_disk_add Ian Campbell
2011-04-07  9:52 ` [PATCH 6 of 7] libxl: handle the tail end of a tap device using the phy backend handling code Ian Campbell
2011-04-07  9:52 ` [PATCH 7 of 7] libxl: Drop internal DEVICE_TAP backend type Ian Campbell
2011-04-08 15:46   ` Ian Jackson
2011-04-07 12:31 ` [PATCH 0 of 7] libxl: refactor tap disk handling Christoph Egger
2011-04-07 14:12   ` Ian Campbell

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).