|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V5 32/32] libxl: synchronize configuration when we plug / unplug device
On Tue, 2014-05-13 at 22:54 +0100, Wei Liu wrote:
> We synchronize domain configuration in the callback, right before we
> return to caller. We depends solely on the return value of AO to
> determine if AO is successful.
>
> This approach cannot deal with half-baked device state (say, if AO
> returns success but for various reasons other parts just fail and we
> have some xenstore entries written / resource allocated). However we
> cannot make thing worse as the original code cannot handle that either.
> The good side of this approach is that in the future we can compare the
> stored configuration and xenstore entries to spot any half-baked device
> state so it can help fix that issue in the future.
>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
This looks broadly sensible to me, but I generally defer to Ian J on
anything involving the ao machinery...
> ---
> tools/libxl/libxl.c | 152
> +++++++++++++++++++++++++++++++++++-------
> tools/libxl/libxl_internal.h | 2 +
> 2 files changed, 129 insertions(+), 25 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index f93096b..a7bf0a4 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1794,29 +1794,6 @@ out:
>
>
> /******************************************************************************/
>
> -/* 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) {
> - if (aodev->dev) {
> - LOG(ERROR, "unable to %s %s with id %u",
> - libxl__device_action_to_string(aodev->action),
> - libxl__device_kind_to_string(aodev->dev->kind),
> - aodev->dev->devid);
> - } else {
> - LOG(ERROR, "unable to %s device",
> - libxl__device_action_to_string(aodev->action));
> - }
> - goto out;
> - }
> -
> -out:
> - libxl__ao_complete(egc, ao, aodev->rc);
> - return;
> -}
> -
> /* common function to get next device id */
> static int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device)
> {
> @@ -3548,6 +3525,126 @@ out:
> } \
> libxl_domain_config_dispose(&d_config); \
>
> +static void device_nic_fixup(libxl_ctx *ctx, libxl_device_nic *dst,
> + libxl_device_nic *src)
> +{
> + dst->devid = src->devid;
> + libxl_mac_copy(ctx, &dst->mac, &src->mac);
> +}
> +
> +static void device_vtpm_fixup(libxl_ctx *ctx, libxl_device_vtpm *dst,
> + libxl_device_vtpm *src)
> +{
> + libxl_uuid_copy(ctx, &dst->uuid, &src->uuid);
> +}
> +
> +static void device_disk_fixup(libxl_ctx *ctx, libxl_device_disk *dst,
> + libxl_device_disk *src)
> +{
> +}
> +
> +#define DEVICE_AO_FAILED_MSG \
> + do { \
> + if (aodev->dev) { \
> + LOG(ERROR, "unable to %s %s with id %u", \
> + libxl__device_action_to_string(aodev->action), \
> + libxl__device_kind_to_string(aodev->dev->kind), \
> + aodev->dev->devid); \
> + } else { \
> + LOG(ERROR, "unable to %s device", \
> + libxl__device_action_to_string(aodev->action)); \
> + } \
> + } while (0)
> +
> +
> +#define DEFINE_DEVICE_ADD_AOCOMPLETE(type,ptr,cnt) \
> + static void device_add_##type##_aocomplete(libxl__egc *egc, \
> + libxl__ao_device *aodev) \
> + { \
> + STATE_AO_GC(aodev->ao); \
> + int rc; \
> + \
> + if (aodev->rc) { \
> + DEVICE_AO_FAILED_MSG; \
> + goto out; \
> + } else { \
> + libxl_device_##type *p; \
> + \
> + LOAD_DOMAIN_CONFIG(aodev->dev->domid); \
> + \
> + d_config.ptr = \
> + libxl__realloc(gc, d_config.ptr, \
> + (d_config.cnt + 1) * \
> + sizeof(libxl_device_##type)); \
> + p = &d_config.ptr[d_config.cnt]; \
> + d_config.cnt++; \
> + \
> + libxl_device_##type##_copy(CTX, p, aodev->pdev_copy); \
> + \
> + device_##type##_fixup(CTX, p, aodev->pdev); \
> + \
> + STORE_DOMAIN_CONFIG(aodev->dev->domid); \
> + } \
> + \
> + out: \
> + /* Dispose our internal copy, which is allocated at the entry of \
> + * this AO. */ \
> + libxl_device_##type##_dispose(aodev->pdev_copy); \
> + libxl__ao_complete(egc, ao, aodev->rc); \
> + return; \
> + }
> +
> +DEFINE_DEVICE_ADD_AOCOMPLETE(nic, nics, num_nics);
> +DEFINE_DEVICE_ADD_AOCOMPLETE(vtpm, vtpms, num_vtpms);
> +DEFINE_DEVICE_ADD_AOCOMPLETE(disk, disks, num_disks);
> +
> +#define COMPARE_DEVID(a, b) ((a)->devid == (b)->devid)
> +#define COMPARE_DISK(a, b) (!strcmp((a)->vdev, (b)->vdev))
> +
> +#define DEFINE_DEVICE_RM_AOCOMPLETE(type,ptr,cnt,compare) \
> + static void device_rm_##type##_aocomplete(libxl__egc *egc, \
> + libxl__ao_device *aodev) \
> + { \
> + STATE_AO_GC(aodev->ao); \
> + int rc; \
> + \
> + if (aodev->rc) { \
> + DEVICE_AO_FAILED_MSG; \
> + goto out; \
> + } else { \
> + int i, j; \
> + libxl_device_##type *p = aodev->pdev; \
> + LOAD_DOMAIN_CONFIG(aodev->dev->domid); \
> + \
> + for (i = j = 0; i < d_config.cnt; i++) { \
> + if (!compare(&d_config.ptr[i], p)) { \
> + if (i != j) { \
> + libxl_device_##type##_dispose(&d_config.ptr[j]);\
> + d_config.ptr[j] = d_config.ptr[i]; \
> + } \
> + j++; \
> + } \
> + } \
> + \
> + d_config.ptr = \
> + libxl__realloc(gc, d_config.ptr, \
> + j * sizeof(libxl_device_##type)); \
> + d_config.cnt = j; \
> + \
> + STORE_DOMAIN_CONFIG(aodev->dev->domid); \
> + } \
> + \
> + out: \
> + libxl__ao_complete(egc, ao, aodev->rc); \
> + return; \
> + }
> +
> +DEFINE_DEVICE_RM_AOCOMPLETE(nic, nics, num_nics, COMPARE_DEVID);
> +DEFINE_DEVICE_RM_AOCOMPLETE(vtpm, vtpms, num_vtpms, COMPARE_DEVID);
> +DEFINE_DEVICE_RM_AOCOMPLETE(disk, disks, num_disks, COMPARE_DISK);
> +DEFINE_DEVICE_RM_AOCOMPLETE(vkb, vkbs, num_vkbs, COMPARE_DEVID);
> +DEFINE_DEVICE_RM_AOCOMPLETE(vfb, vfbs, num_vfbs, COMPARE_DEVID);
> +
> /* Macro for defining device remove/destroy functions in a compact way */
> /* The following functions are defined:
> * libxl_device_disk_remove
> @@ -3577,9 +3674,11 @@ out:
> \
> GCNEW(aodev); \
> libxl__prepare_ao_device(ao, aodev); \
> + aodev->pdev = type; \
> + aodev->pdev_copy = NULL; /* unused */ \
> aodev->action = LIBXL__DEVICE_ACTION_REMOVE; \
> aodev->dev = device; \
> - aodev->callback = device_addrm_aocomplete; \
> + aodev->callback = device_rm_##type##_aocomplete; \
> aodev->force = f; \
> libxl__initiate_device_remove(egc, aodev); \
> \
> @@ -3631,8 +3730,11 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
> libxl__ao_device *aodev; \
> \
> GCNEW(aodev); \
> + aodev->pdev = type; \
> + aodev->pdev_copy = libxl__zalloc(gc, sizeof(*type)); \
> + libxl_device_##type##_copy(CTX, aodev->pdev_copy, aodev->pdev); \
> libxl__prepare_ao_device(ao, aodev); \
> - aodev->callback = device_addrm_aocomplete; \
> + aodev->callback = device_add_##type##_aocomplete; \
> libxl__device_##type##_add(egc, domid, type, aodev); \
> \
> return AO_INPROGRESS; \
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 21bb774..06503df 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2106,6 +2106,8 @@ struct libxl__ao_device {
> const char *what;
> int num_exec;
> libxl__ev_child child;
> + void *pdev; /* pointer to libxl_device_nic/vtpm/disk etc. */
> + void *pdev_copy; /* a copy of vanilla structure pointed to by pdev. */
> };
>
> /*
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |