libxl: fix devd removal path

The current flow of the devd helper (in charge of launching hotplug scripts
inside of driver domains) is to wait for the device backend to switch to
state 6  (XenbusStateClosed) and then remove it. This is not correct, since
a domain can reconnect it's PV devices as many times as it wants.

In order to fix this, introduce the following logic: the control domain will
set the "online" backend node to 0 when it wants the driver domain to
disconnect the device, so now the condition applied in devd is that "state"
must be 6 and "online" 0 in order to proceed with the disconnection.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reported-by: Alex Velazquez <alex.j.velazquez@gmail.com>
Cc: Alex Velazquez <alex.j.velazquez@gmail.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4d27891a8caf401c66768241a01ec204bf8ff356..d38d0c76ca02f5e29b69c534e103f501780b579a 100644 (file)
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4421,32 +4421,36 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
     libxl__ao *nested_ao = libxl__nested_ao_create(ddomain->ao);
     STATE_AO_GC(nested_ao);
     char *p, *path;
-    const char *sstate;
-    int state, rc, num_devs;
+    const char *sstate, *sonline;
+    int state, online, rc, num_devs;
     libxl__device *dev = NULL;
     libxl__ddomain_device *ddev = NULL;
     libxl__ddomain_guest *dguest = NULL;
     bool free_ao = false;
 
-    /* Check if event_path ends with "state" and truncate it */
-    if (strlen(event_path) < strlen("state"))
-        goto skip;
-
+    /* Check if event_path ends with "state" or "online" and truncate it. */
     path = libxl__strdup(gc, event_path);
-    p = path + strlen(path) - strlen("state") - 1;
-    if (*p != '/')
+    p = strrchr(path, '/');
+    if (p == NULL)
         goto skip;
-    *p = '\0';
-    p++;
-    if (strcmp(p, "state") != 0)
+    if (strcmp(p, "/state") != 0 && strcmp(p, "/online") != 0)
         goto skip;
+    /* Truncate the string so it points to the backend directory. */
+    *p = '\0';
 
-    /* Check if the state is 1 (XenbusStateInitialising) or greater */
-    rc = libxl__xs_read_checked(gc, XBT_NULL, event_path, &sstate);
+    /* Fetch the value of the state and online nodes. */
+    rc = libxl__xs_read_checked(gc, XBT_NULL, GCSPRINTF("%s/state", path),
+                                &sstate);
     if (rc || !sstate)
         goto skip;
     state = atoi(sstate);
 
+    rc = libxl__xs_read_checked(gc, XBT_NULL, GCSPRINTF("%s/online", path),
+                                &sonline);
+    if (rc || !sonline)
+        goto skip;
+    online = atoi(sonline);
+
     dev = libxl__zalloc(NOGC, sizeof(*dev));
     rc = libxl__parse_backend_path(gc, path, dev);
     if (rc)
@@ -4488,7 +4492,7 @@ static void backend_watch_callback(libxl__egc *egc, libxl__ev_xswatch *watch,
         rc = add_device(egc, nested_ao, dguest, ddev);
         if (rc > 0)
             free_ao = true;
-    } else if (state == XenbusStateClosed) {
+    } else if (state == XenbusStateClosed && online == 0) {
         /*
          * Removal of an active device, remove it from the list and
          * free it's data structures if they are no longer needed.

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index bee5ed564b04f1ca3d4a9e5e1e783f61561acefc..85a3662cdf793013f006a17a8f2d11a93576e1aa 100644 (file)
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -837,16 +837,15 @@ void libxl__initiate_device_remove(libxl__egc *egc,
             goto out;
         }
 
+        rc = libxl__xs_write_checked(gc, t, online_path, "0");
+        if (rc)
+            goto out;
+
         /*
          * Check if device is already in "closed" state, in which case
          * it should not be changed.
          */
          if (state && atoi(state) != XenbusStateClosed) {
-            rc = libxl__xs_write_checked(gc, t, online_path, "0");
-            if (rc) {
-                LOG(ERROR, "unable to write to xenstore path %s", online_path);
-                goto out;
-            }
             rc = libxl__xs_write_checked(gc, t, state_path, GCSPRINTF("%d", XenbusStateClosing));
             if (rc) {
                 LOG(ERROR, "unable to write to xenstore path %s", state_path);