Xen devel - Apr 2013 - [PATCH 0 of 3] blktap3/libxl: add support for blktap3 in libxl

This patch series implements support for blktap3 in libxl.

Supporting blktap3 requires rather few changes:
1. We introduce a new disk back-end type (TAP3) and a new device kind (VBD3) to
   allow blktap3 to co-exist with blktap3. blktap2 remains the default back-end
   for tap devices. Switching in the future to blktap3 as the default handler
   of tap devices should be trivial.
2. libxl doesn''t spawn the tapdisk process any more, as is the case for
   blktap2, since the tapback daemon is responsible for that. Thus, libxl only
   needs to write to XenStore the file/partition/whatever backing the virtual
   disk so that the tapback daemon can pass it to tapdisk.
3. Since there is no block device in dom0 any more, pygrub won''t be
able to
   boot from VHD files. To get around this problem, pygrub can use the NBD
   functionality (existing in blktap2.5): it can explicitly ask tapback to
   create a NDB in order to access the virtual disk. This functionality will be
   implemented in a future patch series. However, this will only work for Linux
   dom0''s. As a generic solution that would work on any dom0, we could
   implement a simple protocol for data exchange between pygrub and tapdisk.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>

---
Changed since v1:
  * Check whether tapback''s control socket exists instead of checking
whether
    a process named "tapback" exists.
  * Don''t duplicate code for writing the type:/path/to/file to
XenStore.

We use new identifiers so that blktap2 and blktap3 can co-exist.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -58,6 +58,7 @@ libxl_disk_backend = Enumeration("disk_b
     (1, "PHY"),
     (2, "TAP"),
     (3, "QDISK"),
+    (4, "TAP3"),
     ])
 
 libxl_nic_type = Enumeration("nic_type", [
diff --git a/tools/libxl/libxl_types_internal.idl
b/tools/libxl/libxl_types_internal.idl
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -20,6 +20,7 @@ libxl__device_kind = Enumeration("device
     (6, "VKBD"),
     (7, "CONSOLE"),
     (8, "VTPM"),
+    (9, "VBD3"),
     ])
 
 libxl__console_backend = Enumeration("console_backend", [
diff --git a/tools/libxl/libxlu_disk_l.c b/tools/libxl/libxlu_disk_l.c
--- a/tools/libxl/libxlu_disk_l.c
+++ b/tools/libxl/libxlu_disk_l.c
@@ -859,6 +859,7 @@ static void setformat(DiskParseContext *
 static void setbackendtype(DiskParseContext *dpc, const char *str) {
     if (     !strcmp(str,"phy"))   DSET(dpc,backend,BACKEND,str,PHY);
     else if (!strcmp(str,"tap"))   DSET(dpc,backend,BACKEND,str,TAP);
+    else if (!strcmp(str,"tap3"))  
DSET(dpc,backend,BACKEND,str,TAP3);
     else if (!strcmp(str,"qdisk"))
DSET(dpc,backend,BACKEND,str,QDISK);
     else xlu__disk_err(dpc,str,"unknown value for backendtype");
 }
diff --git a/tools/libxl/libxlu_disk_l.l b/tools/libxl/libxlu_disk_l.l
--- a/tools/libxl/libxlu_disk_l.l
+++ b/tools/libxl/libxlu_disk_l.l
@@ -109,6 +109,7 @@ static void setformat(DiskParseContext *
 static void setbackendtype(DiskParseContext *dpc, const char *str) {
     if (     !strcmp(str,"phy"))   DSET(dpc,backend,BACKEND,str,PHY);
     else if (!strcmp(str,"tap"))   DSET(dpc,backend,BACKEND,str,TAP);
+    else if (!strcmp(str,"tap3"))  
DSET(dpc,backend,BACKEND,str,TAP3);
     else if (!strcmp(str,"qdisk"))
DSET(dpc,backend,BACKEND,str,QDISK);
     else xlu__disk_err(dpc,str,"unknown value for backendtype");
 }

This patch implements function libxl__blktap3_enabled, the equivalent of the
existing libxl__blktap_enabled for blktap2. The checks performed are rather
simple and should be extended.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
---
Changed since v1:
  * Check whether tapback''s control socket exists instead of checking
whether
    the process exists.
  * Include the blktap3 header when compiling as this is where the path of
    tapback''s control socket is defined.

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -37,8 +37,11 @@ LIBXLU_LIBS  LIBXL_OBJS-y = osdeps.o libxl_paths.o
libxl_bootloader.o flexarray.o
 ifeq ($(LIBXL_BLKTAP),y)
 LIBXL_OBJS-y += libxl_blktap2.o
+LIBXL_OBJS-y += libxl_blktap3.o
+CFLAGS += -I$(XEN_ROOT)/tools/blktap3/include
 else
 LIBXL_OBJS-y += libxl_noblktap2.o
+LIBXL_OBJS-y += libxl_noblktap3.o
 endif
 LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o
 LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o libxl_noarch.o
diff --git a/tools/libxl/libxl_blktap3.c b/tools/libxl/libxl_blktap3.c
new file mode 100644
--- /dev/null
+++ b/tools/libxl/libxl_blktap3.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2012      Citrix Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * 
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+ * USA.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include "libxl_internal.h"
+#include <unistd.h>
+#include "blktap3.h"
+
+/*
+ * Simple sanity checks. Most of these checks are not race-free (e.g. checking
+ * wether the tapdisk binary exists), but at least we get some protection
+ * against spectacularly silly mistakes.
+ *
+ * We don''t check whether the tapdisk binary exists as this is done by
the
+ * tapback daemon.
+ */
+int libxl__blktap3_enabled(libxl__gc *gc)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int err;
+
+    /*
+     * Check whether the tapback daemon is running.
+     */
+    err = access(TAPBACK_CTL_SOCK_PATH, F_OK);
+    if (err) {
+        if (errno != ENOENT)
+            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+                "failed to check whether the tapback daemon is
running\n");
+        return 0;
+    }
+
+    /*
+     * TODO Check for evtchn, gntdev. How do we do that!?
+     */
+
+    return 1;
+}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1338,6 +1338,14 @@ struct libxl__cpuid_policy {
 };
 
 /*
+ * blktap3 support
+ */
+/* libxl__blktap_enabled:
+ *    return true if blktap3 support is available.
+ */
+_hidden int libxl__blktap3_enabled(libxl__gc *gc);
+
+/*
  * blktap2 support
  */
 
diff --git a/tools/libxl/libxl_noblktap3.c b/tools/libxl/libxl_noblktap3.c
new file mode 100644
--- /dev/null
+++ b/tools/libxl/libxl_noblktap3.c
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2012      Citrix Ltd.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * 
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+ * USA.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include "libxl_internal.h"
+
+int libxl__blktap3_enabled(libxl__gc *gc)
+{
+    return 0;
+}

Handling of blktap3 devices is similar to blktap2, except that libxl
doesn''t
spawn the tapdisk and doesn''t create the physical device in dom0.

Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>

---
Changed since v1:
  * Don''t duplicate code for writing the type:/path/to/file to
XenStore.

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1159,6 +1159,8 @@ static void disk_eject_xswatch_callback(
         disk->backend = LIBXL_DISK_BACKEND_TAP;
     } else if (!strcmp(backend_type, "qdisk")) {
         disk->backend = LIBXL_DISK_BACKEND_QDISK;
+    } else if (!strcmp(backend_type, "tap3")) {
+        disk->backend = LIBXL_DISK_BACKEND_TAP3;
     } else {
         disk->backend = LIBXL_DISK_BACKEND_UNKNOWN;
     }
@@ -2012,6 +2014,9 @@ int libxl__device_from_disk(libxl__gc *g
         case LIBXL_DISK_BACKEND_QDISK:
             device->backend_kind = LIBXL__DEVICE_KIND_QDISK;
             break;
+        case LIBXL_DISK_BACKEND_TAP3:
+            device->backend_kind = LIBXL__DEVICE_KIND_VBD3;
+            break;
         default:
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk backend
type: %d\n",
                        disk->backend);
@@ -2130,11 +2135,15 @@ static void device_disk_add(libxl__egc *
 
                 /* now create a phy device to export the device to the guest */
                 goto do_backend_phy;
+            case LIBXL_DISK_BACKEND_TAP3:
             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);
+                assert(
+                        (disk->backend == LIBXL_DISK_BACKEND_QDISK
+                         && device->backend_kind ==
LIBXL__DEVICE_KIND_QDISK)
+                        || (disk->backend == LIBXL_DISK_BACKEND_TAP3));
                 break;
             default:
                 LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "unrecognized disk
backend type: %d\n", disk->backend);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -190,6 +190,23 @@ static int disk_try_backend(disk_try_bac
         }
         return backend;
 
+    case LIBXL_DISK_BACKEND_TAP3:
+        /* TODO What''s that script thing? */
+        if (a->disk->script) goto bad_script;
+
+        if (!libxl__blktap3_enabled(a->gc)) {
+            LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, backend
tap"
+                       " unsuitable because blktap3 not available",
+                       a->disk->vdev);
+            return 0;
+        }
+        /* TODO other formats supported by blktap3? */
+        if (!(a->disk->format == LIBXL_DISK_FORMAT_RAW ||
+              a->disk->format == LIBXL_DISK_FORMAT_VHD)) {
+            goto bad_format;
+        }
+        return backend;
+
     case LIBXL_DISK_BACKEND_QDISK:
         if (a->disk->script) goto bad_script;
         return backend;
@@ -258,6 +275,7 @@ int libxl__device_disk_set_backend(libxl
         ok             disk_try_backend(&a, LIBXL_DISK_BACKEND_PHY) ?:
             disk_try_backend(&a, LIBXL_DISK_BACKEND_TAP) ?:
+            disk_try_backend(&a, LIBXL_DISK_BACKEND_TAP3) ?:
             disk_try_backend(&a, LIBXL_DISK_BACKEND_QDISK);
         if (ok)
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, using backend
%s",
@@ -290,6 +308,7 @@ char *libxl__device_disk_string_of_backe
     switch (backend) {
         case LIBXL_DISK_BACKEND_QDISK: return "qdisk";
         case LIBXL_DISK_BACKEND_TAP: return "phy";
+        case LIBXL_DISK_BACKEND_TAP3: return "phy";
         case LIBXL_DISK_BACKEND_PHY: return "phy";
         default: return NULL;
     }

On Fri, Apr 19, 2013 at 04:40:48PM +0100, Thanos Makatos
wrote:
> This patch series implements support for blktap3 in libxl.
> 
> Supporting blktap3 requires rather few changes:
> 1. We introduce a new disk back-end type (TAP3) and a new device kind
(VBD3) to
>    allow blktap3 to co-exist with blktap3.
"allow blktap3 to co-exist with blktap3" ? 

I guess the the other one should be blktap2 .. 

-- Pasi

>  blktap2 remains the default back-end
>    for tap devices. Switching in the future to blktap3 as the default
handler
>    of tap devices should be trivial.
> 2. libxl doesn''t spawn the tapdisk process any more, as is the
case for
>    blktap2, since the tapback daemon is responsible for that. Thus, libxl
only
>    needs to write to XenStore the file/partition/whatever backing the
virtual
>    disk so that the tapback daemon can pass it to tapdisk.
> 3. Since there is no block device in dom0 any more, pygrub won''t
be able to
>    boot from VHD files. To get around this problem, pygrub can use the NBD
>    functionality (existing in blktap2.5): it can explicitly ask tapback to
>    create a NDB in order to access the virtual disk. This functionality
will be
>    implemented in a future patch series. However, this will only work for
Linux
>    dom0''s. As a generic solution that would work on any dom0, we
could
>    implement a simple protocol for data exchange between pygrub and
tapdisk.
> 
> Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
> 
> ---
> Changed since v1:
>   * Check whether tapback''s control socket exists instead of
checking whether
>     a process named "tapback" exists.
>   * Don''t duplicate code for writing the type:/path/to/file to
XenStore.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

> > Supporting blktap3 requires rather few changes:
> > 1. We introduce a new disk back-end type (TAP3) and a new device kind
> (VBD3) to
> >    allow blktap3 to co-exist with blktap3.
> 
> "allow blktap3 to co-exist with blktap3" ?
> 
> I guess the the other one should be blktap2 ..
Yes that was a typo.

Hi,
> This patch series implements support for blktap3 in libxl.
Do you have a public git tree somewhere with all of these ?

> 3. Since there is no block device in dom0 any more, pygrub won''t
be able to
>    boot from VHD files. To get around this problem, pygrub can use the NBD
>    functionality (existing in blktap2.5): it can explicitly ask tapback to
>    create a NDB in order to access the virtual disk. This functionality
will be
>    implemented in a future patch series. However, this will only work for
Linux
>    dom0''s. As a generic solution that would work on any dom0, we
could
>    implement a simple protocol for data exchange between pygrub and
tapdisk.
Isn''t it possible to just "attach" a blkfront in the dom0 if
you want
a block device ?


Cheers,

    Sylvain

> > This patch series implements support for blktap3 in libxl.
> 
> Do you have a public git tree somewhere with all of these ?
I can''t really keep it in a tree because of the upstream process; I
keep all the patches on bitbucket:
https://bitbucket.org/tmakatos/blktap3-patches.
> Isn''t it possible to just "attach" a blkfront in the
dom0 if you want a
> block device ?
Not really because tapdisk would have to support multiple frontend/backend pairs
per VDI. Supporting multiple frontend/backend pairs per VDI is something
desirable, so when it''s implemented we can switch to this mechanism
instead of the NBD one.

> > 3. Since there is no block device in dom0 any more, pygrub
won''t be
> able to
> >    boot from VHD files. To get around this problem, pygrub can use
> the NBD
> >    functionality (existing in blktap2.5): it can explicitly ask
> tapback to
> >    create a NDB in order to access the virtual disk. This
> functionality will be
> >    implemented in a future patch series. However, this will only work
> for Linux
> >    dom0''s. As a generic solution that would work on any dom0,
we
> could
> >    implement a simple protocol for data exchange between pygrub and
> tapdisk.
> 
> Isn''t it possible to just "attach" a blkfront in the
dom0 if you want a
> block device ?
> 
Actually I was wrong about this, since libxl explicitly detaches the block
device once done with pygrub. There was another reason using blkfront in dom0
wouldn''t work but I''ve been told that it got fixed now. So,
using blkfront with blktap3 in dom0 could work.