Re: libxl device_disk_add orphans blktap devices on transaction error

["Roger Pau Monné" <roger.pau@citrix.com>]

On 02/03/13 23:23, Darren Shepherd wrote:
> I'm using the CentOS 6 bundle of xen from
> http://dev.centos.org/centos/6/xen-c6/ and ran into an issue when
> creating domains with multiple VHD tap disks.  Comparing unstable to
> the 4.2.1 code I'm using, it seems this issue still applies.  I'm
> using a configuration line that looks something like
> 
> disk = [ "tap:vhd:/var/norm/pools/agentTest/d894b704-b890-488d-b66e-1422b2b9a7a5.vhd,xvda,w",
> "tap:vhd:/var/norm/pools/agentTest/70bd3927-0e27-4830-9bf0-5b66ba290547.vhd,xvdb,w"
> ]
> 
> What I noticed happening in the code is that in device_disk_add the
> call to libxl__xs_transaction_commit returns rc=1 so the "for (;;)"
> loop just tries again.  The problem though is that the blktap device
> was already created and assigned to a tapdisk process.  So it will
> just leave that process hanging out there and allocated a new device
> on the second pass through the for loop.  The second run through
> succeeds.  What I typically see is that if I have more than 1 disk on
> the domain, the first disk gets created fine and then each subsequent
> disk gets two tapdisk processes.

Hello Darren

Right now I don't have a system with tapdisk, but I think the following 
patch should fix the problem, could you please try it and report back?

Thanks, Roger.

---
>From e142569922ab810d960e05d26440fa1284e759e7 Mon Sep 17 00:00:00 2001
From: Roger Pau Monne <roger.pau@citrix.com>
Date: Mon, 4 Mar 2013 11:36:41 +0100
Subject: [PATCH] libxl: remove tapdisk process if adding disk fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Destroy tapdisk process if transaction fails, and also destroy tapdisk
process if disk adding fails.

Reported-by: Darren Shepherd <darren.s.shepherd@gmail.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/libxl/libxl.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 73e0dc3..30f2b04 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2160,6 +2160,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
         rc = libxl__xs_transaction_commit(gc, &t);
         if (!rc) break;
         if (rc < 0) goto out;
+        if (disk->backend == LIBXL_DISK_BACKEND_TAP)
+            libxl__device_destroy_tapdisk(gc, GCSPRINTF("%s:%s",
+                    libxl__device_disk_string_of_format(disk->format),
+                    disk->pdev_path));
     }
 
     aodev->dev = device;
@@ -2171,6 +2175,10 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
 out:
     libxl__xs_transaction_abort(gc, &t);
     aodev->rc = rc;
+    if (disk->backend == LIBXL_DISK_BACKEND_TAP)
+        libxl__device_destroy_tapdisk(gc, GCSPRINTF("%s:%s",
+                    libxl__device_disk_string_of_format(disk->format),
+                    disk->pdev_path));
     if (rc) aodev->callback(egc, aodev);
     return;
 }
-- 
1.7.7.5 (Apple Git-26)