xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: George Dunlap <george.dunlap@eu.citrix.com>
To: xen-devel@lists.xen.org
Cc: Wei Liu <wei.liu2@citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Wen Congyang <wency@cn.fujitsu.com>,
	George Dunlap <george.dunlap@eu.citrix.com>,
	Felipe Franciosi <felipe.franciosi@citrix.com>,
	Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>,
	Ian Jackson <ian.jackson@citrix.com>,
	Yang Hongyang <yanghy@cn.fujitsu.com>,
	Dave Scott <Dave.Scott@eu.citrix.com>
Subject: [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP
Date: Mon, 6 Jul 2015 11:51:41 +0100	[thread overview]
Message-ID: <1436179903-449-5-git-send-email-george.dunlap@eu.citrix.com> (raw)
In-Reply-To: <1436179903-449-1-git-send-email-george.dunlap@eu.citrix.com>

The block-tap script can now do everything needed for libxl; no need
to link against the blktap library.

To do this:

* Set disk->script to "block-tap" and dev to "format:pdev_path" in
  device_disk_add for LIBXL_DISK_BACKEND_TAP

* Remove libxl_blktap2.o and libxl_noblktap2.o and all code depending
  on them

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Dave Scott <Dave.Scott@eu.citrix.com>
CC: Jonathan Ludlam <Jonathan.Ludlam@eu.citrix.com>
CC: Wen Congyang <wency@cn.fujitsu.com>
CC: Yang Hongyang <yanghy@cn.fujitsu.com>
CC: Felipe Franciosi <felipe.franciosi@citrix.com>

Note: This is broken for HVM domains at the moment because all block
scripts are broken for HVM domains.  That is a bug which should be a
blocker for the 4.6 release.  As such, I think there is justification
for checking in this "feature" (enabling use of a 'system' blktap)
with the assumption that the FIXME will go away before the release.

That will technically break bisectability; but only for users of
blktap (which does not include osstest).
---
 tools/libxl/Makefile          |  5 ---
 tools/libxl/libxl.c           | 28 ++++---------
 tools/libxl/libxl_blktap2.c   | 93 -------------------------------------------
 tools/libxl/libxl_device.c    | 14 -------
 tools/libxl/libxl_dm.c        |  5 ++-
 tools/libxl/libxl_internal.h  | 26 ------------
 tools/libxl/libxl_noblktap2.c | 42 -------------------
 7 files changed, 11 insertions(+), 202 deletions(-)

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index cc9c152..43ab243 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -44,11 +44,6 @@ LIBXL_LIBS += $(LIBXL_LIBS-y)
 LIBXLU_LIBS = libxenlight.so
 
 LIBXL_OBJS-y = osdeps.o libxl_paths.o libxl_bootloader.o flexarray.o
-ifeq ($(LIBXL_BLKTAP),y)
-LIBXL_OBJS-y += libxl_blktap2.o
-else
-LIBXL_OBJS-y += libxl_noblktap2.o
-endif
 
 ifeq ($(CONFIG_REMUS_NETBUF),y)
 LIBXL_OBJS-y += libxl_netbuffer.o
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3a83903..2aab0c2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2469,13 +2469,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         switch (disk->backend) {
             case LIBXL_DISK_BACKEND_PHY:
                 dev = disk->pdev_path;
-
-        do_backend_phy:
-                flexarray_append(back, "params");
-                flexarray_append(back, dev);
-
                 script = libxl__abs_path(gc, disk->script?: "block",
                                          libxl__xen_script_dir_path());
+        do_backend_phy:
+                flexarray_append_pair(back, "params", dev);
                 flexarray_append_pair(back, "script", script);
 
                 /* If the user did not supply a block script then we
@@ -2497,25 +2494,16 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
                 break;
 
             case LIBXL_DISK_BACKEND_TAP:
-                if (dev == NULL) {
-                    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;
-                    }
-                }
-                flexarray_append(back, "tapdisk-params");
-                flexarray_append(back, libxl__sprintf(gc, "%s:%s",
-                    libxl__device_disk_string_of_format(disk->format),
-                    disk->pdev_path));
-
                 /* tap backends with scripts are rejected by
                  * libxl__device_disk_set_backend */
                 assert(!disk->script);
 
+                script = libxl__abs_path(gc, "block-tap",
+                                         libxl__xen_script_dir_path());
+                dev = 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:
diff --git a/tools/libxl/libxl_blktap2.c b/tools/libxl/libxl_blktap2.c
deleted file mode 100644
index 2053403..0000000
--- a/tools/libxl/libxl_blktap2.c
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * Copyright (C) 2010      Advanced Micro Devices
- * Author Christoph Egger <Christoph.Egger@amd.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published
- * by the Free Software Foundation; version 2.1 only.
- *
- * 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 Lesser General Public License for more details.
- */
-
-#include "libxl_osdeps.h" /* must come before any other headers */
-#include "libxl_internal.h"
-
-#include "tap-ctl.h"
-
-int libxl__blktap_enabled(libxl__gc *gc)
-{
-    const char *msg;
-    return !tap_ctl_check(&msg);
-}
-
-char *libxl__blktap_devpath(libxl__gc *gc,
-                            const char *disk,
-                            libxl_disk_format format)
-{
-    const char *type;
-    char *params, *devname = NULL;
-    tap_list_t tap;
-    int err;
-
-    type = libxl__device_disk_string_of_format(format);
-    err = tap_ctl_find(type, disk, &tap);
-    if (err == 0) {
-        devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", tap.minor);
-        if (devname)
-            return devname;
-    }
-
-    params = libxl__sprintf(gc, "%s:%s", type, disk);
-    err = tap_ctl_create(params, &devname);
-    if (!err) {
-        libxl__ptr_add(gc, devname);
-        return devname;
-    }
-
-    return NULL;
-}
-
-
-int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params)
-{
-    char *type, *disk;
-    int err;
-    tap_list_t tap;
-
-    type = libxl__strdup(gc, params);
-
-    disk = strchr(type, ':');
-    if (!disk) {
-        LOG(ERROR, "Unable to parse params %s", params);
-        return ERROR_INVAL;
-    }
-
-    *disk++ = '\0';
-
-    err = tap_ctl_find(type, disk, &tap);
-    if (err < 0) {
-        /* returns -errno */
-        LOGEV(ERROR, -err, "Unable to find type %s disk %s", type, disk);
-        return ERROR_FAIL;
-    }
-
-    err = tap_ctl_destroy(tap.id, tap.minor);
-    if (err < 0) {
-        LOGEV(ERROR, -err, "Failed to destroy tap device id %d minor %d",
-              tap.id, tap.minor);
-        return ERROR_FAIL;
-    }
-
-    return 0;
-}
-
-/*
- * Local variables:
- * mode: C
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 8209629..c1ca07b 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -223,11 +223,6 @@ static int disk_try_backend(disk_try_backend_args *a,
                        a->disk->vdev);
             return 0;
         }
-        if (!libxl__blktap_enabled(a->gc)) {
-            LOG(DEBUG, "Disk vdev=%s, backend tap unsuitable because blktap "
-                       "not available", a->disk->vdev);
-            return 0;
-        }
         if (!(a->disk->format == LIBXL_DISK_FORMAT_RAW ||
               a->disk->format == LIBXL_DISK_FORMAT_VHD)) {
             goto bad_format;
@@ -572,8 +567,6 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
 {
     const char *be_path = libxl__device_backend_path(gc, dev);
     const char *fe_path = libxl__device_frontend_path(gc, dev);
-    const char *tapdisk_path = GCSPRINTF("%s/%s", be_path, "tapdisk-params");
-    const char *tapdisk_params;
     xs_transaction_t t = 0;
     int rc;
     uint32_t domid;
@@ -585,10 +578,6 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
         rc = libxl__xs_transaction_start(gc, &t);
         if (rc) goto out;
 
-        /* May not exist if this is not a tap device */
-        rc = libxl__xs_read_checked(gc, t, tapdisk_path, &tapdisk_params);
-        if (rc) goto out;
-
         if (domid == LIBXL_TOOLSTACK_DOMID) {
             /*
              * The toolstack domain is in charge for removing both the
@@ -609,9 +598,6 @@ int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
         if (rc < 0) goto out;
     }
 
-    if (tapdisk_params)
-        rc = libxl__device_destroy_tapdisk(gc, tapdisk_params);
-
 out:
     libxl__xs_transaction_abort(gc, &t);
     return rc;
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 317a8eb..9ac6029 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -856,8 +856,9 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
 
                 if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
                     format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
-                    pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
-                                                      disks[i].format);
+                    /* FIXME This won't work at the moment, because
+                     * all block scripts are broken for qemu */
+                    pdev_path = disks[i].pdev_path;
                 } else {
                     pdev_path = disks[i].pdev_path;
                 }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 7129aee..0d15397 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1615,32 +1615,6 @@ struct libxl__cpuid_policy {
     char *policy[4];
 };
 
-/*
- * blktap2 support
- */
-
-/* libxl__blktap_enabled:
- *    return true if blktap/blktap2 support is available.
- */
-_hidden int libxl__blktap_enabled(libxl__gc *gc);
-
-/* libxl__blktap_devpath:
- *    Argument: path and disk image as specified in config file.
- *      The type specifies whether this is aio, qcow, qcow2, etc.
- *    returns device path xenstore wants to have. returns NULL
- *      if no device corresponds to the disk.
- */
-_hidden char *libxl__blktap_devpath(libxl__gc *gc,
-                                    const char *disk,
-                                    libxl_disk_format format);
-
-/* libxl__device_destroy_tapdisk:
- *   Destroys any tapdisk process associated with the backend represented
- *   by be_path.
- *   Always logs on failure.
- */
-_hidden int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params);
-
 _hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
                                    libxl_device_disk *disk,
                                    libxl__device *device);
diff --git a/tools/libxl/libxl_noblktap2.c b/tools/libxl/libxl_noblktap2.c
deleted file mode 100644
index 5a86ed1..0000000
--- a/tools/libxl/libxl_noblktap2.c
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * Copyright (C) 2010      Advanced Micro Devices
- * Author Christoph Egger <Christoph.Egger@amd.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published
- * by the Free Software Foundation; version 2.1 only.
- *
- * 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 Lesser General Public License for more details.
- */
-
-#include "libxl_osdeps.h" /* must come before any other headers */
-
-#include "libxl_internal.h"
-
-int libxl__blktap_enabled(libxl__gc *gc)
-{
-    return 0;
-}
-
-char *libxl__blktap_devpath(libxl__gc *gc,
-                            const char *disk,
-                            libxl_disk_format format)
-{
-    return NULL;
-}
-
-int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params)
-{
-    return 0;
-}
-
-/*
- * Local variables:
- * mode: C
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
-- 
1.9.1

  parent reply	other threads:[~2015-07-06 10:51 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-06 10:51 [PATCH 0/6] Use system blktap George Dunlap
2015-07-06 10:51 ` [PATCH 1/6] libxl: Make local_initiate_attach more rational George Dunlap
2015-07-06 10:51 ` [PATCH 2/6] libxl: Remove linux udev rules George Dunlap
2015-07-07 11:39   ` Wei Liu
2015-07-14 16:13     ` Konrad Rzeszutek Wilk
2015-07-14 16:21       ` Ian Campbell
2015-07-14 16:35         ` George Dunlap
2015-07-14 16:40           ` Wei Liu
2015-07-14 16:48             ` Ian Campbell
2015-07-23 11:55               ` Roger Pau Monné
2015-07-23 12:30                 ` Ian Campbell
2015-07-15 11:07         ` [PATCH for-4.6] tools/hotplug: Add an initscript to start "xl devd" in a driver domain Ian Campbell
2015-07-15 13:26           ` Wei Liu
2015-07-15 13:40             ` Ian Campbell
2015-07-15 15:25           ` George Dunlap
2015-07-15 15:32           ` Ian Jackson
2015-07-15 15:35             ` George Dunlap
2015-07-15 15:37               ` Ian Campbell
2015-07-16 16:58         ` [PATCH for-4.6 v2] " Ian Campbell
2015-07-16 17:09           ` Ian Jackson
2015-07-16 17:48           ` Wei Liu
2015-07-17  9:05           ` Wei Liu
2015-07-17 11:36             ` Ian Campbell
2015-07-20 14:16           ` Roger Pau Monné
2015-07-20 14:28             ` Ian Campbell
2015-07-06 10:51 ` [PATCH 3/6] tools: Add a block-tap script for setting up tapdisks via tap-ctl George Dunlap
2015-07-07 12:03   ` Wei Liu
2015-07-07 12:35     ` Wei Liu
2015-07-06 10:51 ` George Dunlap [this message]
2015-07-06 11:01   ` [PATCH 4/6] libxl: Use the block-tap script for LIBXL_DISK_BACKEND_TAP Andrew Cooper
2015-07-06 12:39     ` George Dunlap
2015-07-07 12:29   ` Wei Liu
2015-07-07 13:41     ` George Dunlap
2015-07-07 14:20       ` Ian Campbell
2015-07-07 14:27         ` George Dunlap
2015-07-06 10:51 ` [PATCH 5/6] tools: Remove in-tree blktap2 George Dunlap
2015-07-07 11:55   ` Wei Liu
2015-07-06 10:51 ` [PATCH 6/6] libxl: Add more logging to hotplug script path George Dunlap
2015-07-07 11:55   ` Wei Liu
2015-07-07 14:21 ` [PATCH 0/6] Use system blktap Ian Campbell
2015-07-07 14:24   ` George Dunlap
2015-07-07 14:52     ` Ian Campbell
2015-07-07 14:59       ` George Dunlap
2015-07-07 15:04         ` Ian Campbell
2015-07-07 15:20 ` Ian Campbell

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=1436179903-449-5-git-send-email-george.dunlap@eu.citrix.com \
    --to=george.dunlap@eu.citrix.com \
    --cc=Dave.Scott@eu.citrix.com \
    --cc=Jonathan.Ludlam@eu.citrix.com \
    --cc=felipe.franciosi@citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=wency@cn.fujitsu.com \
    --cc=xen-devel@lists.xen.org \
    --cc=yanghy@cn.fujitsu.com \
    /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).