All of lore.kernel.org
 help / color / mirror / Atom feed
* [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak.
@ 2012-08-05 14:48 Dr. Greg Wettstein
  2012-08-07 15:14 ` Ian Jackson
  0 siblings, 1 reply; 5+ messages in thread
From: Dr. Greg Wettstein @ 2012-08-05 14:48 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, keir

libxl: Seal tapdisk minor leak.

This patch needs to be applied on top of the following patch
to establish correct cleanup of a blktap2 based virtual disk:

libxl: attempt to cleanup tapdisk processes on disk backend destroy.

To implement correct cleanup of blktap devices in Xen 4.1.2.

This patch implements the release of the backend device before calling
for the destruction of the userspace component of the tapdisk device.

Without this patch the kernel xen-blkback driver deadlocks with
the tapdisk user control plane until the IPC channel is terminated by the
timeout on the select() call.  This results in a noticeable delay
in the termination of the guest and causes the blktap minor
number which had been allocated for the device to be orphaned.

Signed-off-by: Greg Wettstein <greg@enjellic.com>

diff -r b2b7a7a49af5 tools/libxl/libxl_blktap2.c
--- a/tools/libxl/libxl_blktap2.c	Sat Aug 04 16:17:08 2012 -0500
+++ b/tools/libxl/libxl_blktap2.c	Sun Aug 05 09:22:35 2012 -0500
@@ -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;
@@ -75,5 +76,11 @@ 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 -r b2b7a7a49af5 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c	Sat Aug 04 16:17:08 2012 -0500
+++ b/tools/libxl/libxl_device.c	Sun Aug 05 09:22:35 2012 -0500
@@ -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;
     }
 
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@enjellic.com
------------------------------------------------------------------------------
"More people are killed every year by pigs than by sharks, which shows
 you how good we are at evaluating risk."
                                -- Bruce Schneier
                                   Beyond Fear

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak.
  2012-08-05 14:48 [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak Dr. Greg Wettstein
@ 2012-08-07 15:14 ` Ian Jackson
  2012-08-07 15:34   ` Ian Campbell
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Jackson @ 2012-08-07 15:14 UTC (permalink / raw)
  To: greg; +Cc: Keir (Xen.org), xen-devel

Dr. Greg Wettstein writes ("[Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak."):
> libxl: Seal tapdisk minor leak.
...
> To implement correct cleanup of blktap devices in Xen 4.1.2.

Is this patch is supposed to be against xen-4.1-testing.hg ?

> diff -r b2b7a7a49af5 tools/libxl/libxl_blktap2.c
> --- a/tools/libxl/libxl_blktap2.c	Sat Aug 04 16:17:08 2012 -0500
> +++ b/tools/libxl/libxl_blktap2.c	Sun Aug 05 09:22:35 2012 -0500
> @@ -59,6 +59,7 @@ void libxl__device_destroy_tapdisk(libxl

This function doesn't seem to exist in my copy of xen-4.1-testing.hg
tip nor in 23172:3eca5bf65e6c.

Ian.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak.
  2012-08-07 15:14 ` Ian Jackson
@ 2012-08-07 15:34   ` Ian Campbell
  2012-08-07 15:45     ` Ian Jackson
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2012-08-07 15:34 UTC (permalink / raw)
  To: Ian Jackson; +Cc: greg, Keir (Xen.org), xen-devel

On Tue, 2012-08-07 at 16:14 +0100, Ian Jackson wrote:
> Dr. Greg Wettstein writes ("[Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak."):
> > libxl: Seal tapdisk minor leak.
> ...
> > To implement correct cleanup of blktap devices in Xen 4.1.2.
> 
> Is this patch is supposed to be against xen-4.1-testing.hg ?
> 
> > diff -r b2b7a7a49af5 tools/libxl/libxl_blktap2.c
> > --- a/tools/libxl/libxl_blktap2.c	Sat Aug 04 16:17:08 2012 -0500
> > +++ b/tools/libxl/libxl_blktap2.c	Sun Aug 05 09:22:35 2012 -0500
> > @@ -59,6 +59,7 @@ void libxl__device_destroy_tapdisk(libxl
> 
> This function doesn't seem to exist in my copy of xen-4.1-testing.hg
> tip nor in 23172:3eca5bf65e6c.

It comes from <201208051440.q75Een7F008501@wind.enjellic.com> which is a
backport request that introduces this function.

Ian.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak.
  2012-08-07 15:34   ` Ian Campbell
@ 2012-08-07 15:45     ` Ian Jackson
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Jackson @ 2012-08-07 15:45 UTC (permalink / raw)
  To: Ian Campbell; +Cc: greg, Keir (Xen.org), xen-devel

Ian Campbell writes ("Re: [Xen-devel] [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak."):
> On Tue, 2012-08-07 at 16:14 +0100, Ian Jackson wrote:
> > Dr. Greg Wettstein writes ("[Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak."):
> > > libxl: Seal tapdisk minor leak.
> > ...
> > > To implement correct cleanup of blktap devices in Xen 4.1.2.
> > 
> > Is this patch is supposed to be against xen-4.1-testing.hg ?
> > 
> > > diff -r b2b7a7a49af5 tools/libxl/libxl_blktap2.c
> > > --- a/tools/libxl/libxl_blktap2.c	Sat Aug 04 16:17:08 2012 -0500
> > > +++ b/tools/libxl/libxl_blktap2.c	Sun Aug 05 09:22:35 2012 -0500
> > > @@ -59,6 +59,7 @@ void libxl__device_destroy_tapdisk(libxl
> > 
> > This function doesn't seem to exist in my copy of xen-4.1-testing.hg
> > tip nor in 23172:3eca5bf65e6c.
> 
> It comes from <201208051440.q75Een7F008501@wind.enjellic.com> which is a
> backport request that introduces this function.

Oh I see, these are two interdependent patches.  Let me read them
again.

Ian.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak.
@ 2012-08-08  1:16 Dr. Greg Wettstein
  0 siblings, 0 replies; 5+ messages in thread
From: Dr. Greg Wettstein @ 2012-08-08  1:16 UTC (permalink / raw)
  To: Ian Jackson, Ian Campbell; +Cc: greg, Keir (Xen.org), xen-devel

On Aug 7,  4:45pm, Ian Jackson wrote:
} Subject: Re: [Xen-devel] [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk mi

Hi, hope the day is going well for everyone.

> Ian Campbell writes ("Re: [Xen-devel] [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak."):
> > On Tue, 2012-08-07 at 16:14 +0100, Ian Jackson wrote:
> > > Dr. Greg Wettstein writes ("[Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak."):
> > > > libxl: Seal tapdisk minor leak.
> > > ...
> > > > To implement correct cleanup of blktap devices in Xen 4.1.2.
> > > 
> > > Is this patch is supposed to be against xen-4.1-testing.hg ?
> > > 
> > > > diff -r b2b7a7a49af5 tools/libxl/libxl_blktap2.c
> > > > --- a/tools/libxl/libxl_blktap2.c	Sat Aug 04 16:17:08 2012 -0500
> > > > +++ b/tools/libxl/libxl_blktap2.c	Sun Aug 05 09:22:35 2012 -0500
> > > > @@ -59,6 +59,7 @@ void libxl__device_destroy_tapdisk(libxl
> > > 
> > > This function doesn't seem to exist in my copy of xen-4.1-testing.hg
> > > tip nor in 23172:3eca5bf65e6c.
> > 
> > It comes from <201208051440.q75Een7F008501@wind.enjellic.com> which is a
> > backport request that introduces this function.
> 
> Oh I see, these are two interdependent patches.  Let me read them
> again.

Sorry for the confusion, I should have explicitly called out the
inter-dependency.

The first patch is a backport of one which Ian did for xen-unstable,
corrected primarily, for the change in calling convention for the
context arguement to libxl__device_destroy_tapdisk().

By itself this patch corrects the problem with the tapdisk2 userspace
process not being kill when the guest exists.

The second builds on top of that patch and fixes an ordering problem
which causes a deadlock between xen-blkback and blktap2 when the user
space process attempts to unmap its ring buffer.  The major effect is
orphaning of the tapdisk minor.

The more subtle effect is the deadlock which stalls shutdown of the
guest and actually livelocks the kernel for the duration of the
timeout on the select() call used for the IPC socket communications.

> Ian.

Have a good day.

}-- End of excerpt from Ian Jackson

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@enjellic.com
------------------------------------------------------------------------------
"Some of them are.  A surprising number aren't.  A personal favorite of
 mine was the log from a cracker who couldn't figure out how to untar
 and install the trojan package he'd ftped onto the machine.  He tried a
 few times, and then eventually gave up and logged out."
                                -- Nat Lanza

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-08-08  1:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-05 14:48 [Patch 1 of 1] [xen-4.1.2] libxl: Seal tapdisk minor leak Dr. Greg Wettstein
2012-08-07 15:14 ` Ian Jackson
2012-08-07 15:34   ` Ian Campbell
2012-08-07 15:45     ` Ian Jackson
2012-08-08  1:16 Dr. Greg Wettstein

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.