Re: [Xen-devel] Basic blktap2 functionality issues.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-03-30 at 09:17 +0100, Ian Campbell wrote:
> I think an approach worth trying would be to have
> tapdisk_control_detach_vbd respond to TAPDISK_MESSAGE_DETACH before
> doing the actual detach. i.e. it would respond with "Yes, I will do
> that" rather than "Yes, I have done that". My speculation is that this
> will allow libxl to continue and hopefully avoid the deadlock.

This seems to be the case as the following fixes things for me. Thanks
very much for your analysis which lead me to this solution...

Ian.

8<--------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1333106445 -3600
# Node ID 79e3dbe4b659e78408a9eea76c51a601bd4a383a
# Parent  947e095a6d29b5e90a01b7c06b298d4c91dcf3ac
tapdisk: respond to destroy request before tearing down the commuication channel

This avoids a deadlock while sending the response which leads to a 5s timeout
and leaked tapdisk processes. Instead send the response after getting the VBD
which appears to be the only step wich can fail (or at least the only one which
returns an error code).

Also call libxl__device_destroy_tapdisk before removing the backend directory
in xenstore since we need the tapdisk-params value.

Reported-by: Greg Wettstein <greg@xxxxxxxxxxxx>
Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r 947e095a6d29 -r 79e3dbe4b659 tools/blktap2/drivers/tapdisk-control.c
--- a/tools/blktap2/drivers/tapdisk-control.c   Tue Mar 27 14:09:11 2012 +0100
+++ b/tools/blktap2/drivers/tapdisk-control.c   Fri Mar 30 12:20:45 2012 +0100
@@ -379,10 +379,18 @@ tapdisk_control_detach_vbd(struct tapdis
        int err;
 
        vbd = tapdisk_server_get_vbd(request->cookie);
-       if (!vbd) {
-               err = -EINVAL;
-               goto out;
-       }
+       err = !vbd ? -EINVAL : 0;
+
+       memset(&response, 0, sizeof(response));
+       response.type = TAPDISK_MESSAGE_DETACH_RSP;
+       response.cookie = request->cookie;
+       response.u.response.error = -err;
+
+       tapdisk_control_write_message(connection->socket, &response, 2);
+       tapdisk_control_close_connection(connection);
+
+       if (err)
+               return;
 
        tapdisk_vbd_detach(vbd);
 
@@ -390,16 +398,6 @@ tapdisk_control_detach_vbd(struct tapdis
                tapdisk_server_remove_vbd(vbd);
                free(vbd);
        }
-
-       err = 0;
-out:
-       memset(&response, 0, sizeof(response));
-       response.type = TAPDISK_MESSAGE_DETACH_RSP;
-       response.cookie = request->cookie;
-       response.u.response.error = -err;
-
-       tapdisk_control_write_message(connection->socket, &response, 2);
-       tapdisk_control_close_connection(connection);
 }
 
 static void
diff -r 947e095a6d29 -r 79e3dbe4b659 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Tue Mar 27 14:09:11 2012 +0100
+++ b/tools/libxl/libxl_device.c        Fri Mar 30 12:20:45 2012 +0100
@@ -432,11 +432,11 @@ int libxl__device_destroy(libxl__gc *gc,
     char *be_path = libxl__device_backend_path(gc, dev);
     char *fe_path = libxl__device_frontend_path(gc, dev);
 
+    libxl__device_destroy_tapdisk(gc, be_path);
+
     xs_rm(ctx->xsh, XBT_NULL, be_path);
     xs_rm(ctx->xsh, XBT_NULL, fe_path);
 
-    libxl__device_destroy_tapdisk(gc, be_path);
-
     return 0;
 }
 



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