From 5c9a00c57172182652c432e80cc278bebc044dc4 Mon Sep 17 00:00:00 2001 From: Roger Pau Monne Date: Wed, 23 Sep 2015 12:06:55 +0200 Subject: [PATCH] libxl: fix devd removal path MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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é Reported-by: Alex Velazquez Cc: Alex Velazquez Cc: Ian Jackson Cc: Ian Campbell Cc: Wei Liu Acked-by: Ian Campbell --- 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 4d27891a8c..d38d0c76ca 100644 --- 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 bee5ed564b..85a3662cdf 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); -- 2.30.2