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....@citrix.com> Reported-by: Alex Velazquez <alex.j.velazq...@gmail.com> Cc: Alex Velazquez <alex.j.velazq...@gmail.com> Cc: Ian Jackson <ian.jack...@eu.citrix.com> Cc: Ian Campbell <ian.campb...@citrix.com> Cc: Wei Liu <wei.l...@citrix.com> --- Changes since v1: - Use strrchr and strcmp in order to check the event path. - Remove superfluous LOG in case of failed xenstore write. - Remove uneeded strlen checks. --- tools/libxl/libxl.c | 32 ++++++++++++++++++-------------- tools/libxl/libxl_device.c | 9 ++++----- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c index 10d1909..6c4ef6d 100644 --- a/tools/libxl/libxl.c +++ b/tools/libxl/libxl.c @@ -4410,32 +4410,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) @@ -4477,7 +4481,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..85a3662 100644 --- 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); -- 1.9.5 (Apple Git-50.3) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel