Re: [PATCH 3/5] libxl: add support for vscsi

[Olaf Hering <olaf@aepfle.de>]

On Fri, Nov 13, Olaf Hering wrote:

> Port pvscsi support from xend to libxl:

How should code which converts devices from xenstore to json handle
devices which got marked as "to be removed"?

In my pvscsi code I set XenbusStateClosing in the vscsidev, then
XenbusStateReconfiguring in vscsictrl. This causes the backend/frontend
to reevaluate the vscsi bus. During that timeframe some other code can
walk xenstore and find such a device. In libxl__vscsi_fill_host below
such device gets the ->remove flag. This flag is handled elsewhere to
indicate that its not an active device anymore. IanC asked to remove the
flag and I'm converting the code to work without such flag. The question
still stands what to do with such devices when filling the list of
vscsidevs on a vscsictrl. Is it up to the caller to inspect the state of
each vscsidev to decide what to do with it? Or should the function below
just skip the device right away?

I think in the current state the device would endup in d_config and in json.

Olaf


> +static void libxl__vscsi_fill_host(libxl__gc *gc,
> +                                   const char *devs_path,
> +                                   char **dev_dirs,
> +                                   unsigned int ndev_dirs,
> +                                   libxl_device_vscsi *v_hst)
> +{
> +    libxl_vscsi_dev *v_dev;
> +    bool parsed_ok;
> +    char *c, *p, *v, *s, *dev;
> +    unsigned int vscsi_dev_id;
> +    int i, r;
> +
> +    v_hst->vscsi_devs = libxl__malloc(NOGC, ndev_dirs * sizeof(*v_dev));
> +    v_hst->num_vscsi_devs = ndev_dirs;
> +    /* Fill each device connected to the host */
> +    for (i = 0; i < ndev_dirs; i++, dev_dirs++) {
> +        v_dev = v_hst->vscsi_devs + i;
> +        libxl_vscsi_dev_init(v_dev);
> +        parsed_ok = false;
> +        r = sscanf(*dev_dirs, "dev-%u", &vscsi_dev_id);
> +        if (r != 1) {
> +            LOG(ERROR, "expected dev-N, got '%s'", *dev_dirs);
> +            continue;
> +        }
> +
> +        dev = GCSPRINTF("%s/%s", devs_path, *dev_dirs);
> +        c = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/p-devname", dev));
> +        p = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/p-dev", dev));
> +        v = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/v-dev", dev));
> +        s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", dev));
> +        LOG(DEBUG, "%s/state is %s", dev, s);
> +        if (!(c && p && v && s)) {
> +            LOG(ERROR, "p-devname '%s' p-dev '%s' v-dev '%s'", c, p, v);
> +            continue;
> +        }
> +
> +        parsed_ok = vscsi_parse_pdev(gc, v_dev, c, p, v);
> +        switch (atoi(s)) {
> +            case XenbusStateUnknown:
> +            case XenbusStateInitialising:
> +            case XenbusStateInitWait:
> +            case XenbusStateInitialised:
> +            case XenbusStateConnected:
> +            case XenbusStateReconfiguring:
> +            case XenbusStateReconfigured:
> +                break;
> +            case XenbusStateClosing:
> +            case XenbusStateClosed:
> +                v_dev->remove = true;
> +                break;
> +        }