Re: [Xen-devel] [BUG] xl devd segmentation fault on xl block-detach

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, May 03, 2017 at 03:31:58PM +0100, Wei Liu wrote:
> On Wed, May 03, 2017 at 04:27:41PM +0300, Reinis Martinsons wrote:
> > On 03.05.2017 13:27, Wei Liu wrote:
> > libxl: debug: libxl_linux.c:200:libxl__get_hotplug_script_info: num_exec 1, 
> > not running hotplug scripts
> > libxl: debug: libxl_device.c:1143:device_hotplug: No hotplug script to 
> > execute
> > libxl: debug: libxl_event.c:686:libxl__ev_xswatch_deregister: watch 
> > w=0x206e9a0: deregister unregistered
> > libxl: debug: libxl.c:3721:device_complete: device 
> > /local/domain/1/backend/vbd/0/51712 remove succeed
> > libxl: debug: libxl_event.c:1838:libxl__ao__destroy: ao 0x2070b80: destroy
> > libxl: debug: libxl_event.c:573:watchfd_callback: watch w=0x7fff97ba73a8 
> > wpath=/local/domain/1/backend token=3/0: event 
> > epath=/local/domain/1/backend/vbd/0/51712
> > libxl: debug: libxl_event.c:2223:libxl__nested_ao_create: ao 0x2070b80: 
> > nested ao, parent 0x206e350
> > libxl: debug: libxl_event.c:1838:libxl__ao__destroy: ao 0x2070b80: destroy
> > libxl: debug: libxl_event.c:573:watchfd_callback: watch w=0x7fff97ba73a8 
> > wpath=/local/domain/1/backend token=3/0: event 
> > epath=/local/domain/1/backend/vbd/0/51728/online
> > libxl: debug: libxl_event.c:2223:libxl__nested_ao_create: ao 0x2070b80: 
> > nested ao, parent 0x206e350
> 
> Interesting, so it seems xl devd never removed dguest from the list.
> 
> I wonder what else could go wrong.
> 
> I've managed to reproduce this locally. I will try to fix it for 4.9.
> 
> Wei.

Can you give the following patch a try? This applies to 4.8.

Not sure if there is a better way to fix it though. Ian and Roger?

---8<---
From 8542a1d4f537cb5719ebd245872d0256df816670 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Wed, 3 May 2017 17:55:42 +0100
Subject: [PATCH] libxl: fix backend_watch_callback

That function needs to cope with spurious events. The original "skip"
path blindly freed dguest even when it needed to stay in ddomain list.

Free dguest iff it is newly added to the list. That way we don't free
the one that should stay on the list and we don't unnecessarily add a
stale dguest entry to ddomain list.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 tools/libxl/libxl.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index acf714e1f9..ed542c0977 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3842,6 +3842,7 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
     libxl__device *dev = NULL;
     libxl__ddomain_device *ddev = NULL;
     libxl__ddomain_guest *dguest = NULL;
+    bool new_dguest = false;
     bool free_ao = false;
 
     /* Check if event_path ends with "state" or "online" and truncate it. */
@@ -3888,6 +3889,7 @@ static void backend_watch_callback(libxl__egc *egc, 
libxl__ev_xswatch *watch,
         LIBXL_SLIST_INSERT_HEAD(&ddomain->guests, dguest, next);
         LOG(DEBUG, "added domain %u to the list of active guests",
                    dguest->domid);
+        new_dguest = true;
     }
     ddev = search_for_device(dguest, dev);
     if (ddev == NULL && state == XenbusStateClosed) {
@@ -3947,7 +3949,13 @@ skip:
     libxl__nested_ao_free(nested_ao);
     free(dev);
     free(ddev);
-    free(dguest);
+    if (new_dguest) {
+        LIBXL_SLIST_REMOVE(&ddomain->guests, dguest, libxl__ddomain_guest,
+                           next);
+        LOG(DEBUG, "removed domain %u from the list of active guests",
+                   dguest->domid);
+        free(dguest);
+    }
     return;
 }
 
-- 
2.11.0


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