[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Blktap fixes and kernel patch.



Hi, hope the day is going well for everyone.  Just came in from a 90
mile cycle ride and wanted to get this out since 4.1.3 and/or 4.2 are
looming in the near future.

After becoming far more familiar with the XEN vbd infrastructure then
I had intended I was finally able to run down the problem with the
blktap2 driver which has plagued 4.1.2.  Most specifically the
orphaning of the tapdisk2 driver process and after Ian's fix for that,
the hang and orphaning of a tapdisk minor which occurs when libxl
attempts to shutdown the driver process.

In the process I updated the blktap2 kernel driver to patch cleanly
into the Linux 3.4 kernel.  These fixes have been validated against
the 3.4 kernel as well as the 3.2 kernel.

Locations for the files are as follows:

Kernel driver:
        ftp://ftp.enjellic.com/pub/xen/blktap2-3.4.patch

Xen fixes:
        ftp://ftp.enjellic.com/pub/xen/xen-4.1.2.blktap1.patch
        ftp://ftp.enjellic.com/pub/xen/xen-4.1.2.blktap2.patch

The two XEN patches overlay sequentially onto a virgin 4.1.2
distribution and both are required for correct behavior.

The first patch is one which was done by Ian for the development tree
with minor corrections for 4.1.2.  I'm including it for completeness
for those who want a trouble free patch set for a 4.1.2 distribution.
This patch fixes the orphaning of the tapdisk2 driver process when xl
shuts down.

The second patch corrects the deadlock which occurs between the
blktap2 kernel driver and the blktap2 userspace control plane.  The
deadlock causes a delay in the shutdown of a XEN guest and results in
the 'orphaning' of tapdisk minor number allocations.  As seems to be
typical with these types of things the fix was trivially straight
forward once I finally figured out what was going on.

Ian for your reference the following change which you introduced to
address this issue:

79e3dbe4b659e78408a9eea76c51a601bd4a383a
tapdisk: respond to destroy request before tearing down the commuication channel

Is not needed and does not provide formally correct behavior in the
presence of the two patches noted above.

I don't know where all this went in 4.2 since I heard mention that
someone was giving xl a good work over with respect to device
management.  I also don't know if a 4.1.3 is planned, if it is the
above patches go a long ways toward improving the user experience for
those sites which find blktap2 useful.

Now on to NPIV support since it was the reason I waded into the xl/vbd
in the first place.... :-)

Best wishes for a pleasant remainder of the week.

Greg

Cut here for patch 1: -----------------------------------------------------
diff -urNp v4.1.2/xen-4.1.2/tools/blktap2/control/tap-ctl-list.c 
xen-4.1.2/tools/blktap2/control/tap-ctl-list.c
--- v4.1.2/xen-4.1.2/tools/blktap2/control/tap-ctl-list.c       Thu Oct 20 
12:05:41 2011
+++ xen-4.1.2/tools/blktap2/control/tap-ctl-list.c      Thu Jul 26 09:43:10 2012
@@ -506,17 +506,15 @@ out:
 }
 
 int
-tap_ctl_find_minor(const char *type, const char *path)
+tap_ctl_find(const char *type, const char *path, tap_list_t *tap)
 {
        tap_list_t **list, **_entry;
-       int minor, err;
+       int ret = -ENOENT, err;
 
        err = tap_ctl_list(&list);
        if (err)
                return err;
 
-       minor = -1;
-
        for (_entry = list; *_entry != NULL; ++_entry) {
                tap_list_t *entry  = *_entry;
 
@@ -526,11 +524,13 @@ tap_ctl_find_minor(const char *type, con
                if (path && (!entry->path || strcmp(entry->path, path)))
                        continue;
 
-               minor = entry->minor;
+               *tap = *entry;
+               tap->type = tap->path = NULL;
+               ret = 0;
                break;
        }
 
        tap_ctl_free_list(list);
 
-       return minor >= 0 ? minor : -ENOENT;
+       return ret;
 }
diff -urNp v4.1.2/xen-4.1.2/tools/blktap2/control/tap-ctl.h 
xen-4.1.2/tools/blktap2/control/tap-ctl.h
--- v4.1.2/xen-4.1.2/tools/blktap2/control/tap-ctl.h    Thu Oct 20 12:05:41 2011
+++ xen-4.1.2/tools/blktap2/control/tap-ctl.h   Thu Jul 26 09:43:10 2012
@@ -76,7 +76,7 @@ int tap_ctl_get_driver_id(const char *ha
 
 int tap_ctl_list(tap_list_t ***list);
 void tap_ctl_free_list(tap_list_t **list);
-int tap_ctl_find_minor(const char *type, const char *path);
+int tap_ctl_find(const char *type, const char *path, tap_list_t *tap);
 
 int tap_ctl_allocate(int *minor, char **devname);
 int tap_ctl_free(const int minor);
diff -urNp v4.1.2/xen-4.1.2/tools/libxl/libxl_blktap2.c 
xen-4.1.2/tools/libxl/libxl_blktap2.c
--- v4.1.2/xen-4.1.2/tools/libxl/libxl_blktap2.c        Thu Oct 20 12:05:42 2011
+++ xen-4.1.2/tools/libxl/libxl_blktap2.c       Thu Jul 26 09:43:10 2012
@@ -18,6 +18,8 @@
 
 #include "tap-ctl.h"
 
+#include <string.h>
+
 int libxl__blktap_enabled(libxl__gc *gc)
 {
     const char *msg;
@@ -30,12 +32,13 @@ const char *libxl__blktap_devpath(libxl_
 {
     const char *type;
     char *params, *devname = NULL;
-    int minor, err;
+    tap_list_t tap;
+    int err;
 
     type = libxl__device_disk_string_of_format(format);
-    minor = tap_ctl_find_minor(type, disk);
-    if (minor >= 0) {
-        devname = libxl__sprintf(gc, "/dev/xen/blktap-2/tapdev%d", minor);
+    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;
     }
@@ -48,4 +51,29 @@ const char *libxl__blktap_devpath(libxl_
     }
 
     return NULL;
+}
+
+
+void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path)
+{
+    char *path, *params, *type, *disk;
+    int err;
+    tap_list_t tap;
+
+    path = libxl__sprintf(gc, "%s/tapdisk-params", be_path);
+    if (!path) return;
+
+    params = libxl__xs_read(gc, XBT_NULL, path);
+    if (!params) return;
+
+    type = params;
+    disk = strchr(params, ':');
+    if (!disk) return;
+
+    *disk++ = '\0';
+
+    err = tap_ctl_find(type, disk, &tap);
+    if (err < 0) return;
+
+    tap_ctl_destroy(tap.id, tap.minor);
 }
diff -urNp v4.1.2/xen-4.1.2/tools/libxl/libxl_device.c 
xen-4.1.2/tools/libxl/libxl_device.c
--- v4.1.2/xen-4.1.2/tools/libxl/libxl_device.c Thu Oct 20 12:05:42 2011
+++ xen-4.1.2/tools/libxl/libxl_device.c        Thu Jul 26 09:48:03 2012
@@ -250,6 +250,7 @@ int libxl__device_destroy(libxl_ctx *ctx
     if (!state)
         goto out;
     if (atoi(state) != 4) {
+        libxl__device_destroy_tapdisk(&gc, be_path);
         xs_rm(ctx->xsh, XBT_NULL, be_path);
         goto out;
     }
@@ -368,6 +369,7 @@ int libxl__devices_destroy(libxl_ctx *ct
             }
         }
     }
+    libxl__device_destroy_tapdisk(&gc, be_path);
 out:
     libxl__free_all(&gc);
     return 0;
diff -urNp v4.1.2/xen-4.1.2/tools/libxl/libxl_internal.h 
xen-4.1.2/tools/libxl/libxl_internal.h
--- v4.1.2/xen-4.1.2/tools/libxl/libxl_internal.h       Thu Oct 20 12:05:43 2011
+++ xen-4.1.2/tools/libxl/libxl_internal.h      Thu Jul 26 09:43:10 2012
@@ -314,6 +314,12 @@ _hidden const char *libxl__blktap_devpat
                                  const char *disk,
                                  libxl_disk_format format);
 
+/* libxl__device_destroy_tapdisk:
+ *   Destroys any tapdisk process associated with the backend represented
+ *   by be_path.
+ */
+_hidden void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path);
+
 _hidden char *libxl__uuid2string(libxl__gc *gc, const libxl_uuid uuid);
 
 struct libxl__xen_console_reader {
diff -urNp v4.1.2/xen-4.1.2/tools/libxl/libxl_noblktap2.c 
xen-4.1.2/tools/libxl/libxl_noblktap2.c
--- v4.1.2/xen-4.1.2/tools/libxl/libxl_noblktap2.c      Thu Oct 20 12:05:43 2011
+++ xen-4.1.2/tools/libxl/libxl_noblktap2.c     Thu Jul 26 09:43:10 2012
@@ -27,3 +27,7 @@ const char *libxl__blktap_devpath(libxl_
 {
     return NULL;
 }
+
+void libxl__device_destroy_tapdisk(libxl__gc *gc, char *be_path)
+{
+}
And here for patch1: ------------------------------------------------------


Cut here for patch 2: -----------------------------------------------------
diff -urNp v4.1.2/xen-4.1.2/tools/libxl/libxl_blktap2.c 
xen-4.1.2/tools/libxl/libxl_blktap2.c
--- v4.1.2/xen-4.1.2/tools/libxl/libxl_blktap2.c        Thu Jul 26 09:43:10 2012
+++ xen-4.1.2/tools/libxl/libxl_blktap2.c       Thu Jul 26 10:04:47 2012
@@ -59,6 +59,7 @@ void libxl__device_destroy_tapdisk(libxl
     char *path, *params, *type, *disk;
     int err;
     tap_list_t tap;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
 
     path = libxl__sprintf(gc, "%s/tapdisk-params", be_path);
     if (!path) return;
@@ -74,6 +75,12 @@ void libxl__device_destroy_tapdisk(libxl
 
     err = tap_ctl_find(type, disk, &tap);
     if (err < 0) return;
+
+    /*
+     * Remove the instance of the backend device to avoid a deadlock with the
+     * removal of the tap device.
+     */
+    xs_rm(ctx->xsh, XBT_NULL, be_path);
 
     tap_ctl_destroy(tap.id, tap.minor);
 }
diff -urNp v4.1.2/xen-4.1.2/tools/libxl/libxl_device.c 
xen-4.1.2/tools/libxl/libxl_device.c
--- v4.1.2/xen-4.1.2/tools/libxl/libxl_device.c Thu Jul 26 09:48:03 2012
+++ xen-4.1.2/tools/libxl/libxl_device.c        Thu Jul 26 10:06:20 2012
@@ -250,8 +250,7 @@ int libxl__device_destroy(libxl_ctx *ctx
     if (!state)
         goto out;
     if (atoi(state) != 4) {
-        libxl__device_destroy_tapdisk(&gc, be_path);
-        xs_rm(ctx->xsh, XBT_NULL, be_path);
+       libxl__device_destroy_tapdisk(&gc, be_path);
         goto out;
     }
 
And here for patch 2: -----------------------------------------------------

As always,
Dr. G.W. Wettstein, Ph.D.   Enjellic Systems Development, LLC.
4206 N. 19th Ave.           Specializing in information infra-structure
Fargo, ND  58102            development.
PH: 701-281-1686
FAX: 701-281-3949           EMAIL: greg@xxxxxxxxxxxx
------------------------------------------------------------------------------
"A distributed system is one on which I cannot get any work done because 
 a machine I have never heard of has crashed."
                                -- Leslie Lamport

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.