|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v8 01/15] libxl: change ao_device_remove to ao_device
Introduce a new structure to track state of device backends, that will
be used in following patches on this series.
This structure if used for both device creation and device
destruction and removes libxl__ao_device_remove.
Changes since v6:
* Use libxl__xs_transaction helpers.
Changes since v5:
* Added a common exit point for device addition/destruction that
removes backend and frontend entries (on destruction).
* Posponed the introduction of the base and active fields in the
ao_device struct.
* Don't call libxl__ev_devstate_init, since _wait will do it.
* Removed "action being performed" comment.
Changes sinve v4:
* Added a more detailed comment in _prepare and _initiate header.
* Removed an unnecessary state check from
libxl_initiate_device_remove.
Changes since v2:
* Remove unnecessary comments in libxl__ao_device.
* Change libxl__device_cb to device_addrm_aocomplete.
* Rename aorm parameter in device_addrm_aocomplete to aodev.
* Use a macro to define {nic,disk,vkb,vfb}_{remove,destroy}
functions.
* Rename libxl__init_ao_device to libxl__prepare_ao_device and add a
comment explaining why we need to set active to 1.
* Replace al uses of aorm with aodev.
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxx>
---
tools/libxl/libxl.c | 214 ++++++++++++++----------------------------
tools/libxl/libxl.h | 15 ++-
tools/libxl/libxl_device.c | 149 +++++++++++++++++++++---------
tools/libxl/libxl_internal.h | 58 ++++++++++--
tools/libxl/xl_cmdimpl.c | 2 +-
5 files changed, 239 insertions(+), 199 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 9f301a9..fd44fd7 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1503,6 +1503,26 @@ int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid,
int autopass)
/******************************************************************************/
+/* generic callback for devices that only need to set ao_complete */
+static void device_addrm_aocomplete(libxl__egc *egc, libxl__ao_device *aodev)
+{
+ STATE_AO_GC(aodev->ao);
+
+ if (aodev->rc) {
+ LOGE(ERROR, "unable to %s %s with id %u",
+ aodev->action == DEVICE_CONNECT ? "add" : "remove",
+ libxl__device_kind_to_string(aodev->dev->kind),
+ aodev->dev->devid);
+ goto out;
+ }
+
+out:
+ libxl__ao_complete(egc, ao, aodev->rc);
+ return;
+}
+
+/******************************************************************************/
+
int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk)
{
int rc;
@@ -1680,42 +1700,6 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t
domid, libxl_device_disk *dis
return rc;
}
-int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
- libxl_device_disk *disk,
- const libxl_asyncop_how *ao_how)
-{
- AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_disk(gc, domid, disk, &device);
- if (rc != 0) goto out;
-
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
-
- return AO_INPROGRESS;
-
-out:
- return AO_ABORT(rc);
-}
-
-int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
- libxl_device_disk *disk)
-{
- GC_INIT(ctx);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_disk(gc, domid, disk, &device);
- if (rc != 0) goto out;
-
- rc = libxl__device_destroy(gc, &device);
-out:
- GC_FREE;
- return rc;
-}
-
static void libxl__device_disk_from_xs_be(libxl__gc *gc,
const char *be_path,
libxl_device_disk *disk)
@@ -2080,8 +2064,9 @@ int libxl__device_disk_local_detach(libxl__gc *gc,
libxl_device_disk *disk)
if (disk->vdev != NULL) {
libxl_device_disk_remove(gc->owner, LIBXL_TOOLSTACK_DOMID,
disk, 0);
+ /* fixme-ao */
rc = libxl_device_disk_destroy(gc->owner,
- LIBXL_TOOLSTACK_DOMID, disk);
+ LIBXL_TOOLSTACK_DOMID, disk, 0);
}
break;
default:
@@ -2249,42 +2234,6 @@ out:
return rc;
}
-int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
- libxl_device_nic *nic,
- const libxl_asyncop_how *ao_how)
-{
- AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_nic(gc, domid, nic, &device);
- if (rc != 0) goto out;
-
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
-
- return AO_INPROGRESS;
-
-out:
- return AO_ABORT(rc);
-}
-
-int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
- libxl_device_nic *nic)
-{
- GC_INIT(ctx);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_nic(gc, domid, nic, &device);
- if (rc != 0) goto out;
-
- rc = libxl__device_destroy(gc, &device);
-out:
- GC_FREE;
- return rc;
-}
-
static void libxl__device_nic_from_xs_be(libxl__gc *gc,
const char *be_path,
libxl_device_nic *nic)
@@ -2611,42 +2560,6 @@ out:
return rc;
}
-int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
- libxl_device_vkb *vkb,
- const libxl_asyncop_how *ao_how)
-{
- AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_vkb(gc, domid, vkb, &device);
- if (rc != 0) goto out;
-
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
-
- return AO_INPROGRESS;
-
-out:
- return AO_ABORT(rc);
-}
-
-int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid,
- libxl_device_vkb *vkb)
-{
- GC_INIT(ctx);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_vkb(gc, domid, vkb, &device);
- if (rc != 0) goto out;
-
- rc = libxl__device_destroy(gc, &device);
-out:
- GC_FREE;
- return rc;
-}
-
/******************************************************************************/
int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
@@ -2744,41 +2657,56 @@ out:
return rc;
}
-int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
- libxl_device_vfb *vfb,
- const libxl_asyncop_how *ao_how)
-{
- AO_CREATE(ctx, domid, ao_how);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_vfb(gc, domid, vfb, &device);
- if (rc != 0) goto out;
-
- rc = libxl__initiate_device_remove(egc, ao, &device);
- if (rc) goto out;
-
- return AO_INPROGRESS;
-
-out:
- return AO_ABORT(rc);
-}
-
-int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid,
- libxl_device_vfb *vfb)
-{
- GC_INIT(ctx);
- libxl__device device;
- int rc;
-
- rc = libxl__device_from_vfb(gc, domid, vfb, &device);
- if (rc != 0) goto out;
+/******************************************************************************/
- rc = libxl__device_destroy(gc, &device);
-out:
- GC_FREE;
- return rc;
-}
+/* Macro for defining device remove/destroy functions in a compact way */
+#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \
+ int libxl_device_##type##_##removedestroy(libxl_ctx *ctx, \
+ uint32_t domid, libxl_device_##type *type, \
+ const libxl_asyncop_how *ao_how) \
+ { \
+ AO_CREATE(ctx, domid, ao_how); \
+ libxl__device *device; \
+ libxl__ao_device *aodev; \
+ int rc; \
+ \
+ GCNEW(device); \
+ rc = libxl__device_from_##type(gc, domid, type, device); \
+ if (rc != 0) goto out; \
+ \
+ GCNEW(aodev); \
+ libxl__prepare_ao_device(ao, aodev); \
+ aodev->action = DEVICE_DISCONNECT; \
+ aodev->dev = device; \
+ aodev->callback = device_addrm_aocomplete; \
+ aodev->force = f; \
+ libxl__initiate_device_remove(egc, aodev); \
+ \
+ out: \
+ if (rc) return AO_ABORT(rc); \
+ return AO_INPROGRESS; \
+ }
+
+/* Define all remove/destroy functions and undef the macro */
+
+/* disk */
+DEFINE_DEVICE_REMOVE(disk, remove, 0)
+DEFINE_DEVICE_REMOVE(disk, destroy, 1)
+
+/* nic */
+DEFINE_DEVICE_REMOVE(nic, remove, 0)
+DEFINE_DEVICE_REMOVE(nic, destroy, 1)
+
+/* vkb */
+DEFINE_DEVICE_REMOVE(vkb, remove, 0)
+DEFINE_DEVICE_REMOVE(vkb, destroy, 1)
+
+/* vfb */
+
+DEFINE_DEVICE_REMOVE(vfb, remove, 0)
+DEFINE_DEVICE_REMOVE(vfb, destroy, 1)
+
+#undef DEFINE_DEVICE_REMOVE
/******************************************************************************/
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 371b659..75175c7 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -671,7 +671,8 @@ int libxl_device_disk_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
const libxl_asyncop_how *ao_how);
int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
- libxl_device_disk *disk);
+ libxl_device_disk *disk,
+ const libxl_asyncop_how *ao_how);
libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int
*num);
int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -688,7 +689,9 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_nic *nic);
int libxl_device_nic_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_nic *nic,
const libxl_asyncop_how *ao_how);
-int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_nic
*nic);
+int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_nic *nic,
+ const libxl_asyncop_how *ao_how);
libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int
*num);
int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -699,14 +702,18 @@ int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid,
libxl_device_vkb *vkb);
int libxl_device_vkb_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_vkb *vkb,
const libxl_asyncop_how *ao_how);
-int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb
*vkb);
+int libxl_device_vkb_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_vkb *vkb,
+ const libxl_asyncop_how *ao_how);
/* Framebuffer */
int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb
*vfb);
int libxl_device_vfb_remove(libxl_ctx *ctx, uint32_t domid,
libxl_device_vfb *vfb,
const libxl_asyncop_how *ao_how);
-int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb
*vfb);
+int libxl_device_vfb_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_vfb *vfb,
+ const libxl_asyncop_how *ao_how);
/* PCI Passthrough */
int libxl_device_pci_add(libxl_ctx *ctx, uint32_t domid, libxl_device_pci
*pcidev);
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 3da60e1..8316707 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -361,11 +361,13 @@ int libxl__device_disk_dev_number(const char *virtpath,
int *pdisk,
return -1;
}
+/* Device AO operations */
-typedef struct {
- libxl__ao *ao;
- libxl__ev_devstate ds;
-} libxl__ao_device_remove;
+void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev)
+{
+ aodev->ao = ao;
+ aodev->rc = 0;
+}
int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
{
@@ -441,34 +443,41 @@ out:
/* Callbacks for device related operations */
-static void device_remove_callback(libxl__egc *egc, libxl__ev_devstate *ds,
+/*
+ * device_backend_callback is the main callback entry point, for both device
+ * addition and removal. It gets called if we reach the desired state
+ * (XenbusStateClosed or XenbusStateInitWait). After that, all this
+ * functions get called in the order displayed below.
+ *
+ * If new device types are added, they should only need to modify the
+ * specific hotplug scripts call, which can be found in each OS specific
+ * file. If this new devices don't need a hotplug script, no modification
+ * should be needed.
+ */
+
+static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
int rc);
-static void device_remove_cleanup(libxl__gc *gc,
- libxl__ao_device_remove *aorm);
+static void device_backend_cleanup(libxl__gc *gc,
+ libxl__ao_device *aodev);
-int libxl__initiate_device_remove(libxl__egc *egc, libxl__ao *ao,
- libxl__device *dev)
+static void device_xsentries_remove(libxl__egc *egc, libxl__ao_device *aodev);
+
+void libxl__initiate_device_remove(libxl__egc *egc,
+ libxl__ao_device *aodev)
{
- AO_GC;
+ STATE_AO_GC(aodev->ao);
libxl_ctx *ctx = libxl__gc_owner(gc);
- xs_transaction_t t;
- char *be_path = libxl__device_backend_path(gc, dev);
+ xs_transaction_t t = 0;
+ char *be_path = libxl__device_backend_path(gc, aodev->dev);
char *state_path = libxl__sprintf(gc, "%s/state", be_path);
- char *state = libxl__xs_read(gc, XBT_NULL, state_path);
int rc = 0;
- libxl__ao_device_remove *aorm = 0;
-
- if (!state)
- goto out_ok;
- if (atoi(state) != 4) {
- libxl__device_destroy_tapdisk(gc, be_path);
- xs_rm(ctx->xsh, XBT_NULL, be_path);
- goto out_ok;
- }
retry_transaction:
t = xs_transaction_start(ctx->xsh);
+ if (aodev->force)
+ libxl__xs_path_cleanup(gc, t,
+ libxl__device_frontend_path(gc, aodev->dev));
xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0",
strlen("0"));
xs_write(ctx->xsh, t, state_path, "5", strlen("5"));
if (!xs_transaction_end(ctx->xsh, t, 0)) {
@@ -479,42 +488,94 @@ retry_transaction:
goto out_fail;
}
}
+ /* mark transaction as ended, to prevent double closing it on out_ok */
+ t = 0;
libxl__device_destroy_tapdisk(gc, be_path);
- aorm = libxl__zalloc(gc, sizeof(*aorm));
- aorm->ao = ao;
- libxl__ev_devstate_init(&aorm->ds);
-
- rc = libxl__ev_devstate_wait(gc, &aorm->ds, device_remove_callback,
+ rc = libxl__ev_devstate_wait(gc, &aodev->backend_ds,
+ device_backend_callback,
state_path, XenbusStateClosed,
LIBXL_DESTROY_TIMEOUT * 1000);
- if (rc) goto out_fail;
+ if (rc) {
+ LOG(ERROR, "unable to remove device %s", be_path);
+ goto out_fail;
+ }
- return 0;
+ return;
out_fail:
assert(rc);
- device_remove_cleanup(gc, aorm);
- return rc;
-
- out_ok:
- libxl__ao_complete(egc, ao, 0);
- return 0;
+ aodev->rc = rc;
+ device_xsentries_remove(egc, aodev);
+ return;
}
-static void device_remove_callback(libxl__egc *egc, libxl__ev_devstate *ds,
+static void device_backend_callback(libxl__egc *egc, libxl__ev_devstate *ds,
int rc) {
- libxl__ao_device_remove *aorm = CONTAINER_OF(ds, *aorm, ds);
- libxl__gc *gc = &aorm->ao->gc;
- libxl__ao_complete(egc, aorm->ao, rc);
- device_remove_cleanup(gc, aorm);
+ libxl__ao_device *aodev = CONTAINER_OF(ds, *aodev, backend_ds);
+ STATE_AO_GC(aodev->ao);
+
+ device_backend_cleanup(gc, aodev);
+
+ if (rc == ERROR_TIMEDOUT && aodev->action == DEVICE_DISCONNECT &&
+ !aodev->force) {
+ aodev->force = 1;
+ libxl__initiate_device_remove(egc, aodev);
+ return;
+ }
+
+ /* Some devices (vkbd) fail to disconnect properly,
+ * but we shouldn't alarm the user if it's during
+ * domain destruction.
+ */
+ if (rc && aodev->action == DEVICE_CONNECT) {
+ LOG(ERROR, "unable to connect device with path %s",
+ libxl__device_backend_path(gc, aodev->dev));
+ goto out;
+ } else if (rc) {
+ LOG(DEBUG, "unable to disconnect device with path %s",
+ libxl__device_backend_path(gc, aodev->dev));
+ goto out;
+ }
+
+out:
+ aodev->rc = rc;
+ device_xsentries_remove(egc, aodev);
+ return;
}
-static void device_remove_cleanup(libxl__gc *gc,
- libxl__ao_device_remove *aorm) {
- if (!aorm) return;
- libxl__ev_devstate_cancel(gc, &aorm->ds);
+static void device_backend_cleanup(libxl__gc *gc, libxl__ao_device *aodev)
+{
+ if (!aodev) return;
+ libxl__ev_devstate_cancel(gc, &aodev->backend_ds);
+}
+
+static void device_xsentries_remove(libxl__egc *egc, libxl__ao_device *aodev)
+{
+ STATE_AO_GC(aodev->ao);
+ char *be_path = libxl__device_backend_path(gc, aodev->dev);
+ char *fe_path = libxl__device_frontend_path(gc, aodev->dev);
+ xs_transaction_t t = 0;
+ int rc;
+
+ if (aodev->action == DEVICE_DISCONNECT) {
+ for (;;) {
+ rc = libxl__xs_transaction_start(gc, &t);
+ if (rc) goto out;
+
+ libxl__xs_path_cleanup(gc, t, fe_path);
+ libxl__xs_path_cleanup(gc, t, be_path);
+
+ rc = libxl__xs_transaction_commit(gc, &t);
+ if (!rc) break;
+ if (rc < 0) goto out;
+ }
+ }
+
+out:
+ aodev->callback(egc, aodev);
+ return;
}
int libxl__wait_for_device_model(libxl__gc *gc,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2781398..339f9a6 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -899,13 +899,6 @@ _hidden const char *libxl__device_nic_devname(libxl__gc
*gc,
uint32_t devid,
libxl_nic_type type);
-/* Arranges that dev will be removed from its guest. When
- * this is done, the ao will be completed. An error
- * return from libxl__initiate_device_remove means that the ao
- * will _not_ be completed and the caller must do so. */
-_hidden int libxl__initiate_device_remove(libxl__egc*, libxl__ao*,
- libxl__device *dev);
-
/*
* libxl__ev_devstate - waits a given time for a device to
* reach a given state. Follows the libxl_ev_* conventions.
@@ -2007,6 +2000,57 @@ _hidden void
libxl__bootloader_init(libxl__bootloader_state *bl);
* If callback is passed rc==0, will have updated st->info appropriately */
_hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
+/*----- device addition/removal -----*/
+
+/* Action to perform (either connect or disconnect) */
+typedef enum {
+ DEVICE_CONNECT,
+ DEVICE_DISCONNECT
+} libxl__device_action;
+
+typedef struct libxl__ao_device libxl__ao_device;
+typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
+
+/* This functions sets the necessary libxl__ao_device struct values to use
+ * safely inside functions. It marks the operation as "active"
+ * since we need to be sure that all device status structs are set
+ * to active before start queueing events, or we might call
+ * ao_complete before all devices had finished
+ *
+ * libxl__initiate_device_{remove/addition} should not be called without
+ * calling libxl__prepare_ao_device first, since it initializes the private
+ * fields of the struct libxl__ao_device to what this functions expect.
+ *
+ * Once _prepare has been called on a libxl__ao_device, it is safe to just
+ * discard this struct, there's no need to call any destroy function.
+ * _prepare can also be called multiple times with the same libxl__ao_device.
+ */
+_hidden void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev);
+
+struct libxl__ao_device {
+ /* filled in by user */
+ libxl__ao *ao;
+ libxl__device_action action;
+ libxl__device *dev;
+ int force;
+ libxl__device_callback *callback;
+ /* private for implementation */
+ int rc;
+ libxl__ev_devstate backend_ds;
+};
+
+/* Arranges that dev will be removed to the guest, and the
+ * hotplug scripts will be executed (if necessary). When
+ * this is done (or an error happens), the callback in
+ * aodev->callback will be called.
+ *
+ * The libxl__ao_device passed to this function should be
+ * prepared using libxl__prepare_ao_device prior to calling
+ * this function.
+ */
+_hidden void libxl__initiate_device_remove(libxl__egc *egc,
+ libxl__ao_device *aodev);
+
/*----- Domain creation -----*/
typedef struct libxl__domain_create_state libxl__domain_create_state;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 155d183..5040210 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5381,7 +5381,7 @@ int main_blockdetach(int argc, char **argv)
if (libxl_device_disk_remove(ctx, domid, &disk, 0)) {
fprintf(stderr, "libxl_device_disk_remove failed.\n");
} else
- libxl_device_disk_destroy(ctx, domid, &disk);
+ libxl_device_disk_destroy(ctx, domid, &disk, 0);
return 0;
}
--
1.7.7.5 (Apple Git-26)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |