libxl: treat "dying" domains as destroyed

Rename the DOMAIN_DESTROY event to DOMAIN_DEATH and have it trigger
when the domain goes into the state indicated by the domaininfo flag
"dying".

This fixes a race which could leak a daemonised xl process, which
would have ignored the domain becoming "dying" and would then wait
forever to be told the domain was destroyed.

After the domain becomes "dying" we can't generate an event when it is
actually destroyed because xenstored will eat the relevant
VIRT_DOM_EXC virq and not generate an @releaseDomain, since xenstored
discards its own record of the domain's existence as soon as it sees
the domain "dying" and will not trigger @releaseDomain watches for
domains it knows nothing about.  Arguably this is a bug in xenstored,
and the whole @releaseDomain machinery is rather poor, but let us not
fix that now.

Anyway, xl does not really want to know when the domain is ultimately
destroyed.  It is enough for xl to know that it is on the way out, in
the "dying" state (which leads later to destruction by Xen).

Also fix a bug where domain_death_xswatch_callback might read one
domain beyond the valid data in its domaininfos array, by correctly
ordering the checks for empty domain list, end of domain list, and our
domain being missing.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2758d4c39e7e188e99e6795cdf967df4521a2da8..9bdce529ec4d19e1640cd79b6c2d38ee69e84a3d 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -685,6 +685,30 @@ int libxl_domain_reboot(libxl_ctx *ctx, uint32_t domid)
     return ret;
 }
 
+static void domain_death_occurred(libxl__egc *egc,
+                                  libxl_evgen_domain_death **evg_upd,
+                                  const char *why) {
+    /* Removes **evg_upd from death_list and puts it on death_reported
+     * and advances *evg_upd to the next entry.
+     * Call sites in domain_death_xswatch_callback must use "continue". */
+    EGC_GC;
+    libxl_evgen_domain_death *const evg = *evg_upd;
+
+    LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, "%s", why);
+
+    libxl_evgen_domain_death *evg_next = LIBXL_TAILQ_NEXT(evg, entry);
+    *evg_upd = evg_next;
+
+    libxl_event *ev = NEW_EVENT(egc, DOMAIN_DEATH, evg->domid);
+    if (!ev) return;
+
+    libxl__event_occurred(egc, ev);
+
+    evg->death_reported = 1;
+    LIBXL_TAILQ_REMOVE(&CTX->death_list, evg, entry);
+    LIBXL_TAILQ_INSERT_HEAD(&CTX->death_reported, evg, entry);
+}
+
 static void domain_death_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
                                         const char *wpath, const char *epath) {
     EGC_GC;
@@ -728,32 +752,23 @@ static void domain_death_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
                        evg, evg->domid, (int)(got - domaininfos),
                        got < gotend ? (long)got->domain : -1L);
 
-            if (!rc || got->domain > evg->domid) {
-                /* ie, the list doesn't contain evg->domid any more so
-                 * the domain has been destroyed */
-                libxl_evgen_domain_death *evg_next;
-
-                LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, " destroyed");
-
-                libxl_event *ev = NEW_EVENT(egc, DOMAIN_DESTROY, evg->domid);
-                if (!ev) goto out;
-
-                libxl__event_occurred(egc, ev);
-
-                evg->death_reported = 1;
-                evg_next = LIBXL_TAILQ_NEXT(evg, entry);
-                LIBXL_TAILQ_REMOVE(&CTX->death_list, evg, entry);
-                LIBXL_TAILQ_INSERT_HEAD(&CTX->death_reported, evg, entry);
-                evg = evg_next;
-
+            if (!rc) {
+                domain_death_occurred(egc, &evg, "empty list");
                 continue;
             }
-            
+
             if (got == gotend) {
                 LIBXL__LOG(CTX, LIBXL__LOG_DEBUG, " got==gotend");
                 break;
             }
 
+            if (got->domain > evg->domid) {
+                /* ie, the list doesn't contain evg->domid any more so
+                 * the domain has been destroyed */
+                domain_death_occurred(egc, &evg, "missing from list");
+                continue;
+            }
+
             if (got->domain < evg->domid) {
                 got++;
                 continue;
@@ -764,6 +779,11 @@ static void domain_death_xswatch_callback(libxl__egc *egc, libxl__ev_xswatch *w,
                        " dominf.flags=%x",
                        evg->shutdown_reported, got->flags);
 
+            if (got->flags & XEN_DOMINF_dying) {
+                domain_death_occurred(egc, &evg, "dying");
+                continue;
+            }
+
             if (!evg->shutdown_reported &&
                 (got->flags & XEN_DOMINF_shutdown)) {
                 libxl_event *ev = NEW_EVENT(egc, DOMAIN_SHUTDOWN, got->domain);

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f438c9f985b4f091c5327a2774d6c918261e0df0..5492ce9f033c46844db23e8657b5c58792944533 100644 (file)
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -396,7 +396,7 @@ libxl_sched_sedf = Struct("sched_sedf", [
 
 libxl_event_type = Enumeration("event_type", [
     (1, "DOMAIN_SHUTDOWN"),
-    (2, "DOMAIN_DESTROY"),
+    (2, "DOMAIN_DEATH"),
     (3, "DISK_EJECT"),
     (4, "OPERATION_COMPLETE"),
     ])
@@ -417,7 +417,7 @@ libxl_event = Struct("event",[
           [("domain_shutdown", Struct(None, [
                                              ("shutdown_reason", uint8),
                                       ])),
-           ("domain_destroy", Struct(None, [])),
+           ("domain_death", Struct(None, [])),
            ("disk_eject", Struct(None, [
                                         ("vdev", string),
                                         ("disk", libxl_device_disk),

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 8832b5a202ebbb5168358769f60c1acd6efd7f69..d3265889b171624c51f1cba972fc86d7ca97b500 100644 (file)
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1909,7 +1909,7 @@ start:
                 abort();
             }
 
-        case LIBXL_EVENT_TYPE_DOMAIN_DESTROY:
+        case LIBXL_EVENT_TYPE_DOMAIN_DEATH:
             LOG("Domain %d has been destroyed.", domid);
             ret = 0;
             goto out;
@@ -2445,7 +2445,7 @@ static void shutdown_domain(const char *p, int wait)
 
             switch (event->type) {
 
-            case LIBXL_EVENT_TYPE_DOMAIN_DESTROY:
+            case LIBXL_EVENT_TYPE_DOMAIN_DEATH:
                 LOG("Domain %d has been destroyed", domid);
                 goto done;