diff mbox

[v8,3/5] libxl: add support for vscsi

Message ID 1455205411-25460-4-git-send-email-olaf@aepfle.de (mailing list archive)
State New, archived
Headers show

Commit Message

Olaf Hering Feb. 11, 2016, 3:43 p.m. UTC
Port pvscsi support from xend to libxl:

 vscsi=['pdev,vdev{,options}']
 xl scsi-attach
 xl scsi-detach
 xl scsi-list

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 docs/man/xl.cfg.pod.5                |  55 +++
 docs/man/xl.pod.1                    |  18 +
 tools/libxl/Makefile                 |   2 +
 tools/libxl/libxl.c                  | 420 +++++++++++++++++++++
 tools/libxl/libxl.h                  |  35 ++
 tools/libxl/libxl_create.c           |   1 +
 tools/libxl/libxl_device.c           |   2 +
 tools/libxl/libxl_internal.h         |  19 +
 tools/libxl/libxl_types.idl          |  53 +++
 tools/libxl/libxl_types_internal.idl |   1 +
 tools/libxl/libxl_vscsi.c            | 615 +++++++++++++++++++++++++++++++
 tools/libxl/libxlu_vscsi.c           | 697 +++++++++++++++++++++++++++++++++++
 tools/libxl/libxlutil.h              |  18 +
 tools/libxl/xl.h                     |   3 +
 tools/libxl/xl_cmdimpl.c             | 208 ++++++++++-
 tools/libxl/xl_cmdtable.c            |  15 +
 16 files changed, 2161 insertions(+), 1 deletion(-)

Comments

Wei Liu Feb. 12, 2016, 5:27 p.m. UTC | #1
I notice you posted several enquiries in your previous patch, but I'm
not sure if any of them still requires answers because you seemed to
have posted answers to your own enquiries. I skip all of them and start
from this version.

There seems to be a lot of places in this patch where the lines are
longer than 80 characters. I'm not going to point them out one by one.
Can you fix as many of those as you can where sensible?

Because VSCSI is similar to PVUSB and PVUSB has mostly been reviewed, I
basically did a side by side comparison between these two. My goal is to
keep them as close as possible.

I'm also aware VSCSI is not complete the same as PVUSB.  If my comments
are made based on wrong assumptions, please let me know.

On Thu, Feb 11, 2016 at 03:43:29PM +0000, Olaf Hering wrote:
> Port pvscsi support from xend to libxl:
> 
>  vscsi=['pdev,vdev{,options}']
>  xl scsi-attach
>  xl scsi-detach
>  xl scsi-list
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  docs/man/xl.cfg.pod.5                |  55 +++
>  docs/man/xl.pod.1                    |  18 +
>  tools/libxl/Makefile                 |   2 +
>  tools/libxl/libxl.c                  | 420 +++++++++++++++++++++
>  tools/libxl/libxl.h                  |  35 ++
>  tools/libxl/libxl_create.c           |   1 +
>  tools/libxl/libxl_device.c           |   2 +
>  tools/libxl/libxl_internal.h         |  19 +
>  tools/libxl/libxl_types.idl          |  53 +++
>  tools/libxl/libxl_types_internal.idl |   1 +
>  tools/libxl/libxl_vscsi.c            | 615 +++++++++++++++++++++++++++++++
>  tools/libxl/libxlu_vscsi.c           | 697 +++++++++++++++++++++++++++++++++++
>  tools/libxl/libxlutil.h              |  18 +
>  tools/libxl/xl.h                     |   3 +
>  tools/libxl/xl_cmdimpl.c             | 208 ++++++++++-
>  tools/libxl/xl_cmdtable.c            |  15 +
>  16 files changed, 2161 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 8899f75..3b5b1c5 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -517,6 +517,61 @@ value is optional if this is a guest domain.
>  
>  =back
>  
> +=item B<vscsi=[ "VSCSI_SPEC_STRING", "VSCSI_SPEC_STRING", ...]>
> +
> +Specifies the PVSCSI devices to be provided to the guest. PVSCSI passes
> +SCSI devices from the backend domain to the guest.
> +
> +Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
> +'pdev' describes the physical device, preferable in a persistent format such as /dev/disk/by-*/*.

Small nit: please wrap this line a bit.

> +'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
> +'options' lists additional flags which a backend may recognize.
> +
> +The supported values for "pdev" and "options" depends on the backend driver used:
> +
> +=over 4
> +
> +=item B<Linux>
> +
> +=over 4
> +
> +=item C<pdev>
> +
> +The backend driver in the pvops kernel is part of the Linux-IO Target framework
> +(LIO). As such the SCSI devices have to be configured first with the tools
> +provided by this framework, such as a xen-scsiback aware targetcli. The "pdev"
> +in domU.cfg has to refer to a config item in that framework instead of the raw
> +device. Usually this is a WWN in the form of "na.WWN:LUN".
> +

naa

> +=item C<options>
> +
> +No options recognized.
> +
> +=back
> +
> +=item B<Linux based on classic Xen kernel>
> +
> +=over 4
> +
> +=item C<pdev>
> +
[...]
>  
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2bde0f5..7583b4b 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2059,6 +2059,417 @@ static int libxl__resolve_domid(libxl__gc *gc, const char *name,
>  }
>  
>  /******************************************************************************/
> +
> +static int libxl__device_vscsidev_backend_set_add(libxl__gc *gc,
> +                                               libxl_device_vscsidev *v,
> +                                               flexarray_t *back)

Indentation.

> +{
> +    int rc;
> +    char *dir;
> +    unsigned int hst, chn, tgt;
> +    unsigned long long lun;
> +
> +
> +    dir = GCSPRINTF("vscsi-devs/dev-%u", v->vscsidev_id);
> +    switch (v->pdev.type) {
> +        case LIBXL_VSCSI_PDEV_TYPE_WWN:
> +            flexarray_append_pair(back,
> +                                  GCSPRINTF("%s/p-dev", dir),
> +                                  v->pdev.u.wwn.m);
> +            break;
> +        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
> +            hst = v->pdev.u.hctl.m.hst;
> +            chn = v->pdev.u.hctl.m.chn;
> +            tgt = v->pdev.u.hctl.m.tgt;
> +            lun = v->pdev.u.hctl.m.lun;
> +            flexarray_append_pair(back,
> +                                  GCSPRINTF("%s/p-dev", dir),
> +                                  GCSPRINTF("%u:%u:%u:%llu", hst, chn, tgt, lun));
> +            break;
> +        case LIBXL_VSCSI_PDEV_TYPE_INVALID:

Please add /* fallthrough */ here, otherwise Coverity scan might
complain. :-/

> +        default:
> +            rc = ERROR_FAIL;
> +            goto out;
> +    }
> +    flexarray_append_pair(back,
> +                          GCSPRINTF("%s/p-devname", dir),
> +                          v->pdev.p_devname);
> +    hst = v->vdev.hst;
> +    chn = v->vdev.chn;
> +    tgt = v->vdev.tgt;
> +    lun = v->vdev.lun;
> +    flexarray_append_pair(back,
> +                          GCSPRINTF("%s/v-dev", dir),
> +                          GCSPRINTF("%u:%u:%u:%llu", hst, chn, tgt, lun));
> +    flexarray_append_pair(back,
> +                          GCSPRINTF("%s/state", dir),
> +                          GCSPRINTF("%d", XenbusStateInitialising));
> +    rc = 0;
> +out:
> +    return rc;
> +}
> +
> +static int libxl__device_vscsi_new_backend(libxl__egc *egc,
> +                                           libxl__ao_device *aodev,
> +                                           libxl_device_vscsictrl *vscsi,
> +                                           libxl_domain_config *d_config)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    int rc, i;
> +    flexarray_t *back;
> +    flexarray_t *front;
> +    libxl_device_vscsidev *v;
> +    xs_transaction_t t = XBT_NULL;
> +
> +    /* Prealloc key+value: 4 toplevel + 4 per device */
> +    i = 2 * (4 + (4 * vscsi->num_vscsidevs));
> +    back = flexarray_make(gc, i, 1);
> +    front = flexarray_make(gc, 2 * 2, 1);
> +
> +    flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", aodev->dev->domid));
> +    flexarray_append_pair(back, "online", "1");
> +    flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateInitialising));
> +    flexarray_append_pair(back, "feature-host",
> +                          libxl_defbool_val(vscsi->scsi_raw_cmds) ?
> +                          "1" : "0");
> +
> +    flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", vscsi->backend_domid));
> +    flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
> +

Wrap long lines please.

> +    for (i = 0; i < vscsi->num_vscsidevs; i++) {
> +        v = vscsi->vscsidevs + i;
> +        rc = libxl__device_vscsidev_backend_set_add(gc, v, back);
> +        if (rc) return rc;

goto out please.

> +    }
> +
> +    for (;;) {
> +        rc = libxl__xs_transaction_start(gc, &t);
> +        if (rc) goto out;
> +
> +        rc = libxl__device_exists(gc, t, aodev->dev);
> +        if (rc < 0) goto out;
> +        if (rc == 1) {              /* already exists in xenstore */
> +            LOG(ERROR, "device already exists in xenstore");
> +            rc = ERROR_DEVICE_EXISTS;
> +            goto out;
> +        }
> +
> +        if (aodev->update_json) {
> +            rc = libxl__set_domain_configuration(gc, aodev->dev->domid, d_config);
> +            if (rc) goto out;
> +        }
> +
> +        libxl__device_generic_add(gc, t, aodev->dev,
> +                                  libxl__xs_kvs_of_flexarray(gc, back,
> +                                                             back->count),
> +                                  libxl__xs_kvs_of_flexarray(gc, front,
> +                                                             front->count),
> +                                  NULL);
> +
> +        rc = libxl__xs_transaction_commit(gc, &t);
> +        if (!rc) break;
> +        if (rc < 0) goto out;
> +    }
> +
> +    libxl__wait_device_connection(egc, aodev);
> +    rc = 0;
> +
> +out:
> +    libxl__xs_transaction_abort(gc, &t);
> +    return rc;
> +}
> +
[...]
> +}
> +void libxl__device_vscsictrl_add(libxl__egc *egc, uint32_t domid,
> +                                 libxl_device_vscsictrl *vscsi,
> +                                 libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    libxl__device *device;
> +    const char *be_path;
> +    unsigned int be_dirs = 0;
> +    int rc;
> +    libxl_domain_config d_config;
> +    libxl_device_vscsictrl vscsi_saved;
> +    libxl__domain_userdata_lock *lock = NULL;
> +
> +    libxl_domain_config_init(&d_config);
> +
> +    libxl_device_vscsictrl_init(&vscsi_saved);
> +    libxl_device_vscsictrl_copy(CTX, &vscsi_saved, vscsi);
> +
> +    if (vscsi->devid == -1) {
> +        LOGE(ERROR, "new vscsictrl for domid %u has uninitialized devid", domid);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    GCNEW(device);
> +    rc = libxl__device_from_vscsictrl(gc, domid, vscsi, device);
> +    if (rc) goto out;
> +
> +    aodev->dev = device;
> +    be_path = libxl__device_backend_path(gc, aodev->dev);
> +
> +    rc = libxl__vscsi_assign_vscsidev_ids(gc, domid, &vscsi_saved);
> +    if (rc) goto out;
> +
> +    if (aodev->update_json) {
> +        lock = libxl__lock_domain_userdata(gc, domid);
> +        if (!lock) {
> +            rc = ERROR_LOCK_FAIL;
> +            goto out;
> +        }
> +
> +        rc = libxl__get_domain_configuration(gc, domid, &d_config);
> +        if (rc) goto out;
> +
> +        /* Replace or append the copy to the domain config */
> +        DEVICE_ADD(vscsictrl, vscsictrls, domid, &vscsi_saved, COMPARE_VSCSI, &d_config);
> +    }
> +
> +    if (libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs)) {
> +        rc = libxl__device_vscsi_reconfigure_add(egc, aodev, &vscsi_saved, &d_config, be_path);
> +        if (rc)
> +            goto out;
> +        /* Notify that this is done */
> +        aodev->callback(egc, aodev);

Wouldn't it be better if you call aodev->callback unconditionally at the
end of this function? BTW you seem to have forgotten to set aodev->rc.

> +    } else {
> +        rc = libxl__device_vscsi_new_backend(egc, aodev, &vscsi_saved, &d_config);
> +        if (rc)
> +            goto out;
> +    }
> +
> +    rc = 0;
> +out:
> +    if (lock) libxl__unlock_domain_userdata(lock);
> +    libxl_device_vscsictrl_dispose(&vscsi_saved);
> +    libxl_domain_config_dispose(&d_config);
> +    aodev->rc = rc;
> +    if (rc) aodev->callback(egc, aodev);
> +    return;
> +}
> +
> +
[...]
> +/* Virtual SCSI */
> +int libxl_device_vscsictrl_add(libxl_ctx *ctx, uint32_t domid,
> +                               libxl_device_vscsictrl *vscsi,
> +                               const libxl_asyncop_how *ao_how)
> +                               LIBXL_EXTERNAL_CALLERS_ONLY;
> +int libxl_device_vscsictrl_remove(libxl_ctx *ctx, uint32_t domid,
> +                                  libxl_device_vscsictrl *vscsi,
> +                                  const libxl_asyncop_how *ao_how)
> +                                  LIBXL_EXTERNAL_CALLERS_ONLY;
> +int libxl_device_vscsictrl_destroy(libxl_ctx *ctx, uint32_t domid,
> +                                   libxl_device_vscsictrl *vscsi,
> +                                   const libxl_asyncop_how *ao_how)
> +                                   LIBXL_EXTERNAL_CALLERS_ONLY;
> +
> +libxl_device_vscsictrl *libxl_device_vscsictrl_list(libxl_ctx *ctx, uint32_t domid, int *num);
> +int libxl_device_vscsictrl_getinfo(libxl_ctx *ctx, uint32_t domid,
> +                                   libxl_device_vscsictrl *vscsictrl,
> +                                   libxl_device_vscsidev *vscsidev,
> +                                   libxl_vscsiinfo *vscsiinfo);
> +/* Remove vscsidev connected to vscsictrl */
> +int libxl_device_vscsidev_remove(libxl_ctx *ctx, uint32_t domid,
> +                                 libxl_device_vscsidev *dev,
> +                                 const libxl_asyncop_how *ao_how)
> +                                 LIBXL_EXTERNAL_CALLERS_ONLY;

Where is libxl_device_vscsidev_add?

> +void libxl_device_vscsictrl_append_vscsidev(libxl_ctx *ctx,
> +                                            libxl_device_vscsictrl *ctrl,
> +                                            libxl_device_vscsidev *dev);
> +
>  /* Virtual TPMs */
>  int libxl_device_vtpm_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vtpm *vtpm,
>                            const libxl_asyncop_how *ao_how)
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index e491d83..64f3c70 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -1166,6 +1166,7 @@ static void domcreate_rebuild_done(libxl__egc *egc,
>      libxl__multidev_begin(ao, &dcs->multidev);
>      dcs->multidev.callback = domcreate_launch_dm;
>      libxl__add_disks(egc, ao, domid, d_config, &dcs->multidev);
> +    libxl__add_vscsictrls(egc, ao, domid, d_config, &dcs->multidev);

This is because you need vscsi disk early?


>      libxl__multidev_prepared(egc, &dcs->multidev, 0);
>  
>      return;
[...]
>  libxl__console_backend = Enumeration("console_backend", [
> diff --git a/tools/libxl/libxl_vscsi.c b/tools/libxl/libxl_vscsi.c
[...]
> +static bool libxl__vscsi_fill_dev(libxl__gc *gc,
> +                                  xs_transaction_t t,
> +                                  const char *devs_path,
> +                                  const char *dev_dir,
> +                                  libxl_device_vscsidev *dev)
> +{
> +    char *path, *c, *p, *v, *s;
> +    unsigned int devid;
> +    int r;
> +
> +    r = sscanf(dev_dir, "dev-%u", &devid);
> +    if (r != 1) {
> +        LOG(ERROR, "expected dev-N, got '%s'", dev_dir);
> +        return false;
> +    }
> +    dev->vscsidev_id = devid;
> +
> +    path = GCSPRINTF("%s/%s", devs_path, dev_dir);
> +    c = libxl__xs_read(gc, t, GCSPRINTF("%s/p-devname", path));
> +    p = libxl__xs_read(gc, t, GCSPRINTF("%s/p-dev", path));
> +    v = libxl__xs_read(gc, t, GCSPRINTF("%s/v-dev", path));
> +    s = libxl__xs_read(gc, t, GCSPRINTF("%s/state", path));
> +    LOG(DEBUG, "%s/state is %s", path, s);
> +    if (!(c && p && v && s)) {
> +        LOG(ERROR, "p-devname '%s' p-dev '%s' v-dev '%s'", c, p, v);
> +        return false;
> +    }
> +
> +    if (!vscsi_parse_pdev(gc, dev, c, p, v)) {
> +        LOG(ERROR, "failed to parse %s: %s %s %s %s", path, c, p, v, s);
> +        return false;
> +    }
> +
> +    switch (atoi(s)) {
> +        case XenbusStateUnknown:
> +        case XenbusStateInitialising:
> +        case XenbusStateInitWait:
> +        case XenbusStateInitialised:
> +        case XenbusStateConnected:
> +        case XenbusStateReconfiguring:
> +        case XenbusStateReconfigured:
> +            break;
> +        case XenbusStateClosing:
> +        case XenbusStateClosed:
> +            LOG(DEBUG, "unexpected state in %s: %s", path, s);
> +            break;
> +    }
> +

This doesn't seem useful in the context of this function.

> +    return true;
> +}
> +
> +static bool libxl__vscsi_fill_ctrl(libxl__gc *gc,
> +                                   xs_transaction_t t,
> +                                   const char *fe_path,
> +                                   const char *dir,
> +                                   libxl_device_vscsictrl *ctrl)
> +{
> +    libxl_device_vscsidev dev;
> +    char *tmp, *be_path, *devs_path;
> +    char **dev_dirs;
> +    unsigned int ndev_dirs, dev_dir;
> +    bool parsed_ok;
> +
> +    ctrl->devid = atoi(dir);
> +
> +    be_path = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend", fe_path, dir));
> +    /* FIXME what if xenstore is broken? */

I'm not sure what kind of "broken" you referred to, but it is most
likely we can't recover from such state.

It's reasonable to assume xenstore is robust.

> +    if (!be_path) {
> +        libxl_defbool_set(&ctrl->scsi_raw_cmds, false);
> +        return false;
> +    }
> +
> +    tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend-id", fe_path, dir));
> +    /* FIXME what if xenstore is broken? */
> +    if (tmp)
> +        ctrl->backend_domid = atoi(tmp);
> +
> +    parsed_ok = false;
> +    tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/feature-host", be_path));
> +    if (tmp)
> +        parsed_ok = atoi(tmp) != 0;
> +    libxl_defbool_set(&ctrl->scsi_raw_cmds, parsed_ok);
> +
> +    parsed_ok = true;
> +    devs_path = GCSPRINTF("%s/vscsi-devs", be_path);
> +    dev_dirs = libxl__xs_directory(gc, t, devs_path, &ndev_dirs);
> +    for (dev_dir = 0; dev_dirs && dev_dir < ndev_dirs; dev_dir++) {
> +        libxl_device_vscsidev_init(&dev);
> +        parsed_ok = libxl__vscsi_fill_dev(gc, t, devs_path, dev_dirs[dev_dir], &dev);
> +        if (parsed_ok == true)
> +            libxl_device_vscsictrl_append_vscsidev(CTX, ctrl, &dev);
> +        libxl_device_vscsidev_dispose(&dev);
> +        if (parsed_ok == false)
> +            break;
> +    }
> +
> +    return parsed_ok;
> +}
> +
> +int libxl__vscsi_collect_ctrls(libxl__gc *gc,
> +                               uint32_t domid,
> +                               libxl_device_vscsictrl **ctrls,
> +                               int *num)
> +{
> +    xs_transaction_t t = XBT_NULL;
> +    libxl_device_vscsictrl ctrl;
> +	char *fe_path;

Hard tab here.

[...]
> +static void libxl__device_vscsidev_rm(libxl__egc *egc,
> +                                       libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    libxl__vscsidev_rm *vscsidev_rm = CONTAINER_OF(aodev, *vscsidev_rm, aodev);
> +    char *state_path;
> +    int rc, be_wait;
> +
> +    vscsidev_rm->be_path = libxl__device_backend_path(gc, aodev->dev);
> +    state_path = GCSPRINTF("%s/state", vscsidev_rm->be_path);
> +
> +    rc = libxl__device_vscsi_reconfigure_rm(aodev, state_path, &be_wait);
> +    if (rc) goto out;
> +
> +    rc = libxl__ev_devstate_wait(ao, &aodev->backend_ds,
> +                                 libxl__device_vscsidev_rm_be,
> +                                 state_path, be_wait,
> +                                 LIBXL_INIT_TIMEOUT * 1000);

Should be LIBXL_DESTROY_TIMEOUT?

> +    if (rc) {
> +        LOG(ERROR, "unable to wait for %s", state_path);
> +        goto out;
> +    }
> +
> +    return;
> +
> +out:
> +    aodev->rc = rc;
> +    /* Notify that this is done */
> +    aodev->callback(egc, aodev);
> +}
> +
[...]
> diff --git a/tools/libxl/libxlu_vscsi.c b/tools/libxl/libxlu_vscsi.c

I skipped this for now. But please be aware libxlu functions are
considered to be stable API, so please don't expose more functions than
necessary.

> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index bdab125..877c695 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -89,6 +89,9 @@ int main_channellist(int argc, char **argv);
>  int main_blockattach(int argc, char **argv);
>  int main_blocklist(int argc, char **argv);
>  int main_blockdetach(int argc, char **argv);
> +int main_vscsiattach(int argc, char **argv);
> +int main_vscsilist(int argc, char **argv);
> +int main_vscsidetach(int argc, char **argv);

What about other commands? Looking at PVUSB series:

+int main_usbctrl_attach(int argc, char **argv);
+int main_usbctrl_detach(int argc, char **argv);
+int main_usbdev_attach(int argc, char **argv);
+int main_usbdev_detach(int argc, char **argv);
+int main_usblist(int argc, char **argv);

so we should be able to attach / detach both controller and specific
device?

Wei.
Olaf Hering Feb. 12, 2016, 6:24 p.m. UTC | #2
On Fri, Feb 12, Wei Liu wrote:

> There seems to be a lot of places in this patch where the lines are
> longer than 80 characters. I'm not going to point them out one by one.
> Can you fix as many of those as you can where sensible?

Many lines are slightly longer than 80 chars already, as can be seen
with 'set colorcolumn=80'. I already wrapped most of them in my patch.

> Because VSCSI is similar to PVUSB and PVUSB has mostly been reviewed, I
> basically did a side by side comparison between these two. My goal is to
> keep them as close as possible.

Unfortnuately they are not comparable. While I have now split the logic
into ctrl and dev, the code using a ctrl is just the one doing the
xenstore device work.

> > +    flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", vscsi->backend_domid));
> > +    flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
> Wrap long lines please.

Done. I dont like the result.


> > +    if (libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs)) {
> > +        rc = libxl__device_vscsi_reconfigure_add(egc, aodev, &vscsi_saved, &d_config, be_path);
> > +        if (rc)
> > +            goto out;
> > +        /* Notify that this is done */
> > +        aodev->callback(egc, aodev);
> 
> Wouldn't it be better if you call aodev->callback unconditionally at the
> end of this function? BTW you seem to have forgotten to set aodev->rc.

I have to rework this code, its still using libxl__wait_for_backend.
This is already fixed in the detach case. Somehow I was only
concentrating on the detach case this week, and did not notice this.


> > +/* Remove vscsidev connected to vscsictrl */
> > +int libxl_device_vscsidev_remove(libxl_ctx *ctx, uint32_t domid,
> > +                                 libxl_device_vscsidev *dev,
> > +                                 const libxl_asyncop_how *ao_how)
> > +                                 LIBXL_EXTERNAL_CALLERS_ONLY;
> 
> Where is libxl_device_vscsidev_add?

This is done in scsi-attach, via libxl_device_vscsictrl_add. Its not
even, compared to the scsi-detach case. I will see if that can be
changed.


> > +    libxl__add_vscsictrls(egc, ao, domid, d_config, &dcs->multidev);
> 
> This is because you need vscsi disk early?

I think yes, DEFINE_DEVICES_ADD has to be used somewhere.


> > +        case XenbusStateClosing:
> > +        case XenbusStateClosed:
> > +            LOG(DEBUG, "unexpected state in %s: %s", path, s);
> > +            break;

> This doesn't seem useful in the context of this function.

A few lines up "state" is already printed. Looks like the whole state
checking can be removed.

> > +    return true;
> > +}
> > +
> > +static bool libxl__vscsi_fill_ctrl(libxl__gc *gc,
> > +                                   xs_transaction_t t,
> > +                                   const char *fe_path,
> > +                                   const char *dir,
> > +                                   libxl_device_vscsictrl *ctrl)
> > +{
> > +    libxl_device_vscsidev dev;
> > +    char *tmp, *be_path, *devs_path;
> > +    char **dev_dirs;
> > +    unsigned int ndev_dirs, dev_dir;
> > +    bool parsed_ok;
> > +
> > +    ctrl->devid = atoi(dir);
> > +
> > +    be_path = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend", fe_path, dir));
> > +    /* FIXME what if xenstore is broken? */
> 
> I'm not sure what kind of "broken" you referred to, but it is most
> likely we can't recover from such state.
> 
> It's reasonable to assume xenstore is robust.

I will remove the FIXME comments ...

> > +    if (!be_path) {
> > +        libxl_defbool_set(&ctrl->scsi_raw_cmds, false);
> > +        return false;
> > +    }
> > +
> > +    tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend-id", fe_path, dir));
> > +    /* FIXME what if xenstore is broken? */

... and I will return failure here as well.

> > +    if (tmp)
> > +        ctrl->backend_domid = atoi(tmp);


> > +int main_vscsiattach(int argc, char **argv);
> > +int main_vscsilist(int argc, char **argv);
> > +int main_vscsidetach(int argc, char **argv);
> 
> What about other commands? Looking at PVUSB series:
> 
> +int main_usbctrl_attach(int argc, char **argv);
> +int main_usbctrl_detach(int argc, char **argv);
> +int main_usbdev_attach(int argc, char **argv);
> +int main_usbdev_detach(int argc, char **argv);
> +int main_usblist(int argc, char **argv);
> 
> so we should be able to attach / detach both controller and specific
> device?

There are no empty scsi controllers, just controllers with at least one
device. IMO its not useful to define empty controllers, what would be
the point? While toying around I noticed that removing all vscsidevs and
leaving the vscsictrl in xenstore the frontend preserved its SCSI
controller entry in sysfs.

Olaf
Wei Liu Feb. 15, 2016, 3:16 p.m. UTC | #3
On Fri, Feb 12, 2016 at 07:24:59PM +0100, Olaf Hering wrote:
[...]
> 
> > > +    if (libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs)) {
> > > +        rc = libxl__device_vscsi_reconfigure_add(egc, aodev, &vscsi_saved, &d_config, be_path);
> > > +        if (rc)
> > > +            goto out;
> > > +        /* Notify that this is done */
> > > +        aodev->callback(egc, aodev);
> > 
> > Wouldn't it be better if you call aodev->callback unconditionally at the
> > end of this function? BTW you seem to have forgotten to set aodev->rc.
> 
> I have to rework this code, its still using libxl__wait_for_backend.
> This is already fixed in the detach case. Somehow I was only
> concentrating on the detach case this week, and did not notice this.
>

OK.

> 
> > > +/* Remove vscsidev connected to vscsictrl */
> > > +int libxl_device_vscsidev_remove(libxl_ctx *ctx, uint32_t domid,
> > > +                                 libxl_device_vscsidev *dev,
> > > +                                 const libxl_asyncop_how *ao_how)
> > > +                                 LIBXL_EXTERNAL_CALLERS_ONLY;
> > 
> > Where is libxl_device_vscsidev_add?
> 
> This is done in scsi-attach, via libxl_device_vscsictrl_add. Its not
> even, compared to the scsi-detach case. I will see if that can be
> changed.
> 
> 
> > > +    libxl__add_vscsictrls(egc, ao, domid, d_config, &dcs->multidev);
> > 
> > This is because you need vscsi disk early?
> 
> I think yes, DEFINE_DEVICES_ADD has to be used somewhere.
>

I'm confused. You're joking, right? "Has to be used somewhere" is not
a justification for having it in this particular place.

> 
> 
> > > +int main_vscsiattach(int argc, char **argv);
> > > +int main_vscsilist(int argc, char **argv);
> > > +int main_vscsidetach(int argc, char **argv);
> > 
> > What about other commands? Looking at PVUSB series:
> > 
> > +int main_usbctrl_attach(int argc, char **argv);
> > +int main_usbctrl_detach(int argc, char **argv);
> > +int main_usbdev_attach(int argc, char **argv);
> > +int main_usbdev_detach(int argc, char **argv);
> > +int main_usblist(int argc, char **argv);
> > 
> > so we should be able to attach / detach both controller and specific
> > device?
> 
> There are no empty scsi controllers, just controllers with at least one
> device. IMO its not useful to define empty controllers, what would be
> the point?

If it is mandated by hardware that empty scsi controller doesn't
exist, that's of course fine. But I don't think it is mandated in
reality? I can have no disk attached to a controller and that should
be fine.

> While toying around I noticed that removing all vscsidevs and
> leaving the vscsictrl in xenstore the frontend preserved its SCSI
> controller entry in sysfs.
>

That's what I would expect, too.

Wei.

> Olaf
Olaf Hering Feb. 15, 2016, 3:24 p.m. UTC | #4
On Mon, Feb 15, Wei Liu wrote:

> > I think yes, DEFINE_DEVICES_ADD has to be used somewhere.
> I'm confused. You're joking, right? "Has to be used somewhere" is not
> a justification for having it in this particular place.

What would be the appropriate place? I think its there since I started
working on it at 4.4 times.

Olaf
Wei Liu Feb. 15, 2016, 3:52 p.m. UTC | #5
On Mon, Feb 15, 2016 at 04:24:38PM +0100, Olaf Hering wrote:
> On Mon, Feb 15, Wei Liu wrote:
> 
> > > I think yes, DEFINE_DEVICES_ADD has to be used somewhere.
> > I'm confused. You're joking, right? "Has to be used somewhere" is not
> > a justification for having it in this particular place.
> 
> What would be the appropriate place? I think its there since I started
> working on it at 4.4 times.
>

Depending on what you want to do.

For example, if you grep for libxl__add_nics (defined with that macro)
you can see it being called in libxl_create.c at a later point and
libxl_dm.c when configuring backend QEMU for stubdom.

So there is two aspects of this:

1. At which point do you want the attachment to happen?
2. How should this device attachment be done?

For #1, you need to look on a higher level than that function you
modified in the patch. In you patch, the attachment is done at the
same time disks are attached before creating device model.  So my
first reply was to ask why you would that to happen at that
particular point.

For #2, in you patch you lump disk and vscsi together in one step. I
think multidev can handle your changes just fine. But I'm not sure if
that's the best practice -- for example, if either of them fails, you
can only get error message "unable to add disk devices". You can
introduce yet another step in the callback chain.

Wei.

> Olaf
Ian Jackson Feb. 15, 2016, 5:09 p.m. UTC | #6
Wei Liu writes ("Re: [PATCH v8 3/5] libxl: add support for vscsi"):
> If it is mandated by hardware that empty scsi controller doesn't
> exist, that's of course fine. But I don't think it is mandated in
> reality? I can have no disk attached to a controller and that should
> be fine.

For the avoidance of any doubt: there is no difficulty with "empty"
scsi controllers.  Such things are entirely normal when talking about
physical controllers.

One reason you might define a virtual controller with no devices yet
is so that you have a stable and pre-expected device path for any
actual targets you choose to hotplug later.

That doesn't mean that the default workflow for a user should be for
them to explicitly have to create an empty controller and then hotplug
the devices into it.  But there is no inherent semantic problem with
allowing an empty controller to exist.

Allowing empty controllers to exist is likely to considerably simplify
the error handling.

Ian.
Olaf Hering Feb. 16, 2016, 3:23 p.m. UTC | #7
On Mon, Feb 15, Ian Jackson wrote:

> One reason you might define a virtual controller with no devices yet
> is so that you have a stable and pre-expected device path for any
> actual targets you choose to hotplug later.

Would it be acceptable to reuse the devid as the "group index"?
The various vdev in vscsi=['pdev,vdev'] will be assigned to the same
vscsictrl if the host part in host:chn:target:lun matches. Right now an
empty vscsictrl has no property to store the "host" part. This could be
handled by either reusing devid, or by introducing a new xenstore
property such as "libxl_vscsictrl_index". The value itself has no
meaning other than being an index or label.

Olaf
Ian Jackson Feb. 16, 2016, 5:48 p.m. UTC | #8
Olaf Hering writes ("Re: [PATCH v8 3/5] libxl: add support for vscsi"):
> On Mon, Feb 15, Ian Jackson wrote:
> > One reason you might define a virtual controller with no devices yet
> > is so that you have a stable and pre-expected device path for any
> > actual targets you choose to hotplug later.
> 
> Would it be acceptable to reuse the devid as the "group index"?
> The various vdev in vscsi=['pdev,vdev'] will be assigned to the same
> vscsictrl if the host part in host:chn:target:lun matches. Right now an
> empty vscsictrl has no property to store the "host" part. This could be
> handled by either reusing devid, or by introducing a new xenstore
> property such as "libxl_vscsictrl_index". The value itself has no
> meaning other than being an index or label.

I haven't been following this design in detail, but: why is the
vscictrl `host' number not part of the xenstore path for the
controller, which in turn contains the devices ?

Ian.
Olaf Hering Feb. 17, 2016, 11:17 a.m. UTC | #9
On Tue, Feb 16, Ian Jackson wrote:

> Olaf Hering writes ("Re: [PATCH v8 3/5] libxl: add support for vscsi"):
> > On Mon, Feb 15, Ian Jackson wrote:
> > > One reason you might define a virtual controller with no devices yet
> > > is so that you have a stable and pre-expected device path for any
> > > actual targets you choose to hotplug later.
> > 
> > Would it be acceptable to reuse the devid as the "group index"?
> > The various vdev in vscsi=['pdev,vdev'] will be assigned to the same
> > vscsictrl if the host part in host:chn:target:lun matches. Right now an
> > empty vscsictrl has no property to store the "host" part. This could be
> > handled by either reusing devid, or by introducing a new xenstore
> > property such as "libxl_vscsictrl_index". The value itself has no
> > meaning other than being an index or label.
> 
> I haven't been following this design in detail, but: why is the
> vscictrl `host' number not part of the xenstore path for the
> controller, which in turn contains the devices ?

Thats exactly what I'm asking, see patch #4 of this series. Each
libxl__device has a devid, which is essentially just an unique counter.
A vscsictrl is a libxl__device, a vscsidev is something below a
vscsictrl. To describe which vscsidev belongs to which vscsictrl the
hctl notation is used. The frontend uses 'ctl', the toolstack 'h'.
The hctl is stored in vscsi-devs/N/v-dev. An empty vscsictrl has no
vscsidev and as a result no 'v-dev' to indicate which 'h' it represents.

During scsi-attach the code has to collect a list of existing
vscsictrls. If an existing vscslctrl with the requested group number 'h'
exists its clear where the new vscsidev belongs to. Otherwise a new
empty vscsictrl has to be created. It needs some property to hold 'h'.
This can be either the devid, or a new xenstore property.


I will use a new xenstore property.

Olaf
diff mbox

Patch

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8899f75..3b5b1c5 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -517,6 +517,61 @@  value is optional if this is a guest domain.
 
 =back
 
+=item B<vscsi=[ "VSCSI_SPEC_STRING", "VSCSI_SPEC_STRING", ...]>
+
+Specifies the PVSCSI devices to be provided to the guest. PVSCSI passes
+SCSI devices from the backend domain to the guest.
+
+Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
+'pdev' describes the physical device, preferable in a persistent format such as /dev/disk/by-*/*.
+'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
+'options' lists additional flags which a backend may recognize.
+
+The supported values for "pdev" and "options" depends on the backend driver used:
+
+=over 4
+
+=item B<Linux>
+
+=over 4
+
+=item C<pdev>
+
+The backend driver in the pvops kernel is part of the Linux-IO Target framework
+(LIO). As such the SCSI devices have to be configured first with the tools
+provided by this framework, such as a xen-scsiback aware targetcli. The "pdev"
+in domU.cfg has to refer to a config item in that framework instead of the raw
+device. Usually this is a WWN in the form of "na.WWN:LUN".
+
+=item C<options>
+
+No options recognized.
+
+=back
+
+=item B<Linux based on classic Xen kernel>
+
+=over 4
+
+=item C<pdev>
+
+The dom0 device in either /dev/scsidev or pHOST:CHANNEL:TARGET:LUN notation.
+
+It's recommended to use persistent names "/dev/disk/by-*/*" to refer to a "pdev".
+The toolstack will translate this internally to "h:c:t:l" notation, which is how
+the backend driver will access the device. Using the "h:c:t:l" notation for
+"pdev" in domU.cfg is discouraged because this value will change across reboots,
+depending on the detection order in the OS.
+
+=item C<options>
+
+Currently only the option value "feature-host" is recognized. SCSI command
+emulation in backend driver is bypassed when "feature-host" is specified.
+
+=back
+
+=back
+
 =item B<vfb=[ "VFB_SPEC_STRING", "VFB_SPEC_STRING", ...]>
 
 Specifies the paravirtual framebuffer devices which should be supplied
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 4279c7c..0674149 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1293,6 +1293,24 @@  List virtual trusted platform modules for a domain.
 
 =back
 
+=head2 PVSCSI DEVICES
+
+=over 4
+
+=item B<scsi-attach> I<domain-id> I<pdev> I<vdev>,I<[feature-host]>
+
+Creates a new vscsi device in the domain specified by I<domain-id>.
+
+=item B<scsi-detach> I<domain-id> I<vdev>
+
+Removes the vscsi device from domain specified by I<domain-id>.
+
+=item B<scsi-list> I<domain-id> I<[domain-id] ...>
+
+List vscsi devices for the domain specified by I<domain-id>.
+
+=back
+
 =head1 PCI PASS-THROUGH
 
 =over 4
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 620720e..a71abf2 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -99,6 +99,7 @@  endif
 LIBXL_LIBS += -lyajl
 
 LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
+			libxl_vscsi.o \
 			libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
 			libxl_internal.o libxl_utils.o libxl_uuid.o \
 			libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o \
@@ -141,6 +142,7 @@  AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h _paths.h \
 AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
 AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
 LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
+	libxlu_vscsi.o \
 	libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o
 $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2bde0f5..7583b4b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2059,6 +2059,417 @@  static int libxl__resolve_domid(libxl__gc *gc, const char *name,
 }
 
 /******************************************************************************/
+
+static int libxl__device_vscsidev_backend_set_add(libxl__gc *gc,
+                                               libxl_device_vscsidev *v,
+                                               flexarray_t *back)
+{
+    int rc;
+    char *dir;
+    unsigned int hst, chn, tgt;
+    unsigned long long lun;
+
+
+    dir = GCSPRINTF("vscsi-devs/dev-%u", v->vscsidev_id);
+    switch (v->pdev.type) {
+        case LIBXL_VSCSI_PDEV_TYPE_WWN:
+            flexarray_append_pair(back,
+                                  GCSPRINTF("%s/p-dev", dir),
+                                  v->pdev.u.wwn.m);
+            break;
+        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
+            hst = v->pdev.u.hctl.m.hst;
+            chn = v->pdev.u.hctl.m.chn;
+            tgt = v->pdev.u.hctl.m.tgt;
+            lun = v->pdev.u.hctl.m.lun;
+            flexarray_append_pair(back,
+                                  GCSPRINTF("%s/p-dev", dir),
+                                  GCSPRINTF("%u:%u:%u:%llu", hst, chn, tgt, lun));
+            break;
+        case LIBXL_VSCSI_PDEV_TYPE_INVALID:
+        default:
+            rc = ERROR_FAIL;
+            goto out;
+    }
+    flexarray_append_pair(back,
+                          GCSPRINTF("%s/p-devname", dir),
+                          v->pdev.p_devname);
+    hst = v->vdev.hst;
+    chn = v->vdev.chn;
+    tgt = v->vdev.tgt;
+    lun = v->vdev.lun;
+    flexarray_append_pair(back,
+                          GCSPRINTF("%s/v-dev", dir),
+                          GCSPRINTF("%u:%u:%u:%llu", hst, chn, tgt, lun));
+    flexarray_append_pair(back,
+                          GCSPRINTF("%s/state", dir),
+                          GCSPRINTF("%d", XenbusStateInitialising));
+    rc = 0;
+out:
+    return rc;
+}
+
+static int libxl__device_vscsi_new_backend(libxl__egc *egc,
+                                           libxl__ao_device *aodev,
+                                           libxl_device_vscsictrl *vscsi,
+                                           libxl_domain_config *d_config)
+{
+    STATE_AO_GC(aodev->ao);
+    int rc, i;
+    flexarray_t *back;
+    flexarray_t *front;
+    libxl_device_vscsidev *v;
+    xs_transaction_t t = XBT_NULL;
+
+    /* Prealloc key+value: 4 toplevel + 4 per device */
+    i = 2 * (4 + (4 * vscsi->num_vscsidevs));
+    back = flexarray_make(gc, i, 1);
+    front = flexarray_make(gc, 2 * 2, 1);
+
+    flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", aodev->dev->domid));
+    flexarray_append_pair(back, "online", "1");
+    flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateInitialising));
+    flexarray_append_pair(back, "feature-host",
+                          libxl_defbool_val(vscsi->scsi_raw_cmds) ?
+                          "1" : "0");
+
+    flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", vscsi->backend_domid));
+    flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
+
+    for (i = 0; i < vscsi->num_vscsidevs; i++) {
+        v = vscsi->vscsidevs + i;
+        rc = libxl__device_vscsidev_backend_set_add(gc, v, back);
+        if (rc) return rc;
+    }
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        rc = libxl__device_exists(gc, t, aodev->dev);
+        if (rc < 0) goto out;
+        if (rc == 1) {              /* already exists in xenstore */
+            LOG(ERROR, "device already exists in xenstore");
+            rc = ERROR_DEVICE_EXISTS;
+            goto out;
+        }
+
+        if (aodev->update_json) {
+            rc = libxl__set_domain_configuration(gc, aodev->dev->domid, d_config);
+            if (rc) goto out;
+        }
+
+        libxl__device_generic_add(gc, t, aodev->dev,
+                                  libxl__xs_kvs_of_flexarray(gc, back,
+                                                             back->count),
+                                  libxl__xs_kvs_of_flexarray(gc, front,
+                                                             front->count),
+                                  NULL);
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    libxl__wait_device_connection(egc, aodev);
+    rc = 0;
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+static int libxl__device_vscsi_reconfigure_add(libxl__egc *egc,
+                                               libxl__ao_device *aodev,
+                                               libxl_device_vscsictrl *vscsi,
+                                               libxl_domain_config *d_config,
+                                               const char *be_path)
+{
+    STATE_AO_GC(aodev->ao);
+    int rc, i, be_state, be_wait;
+    char *dev_path, *state_path, *state_val;
+    flexarray_t *back;
+    libxl_device_vscsidev *v;
+    xs_transaction_t t = XBT_NULL;
+    bool do_reconfigure = false;
+
+    /* Prealloc key+value: 1 toplevel + 4 per device */
+    i = 2 * (1 + (4 * vscsi->num_vscsidevs));
+    back = flexarray_make(gc, i, 1);
+
+    state_path = GCSPRINTF("%s/state", be_path);
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        state_val = libxl__xs_read(gc, t, state_path);
+        LOG(DEBUG, "%s is %s", state_path, state_val);
+        if (!state_val) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+
+        be_state = atoi(state_val);
+        switch (be_state) {
+            case XenbusStateUnknown:
+            case XenbusStateInitialising:
+            case XenbusStateClosing:
+            case XenbusStateClosed:
+            default:
+                /* The backend is in a bad state */
+                rc = ERROR_FAIL;
+                goto out;
+            case XenbusStateInitialised:
+            case XenbusStateReconfiguring:
+            case XenbusStateReconfigured:
+                /* Backend is still busy, caller has to retry */
+                rc = ERROR_NOT_READY;
+                goto out;
+            case XenbusStateInitWait:
+                /* The frontend did not connect yet */
+                be_wait = XenbusStateInitWait;
+                do_reconfigure = false;
+                break;
+            case XenbusStateConnected:
+                /* The backend can handle reconfigure */
+                be_wait = XenbusStateConnected;
+                flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateReconfiguring));
+                do_reconfigure = true;
+                break;
+        }
+
+        /* Append new vscsidev or skip existing  */
+        for (i = 0; i < vscsi->num_vscsidevs; i++) {
+            unsigned int nb = 0;
+            v = vscsi->vscsidevs + i;
+            dev_path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsidev_id);
+            if (libxl__xs_directory(gc, XBT_NULL, dev_path, &nb)) {
+                /* FIXME Sanity check */
+                LOG(DEBUG, "%s exists already with %u entries", dev_path, nb);
+                continue;
+            }
+            rc = libxl__device_vscsidev_backend_set_add(gc, v, back);
+            if (rc) goto out;
+        }
+
+        if (aodev->update_json) {
+            rc = libxl__set_domain_configuration(gc, aodev->dev->domid, d_config);
+            if (rc) goto out;
+        }
+
+        libxl__xs_writev(gc, t, be_path,
+                         libxl__xs_kvs_of_flexarray(gc, back, back->count));
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    if (do_reconfigure) {
+        /* Poll for backend change */
+        rc = libxl__wait_for_backend(gc, be_path, GCSPRINTF("%d", be_wait));
+        if (rc) goto out;
+    }
+    rc = 0;
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+static int libxl__vscsictrl_next_vscsidev_id(libxl__gc *gc,
+                                             const char *libxl_path,
+                                             libxl_devid *vscsidev_id)
+{
+    const char *val;
+    xs_transaction_t t = XBT_NULL;
+    unsigned int id;
+    int rc;
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        val = libxl__xs_read(gc, t, libxl_path);
+        id = val ? strtoul(val, NULL, 10) : 0;
+
+        LOG(DEBUG, "%s = %s vscsidev_id %u", libxl_path, val, id);
+
+        val = GCSPRINTF("%u", id + 1);
+        rc = libxl__xs_write_checked(gc, t, libxl_path, val);
+        if (rc) goto out;
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    *vscsidev_id = id;
+    rc = 0;
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+static int libxl__vscsi_assign_vscsidev_ids(libxl__gc *gc,
+                                            uint32_t domid,
+                                            libxl_device_vscsictrl *vscsi)
+{
+    libxl_device_vscsidev *dev;
+    libxl_devid vscsidev_id;
+    const char *libxl_path;
+    int rc, i;
+
+    libxl_path = GCSPRINTF("%s/vscsi/%u/next_vscsidev_id",
+                           libxl__xs_libxl_path(gc, domid),
+                           vscsi->devid);
+    for (i = 0; i < vscsi->num_vscsidevs; i++) {
+        dev = &vscsi->vscsidevs[i];
+        if (dev->vscsidev_id >= 0)
+            continue;
+        rc = libxl__vscsictrl_next_vscsidev_id(gc, libxl_path, &vscsidev_id);
+        if (rc) {
+            LOG(ERROR, "failed to assign vscsidev_id to %s for %s",
+                libxl_path, dev->pdev.p_devname);
+            goto out;
+        }
+        dev->vscsidev_id = vscsidev_id;
+    }
+
+    rc = 0;
+out:
+    return rc;
+}
+
+static int libxl__device_from_vscsictrl(libxl__gc *gc, uint32_t domid,
+                                        libxl_device_vscsictrl *vscsi,
+                                        libxl__device *device)
+{
+    device->backend_devid = vscsi->devid;
+    device->backend_domid = vscsi->backend_domid;
+    device->devid         = vscsi->devid;
+    device->domid         = domid;
+    device->backend_kind  = LIBXL__DEVICE_KIND_VSCSI;
+    device->kind          = LIBXL__DEVICE_KIND_VSCSI;
+
+    return 0;
+}
+
+void libxl__device_vscsictrl_add(libxl__egc *egc, uint32_t domid,
+                                 libxl_device_vscsictrl *vscsi,
+                                 libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__device *device;
+    const char *be_path;
+    unsigned int be_dirs = 0;
+    int rc;
+    libxl_domain_config d_config;
+    libxl_device_vscsictrl vscsi_saved;
+    libxl__domain_userdata_lock *lock = NULL;
+
+    libxl_domain_config_init(&d_config);
+
+    libxl_device_vscsictrl_init(&vscsi_saved);
+    libxl_device_vscsictrl_copy(CTX, &vscsi_saved, vscsi);
+
+    if (vscsi->devid == -1) {
+        LOGE(ERROR, "new vscsictrl for domid %u has uninitialized devid", domid);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    GCNEW(device);
+    rc = libxl__device_from_vscsictrl(gc, domid, vscsi, device);
+    if (rc) goto out;
+
+    aodev->dev = device;
+    be_path = libxl__device_backend_path(gc, aodev->dev);
+
+    rc = libxl__vscsi_assign_vscsidev_ids(gc, domid, &vscsi_saved);
+    if (rc) goto out;
+
+    if (aodev->update_json) {
+        lock = libxl__lock_domain_userdata(gc, domid);
+        if (!lock) {
+            rc = ERROR_LOCK_FAIL;
+            goto out;
+        }
+
+        rc = libxl__get_domain_configuration(gc, domid, &d_config);
+        if (rc) goto out;
+
+        /* Replace or append the copy to the domain config */
+        DEVICE_ADD(vscsictrl, vscsictrls, domid, &vscsi_saved, COMPARE_VSCSI, &d_config);
+    }
+
+    if (libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs)) {
+        rc = libxl__device_vscsi_reconfigure_add(egc, aodev, &vscsi_saved, &d_config, be_path);
+        if (rc)
+            goto out;
+        /* Notify that this is done */
+        aodev->callback(egc, aodev);
+    } else {
+        rc = libxl__device_vscsi_new_backend(egc, aodev, &vscsi_saved, &d_config);
+        if (rc)
+            goto out;
+    }
+
+    rc = 0;
+out:
+    if (lock) libxl__unlock_domain_userdata(lock);
+    libxl_device_vscsictrl_dispose(&vscsi_saved);
+    libxl_domain_config_dispose(&d_config);
+    aodev->rc = rc;
+    if (rc) aodev->callback(egc, aodev);
+    return;
+}
+
+
+int libxl__device_vscsictrl_remove(libxl_ctx *ctx,
+                                   uint32_t domid,
+                                   libxl_device_vscsictrl *vscsictrl,
+                                   const libxl_asyncop_how *ao_how,
+                                   int force)
+{
+    AO_CREATE(ctx, domid, ao_how);
+    libxl__device *device;
+    libxl__ao_device *aodev;
+    int rc;
+
+    GCNEW(device);
+    rc = libxl__device_from_vscsictrl(gc, domid, vscsictrl, device);
+    if (rc != 0) goto out;
+
+    GCNEW(aodev);
+    libxl__prepare_ao_device(ao, aodev);
+    aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
+    aodev->dev = device;
+    aodev->callback = device_addrm_aocomplete;
+    aodev->force = force;
+    libxl__initiate_device_generic_remove(egc, aodev);
+
+out:
+    if (rc) return AO_CREATE_FAIL(rc);
+    return AO_INPROGRESS;
+}
+
+int libxl_device_vscsictrl_remove(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_vscsictrl *vscsictrl,
+                                  const libxl_asyncop_how *ao_how)
+{
+    return libxl__device_vscsictrl_remove(ctx, domid, vscsictrl, ao_how, 0);
+}
+
+int libxl_device_vscsictrl_destroy(libxl_ctx *ctx, uint32_t domid,
+                                   libxl_device_vscsictrl *vscsictrl,
+                                   const libxl_asyncop_how *ao_how)
+{
+    return libxl__device_vscsictrl_remove(ctx, domid, vscsictrl, ao_how, 1);
+}
+
 int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
 {
     int rc;
@@ -4231,6 +4642,7 @@  DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
 /* The following functions are defined:
  * libxl_device_disk_add
  * libxl_device_nic_add
+ * libxl_device_vscsictrl_add
  * libxl_device_vtpm_add
  */
 
@@ -4260,6 +4672,9 @@  DEFINE_DEVICE_ADD(disk)
 /* nic */
 DEFINE_DEVICE_ADD(nic)
 
+/* vscsi */
+DEFINE_DEVICE_ADD(vscsictrl)
+
 /* vtpm */
 DEFINE_DEVICE_ADD(vtpm)
 
@@ -6799,6 +7214,11 @@  int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
 
     MERGE(nic, nics, COMPARE_DEVID, {});
 
+    MERGE(vscsictrl, vscsictrls, COMPARE_VSCSI, {
+            libxl_device_vscsictrl_dispose(dst);
+            libxl_device_vscsictrl_copy(CTX, dst, src);
+          });
+
     MERGE(vtpm, vtpms, COMPARE_DEVID, {});
 
     MERGE(pci, pcidevs, COMPARE_PCI, {});
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 156c0d5..bb4c772 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -836,6 +836,13 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
 #define LIBXL_HAVE_PCITOPOLOGY 1
 
 /*
+ * LIBXL_HAVE_VSCSI
+ *
+ * If this is defined, the PV SCSI feature is supported.
+ */
+#define LIBXL_HAVE_VSCSI 1
+
+/*
  * LIBXL_HAVE_SOCKET_BITMAP
  *
  * If this is defined, then libxl_socket_bitmap_alloc and
@@ -1539,6 +1546,34 @@  int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
                                  libxl_device_channel *channel,
                                  libxl_channelinfo *channelinfo);
 
+/* Virtual SCSI */
+int libxl_device_vscsictrl_add(libxl_ctx *ctx, uint32_t domid,
+                               libxl_device_vscsictrl *vscsi,
+                               const libxl_asyncop_how *ao_how)
+                               LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vscsictrl_remove(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_device_vscsictrl *vscsi,
+                                  const libxl_asyncop_how *ao_how)
+                                  LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vscsictrl_destroy(libxl_ctx *ctx, uint32_t domid,
+                                   libxl_device_vscsictrl *vscsi,
+                                   const libxl_asyncop_how *ao_how)
+                                   LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_vscsictrl *libxl_device_vscsictrl_list(libxl_ctx *ctx, uint32_t domid, int *num);
+int libxl_device_vscsictrl_getinfo(libxl_ctx *ctx, uint32_t domid,
+                                   libxl_device_vscsictrl *vscsictrl,
+                                   libxl_device_vscsidev *vscsidev,
+                                   libxl_vscsiinfo *vscsiinfo);
+/* Remove vscsidev connected to vscsictrl */
+int libxl_device_vscsidev_remove(libxl_ctx *ctx, uint32_t domid,
+                                 libxl_device_vscsidev *dev,
+                                 const libxl_asyncop_how *ao_how)
+                                 LIBXL_EXTERNAL_CALLERS_ONLY;
+void libxl_device_vscsictrl_append_vscsidev(libxl_ctx *ctx,
+                                            libxl_device_vscsictrl *ctrl,
+                                            libxl_device_vscsidev *dev);
+
 /* Virtual TPMs */
 int libxl_device_vtpm_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vtpm *vtpm,
                           const libxl_asyncop_how *ao_how)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e491d83..64f3c70 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1166,6 +1166,7 @@  static void domcreate_rebuild_done(libxl__egc *egc,
     libxl__multidev_begin(ao, &dcs->multidev);
     dcs->multidev.callback = domcreate_launch_dm;
     libxl__add_disks(egc, ao, domid, d_config, &dcs->multidev);
+    libxl__add_vscsictrls(egc, ao, domid, d_config, &dcs->multidev);
     libxl__multidev_prepared(egc, &dcs->multidev, 0);
 
     return;
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 8bb5e93..0bf882c 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -543,6 +543,7 @@  void libxl__multidev_prepared(libxl__egc *egc,
  * The following functions are defined:
  * libxl__add_disks
  * libxl__add_nics
+ * libxl__add_vscsictrls
  * libxl__add_vtpms
  */
 
@@ -562,6 +563,7 @@  void libxl__multidev_prepared(libxl__egc *egc,
 
 DEFINE_DEVICES_ADD(disk)
 DEFINE_DEVICES_ADD(nic)
+DEFINE_DEVICES_ADD(vscsictrl)
 DEFINE_DEVICES_ADD(vtpm)
 
 #undef DEFINE_DEVICES_ADD
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b194e65..ca67c47 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2576,6 +2576,10 @@  _hidden void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
                                    libxl_device_nic *nic,
                                    libxl__ao_device *aodev);
 
+_hidden void libxl__device_vscsictrl_add(libxl__egc *egc, uint32_t domid,
+                                         libxl_device_vscsictrl *vscsi,
+                                         libxl__ao_device *aodev);
+
 _hidden void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
                                    libxl_device_vtpm *vtpm,
                                    libxl__ao_device *aodev);
@@ -3294,6 +3298,10 @@  _hidden void libxl__add_nics(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
                              libxl_domain_config *d_config,
                              libxl__multidev *multidev);
 
+_hidden void libxl__add_vscsictrls(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+                                   libxl_domain_config *d_config,
+                                   libxl__multidev *multidev);
+
 _hidden void libxl__add_vtpms(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
                              libxl_domain_config *d_config,
                              libxl__multidev *multidev);
@@ -3506,7 +3514,17 @@  _hidden void libxl__domain_suspend(libxl__egc *egc,
 /* used by libxc to suspend the guest during migration */
 _hidden void libxl__domain_suspend_callback(void *data);
 
+/* return an array of vscsictrls with num elements */
+_hidden int libxl__vscsi_collect_ctrls(libxl__gc *gc,
+                                       uint32_t domid,
+                                       libxl_device_vscsictrl **ctrls,
+                                       int *num);
 
+_hidden int libxl__device_vscsictrl_remove(libxl_ctx *ctx,
+                                           uint32_t domid,
+                                           libxl_device_vscsictrl *vscsictrl,
+                                           const libxl_asyncop_how *ao_how,
+                                           int force);
 /*
  * Convenience macros.
  */
@@ -3963,6 +3981,7 @@  static inline void libxl__update_config_vtpm(libxl__gc *gc,
  * devices have same identifier. */
 #define COMPARE_DEVID(a, b) ((a)->devid == (b)->devid)
 #define COMPARE_DISK(a, b) (!strcmp((a)->vdev, (b)->vdev))
+#define COMPARE_VSCSI(a, b) ((a)->devid == (b)->devid)
 #define COMPARE_PCI(a, b) ((a)->func == (b)->func &&    \
                            (a)->bus == (b)->bus &&      \
                            (a)->dev == (b)->dev)
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 9ad7eba..fd8a462 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -630,6 +630,42 @@  libxl_device_channel = Struct("device_channel", [
            ])),
 ])
 
+libxl_vscsi_pdev_type = Enumeration("vscsi_pdev_type", [
+    (0, "INVALID"),
+    (1, "HCTL"),
+    (2, "WWN"),
+    ])
+
+libxl_vscsi_hctl = Struct("vscsi_hctl", [
+    ("hst", uint32),
+    ("chn", uint32),
+    ("tgt", uint32),
+    ("lun", uint64),
+    ])
+
+libxl_vscsi_pdev = Struct("vscsi_pdev", [
+    ("p_devname",        string),
+    ("u", KeyedUnion(None, libxl_vscsi_pdev_type, "type",
+        [
+         ("invalid", None),
+         ("hctl", Struct(None, [("m", libxl_vscsi_hctl)])),
+         ("wwn", Struct(None, [("m", string)])),
+        ])),
+    ])
+
+libxl_device_vscsidev = Struct("device_vscsidev", [
+    ("vscsidev_id",      libxl_devid),
+    ("pdev",             libxl_vscsi_pdev),
+    ("vdev",             libxl_vscsi_hctl),
+    ])
+
+libxl_device_vscsictrl = Struct("device_vscsictrl", [
+    ("backend_domid",    libxl_domid),
+    ("devid",            libxl_devid),
+    ("vscsidevs",        Array(libxl_device_vscsidev, "num_vscsidevs")),
+    ("scsi_raw_cmds",    libxl_defbool),
+    ])
+
 libxl_domain_config = Struct("domain_config", [
     ("c_info", libxl_domain_create_info),
     ("b_info", libxl_domain_build_info),
@@ -641,6 +677,7 @@  libxl_domain_config = Struct("domain_config", [
     ("dtdevs", Array(libxl_device_dtdev, "num_dtdevs")),
     ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
     ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
+    ("vscsictrls", Array(libxl_device_vscsictrl, "num_vscsictrls")),
     ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
     # a channel manifests as a console with a name,
     # see docs/misc/channels.txt
@@ -676,6 +713,22 @@  libxl_nicinfo = Struct("nicinfo", [
     ("rref_rx", integer),
     ], dir=DIR_OUT)
 
+libxl_vscsiinfo = Struct("vscsiinfo", [
+    ("backend", string),
+    ("backend_id", uint32),
+    ("frontend", string),
+    ("frontend_id", uint32),
+    ("devid", libxl_devid),
+    ("pdev", libxl_vscsi_pdev),
+    ("vdev", libxl_vscsi_hctl),
+    ("vscsidev_id", libxl_devid),
+    ("scsi_raw_cmds", bool),
+    ("vscsictrl_state", integer),
+    ("vscsidev_state", integer),
+    ("evtch", integer),
+    ("rref", integer),
+    ], dir=DIR_OUT)
+
 libxl_vtpminfo = Struct("vtpminfo", [
     ("backend", string),
     ("backend_id", uint32),
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index 5e55685..84c44f5 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -22,6 +22,7 @@  libxl__device_kind = Enumeration("device_kind", [
     (6, "VKBD"),
     (7, "CONSOLE"),
     (8, "VTPM"),
+    (9, "VSCSI"),
     ])
 
 libxl__console_backend = Enumeration("console_backend", [
diff --git a/tools/libxl/libxl_vscsi.c b/tools/libxl/libxl_vscsi.c
new file mode 100644
index 0000000..42c214d
--- /dev/null
+++ b/tools/libxl/libxl_vscsi.c
@@ -0,0 +1,615 @@ 
+/*
+ * Copyright (C) 2016      SUSE Linux GmbH
+ * Author Olaf Hering <olaf@aepfle.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include "libxl_internal.h"
+
+#define XLU_WWN_LEN 16
+
+static int vscsi_parse_hctl(char *str, libxl_vscsi_hctl *hctl)
+{
+    unsigned int hst, chn, tgt;
+    unsigned long long lun;
+
+    if (sscanf(str, "%u:%u:%u:%llu", &hst, &chn, &tgt, &lun) != 4)
+        return ERROR_INVAL;
+
+    hctl->hst = hst;
+    hctl->chn = chn;
+    hctl->tgt = tgt;
+    hctl->lun = lun;
+    return 0;
+}
+
+static bool vscsi_wwn_valid(const char *p)
+{
+    bool ret = true;
+    int i = 0;
+
+    for (i = 0; i < XLU_WWN_LEN; i++, p++) {
+        if (*p >= '0' && *p <= '9')
+            continue;
+        if (*p >= 'a' && *p <= 'f')
+            continue;
+        if (*p >= 'A' && *p <= 'F')
+            continue;
+        ret = false;
+        break;
+    }
+    return ret;
+}
+
+/* Translate p-dev back into pdev.type */
+static bool vscsi_parse_pdev(libxl__gc *gc, libxl_device_vscsidev *dev,
+                             char *c, char *p, char *v)
+{
+    libxl_vscsi_hctl hctl;
+    unsigned long long lun;
+    char wwn[XLU_WWN_LEN + 1];
+    bool parsed_ok = false;
+
+    libxl_vscsi_hctl_init(&hctl);
+
+    dev->pdev.p_devname = libxl__strdup(NOGC, c);
+
+    if (strncmp(p, "naa.", 4) == 0) {
+        /* WWN as understood by pvops */
+        memset(wwn, 0, sizeof(wwn));
+        if (sscanf(p, "naa.%16c:%llu", wwn, &lun) == 2 && vscsi_wwn_valid(wwn)) {
+            libxl_vscsi_pdev_init_type(&dev->pdev, LIBXL_VSCSI_PDEV_TYPE_WWN);
+            dev->pdev.u.wwn.m = libxl__strdup(NOGC, p);
+            parsed_ok = true;
+        }
+    } else if (vscsi_parse_hctl(p, &hctl) == 0) {
+        /* Either xenlinux, or pvops with properly configured alias in sysfs */
+        libxl_vscsi_pdev_init_type(&dev->pdev, LIBXL_VSCSI_PDEV_TYPE_HCTL);
+        libxl_vscsi_hctl_copy(CTX, &dev->pdev.u.hctl.m, &hctl);
+        parsed_ok = true;
+    }
+
+    if (parsed_ok && vscsi_parse_hctl(v, &dev->vdev) != 0)
+        parsed_ok = false;
+
+    libxl_vscsi_hctl_dispose(&hctl);
+
+    return parsed_ok;
+}
+
+void libxl_device_vscsictrl_append_vscsidev(libxl_ctx *ctx,
+                                            libxl_device_vscsictrl *ctrl,
+                                            libxl_device_vscsidev *dev)
+{
+    GC_INIT(ctx);
+    ctrl->vscsidevs = libxl__realloc(NOGC, ctrl->vscsidevs, sizeof(*dev) * (ctrl->num_vscsidevs + 1));
+    libxl_device_vscsidev_init(ctrl->vscsidevs + ctrl->num_vscsidevs);
+    libxl_device_vscsidev_copy(CTX, ctrl->vscsidevs + ctrl->num_vscsidevs, dev);
+    ctrl->num_vscsidevs++;
+    GC_FREE;
+}
+
+static bool libxl__vscsi_fill_dev(libxl__gc *gc,
+                                  xs_transaction_t t,
+                                  const char *devs_path,
+                                  const char *dev_dir,
+                                  libxl_device_vscsidev *dev)
+{
+    char *path, *c, *p, *v, *s;
+    unsigned int devid;
+    int r;
+
+    r = sscanf(dev_dir, "dev-%u", &devid);
+    if (r != 1) {
+        LOG(ERROR, "expected dev-N, got '%s'", dev_dir);
+        return false;
+    }
+    dev->vscsidev_id = devid;
+
+    path = GCSPRINTF("%s/%s", devs_path, dev_dir);
+    c = libxl__xs_read(gc, t, GCSPRINTF("%s/p-devname", path));
+    p = libxl__xs_read(gc, t, GCSPRINTF("%s/p-dev", path));
+    v = libxl__xs_read(gc, t, GCSPRINTF("%s/v-dev", path));
+    s = libxl__xs_read(gc, t, GCSPRINTF("%s/state", path));
+    LOG(DEBUG, "%s/state is %s", path, s);
+    if (!(c && p && v && s)) {
+        LOG(ERROR, "p-devname '%s' p-dev '%s' v-dev '%s'", c, p, v);
+        return false;
+    }
+
+    if (!vscsi_parse_pdev(gc, dev, c, p, v)) {
+        LOG(ERROR, "failed to parse %s: %s %s %s %s", path, c, p, v, s);
+        return false;
+    }
+
+    switch (atoi(s)) {
+        case XenbusStateUnknown:
+        case XenbusStateInitialising:
+        case XenbusStateInitWait:
+        case XenbusStateInitialised:
+        case XenbusStateConnected:
+        case XenbusStateReconfiguring:
+        case XenbusStateReconfigured:
+            break;
+        case XenbusStateClosing:
+        case XenbusStateClosed:
+            LOG(DEBUG, "unexpected state in %s: %s", path, s);
+            break;
+    }
+
+    return true;
+}
+
+static bool libxl__vscsi_fill_ctrl(libxl__gc *gc,
+                                   xs_transaction_t t,
+                                   const char *fe_path,
+                                   const char *dir,
+                                   libxl_device_vscsictrl *ctrl)
+{
+    libxl_device_vscsidev dev;
+    char *tmp, *be_path, *devs_path;
+    char **dev_dirs;
+    unsigned int ndev_dirs, dev_dir;
+    bool parsed_ok;
+
+    ctrl->devid = atoi(dir);
+
+    be_path = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend", fe_path, dir));
+    /* FIXME what if xenstore is broken? */
+    if (!be_path) {
+        libxl_defbool_set(&ctrl->scsi_raw_cmds, false);
+        return false;
+    }
+
+    tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend-id", fe_path, dir));
+    /* FIXME what if xenstore is broken? */
+    if (tmp)
+        ctrl->backend_domid = atoi(tmp);
+
+    parsed_ok = false;
+    tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/feature-host", be_path));
+    if (tmp)
+        parsed_ok = atoi(tmp) != 0;
+    libxl_defbool_set(&ctrl->scsi_raw_cmds, parsed_ok);
+
+    parsed_ok = true;
+    devs_path = GCSPRINTF("%s/vscsi-devs", be_path);
+    dev_dirs = libxl__xs_directory(gc, t, devs_path, &ndev_dirs);
+    for (dev_dir = 0; dev_dirs && dev_dir < ndev_dirs; dev_dir++) {
+        libxl_device_vscsidev_init(&dev);
+        parsed_ok = libxl__vscsi_fill_dev(gc, t, devs_path, dev_dirs[dev_dir], &dev);
+        if (parsed_ok == true)
+            libxl_device_vscsictrl_append_vscsidev(CTX, ctrl, &dev);
+        libxl_device_vscsidev_dispose(&dev);
+        if (parsed_ok == false)
+            break;
+    }
+
+    return parsed_ok;
+}
+
+int libxl__vscsi_collect_ctrls(libxl__gc *gc,
+                               uint32_t domid,
+                               libxl_device_vscsictrl **ctrls,
+                               int *num)
+{
+    xs_transaction_t t = XBT_NULL;
+    libxl_device_vscsictrl ctrl;
+	char *fe_path;
+    char **dirs;
+    unsigned int ndirs = 0, dir;
+    int rc;
+
+    fe_path = GCSPRINTF("%s/device/vscsi", libxl__xs_get_dompath(gc, domid));
+
+    for (;;) {
+        *num = 0;
+
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        dirs = libxl__xs_directory(gc, t, fe_path, &ndirs);
+        /* Nothing to do */
+        if (!(dirs && ndirs))
+            break;
+
+        /* List of ctrls to be returned to the caller */
+        *ctrls = libxl__malloc(NOGC, ndirs * sizeof(**ctrls));
+
+        for (dir = 0; dir < ndirs; dir++) {
+            libxl_device_vscsictrl_init(*ctrls + dir);
+
+            libxl_device_vscsictrl_init(&ctrl);
+            if (libxl__vscsi_fill_ctrl(gc, t, fe_path, dirs[dir], &ctrl)) {
+                libxl_device_vscsictrl_copy(CTX, *ctrls + *num, &ctrl);
+                (*num)++;
+            }
+            libxl_device_vscsictrl_dispose(&ctrl);
+        }
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+
+        if (rc < 0) {
+            for (dir = 0; dir < ndirs; dir++)
+                libxl_device_vscsictrl_dispose(*ctrls + dir);
+            free(*ctrls);
+            *ctrls = NULL;
+            *num = 0;
+            goto out;
+        }
+    }
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+libxl_device_vscsictrl *libxl_device_vscsictrl_list(libxl_ctx *ctx,
+                                                    uint32_t domid,
+                                                    int *num)
+{
+    GC_INIT(ctx);
+    libxl_device_vscsictrl *ctrls = NULL;
+    int rc, num_ctrls = 0;
+
+    *num = 0;
+
+    rc = libxl__vscsi_collect_ctrls(gc, domid, &ctrls, &num_ctrls);
+    if (rc == 0)
+        *num = num_ctrls;
+
+    GC_FREE;
+    return ctrls;
+}
+
+int libxl_device_vscsictrl_getinfo(libxl_ctx *ctx, uint32_t domid,
+                                   libxl_device_vscsictrl *vscsictrl,
+                                   libxl_device_vscsidev *vscsidev,
+                                   libxl_vscsiinfo *vscsiinfo)
+{
+    GC_INIT(ctx);
+    char *dompath, *vscsipath;
+    char *val;
+    int rc = ERROR_FAIL;
+
+    libxl_vscsiinfo_init(vscsiinfo);
+    dompath = libxl__xs_get_dompath(gc, domid);
+    vscsiinfo->devid = vscsictrl->devid;
+    vscsiinfo->vscsidev_id = vscsidev->vscsidev_id;
+    libxl_vscsi_pdev_copy(ctx, &vscsiinfo->pdev, &vscsidev->pdev);
+    libxl_vscsi_hctl_copy(ctx, &vscsiinfo->vdev, &vscsidev->vdev);
+
+    vscsipath = GCSPRINTF("%s/device/vscsi/%d", dompath, vscsiinfo->devid);
+    vscsiinfo->backend = xs_read(ctx->xsh, XBT_NULL,
+                                 GCSPRINTF("%s/backend", vscsipath), NULL);
+    if (!vscsiinfo->backend)
+        goto out;
+    if(!libxl__xs_read(gc, XBT_NULL, vscsiinfo->backend))
+        goto out;
+
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/backend-id", vscsipath));
+    vscsiinfo->backend_id = val ? strtoul(val, NULL, 10) : -1;
+
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", vscsipath));
+    vscsiinfo->vscsictrl_state = val ? strtoul(val, NULL, 10) : -1;
+
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/event-channel", vscsipath));
+    vscsiinfo->evtch = val ? strtoul(val, NULL, 10) : -1;
+
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/ring-ref", vscsipath));
+    vscsiinfo->rref = val ? strtoul(val, NULL, 10) : -1;
+
+    vscsiinfo->frontend = xs_read(ctx->xsh, XBT_NULL,
+                                  GCSPRINTF("%s/frontend", vscsiinfo->backend), NULL);
+
+    val = libxl__xs_read(gc, XBT_NULL,
+                         GCSPRINTF("%s/frontend-id", vscsiinfo->backend));
+    vscsiinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1;
+
+    val = libxl__xs_read(gc, XBT_NULL,
+                         GCSPRINTF("%s/vscsi-devs/dev-%u/state",
+                         vscsiinfo->backend, vscsidev->vscsidev_id));
+    vscsiinfo->vscsidev_state = val ? strtoul(val, NULL, 10) : -1;
+
+    rc = 0;
+out:
+    GC_FREE;
+    return rc;
+}
+
+
+struct libxl__vscsidev_rm {
+    libxl_device_vscsictrl *ctrl;
+    char *be_path;
+    int dev_wait;
+    libxl__ao_device aodev;
+};
+typedef struct libxl__vscsidev_rm libxl__vscsidev_rm;
+
+static int libxl__device_vscsidev_backend_set_rm(libxl__gc *gc,
+                                                  libxl_device_vscsidev *v,
+                                                  flexarray_t *back)
+{
+    int rc;
+    char *dir;
+
+    dir = GCSPRINTF("vscsi-devs/dev-%u", v->vscsidev_id);
+    rc = flexarray_append_pair(back,
+                               GCSPRINTF("%s/state", dir),
+                               GCSPRINTF("%d", XenbusStateClosing));
+    return rc;
+}
+
+static int libxl__device_vscsi_reconfigure_rm(libxl__ao_device *aodev,
+                                           const char *state_path,
+                                           int *be_wait)
+
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__vscsidev_rm *vscsidev_rm = CONTAINER_OF(aodev, *vscsidev_rm, aodev);
+    libxl_device_vscsictrl *ctrl = vscsidev_rm->ctrl;
+    const char *be_path = vscsidev_rm->be_path;
+    int rc, i, be_state;
+    char *dev_path, *state_val;
+    flexarray_t *back;
+    libxl_device_vscsidev *v;
+    xs_transaction_t t = XBT_NULL;
+
+    /* Prealloc key+value: 1 toplevel + 4 per device */
+    i = 2 * (1 + (4 * ctrl->num_vscsidevs));
+    back = flexarray_make(gc, i, 1);
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        state_val = libxl__xs_read(gc, t, state_path);
+        LOG(DEBUG, "%s is %s", state_path, state_val);
+        if (!state_val) {
+            rc = ERROR_NOTFOUND;
+            goto out;
+        }
+
+        be_state = atoi(state_val);
+        switch (be_state) {
+            case XenbusStateUnknown:
+            case XenbusStateInitialising:
+            case XenbusStateClosing:
+            case XenbusStateClosed:
+            default:
+                /* The backend is in a bad state */
+                rc = ERROR_FAIL;
+                goto out;
+            case XenbusStateInitialised:
+            case XenbusStateReconfiguring:
+            case XenbusStateReconfigured:
+                /* Backend is still busy, caller has to retry */
+                rc = ERROR_NOT_READY;
+                goto out;
+            case XenbusStateInitWait:
+                /* The frontend did not connect yet */
+                *be_wait = XenbusStateInitWait;
+                vscsidev_rm->dev_wait = XenbusStateClosing;
+                break;
+            case XenbusStateConnected:
+                /* The backend can handle reconfigure */
+                *be_wait = XenbusStateConnected;
+                vscsidev_rm->dev_wait = XenbusStateClosed;
+                flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateReconfiguring));
+                break;
+        }
+
+        /* Append new vscsidev or skip existing  */
+        for (i = 0; i < ctrl->num_vscsidevs; i++) {
+            unsigned int nb = 0;
+            v = ctrl->vscsidevs + i;
+            dev_path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsidev_id);
+            if (!libxl__xs_directory(gc, XBT_NULL, dev_path, &nb)) {
+                /* FIXME Sanity check */
+                LOG(DEBUG, "%s does not exist anymore", dev_path);
+                continue;
+            }
+            rc = libxl__device_vscsidev_backend_set_rm(gc, v, back);
+            if (rc) goto out;
+        }
+
+        libxl__xs_writev(gc, t, be_path,
+                         libxl__xs_kvs_of_flexarray(gc, back, back->count));
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    rc = 0;
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+static void libxl__device_vscsidev_backend_rm(libxl__gc *gc,
+                                              libxl_device_vscsidev *v,
+                                              xs_transaction_t t,
+                                              const char *be_path,
+                                              int dev_wait)
+{
+    char *dir, *path, *val;
+
+    dir = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsidev_id);
+    path = GCSPRINTF("%s/state", dir);
+    val = libxl__xs_read(gc, t, path);
+    LOG(DEBUG, "%s is %s", path, val);
+    if (val && strcmp(val, GCSPRINTF("%d", dev_wait)) == 0) {
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/state", dir));
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/p-devname", dir));
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/p-dev", dir));
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/v-dev", dir));
+        xs_rm(CTX->xsh, t, dir);
+    } else {
+        LOG(ERROR, "%s has %s, expected %d", path, val, dev_wait);
+    }
+}
+
+static void libxl__device_vscsidev_rm_be(libxl__egc *egc,
+                                         libxl__ev_devstate *ds,
+                                         int rc)
+{
+    libxl__ao_device *aodev = CONTAINER_OF(ds, *aodev, backend_ds);
+    STATE_AO_GC(aodev->ao);
+    libxl__vscsidev_rm *vscsidev_rm = CONTAINER_OF(aodev, *vscsidev_rm, aodev);
+    libxl_device_vscsictrl *ctrl = vscsidev_rm->ctrl;
+    xs_transaction_t t = XBT_NULL;
+    int i;
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        for (i = 0; i < ctrl->num_vscsidevs; i++)
+            libxl__device_vscsidev_backend_rm(gc, ctrl->vscsidevs + i, t, vscsidev_rm->be_path, vscsidev_rm->dev_wait);
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+out:
+    aodev->rc = rc;
+    aodev->callback(egc, aodev);
+}
+
+static void libxl__device_vscsidev_rm(libxl__egc *egc,
+                                       libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__vscsidev_rm *vscsidev_rm = CONTAINER_OF(aodev, *vscsidev_rm, aodev);
+    char *state_path;
+    int rc, be_wait;
+
+    vscsidev_rm->be_path = libxl__device_backend_path(gc, aodev->dev);
+    state_path = GCSPRINTF("%s/state", vscsidev_rm->be_path);
+
+    rc = libxl__device_vscsi_reconfigure_rm(aodev, state_path, &be_wait);
+    if (rc) goto out;
+
+    rc = libxl__ev_devstate_wait(ao, &aodev->backend_ds,
+                                 libxl__device_vscsidev_rm_be,
+                                 state_path, be_wait,
+                                 LIBXL_INIT_TIMEOUT * 1000);
+    if (rc) {
+        LOG(ERROR, "unable to wait for %s", state_path);
+        goto out;
+    }
+
+    return;
+
+out:
+    aodev->rc = rc;
+    /* Notify that this is done */
+    aodev->callback(egc, aodev);
+}
+
+static void libxl_vscsidev_rm_aodev_cb(libxl__egc *egc, libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__ao_complete(egc, ao, aodev->rc);
+}
+
+static int libxl__device_vscsidev_remove(libxl_ctx *ctx,
+                                         uint32_t domid,
+                                         libxl_device_vscsictrl *vscsictrl,
+                                         const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, domid, ao_how);
+    libxl__ao_device *aodev;
+    libxl__vscsidev_rm *vscsidev_rm;
+    libxl__device *device;
+
+    GCNEW(vscsidev_rm);
+    aodev = &vscsidev_rm->aodev;
+    vscsidev_rm->ctrl = vscsictrl;
+
+    GCNEW(device);
+    device->backend_devid = vscsictrl->devid;
+    device->backend_domid = vscsictrl->backend_domid;
+    device->devid         = vscsictrl->devid;
+    device->domid         = domid;
+    device->backend_kind  = LIBXL__DEVICE_KIND_VSCSI;
+    device->kind          = LIBXL__DEVICE_KIND_VSCSI;
+
+    libxl__prepare_ao_device(ao, aodev);
+    aodev->dev = device;
+    aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
+    aodev->callback = libxl_vscsidev_rm_aodev_cb;
+
+    libxl__device_vscsidev_rm(egc, aodev);
+
+    return AO_INPROGRESS;
+}
+
+int libxl_device_vscsidev_remove(libxl_ctx *ctx, uint32_t domid,
+                                 libxl_device_vscsidev *vscsidev,
+                                 const libxl_asyncop_how *ao_how)
+{
+    GC_INIT(ctx);
+    libxl_device_vscsictrl ctrl, *vc, *ctrls = NULL;
+    libxl_device_vscsidev *vd;
+    int c, d, rc, num_ctrls = 0, found = 0;
+
+    libxl_device_vscsictrl_init(&ctrl);
+
+    rc = libxl__vscsi_collect_ctrls(gc, domid, &ctrls, &num_ctrls);
+    if (rc != 0) goto out;
+
+
+    for (c = 0; c < num_ctrls; ++c) {
+        vc = ctrls + c;
+        for (d = 0; !found && d < vc->num_vscsidevs; d++) {
+            vd = vc->vscsidevs + d;
+#define CMP(member) (vd->vdev.member == vscsidev->vdev.member)
+            if (!(CMP(hst) && CMP(chn) && CMP(tgt) && CMP(lun)))
+                continue;
+#undef CMP
+            found = 1;
+            if (vc->num_vscsidevs > 1) {
+                /* Remove single vscsidev connected to this vscsictrl */
+                libxl_device_vscsictrl_append_vscsidev(ctx, &ctrl, vscsidev);
+                ctrl.devid = vc->devid;
+                ctrl.vscsidevs[0].vscsidev_id = vd->vscsidev_id;
+                rc = libxl__device_vscsidev_remove(ctx, domid, &ctrl, ao_how);
+            } else {
+                /* Wipe entire vscsictrl */;
+                rc = libxl__device_vscsictrl_remove(ctx, domid, vc, ao_how, 0);
+            }
+        }
+        libxl_device_vscsictrl_dispose(vc);
+    }
+    free(ctrls);
+
+    if (!found)
+        rc = ERROR_NOTFOUND;
+
+out:
+    libxl_device_vscsictrl_dispose(&ctrl);
+    GC_FREE;
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/libxlu_vscsi.c b/tools/libxl/libxlu_vscsi.c
new file mode 100644
index 0000000..704e8f1
--- /dev/null
+++ b/tools/libxl/libxlu_vscsi.c
@@ -0,0 +1,697 @@ 
+/*
+ * libxlu_vscsi.c - xl configuration file parsing: setup and helper functions
+ *
+ * Copyright (C) 2016      SUSE Linux GmbH
+ * Author Olaf Hering <olaf@aepfle.de>
+ * Author Ond?ej Hole?ek <aaannz@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include <unistd.h>
+#include <ctype.h>
+#include <dirent.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include "libxlu_internal.h"
+
+#ifdef __linux__
+#define LOG(_c, _x, _a...) \
+        if((_c) && (_c)->report) fprintf((_c)->report, "%s(%u): " _x "\n", __func__, __LINE__, ##_a)
+
+#define XLU_SYSFS_TARGET_PVSCSI "/sys/kernel/config/target/xen-pvscsi"
+#define XLU_WWN_LEN 16
+struct xlu__vscsi_target {
+    XLU_Config *cfg;
+    libxl_vscsi_hctl *pdev_hctl;
+    libxl_vscsi_pdev *pdev;
+    char path[PATH_MAX];
+    char udev_path[PATH_MAX];
+    char wwn[XLU_WWN_LEN + 1];
+    unsigned long long lun;
+};
+
+static int xlu__vscsi_parse_hctl(char *str, libxl_vscsi_hctl *hctl)
+{
+    unsigned int hst, chn, tgt;
+    unsigned long long lun;
+
+    if (sscanf(str, "%u:%u:%u:%llu", &hst, &chn, &tgt, &lun) != 4)
+        return ERROR_INVAL;
+
+    hctl->hst = hst;
+    hctl->chn = chn;
+    hctl->tgt = tgt;
+    hctl->lun = lun;
+    return 0;
+}
+
+static char *xlu__vscsi_trim_string(char *s)
+{
+    size_t len;
+
+    while (isspace(*s))
+        s++;
+    len = strlen(s);
+    while (len-- > 1 && isspace(s[len]))
+        s[len] = '\0';
+    return s;
+}
+
+
+static int xlu__vscsi_parse_dev(XLU_Config *cfg, char *pdev, libxl_vscsi_hctl *hctl)
+{
+    struct stat dentry;
+    char *sysfs = NULL;
+    const char *type;
+    int rc, found = 0;
+    DIR *dirp;
+    struct dirent *de;
+
+    /* stat pdev to get device's sysfs entry */
+    if (stat (pdev, &dentry) < 0) {
+        LOG(cfg, "%s, device node not found", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    if (S_ISBLK (dentry.st_mode)) {
+        type = "block";
+    } else if (S_ISCHR (dentry.st_mode)) {
+        type = "char";
+    } else {
+        LOG(cfg, "%s, device node not a block or char device", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    /* /sys/dev/type/major:minor symlink added in 2.6.27 */
+    if (asprintf(&sysfs, "/sys/dev/%s/%u:%u/device/scsi_device", type,
+                 major(dentry.st_rdev), minor(dentry.st_rdev)) < 0) {
+        sysfs = NULL;
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+
+    dirp = opendir(sysfs);
+    if (!dirp) {
+        LOG(cfg, "%s, no major:minor link in sysfs", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        if (xlu__vscsi_parse_hctl(de->d_name, hctl))
+            continue;
+
+        found = 1;
+        break;
+    }
+    closedir(dirp);
+
+    if (!found) {
+        LOG(cfg, "%s, no h:c:t:l link in sysfs", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    rc = 0;
+out:
+    free(sysfs);
+    return rc;
+}
+
+static bool xlu__vscsi_wwn_valid(const char *p)
+{
+    bool ret = true;
+    int i = 0;
+
+    for (i = 0; i < XLU_WWN_LEN; i++, p++) {
+        if (*p >= '0' && *p <= '9')
+            continue;
+        if (*p >= 'a' && *p <= 'f')
+            continue;
+        if (*p >= 'A' && *p <= 'F')
+            continue;
+        ret = false;
+        break;
+    }
+    return ret;
+}
+
+static bool xlu__vscsi_compare_hctl(libxl_vscsi_hctl *a, libxl_vscsi_hctl *b)
+{
+    if (a->hst == b->hst &&
+        a->chn == b->chn &&
+        a->tgt == b->tgt &&
+        a->lun == b->lun)
+        return true;
+    return false;
+}
+
+/* Finally at
+ * /sys/kernel/config/target/xen-pvscsi/naa.<wwn>/tpgt_1/lun/lun_0/<X>/udev_path
+ */
+static bool xlu__vscsi_compare_udev(struct xlu__vscsi_target *tgt)
+{
+    bool ret;
+    int fd;
+    ssize_t read_sz;
+    libxl_vscsi_hctl udev_hctl;
+
+    libxl_vscsi_hctl_init(&udev_hctl);
+
+    fd = open(tgt->path, O_RDONLY);
+    if (fd < 0){
+        ret = false;
+        goto out;
+    }
+    read_sz = read(fd, &tgt->udev_path, sizeof(tgt->udev_path) - 1);
+    close(fd);
+
+    if (read_sz <= 0 || read_sz > sizeof(tgt->udev_path) - 1) {
+        ret = false;
+        goto out;
+    }
+    tgt->udev_path[read_sz] = '\0';
+    read_sz--;
+    if (tgt->udev_path[read_sz] == '\n')
+        tgt->udev_path[read_sz] = '\0';
+
+    if (xlu__vscsi_parse_dev(tgt->cfg, tgt->udev_path, &udev_hctl)) {
+        ret = false;
+        goto out;
+    }
+    ret = xlu__vscsi_compare_hctl(tgt->pdev_hctl, &udev_hctl);
+
+out:
+    libxl_vscsi_hctl_dispose(&udev_hctl);
+    return ret;
+}
+
+/* /sys/kernel/config/target/xen-pvscsi/naa.<wwn>/tpgt_1/lun/lun_0/<X>/udev_path */
+static bool xlu__vscsi_walk_dir_lun(struct xlu__vscsi_target *tgt)
+{
+    bool found;
+    DIR *dirp;
+    struct dirent *de;
+    size_t path_len = strlen(tgt->path);
+    char *subdir = &tgt->path[path_len];
+
+    dirp = opendir(tgt->path);
+    if (!dirp)
+        return false;
+
+    found = false;
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        snprintf(subdir, sizeof(tgt->path) - path_len, "/%s/udev_path", de->d_name);
+
+        found = xlu__vscsi_compare_udev(tgt);
+        if (found)
+            break;
+
+        *subdir = '\0';
+    }
+    closedir(dirp);
+    return found;
+}
+
+/* /sys/kernel/config/target/xen-pvscsi/naa.<wwn>/tpgt_1/lun/lun_0 */
+static bool xlu__vscsi_walk_dir_luns(struct xlu__vscsi_target *tgt)
+{
+    bool found;
+    DIR *dirp;
+    struct dirent *de;
+    size_t path_len = strlen(tgt->path);
+    char *subdir = &tgt->path[path_len];
+
+    dirp = opendir(tgt->path);
+    if (!dirp)
+        return false;
+
+    found = false;
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        if (sscanf(de->d_name, "lun_%llu", &tgt->lun) != 1)
+            continue;
+
+
+        snprintf(subdir, sizeof(tgt->path) - path_len, "/%s", de->d_name);
+
+        found = xlu__vscsi_walk_dir_lun(tgt);
+        if (found)
+            break;
+
+        *subdir = '\0';
+    }
+    closedir(dirp);
+    return found;
+}
+
+/* /sys/kernel/config/target/xen-pvscsi/naa.<wwn>/tpgt_1 */
+static bool xlu__vscsi_walk_dir_naa(struct xlu__vscsi_target *tgt)
+{
+    bool found;
+    DIR *dirp;
+    struct dirent *de;
+    size_t path_len = strlen(tgt->path);
+    char *subdir = &tgt->path[path_len];
+    unsigned int tpgt;
+
+    dirp = opendir(tgt->path);
+    if (!dirp)
+        return false;
+
+    found = false;
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        if (sscanf(de->d_name, "tpgt_%u", &tpgt) != 1)
+            continue;
+
+        snprintf(subdir, sizeof(tgt->path) - path_len, "/%s/lun", de->d_name);
+
+        found = xlu__vscsi_walk_dir_luns(tgt);
+        if (found)
+            break;
+
+        *subdir = '\0';
+    }
+    closedir(dirp);
+    return found;
+}
+
+/* /sys/kernel/config/target/xen-pvscsi/naa.<wwn> */
+static bool xlu__vscsi_find_target_wwn(struct xlu__vscsi_target *tgt)
+{
+    bool found;
+    DIR *dirp;
+    struct dirent *de;
+    size_t path_len = strlen(tgt->path);
+    char *subdir = &tgt->path[path_len];
+
+    dirp = opendir(tgt->path);
+    if (!dirp)
+        return false;
+
+    found = false;
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        if (sscanf(de->d_name, "naa.%16c", tgt->wwn) != 1)
+            continue;
+        if (!xlu__vscsi_wwn_valid(tgt->wwn))
+            continue;
+
+        snprintf(subdir, sizeof(tgt->path) - path_len, "/%s", de->d_name);
+
+        found = xlu__vscsi_walk_dir_naa(tgt);
+        if (found)
+            break;
+
+        *subdir = '\0';
+    }
+    closedir(dirp);
+    return found;
+}
+
+/*
+ * Convert pdev from config string into pdev property for backend,
+ * which is either h:c:t:l for xenlinux or naa.wwn:lun for pvops
+ */
+static int xlu__vscsi_dev_to_pdev(XLU_Config *cfg, libxl_ctx *ctx, char *str,
+                                  libxl_vscsi_hctl *pdev_hctl,
+                                  libxl_vscsi_pdev *pdev)
+{
+    int rc = ERROR_INVAL;
+    struct xlu__vscsi_target *tgt;
+    static const char xen_pvscsi[] = XLU_SYSFS_TARGET_PVSCSI;
+
+    /* First get hctl representation of config item */
+    if (xlu__vscsi_parse_dev(cfg, str, pdev_hctl))
+        goto out;
+
+    /* Check if a SCSI target item exists for the config item */
+    if (access(xen_pvscsi, F_OK) == 0) {
+        tgt = calloc(1, sizeof(*tgt));
+        if (!tgt) {
+            rc = ERROR_NOMEM;
+            goto out;
+        }
+        tgt->cfg = cfg;
+        tgt->pdev_hctl = pdev_hctl;
+        tgt->pdev = pdev;
+        snprintf(tgt->path, sizeof(tgt->path), "%s", xen_pvscsi);
+        if (xlu__vscsi_find_target_wwn(tgt) == true) {
+            LOG(cfg, "'%s' maps to '%s(%s)'", str, tgt->path, tgt->udev_path);
+            libxl_vscsi_pdev_init_type(pdev, LIBXL_VSCSI_PDEV_TYPE_WWN);
+            if (asprintf(&pdev->u.wwn.m, "naa.%s:%llu", tgt->wwn, tgt->lun) < 0) {
+                rc = ERROR_NOMEM;
+                goto out;
+            }
+        }
+        free(tgt);
+    } else {
+        /* Assume xenlinux backend */
+        libxl_vscsi_pdev_init_type(pdev, LIBXL_VSCSI_PDEV_TYPE_HCTL);
+        libxl_vscsi_hctl_copy(ctx, &pdev->u.hctl.m, pdev_hctl);
+    }
+    rc = 0;
+
+out:
+    return rc;
+}
+
+/* WWN as understood by pvops */
+static int xlu__vscsi_wwn_to_pdev(XLU_Config *cfg, char *str, libxl_vscsi_pdev *pdev)
+{
+    int rc = ERROR_INVAL;
+    unsigned long long lun;
+    char wwn[XLU_WWN_LEN + 1];
+
+    memset(wwn, 0, sizeof(wwn));
+    if (sscanf(str, "naa.%16c:%llu", wwn, &lun) == 2 && xlu__vscsi_wwn_valid(wwn)) {
+        libxl_vscsi_pdev_init_type(pdev, LIBXL_VSCSI_PDEV_TYPE_WWN);
+        pdev->u.wwn.m = strdup(str);
+        rc = pdev->u.wwn.m ? 0 : ERROR_NOMEM;
+    }
+    return rc;
+}
+
+static int xlu__vscsi_parse_pdev(XLU_Config *cfg, libxl_ctx *ctx, char *str,
+                                 libxl_vscsi_pdev *pdev)
+{
+    int rc = ERROR_INVAL;
+    libxl_vscsi_hctl pdev_hctl;
+
+    libxl_vscsi_hctl_init(&pdev_hctl);
+    if (strncmp(str, "/dev/", 5) == 0) {
+        rc = xlu__vscsi_dev_to_pdev(cfg, ctx, str, &pdev_hctl, pdev);
+    } else if (strncmp(str, "naa.", 4) == 0) {
+        rc = xlu__vscsi_wwn_to_pdev(cfg, str, pdev);
+    } else if (xlu__vscsi_parse_hctl(str, &pdev_hctl) == 0) {
+        /* Either xenlinux, or pvops with properly configured alias in sysfs */
+        libxl_vscsi_pdev_init_type(pdev, LIBXL_VSCSI_PDEV_TYPE_HCTL);
+        libxl_vscsi_hctl_copy(ctx, &pdev->u.hctl.m, &pdev_hctl);
+        rc = 0;
+    }
+
+    if (rc == 0) {
+            pdev->p_devname = strdup(str);
+            if (!pdev->p_devname)
+                rc = ERROR_NOMEM;
+    }
+
+    libxl_vscsi_hctl_dispose(&pdev_hctl);
+    return rc;
+}
+
+int xlu_vscsi_parse(XLU_Config *cfg, libxl_ctx *ctx, const char *str,
+                    libxl_device_vscsictrl *new_ctrl,
+                    libxl_device_vscsidev *new_dev)
+{
+    int rc;
+    char *tmp, *pdev, *vdev, *fhost;
+
+    tmp = strdup(str);
+    if (!tmp) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+
+    pdev = strtok(tmp, ",");
+    vdev = strtok(NULL, ",");
+    fhost = strtok(NULL, ",");
+    if (!(pdev && vdev)) {
+        LOG(cfg, "invalid devspec: '%s'\n", str);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    pdev = xlu__vscsi_trim_string(pdev);
+    vdev = xlu__vscsi_trim_string(vdev);
+
+    rc = xlu__vscsi_parse_pdev(cfg, ctx, pdev, &new_dev->pdev);
+    if (rc) {
+        LOG(cfg, "failed to parse %s, rc == %d", pdev, rc);
+        goto out;
+    }
+
+    if (xlu__vscsi_parse_hctl(vdev, &new_dev->vdev)) {
+        LOG(cfg, "invalid '%s', expecting hst:chn:tgt:lun", vdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    /* Record group index */
+    new_ctrl->devid = new_dev->vdev.hst;
+
+    if (fhost) {
+        fhost = xlu__vscsi_trim_string(fhost);
+        if (strcmp(fhost, "feature-host") == 0) {
+            libxl_defbool_set(&new_ctrl->scsi_raw_cmds, true);
+        } else {
+            LOG(cfg, "invalid option '%s', expecting %s", fhost, "feature-host");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+    } else
+        libxl_defbool_set(&new_ctrl->scsi_raw_cmds, false);
+    rc = 0;
+
+out:
+    free(tmp);
+    return rc;
+}
+
+int xlu_vscsi_get_ctrl(XLU_Config *cfg, libxl_ctx *ctx, uint32_t domid,
+                       const char *str, libxl_device_vscsictrl *vscsictrl)
+{
+    libxl_device_vscsidev *new_dev = NULL;
+    libxl_device_vscsictrl *new_ctrl, *vscsictrls = NULL, *tmp;
+    int rc, found_ctrl = -1, i;
+    int num_ctrls;
+
+    new_ctrl = malloc(sizeof(*new_ctrl));
+    new_dev = malloc(sizeof(*new_dev));
+    if (!(new_ctrl && new_dev)) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+    libxl_device_vscsictrl_init(new_ctrl);
+    libxl_device_vscsidev_init(new_dev);
+
+    rc = xlu_vscsi_parse(cfg, ctx, str, new_ctrl, new_dev);
+    if (rc)
+        goto out;
+
+    /* Look for existing vscsictrl for given domain */
+    vscsictrls = libxl_device_vscsictrl_list(ctx, domid, &num_ctrls);
+    if (vscsictrls) {
+        for (i = 0; i < num_ctrls; ++i) {
+            if (vscsictrls[i].devid == new_ctrl->devid) {
+                found_ctrl = i;
+                break;
+            }
+        }
+    }
+
+    if (found_ctrl == -1) {
+        /* Not found, create new ctrl */
+        tmp = new_ctrl;
+    } else {
+        tmp = vscsictrls + found_ctrl;
+
+        /* Check if the vdev address is already taken */
+        for (i = 0; i < tmp->num_vscsidevs; ++i) {
+            if (tmp->vscsidevs[i].vdev.chn == new_dev->vdev.chn &&
+                tmp->vscsidevs[i].vdev.tgt == new_dev->vdev.tgt &&
+                tmp->vscsidevs[i].vdev.lun == new_dev->vdev.lun) {
+                unsigned long long lun = new_dev->vdev.lun;
+                LOG(cfg, "vdev '%u:%u:%u:%llu' is already used.\n",
+                    new_dev->vdev.hst, new_dev->vdev.chn, new_dev->vdev.tgt, lun);
+                rc = ERROR_INVAL;
+                goto out;
+            }
+        }
+
+        if (libxl_defbool_val(new_ctrl->scsi_raw_cmds) !=
+            libxl_defbool_val(tmp->scsi_raw_cmds)) {
+            LOG(cfg, "different feature-host setting: "
+                      "existing ctrl has it %s, new ctrl has it %s\n",
+                libxl_defbool_val(new_ctrl->scsi_raw_cmds) ? "set" : "unset",
+                libxl_defbool_val(tmp->scsi_raw_cmds) ? "set" : "unset");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+    }
+
+    libxl_device_vscsictrl_copy(ctx, vscsictrl, tmp);
+    libxl_device_vscsictrl_append_vscsidev(ctx, vscsictrl, new_dev);
+
+    rc = 0;
+
+out:
+    if (vscsictrls) {
+        for (i = 0; i < num_ctrls; ++i)
+            libxl_device_vscsictrl_dispose(&vscsictrls[i]);
+        free(vscsictrls);
+    }
+    libxl_device_vscsidev_dispose(new_dev);
+    libxl_device_vscsictrl_dispose(new_ctrl);
+    free(new_dev);
+    free(new_ctrl);
+    return rc;
+}
+
+int xlu_vscsi_detach(XLU_Config *cfg, libxl_ctx *ctx, uint32_t domid, char *str)
+{
+    libxl_device_vscsidev dev = { };
+    libxl_device_vscsictrl ctrl = { };
+    int rc;
+    char *tmp = NULL;
+
+    libxl_device_vscsictrl_init(&ctrl);
+    libxl_device_vscsidev_init(&dev);
+
+    /* Create a dummy cfg */
+    if (asprintf(&tmp, "0:0:0:0,%s", str) < 0) {
+        LOG(cfg, "asprintf failed while removing %s from domid %u", str, domid);
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = xlu_vscsi_parse(cfg, ctx, tmp, &ctrl, &dev);
+    if (rc) goto out;
+
+    rc = libxl_device_vscsidev_remove(ctx, domid, &dev, NULL);
+    switch (rc) {
+    case ERROR_NOTFOUND:
+        LOG(cfg, "detach failed: %s does not exist in domid %u", str, domid);
+        break;
+    default:
+        break;
+    }
+
+out:
+    free(tmp);
+    libxl_device_vscsidev_dispose(&dev);
+    libxl_device_vscsictrl_dispose(&ctrl);
+    return rc;
+}
+
+int xlu_vscsi_config_add(XLU_Config *cfg,
+                         libxl_ctx *ctx,
+                         const char *str,
+                         int *num_vscsis,
+                         libxl_device_vscsictrl **vscsis)
+{
+    int rc, i;
+    libxl_device_vscsidev dev = { };
+    libxl_device_vscsictrl *tmp_ctrl, ctrl = { };
+    bool ctrl_found = false;
+
+    /*
+     * #1: parse the devspec and place it in temporary ctrl+dev part
+     * #2: find existing vscsictrl with number vdev.hst
+     *     if found, append the vscsidev to this vscsictrl
+     * #3: otherwise, create new vscsictrl and append vscsidev
+     * Note: vdev.hst does not represent the index named "num_vscsis",
+     *       it is a private index used just in the config file
+     */
+    libxl_device_vscsictrl_init(&ctrl);
+    libxl_device_vscsidev_init(&dev);
+
+    rc = xlu_vscsi_parse(cfg, ctx, str, &ctrl, &dev);
+    if (rc)
+        goto out;
+
+    if (*num_vscsis) {
+        for (i = 0; i < *num_vscsis; i++) {
+            tmp_ctrl = *vscsis + i;
+            if (tmp_ctrl->devid == ctrl.devid) {
+                libxl_device_vscsictrl_append_vscsidev(ctx, tmp_ctrl, &dev);
+                ctrl_found = true;
+                break;
+	           }
+        }
+    }
+
+    if (!ctrl_found || !*num_vscsis) {
+        tmp_ctrl = realloc(*vscsis, sizeof(ctrl) * (*num_vscsis + 1));
+        if (!tmp_ctrl) {
+            LOG(cfg, "realloc #%d failed", *num_vscsis + 1);
+            rc = ERROR_NOMEM;
+            goto out;
+        }
+        *vscsis = tmp_ctrl;
+        tmp_ctrl = *vscsis + *num_vscsis;
+        libxl_device_vscsictrl_init(tmp_ctrl);
+
+        libxl_device_vscsictrl_copy(ctx, tmp_ctrl, &ctrl);
+
+        libxl_device_vscsictrl_append_vscsidev(ctx, tmp_ctrl, &dev);
+
+        (*num_vscsis)++;
+    }
+
+    rc = 0;
+out:
+    libxl_device_vscsidev_dispose(&dev);
+    libxl_device_vscsictrl_dispose(&ctrl);
+    return rc;
+}
+#else /* ! __linux__ */
+int xlu_vscsi_get_ctrl(XLU_Config *config,
+                       libxl_ctx *ctx,
+                       uint32_t domid,
+                       const char *str,
+                       libxl_device_vscsictrl *vscsictrl)
+{
+    return ERROR_INVAL;
+}
+
+int xlu_vscsi_parse(XLU_Config *cfg,
+                    libxl_ctx *ctx,
+                    const char *str,
+                    libxl_device_vscsictrl *new_ctrl,
+                    libxl_device_vscsidev *new_dev)
+{
+    return ERROR_INVAL;
+}
+
+int xlu_vscsi_detach(XLU_Config *cfg,
+                     libxl_ctx *ctx,
+                     uint32_t domid,
+                     char *str)
+{
+    return ERROR_INVAL;
+}
+
+int xlu_vscsi_config_add(XLU_Config *cfg,
+                         libxl_ctx *ctx,
+                         const char *str,
+                         int *num_vscsis,
+                         libxl_device_vscsictrl **vscsis)
+{
+    return ERROR_INVAL;
+}
+#endif
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
index e81b644..886c625 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -118,6 +118,24 @@  int xlu_rdm_parse(XLU_Config *cfg, libxl_rdm_reserve *rdm, const char *str);
 int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate,
                        libxl_device_nic *nic);
 
+/* Fill vscsictrl with device described in str (pdev,vdev[,options]) */
+int xlu_vscsi_get_ctrl(XLU_Config *config,
+                       libxl_ctx *ctx,
+                       uint32_t domid,
+                       const char *str,
+                       libxl_device_vscsictrl *vscsictrl);
+/* Parse config string and fill provided vscsi ctrl and vscsi device */
+int xlu_vscsi_parse(XLU_Config *cfg, libxl_ctx *ctx, const char *str,
+                    libxl_device_vscsictrl *new_ctrl,
+                    libxl_device_vscsidev *new_dev);
+/* Detach vscsi device described in config string (pdev,vdev[,options]) */
+int xlu_vscsi_detach(XLU_Config *cfg, libxl_ctx *ctx, uint32_t domid, char *str);
+/* Add vscsi device described in config string (pdev,vdev[,options]) to d_config */
+int xlu_vscsi_config_add(XLU_Config *cfg,
+                         libxl_ctx *ctx,
+                         const char *str,
+                         int *num_vscsis,
+                         libxl_device_vscsictrl **vscsis);
 #endif /* LIBXLUTIL_H */
 
 /*
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index bdab125..877c695 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -89,6 +89,9 @@  int main_channellist(int argc, char **argv);
 int main_blockattach(int argc, char **argv);
 int main_blocklist(int argc, char **argv);
 int main_blockdetach(int argc, char **argv);
+int main_vscsiattach(int argc, char **argv);
+int main_vscsilist(int argc, char **argv);
+int main_vscsidetach(int argc, char **argv);
 int main_vtpmattach(int argc, char **argv);
 int main_vtpmlist(int argc, char **argv);
 int main_vtpmdetach(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 25507c7..78241d8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1263,7 +1263,7 @@  static void parse_config_data(const char *config_source,
     const char *buf;
     long l, vcpus = 0;
     XLU_Config *config;
-    XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
+    XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms, *vscsictrls;
     XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs;
     int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian;
     int pci_power_mgmt = 0;
@@ -1793,6 +1793,17 @@  static void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_list(config, "vscsi", &vscsictrls, 0, 0)) {
+        int num_vscsi_items = 0;
+        d_config->num_vscsictrls = 0;
+        d_config->vscsictrls = NULL;
+        while ((buf = xlu_cfg_get_listitem (vscsictrls, num_vscsi_items)) != NULL) {
+            if (xlu_vscsi_config_add(config, ctx, buf, &d_config->num_vscsictrls, &d_config->vscsictrls))
+                exit(1);
+            num_vscsi_items++;
+        }
+    }
+
     if (!xlu_cfg_get_list(config, "vtpm", &vtpms, 0, 0)) {
         d_config->num_vtpms = 0;
         d_config->vtpms = NULL;
@@ -6744,6 +6755,201 @@  int main_blockdetach(int argc, char **argv)
     return rc;
 }
 
+int main_vscsiattach(int argc, char **argv)
+{
+    uint32_t domid;
+    int opt, rc;
+    XLU_Config *config = NULL;
+    libxl_device_vscsictrl *vscsictrl = NULL;
+    char *str = NULL, *feat_buf = NULL;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "scsi-attach", 1) {
+        /* No options */
+    }
+
+    if (argc < 4 || argc > 5) {
+        help("scsi-attach");
+        return 1;
+    }
+
+    if (libxl_domain_qualifier_to_domid(ctx, argv[optind], &domid) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+        return 1;
+    }
+
+    optind++;
+
+    if (argc == 5) {
+        if (asprintf(&feat_buf, ",%s", argv[4]) < 0) {
+            perror("asprintf");
+            return 1;
+        }
+    }
+
+    if (asprintf(&str, "%s,%s%s", argv[2], argv[3], feat_buf ?: "") < 0) {
+        perror("asprintf");
+        rc = 1;
+        goto out;;
+    }
+
+    vscsictrl = xmalloc(sizeof(*vscsictrl));
+    libxl_device_vscsictrl_init(vscsictrl);
+
+    config = xlu_cfg_init(stderr, "command line");
+    if (!config) {
+        fprintf(stderr, "Failed to allocate for configuration\n");
+        rc = 1;
+        goto out;
+    }
+
+    /* Parse config string and store result */
+    rc = xlu_vscsi_get_ctrl(config, ctx, domid, str, vscsictrl);
+    if (rc < 0)
+        goto out;
+
+    if (dryrun_only) {
+        char *json = libxl_device_vscsictrl_to_json(ctx, vscsictrl);
+        printf("vscsi: %s\n", json);
+        free(json);
+        if (ferror(stdout) || fflush(stdout)) { perror("stdout"); exit(-1); }
+        rc = 0;
+        goto out;
+    }
+
+    /* Finally add the device */
+    if (libxl_device_vscsictrl_add(ctx, domid, vscsictrl, NULL)) {
+        fprintf(stderr, "libxl_device_vscsictrl_add failed.\n");
+        rc = 1;
+        goto out;
+    }
+
+    rc = 0;
+out:
+    if (config)
+        xlu_cfg_destroy(config);
+    libxl_device_vscsictrl_dispose(vscsictrl);
+    free(vscsictrl);
+    free(str);
+    free(feat_buf);
+    return rc;
+}
+
+int main_vscsilist(int argc, char **argv)
+{
+    int opt;
+    uint32_t domid;
+    libxl_device_vscsictrl *vscsictrls;
+    libxl_vscsiinfo vscsiinfo;
+    int num_ctrls, h, d;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "scsi-list", 1) {
+        /* No options */
+    }
+    if (argc < 2) {
+        help("scsi-list");
+        return 1;
+    }
+
+    /*      Idx  BE  state ctrl p_hst v_hst state */
+    printf("%-3s %-3s %-5s %-5s %-10s %-10s %-5s\n",
+           "Idx", "BE", "state", "ctrl", "phy-hctl", "vir-hctl", "devstate");
+    for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
+        if (libxl_domain_qualifier_to_domid(ctx, *argv, &domid) < 0) {
+            fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
+            continue;
+        }
+        if (!(vscsictrls = libxl_device_vscsictrl_list(ctx, domid, &num_ctrls))) {
+            continue;
+        }
+        for (h = 0; h < num_ctrls; ++h) {
+            for (d = 0; d < vscsictrls[h].num_vscsidevs; d++) {
+                if (!libxl_device_vscsictrl_getinfo(ctx, domid, &vscsictrls[h],
+                                                &vscsictrls[h].vscsidevs[d],
+                                                &vscsiinfo)) {
+                    char pdev[64], vdev[64];
+                    unsigned long long lun;
+                    switch (vscsiinfo.pdev.type) {
+                        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
+                            lun = vscsiinfo.pdev.u.hctl.m.lun;
+                            snprintf(pdev, sizeof(pdev), "%u:%u:%u:%llu",
+                                     vscsiinfo.pdev.u.hctl.m.hst,
+                                     vscsiinfo.pdev.u.hctl.m.chn,
+                                     vscsiinfo.pdev.u.hctl.m.tgt,
+                                     lun);
+                            break;
+                        case LIBXL_VSCSI_PDEV_TYPE_WWN:
+                            snprintf(pdev, sizeof(pdev), "%s",
+                                     vscsiinfo.pdev.u.wwn.m);
+                            break;
+                        default:
+                            pdev[0] = '\0';
+                            break;
+                    }
+                    lun = vscsiinfo.vdev.lun;
+                    snprintf(vdev, sizeof(vdev), "%u:%u:%u:%llu",
+                             vscsiinfo.vdev.hst,
+                             vscsiinfo.vdev.chn,
+                             vscsiinfo.vdev.tgt,
+                             lun);
+                    /*      Idx  BE  state Sta */
+                    printf("%-3d %-3d %-5d %-5d %-10s %-10s %d\n",
+                           vscsiinfo.devid,
+                           vscsiinfo.backend_id,
+                           vscsiinfo.vscsictrl_state,
+                           vscsiinfo.backend_id,
+                           pdev, vdev,
+                           vscsiinfo.vscsidev_state);
+
+                }
+                libxl_vscsiinfo_dispose(&vscsiinfo);
+            }
+            libxl_device_vscsictrl_dispose(&vscsictrls[h]);
+        }
+        free(vscsictrls);
+
+    }
+
+    return 0;
+}
+
+int main_vscsidetach(int argc, char **argv)
+{
+    int opt;
+    char *dom = argv[1], *str = argv[2];
+    uint32_t domid;
+    XLU_Config *config = NULL;
+    int rc = 0;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "scsi-detach", 1) {
+        /* No options */
+    }
+
+    if (argc < 3) {
+        help("scsi-detach");
+        return 1;
+    }
+
+    if (libxl_domain_qualifier_to_domid(ctx, dom, &domid) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", dom);
+        return 1;
+    }
+
+    config = xlu_cfg_init(stderr, "command line");
+    if (!config) {
+        fprintf(stderr, "Failed to allocate for configuration\n");
+        goto out;
+    }
+
+    rc = xlu_vscsi_detach(config, ctx, domid, str);
+    if (rc)
+        fprintf(stderr, "scsi-detach %s %s failed: %d\n", dom, str, rc);
+
+out:
+    if (config)
+        xlu_cfg_destroy(config);
+    return !!rc;
+}
+
 int main_vtpmattach(int argc, char **argv)
 {
     int opt;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index fdc1ac6..11e1c26 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -351,6 +351,21 @@  struct cmd_spec cmd_table[] = {
       "Destroy a domain's virtual block device",
       "<Domain> <DevId>",
     },
+    { "scsi-attach",
+      &main_vscsiattach, 1, 1,
+      "Attach a dom0 SCSI device to a domain.",
+      "<Domain> <PhysDevice> <VirtDevice>",
+    },
+    { "scsi-list",
+      &main_vscsilist, 0, 0,
+      "List all dom0 SCSI devices currently attached to a domain.",
+      "<Domain(s)>",
+    },
+    { "scsi-detach",
+      &main_vscsidetach, 0, 1,
+      "Detach a specified SCSI device from a domain.",
+      "<Domain> <VirtDevice>",
+    },
     { "vtpm-attach",
       &main_vtpmattach, 1, 1,
       "Create a new virtual TPM device",