[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |