[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH for-4.6 1/2] 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@xxxxxxxxxx>
Reported-by: Alex Velazquez <alex.j.velazquez@xxxxxxxxx>
Cc: Alex Velazquez <alex.j.velazquez@xxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
---
 tools/libxl/libxl.c        | 38 ++++++++++++++++++++++++--------------
 tools/libxl/libxl_device.c | 11 ++++++-----
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 10d1909..1d1917e 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4410,32 +4410,42 @@ 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"))
+    /* Check if event_path ends with "state" or "online" and truncate it. */
+    if (strlen(event_path) < strlen("state") ||
+        strlen(event_path) < strlen("online"))
         goto skip;
 
     path = libxl__strdup(gc, event_path);
-    p = path + strlen(path) - strlen("state") - 1;
-    if (*p != '/')
-        goto skip;
-    *p = '\0';
-    p++;
-    if (strcmp(p, "state") != 0)
-        goto skip;
+    p = strstr(path, "/state");
+    if (p != NULL) {
+        *p = '\0';
+    } else {
+        p = strstr(path, "/online");
+        if (p == NULL)
+            goto skip;
+        *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)
@@ -4477,7 +4487,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 bee5ed5..51da10e 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -837,16 +837,17 @@ void libxl__initiate_device_remove(libxl__egc *egc,
             goto out;
         }
 
+        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;
+        }
+
         /*
          * 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);
-- 
1.9.5 (Apple Git-50.3)


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.