wating for backend changes (was Re: [PATCH v3 4/4] libxl: add support for vscsi)

[Olaf Hering <olaf@aepfle.de>]

How is new code supposed to wait for backend changes?

Right now there are two APIs for that:
- libxl__wait_for_backend loops for a while until it returns an error.
- libxl__ev_devstate_wait registers a watch and a timer.


In case of pvscsi there are three variants:
- new, can use libxl__wait_device_connection
- reconfigure, can use both of the above
- remove, may use DEFINE_DEVICE_REMOVE and its
  libxl__initiate_device_remove

The reconfigure case has to wait for various states, depending on the
state before the reconfiguration.

The pci code in libxl uses just libxl__wait_for_backend. Looking through
the history it seems the function was added just for pci. Is it a
deprecated function, should callers get converted to
libxl__ev_devstate_wait?

Olaf

On Fri, Mar 06, Olaf Hering wrote:

> +void libxl__device_vscsi_add(libxl__egc *egc, uint32_t domid,
> +                             libxl_device_vscsi *vscsi,
> +                             libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +    flexarray_t *front;
> +    flexarray_t *back;
> +    libxl__device *device;
> +    char *be_path;
> +    unsigned int be_dirs = 0, rc, i;
> +
> +    if (vscsi->devid == -1) {
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    /* Prealloc key+value: 4 toplevel + 4 per device */
> +    i = 2 * (4 + (4 * vscsi->num_vscsi_devs));
> +    back = flexarray_make(gc, i, 1);
> +    front = flexarray_make(gc, 2 * 2, 1);
> +
> +    GCNEW(device);
> +    rc = libxl__device_from_vscsi(gc, domid, vscsi, device);
> +    if ( rc != 0 ) goto out;
> +
> +    /* Check if backend device path is already present */
> +    be_path = libxl__device_backend_path(gc, device);
> +    if (!libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs) || !be_dirs) {
> +        /* backend does not exist, create a new one */
> +        flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
> +        flexarray_append_pair(back, "online", "1");
> +        flexarray_append_pair(back, "state", "1");
> +        flexarray_append_pair(back, "feature-host", GCSPRINTF("%d", !!vscsi->feature_host));
> +
> +        flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", vscsi->backend_domid));
> +        flexarray_append_pair(front, "state", "1");
> +    }
> +
> +    for (i = 0; i < vscsi->num_vscsi_devs; i++) {
> +        libxl_vscsi_dev *v = vscsi->vscsi_devs + i;
> +        /* Trigger removal, otherwise create new device */
> +        if (be_dirs) {
> +            unsigned int nb = 0;
> +            /* Preserve existing device */
> +            if (libxl__xs_directory(gc, XBT_NULL, GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id), &nb) && nb) {
> +                /* Trigger device removal by forwarding state to XenbusStateClosing */
> +                if (v->remove)
> +                    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id), "5");
> +                continue;
> +            }
> +        }
> +        flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/p-devname", v->vscsi_dev_id), v->p_devname);
> +        switch (v->pdev_type) {
> +            case LIBXL_VSCSI_PDEV_TYPE_WWN:
> +                flexarray_append_pair(back,
> +                                      GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
> +                                      v->p_devname);
> +                break;
> +            case LIBXL_VSCSI_PDEV_TYPE_DEV:
> +            case LIBXL_VSCSI_PDEV_TYPE_HCTL:
> +                flexarray_append_pair(back,
> +                                      GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
> +                                      GCSPRINTF("%u:%u:%u:%u", v->pdev.hst, v->pdev.chn, v->pdev.tgt, v->pdev.lun));
> +                break;
> +            case LIBXL_VSCSI_PDEV_TYPE_INVALID:
> +            default:
> +                rc = ERROR_FAIL;
> +                goto out;
> +        }
> +        flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/v-dev", v->vscsi_dev_id),
> +                              GCSPRINTF("%u:%u:%u:%u", v->vdev.hst, v->vdev.chn, v->vdev.tgt, v->vdev.lun));
> +        flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id), "1");
> +    }
> +
> +    aodev->dev = device;
> +    /* Either create new host or reconfigure existing host */
> +    if (be_dirs == 0) {
> +        libxl__device_generic_add(gc, XBT_NULL, device,
> +                                  libxl__xs_kvs_of_flexarray(gc, back, back->count),
> +                                  libxl__xs_kvs_of_flexarray(gc, front, front->count),
> +                                  NULL);
> +        aodev->action = LIBXL__DEVICE_ACTION_ADD;
> +        libxl__wait_device_connection(egc, aodev);
> +        rc = 0;
> +        /* Done with new host */
> +        goto out;
> +    }
> +
> +    /* Only new devices, write them and do vscsi host reconfiguration */
> +    xs_transaction_t t;
> +retry_transaction:
> +    t = xs_transaction_start(ctx->xsh);
> +    libxl__xs_writev(gc, t, be_path,
> +                     libxl__xs_kvs_of_flexarray(gc, back, back->count));
> +    xs_write(ctx->xsh, t, GCSPRINTF("%s/state", be_path), "7", 2);
> +    if (!xs_transaction_end(ctx->xsh, t, 0)) {
> +        if (errno == EAGAIN)
> +            goto retry_transaction;
> +        LOGE(ERROR, "xs transaction failed");
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +    libxl__wait_for_backend(gc, be_path, "4");
> +
> +retry_transaction2:
> +    t = xs_transaction_start(ctx->xsh);
> +    for (i = 0; i < vscsi->num_vscsi_devs; i++) {
> +        libxl_vscsi_dev *v = vscsi->vscsi_devs + i;
> +        if (v->remove) {
> +            char *path, *val;
> +            path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> +            val = libxl__xs_read(gc, t, path);
> +            if (val && strcmp(val, "6") == 0) {
> +                path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> +                xs_rm(ctx->xsh, t, path);
> +                path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-devname", be_path, v->vscsi_dev_id);
> +                xs_rm(ctx->xsh, t, path);
> +                path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-dev", be_path, v->vscsi_dev_id);
> +                xs_rm(ctx->xsh, t, path);
> +                path = GCSPRINTF("%s/vscsi-devs/dev-%u/v-dev", be_path, v->vscsi_dev_id);
> +                xs_rm(ctx->xsh, t, path);
> +                path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
> +                xs_rm(ctx->xsh, t, path);
> +            } else {
> +                LOGE(ERROR, "%s: %s has %s, expected 6", __func__, path, val);
> +            }
> +        }
> +    }
> +
> +    if (!xs_transaction_end(ctx->xsh, t, 0)) {
> +        if (errno == EAGAIN)
> +            goto retry_transaction2;
> +        LOGE(ERROR, "xs transaction failed");
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +    /* As we are not adding new device, skip waiting for it */
> +    libxl__ao_complete(egc, aodev->ao, 0);
> +
> +    rc = 0;
> +out:
> +    aodev->rc = rc;
> +    if(rc) aodev->callback(egc, aodev);
> +    return;
> +}