[PATCH v4 0/5] libbxl: add support for pvscsi, iteration 4

[PATCH v4 0/5] libbxl: add support for pvscsi, iteration 4

[Olaf Hering]

Port vscsi=[] and scsi-{attach,detach,list} commands from xend to libxl.

TODO:
 - find better name for ->feature_host (perhaps ->sg_io as used in libvirt?)
 - maybe use events instead of polling for "state" changes in reconfigure
   (libxl__wait_for_backend vs. libxl__ev_devstate_wait)
 - add code to libvirt to actually allow SCSI and target passthrough. The
   current code is still a decade behind: it expects h:c:t:l notation...
 - maybe adjust API for xl and libvirt to work directly with .idl provided
   structs instead of pass domU.cfg strings around

Changes between v3 and v4:
 - Use libxl__device_nextid in libxl__device_vscsi_add
 - Remove check for duplicate pdev assignment from libxl_device_vscsi_get_host
 - Caller provides libxl_device_vscsi to libxl_device_vscsi_get_host
 - Define LIBXL_HAVE_VSCSI
 - Remove init_val from libxl_vscsi_pdev_type
 - Move some functions from libxl to libxlu
 - Introduce libxl_device_vscsi->next_vscsi_dev_id to handle holes
 - Use Struct in KeyedUnion for ocaml idl
 - docs: Mention pvscsi in xl and xl.cfg
 - Turn feature_host into a defbool and add checking
 - Support pvops and /dev/ nodes in config
 - Wrap entire libxlu_vscsi.c with ifdef linux
 - Set remove flag in libxl_device_vscsi_list
 - Fix vscsiif path in xenstore-paths.markdown
 - vscsiif.h: add some notes about xenstore layout
 - Add copyright to libxlu_vscsi.c and libxl_vscsi.c
 - Scripts to create and delete xen-scsiback nodes in Linux target framework
 - Remove pvscsi.txt

Changes between v2 and v3:
 - Adjust change for vscsiif.h
 - Support "naa.wwn:lun" notation in pvops kernel
 - Add example for pvops kernel using targetcli
   patch required for python-rtslib:
   http://article.gmane.org/gmane.linux.scsi.target.devel/8146
 - Use vdev variable in libxl_device_vscsi_parse
http://lists.xenproject.org/archives/html/xen-devel/2015-03/msg00734.html

Changes between v1 and v2:
 - ported to current staging
http://lists.xenproject.org/archives/html/xen-devel/2015-03/msg00030.html

Initial attempt was sent a year ago:
http://lists.xenproject.org/archives/html/xen-devel/2014-04/msg03958.html
Most comments are addressed.

This version has been tested with SLES as backend and frontend.
This version has been tested with pvops as backend and SLES as frontend.



Olaf Hering (5):
  vscsiif.h: fix WWN notation for p-dev property
  docs: add vscsi to xenstore-paths.markdown
  libxl: add support for vscsi
  vscsiif.h: add some notes about xenstore layout
  Scripts to create and delete xen-scsiback nodes in Linux target
    framework

 docs/man/xl.cfg.pod.5                    |  55 +++
 docs/man/xl.pod.1                        |  18 +
 docs/misc/xenstore-paths.markdown        |  10 +
 tools/libxl/Makefile                     |   2 +
 tools/libxl/libxl.c                      | 441 ++++++++++++++++++
 tools/libxl/libxl.h                      |  27 ++
 tools/libxl/libxl_create.c               |   1 +
 tools/libxl/libxl_device.c               |   2 +
 tools/libxl/libxl_internal.h             |  16 +
 tools/libxl/libxl_types.idl              |  56 +++
 tools/libxl/libxl_types_internal.idl     |   1 +
 tools/libxl/libxl_vscsi.c                | 271 +++++++++++
 tools/libxl/libxlu_vscsi.c               | 750 +++++++++++++++++++++++++++++++
 tools/libxl/libxlutil.h                  |  21 +
 tools/libxl/xl.h                         |   3 +
 tools/libxl/xl_cmdimpl.c                 | 184 +++++++-
 tools/libxl/xl_cmdtable.c                |  15 +
 tools/misc/Makefile                      |   4 +
 tools/misc/target-create-xen-scsiback.sh |  94 ++++
 tools/misc/target-delete-xen-scsiback.sh |  39 ++
 xen/include/public/io/vscsiif.h          |  70 ++-
 21 files changed, 2078 insertions(+), 2 deletions(-)
 create mode 100644 tools/libxl/libxl_vscsi.c
 create mode 100644 tools/libxl/libxlu_vscsi.c
 create mode 100644 tools/misc/target-create-xen-scsiback.sh
 create mode 100644 tools/misc/target-delete-xen-scsiback.sh

[PATCH v4 1/5] vscsiif.h: fix WWN notation for p-dev property

[Olaf Hering]

The pvops kernel expects either "naa.WWN:LUN" or "h:c:t:l" in the p-dev
property. Add the missing :LUN part to the comment.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/public/io/vscsiif.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h
index 7a1db05..e8e38a9 100644
--- a/xen/include/public/io/vscsiif.h
+++ b/xen/include/public/io/vscsiif.h
@@ -60,7 +60,7 @@
  *
  *      A string specifying the backend device: either a 4-tuple "h:c:t:l"
  *      (host, controller, target, lun, all integers), or a WWN (e.g.
- *      "naa.60014054ac780582").
+ *      "naa.60014054ac780582:0").
  *
  * v-dev
  *      Values:         string

Re: [PATCH v4 1/5] vscsiif.h: fix WWN notation for p-dev property

[Konrad Rzeszutek Wilk]

On Fri, Apr 17, 2015 at 08:30:56AM +0000, Olaf Hering wrote:
> The pvops kernel expects either "naa.WWN:LUN" or "h:c:t:l" in the p-dev
> property. Add the missing :LUN part to the comment.

What about the older kernels that had said driver?

> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> ---
>  xen/include/public/io/vscsiif.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h
> index 7a1db05..e8e38a9 100644
> --- a/xen/include/public/io/vscsiif.h
> +++ b/xen/include/public/io/vscsiif.h
> @@ -60,7 +60,7 @@
>   *
>   *      A string specifying the backend device: either a 4-tuple "h:c:t:l"
>   *      (host, controller, target, lun, all integers), or a WWN (e.g.
> - *      "naa.60014054ac780582").
> + *      "naa.60014054ac780582:0").
>   *
>   * v-dev
>   *      Values:         string
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v4 1/5] vscsiif.h: fix WWN notation for p-dev property

[Olaf Hering]

On Tue, Apr 21, Konrad Rzeszutek Wilk wrote:

> On Fri, Apr 17, 2015 at 08:30:56AM +0000, Olaf Hering wrote:
> > The pvops kernel expects either "naa.WWN:LUN" or "h:c:t:l" in the p-dev
> > property. Add the missing :LUN part to the comment.
> 
> What about the older kernels that had said driver?

Which older kernels? There is just 3.18+ of pvops.

Olaf

Re: [PATCH v4 1/5] vscsiif.h: fix WWN notation for p-dev property

[Konrad Rzeszutek Wilk]

On Tue, Apr 21, 2015 at 04:35:17PM +0200, Olaf Hering wrote:
> On Tue, Apr 21, Konrad Rzeszutek Wilk wrote:
> 
> > On Fri, Apr 17, 2015 at 08:30:56AM +0000, Olaf Hering wrote:
> > > The pvops kernel expects either "naa.WWN:LUN" or "h:c:t:l" in the p-dev
> > > property. Add the missing :LUN part to the comment.
> > 
> > What about the older kernels that had said driver?
> 
> Which older kernels? There is just 3.18+ of pvops.

Xen classic kernels.

> 
> Olaf

Re: [PATCH v4 1/5] vscsiif.h: fix WWN notation for p-dev property

[Olaf Hering]

On Tue, Apr 21, Konrad Rzeszutek Wilk wrote:

> On Tue, Apr 21, 2015 at 04:35:17PM +0200, Olaf Hering wrote:
> > On Tue, Apr 21, Konrad Rzeszutek Wilk wrote:
> > 
> > > On Fri, Apr 17, 2015 at 08:30:56AM +0000, Olaf Hering wrote:
> > > > The pvops kernel expects either "naa.WWN:LUN" or "h:c:t:l" in the p-dev
> > > > property. Add the missing :LUN part to the comment.
> > > 
> > > What about the older kernels that had said driver?
> > 
> > Which older kernels? There is just 3.18+ of pvops.
> 
> Xen classic kernels.

They handle just h:c:t:l in "p-dev".

Olaf

[PATCH v4 2/5] docs: add vscsi to xenstore-paths.markdown

[Olaf Hering]

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 docs/misc/xenstore-paths.markdown | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index d94ea9d..d7a5e30 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -232,6 +232,11 @@ A virtual keyboard device frontend. Described by
 A virtual network device frontend. Described by
 [xen/include/public/io/netif.h][NETIF]
 
+#### ~/device/vscsi/$DEVID/* []
+
+A virtual scsi device frontend. Described by
+[xen/include/public/io/vscsiif.h][SCSIIF]
+
 #### ~/console/* []
 
 The primary PV console device. Described in [console.txt](console.txt)
@@ -302,6 +307,10 @@ A virtual keyboard device backend. Described by
 A virtual network device backend. Described by
 [xen/include/public/io/netif.h][NETIF]
 
+#### ~/backend/vscsi/$DOMID/$DEVID/* []
+
+A PV SCSI backend. Described in [pvscsi.txt](pvscsi.txt)
+
 #### ~/backend/console/$DOMID/$DEVID/* []
 
 A PV console backend. Described in [console.txt](console.txt)
@@ -403,6 +412,7 @@ ifb device used by Remus to buffer network output from the associated vif.
 [KBDIF]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,kbdif.h.html
 [LIBXLMEM]: http://xenbits.xen.org/docs/unstable/misc/libxl_memory.txt
 [NETIF]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,netif.h.html
+[SCSIIF]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,vscsiif.h.html
 [SI]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,xen.h.html#Struct_start_info
 [VCPU]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,vcpu.h.html
 [XSWIRE]: http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,xs_wire.h.html

Re: [PATCH v4 2/5] docs: add vscsi to xenstore-paths.markdown

[Olaf Hering]

On Fri, Apr 17, Olaf Hering wrote:

> @@ -302,6 +307,10 @@ A virtual keyboard device backend. Described by
>  A virtual network device backend. Described by
>  [xen/include/public/io/netif.h][NETIF]
>  
> +#### ~/backend/vscsi/$DOMID/$DEVID/* []
> +
> +A PV SCSI backend. Described in [pvscsi.txt](pvscsi.txt)
> +
>  #### ~/backend/console/$DOMID/$DEVID/* []
>  
>  A PV console backend. Described in [console.txt](console.txt)

This hunk still refers to the removed pvscsi.txt. Will be fixed in next
version.


Olaf

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

[Olaf Hering]

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                  | 441 ++++++++++++++++++++
 tools/libxl/libxl.h                  |  27 ++
 tools/libxl/libxl_create.c           |   1 +
 tools/libxl/libxl_device.c           |   2 +
 tools/libxl/libxl_internal.h         |  16 +
 tools/libxl/libxl_types.idl          |  56 +++
 tools/libxl/libxl_types_internal.idl |   1 +
 tools/libxl/libxl_vscsi.c            | 271 +++++++++++++
 tools/libxl/libxlu_vscsi.c           | 750 +++++++++++++++++++++++++++++++++++
 tools/libxl/libxlutil.h              |  21 +
 tools/libxl/xl.h                     |   3 +
 tools/libxl/xl_cmdimpl.c             | 184 ++++++++-
 tools/libxl/xl_cmdtable.c            |  15 +
 16 files changed, 1862 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index f936dfc..d395e56 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -510,6 +510,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
+dom0 SCSI devices as-is to the guest.
+
+Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
+'pdev' describes the physical device, preferable in a persistant format.
+'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
+'option' lists additional flags which a backend may recognize.
+
+The supported values for "pdev" and "option" depends on the used backend driver:
+
+=over 4
+
+=item B<Linux pvops>
+
+=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. Ususally this is a WWN in the form of "na.WWN:LUN".
+
+=item C<option>
+
+No options recognized.
+
+=back
+
+=item B<Linux xenlinux>
+
+=over 4
+
+=item C<pdev>
+
+The dom0 device in either /dev/scsidev or pHOST:CHANNEL:TARGET:LUN notation.
+
+Its recommended to use persistant 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<option>
+
+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 16783c8..19bdbfa 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1328,6 +1328,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 1b16598..79b3867 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -91,6 +91,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 \
@@ -122,6 +123,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 511eef1..abe7d75 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2014,6 +2014,437 @@ static int libxl__resolve_domid(libxl__gc *gc, const char *name,
 }
 
 /******************************************************************************/
+
+static void libxl__device_vscsi_dev_backend_rm(libxl__gc *gc,
+                                              libxl_vscsi_dev *v,
+                                              xs_transaction_t t,
+                                              char *be_path,
+                                              int dev_wait)
+{
+    char *path, *val;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+
+    path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
+    val = libxl__xs_read(gc, t, path);
+    LOG(DEBUG, "%s is %s", path, val);
+    if (val && strcmp(val, GCSPRINTF("%d", dev_wait)) == 0) {
+        path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
+        xs_rm(ctx->xsh, t, path);
+        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-devname", be_path, v->vscsi_dev_id);
+        xs_rm(ctx->xsh, t, path);
+        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-dev", be_path, v->vscsi_dev_id);
+        xs_rm(ctx->xsh, t, path);
+        path = GCSPRINTF("%s/vscsi-devs/dev-%u/v-dev", be_path, v->vscsi_dev_id);
+        xs_rm(ctx->xsh, t, path);
+        path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
+        xs_rm(ctx->xsh, t, path);
+    } else {
+        LOG(ERROR, "%s has %s, expected %d", path, val, dev_wait);
+    }
+}
+
+static int libxl__device_vscsi_dev_backend_set(libxl__gc *gc,
+                                               libxl_vscsi_dev *v,
+                                               flexarray_t *back)
+{
+    int rc;
+    libxl_vscsi_hctl *hctl;
+
+    switch (v->pdev.type) {
+        case LIBXL_VSCSI_PDEV_TYPE_WWN:
+            flexarray_append_pair(back,
+                                  GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
+                                  v->pdev.u.wwn.m);
+            break;
+        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
+            hctl = &v->pdev.u.hctl.m;
+            flexarray_append_pair(back,
+                                  GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
+                                  GCSPRINTF("%u:%u:%u:%u", hctl->hst, hctl->chn, hctl->tgt, hctl->lun));
+            break;
+        case LIBXL_VSCSI_PDEV_TYPE_INVALID:
+        default:
+            rc = ERROR_FAIL;
+            goto out;
+    }
+    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/p-devname", v->vscsi_dev_id),
+                          v->pdev.p_devname);
+    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/v-dev", v->vscsi_dev_id),
+                          GCSPRINTF("%u:%u:%u:%u", v->vdev.hst, v->vdev.chn, v->vdev.tgt, v->vdev.lun));
+    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id),
+                          GCSPRINTF("%d", XenbusStateInitialising));
+    rc = 0;
+out:
+    return rc;
+}
+
+static int libxl__device_vscsi_new_backend(libxl__egc *egc,
+                                           libxl__ao_device *aodev,
+                                           libxl_device_vscsi *vscsi,
+                                           libxl_domain_config *d_config)
+{
+    STATE_AO_GC(aodev->ao);
+    int rc, i;
+    flexarray_t *back;
+    flexarray_t *front;
+    libxl_vscsi_dev *v;
+    xs_transaction_t t = XBT_NULL;
+
+    /* Prealloc key+value: 4 toplevel + 4 per device */
+    i = 2 * (4 + (4 * vscsi->num_vscsi_devs));
+    back = flexarray_make(gc, i, 1);
+    front = flexarray_make(gc, 2 * 2, 1);
+
+    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->feature_host) ?
+                          "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_vscsi_devs; i++) {
+        v = vscsi->vscsi_devs + i;
+        if (v->remove)
+            continue;
+        rc = libxl__device_vscsi_dev_backend_set(gc, v, back);
+        if (rc) goto out;
+    }
+
+    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(libxl__egc *egc,
+                                           libxl__ao_device *aodev,
+                                           libxl_device_vscsi *vscsi,
+                                           libxl_domain_config *d_config,
+                                           char *be_path,
+                                           int *dev_wait)
+{
+    STATE_AO_GC(aodev->ao);
+    int rc, i, be_state, be_wait;
+    char *dev_path, *state_path, *state_val;
+    flexarray_t *back;
+    libxl_vscsi_dev *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_vscsi_devs));
+    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;
+                if (dev_wait)
+                    *dev_wait = XenbusStateInitialising;
+                do_reconfigure = false;
+                break;
+            case XenbusStateConnected:
+                /* The backend can handle reconfigure */
+                be_wait = XenbusStateConnected;
+                if (dev_wait)
+                    *dev_wait = XenbusStateClosed;
+                flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateReconfiguring));
+                do_reconfigure = true;
+                break;
+        }
+
+        /* Append new device or trigger removal */
+        for (i = 0; i < vscsi->num_vscsi_devs; i++) {
+            v = vscsi->vscsi_devs + i;
+            unsigned int nb = 0;
+            dev_path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
+            /* Preserve existing device */
+            if (libxl__xs_directory(gc, XBT_NULL, dev_path, &nb) && nb) {
+                /* Trigger device removal by forwarding state to XenbusStateClosing */
+                if (do_reconfigure && v->remove)
+                    flexarray_append_pair(back,
+                                          GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id),
+                                          GCSPRINTF("%d", XenbusStateClosing));
+                continue;
+            }
+            rc = libxl__device_vscsi_dev_backend_set(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:
+    LOG(ERROR, "%u: rc %d", __LINE__, rc);
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+static int libxl__device_from_vscsi(libxl__gc *gc, uint32_t domid,
+                                    libxl_device_vscsi *vscsi,
+                                    libxl__device *device)
+{
+    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_vscsi_add(libxl__egc *egc, uint32_t domid,
+                             libxl_device_vscsi *vscsi,
+                             libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__device *device;
+    char *be_path;
+    unsigned int be_dirs = 0;
+    int rc;
+    libxl_domain_config d_config;
+    libxl_device_vscsi vscsi_saved;
+    libxl__domain_userdata_lock *lock = NULL;
+
+    libxl_domain_config_init(&d_config);
+
+    /* In case of vscsi the copy remains identical to the provided input */
+    libxl_device_vscsi_init(&vscsi_saved);
+    libxl_device_vscsi_copy(CTX, &vscsi_saved, vscsi);
+
+    if (vscsi->devid == -1) {
+        if ((vscsi->devid = libxl__device_nextid(gc, domid, "vscsi")) < 0) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+    }
+
+    /* Adjust copy */
+    libxl__update_config_vscsi(gc, &vscsi_saved, vscsi);
+
+    GCNEW(device);
+    rc = libxl__device_from_vscsi(gc, domid, vscsi, device);
+    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(vscsi, vscsis, domid, &vscsi_saved, COMPARE_VSCSI, &d_config);
+    }
+
+    aodev->dev = device;
+
+    be_path = libxl__device_backend_path(gc, aodev->dev);
+    if (libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs)) {
+        rc = libxl__device_vscsi_reconfigure(egc, aodev, vscsi, &d_config, be_path, NULL);
+        if (rc)
+            goto out;
+        /* Notify that this is done */
+        aodev->callback(egc, aodev);
+    } else {
+        rc = libxl__device_vscsi_new_backend(egc, aodev, vscsi, &d_config);
+        if (rc)
+            goto out;
+    }
+
+    rc = 0;
+out:
+    if (lock) libxl__unlock_domain_userdata(lock);
+    libxl_device_vscsi_dispose(&vscsi_saved);
+    libxl_domain_config_dispose(&d_config);
+    aodev->rc = rc;
+    if (rc) aodev->callback(egc, aodev);
+    return;
+}
+
+static void libxl__device_vscsi_dev_rm(libxl__egc *egc,
+                                       libxl_device_vscsi *vscsi,
+                                       libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    char *be_path;
+    int rc, i, dev_wait;
+    libxl_domain_config d_config;
+    libxl__domain_userdata_lock *lock = NULL;
+    libxl_vscsi_dev *v;
+    xs_transaction_t t = XBT_NULL;
+
+    libxl_domain_config_init(&d_config);
+
+    if (vscsi->devid == -1) {
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    /* No other code will traverse device list, update json with removal info */
+    if (aodev->update_json) {
+        lock = libxl__lock_domain_userdata(gc, aodev->dev->domid);
+        if (!lock) {
+            rc = ERROR_LOCK_FAIL;
+            goto out;
+        }
+
+        rc = libxl__get_domain_configuration(gc, aodev->dev->domid, &d_config);
+        if (rc) goto out;
+
+        /* Replace the item in the domain config */
+        DEVICE_ADD(vscsi, vscsis, aodev->dev->domid, vscsi, COMPARE_VSCSI, &d_config);
+    }
+
+    be_path = libxl__device_backend_path(gc, aodev->dev);
+    rc = libxl__device_vscsi_reconfigure(egc, aodev, vscsi, &d_config, be_path, &dev_wait);
+    if (rc) goto out;
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        for (i = 0; i < vscsi->num_vscsi_devs; i++) {
+            v = vscsi->vscsi_devs + i;
+            if (v->remove)
+                libxl__device_vscsi_dev_backend_rm(gc, v, t, be_path, dev_wait);
+        }
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    rc = 0;
+
+out:
+    if (lock) libxl__unlock_domain_userdata(lock);
+    libxl_domain_config_dispose(&d_config);
+    aodev->rc = rc;
+    /* Notify that this is done */
+    aodev->callback(egc, aodev);
+}
+
+/* Extended variant of DEFINE_DEVICE_REMOVE to handle reconfigure */
+int libxl_device_vscsi_remove(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_vscsi *vscsi,
+                              const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, domid, ao_how);
+    libxl__device *device;
+    libxl__ao_device *aodev;
+    int rc, i;
+    unsigned int remaining = vscsi->num_vscsi_devs;
+
+    GCNEW(device);
+    rc = libxl__device_from_vscsi(gc, domid, vscsi, 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 = 0;
+
+    for (i = 0; i < vscsi->num_vscsi_devs; i++) {
+        if (vscsi->vscsi_devs[i].remove)
+            remaining--;
+    }
+
+    LOG(DEBUG, "%u: v_hst %u, %u of %u remaining", domid, vscsi->v_hst, remaining, vscsi->num_vscsi_devs);
+    if (remaining)
+        libxl__device_vscsi_dev_rm(egc, vscsi, aodev);
+    else
+        libxl__initiate_device_remove(egc, aodev);
+
+out:
+    if (rc) return AO_ABORT(rc);
+    return AO_INPROGRESS;
+}
+
 int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
 {
     int rc;
@@ -4158,6 +4589,7 @@ out:
  * libxl_device_disk_destroy
  * libxl_device_nic_remove
  * libxl_device_nic_destroy
+ * libxl_device_vscsi_destroy
  * libxl_device_vtpm_remove
  * libxl_device_vtpm_destroy
  * libxl_device_vkb_remove
@@ -4202,6 +4634,9 @@ DEFINE_DEVICE_REMOVE(disk, destroy, 1)
 DEFINE_DEVICE_REMOVE(nic, remove, 0)
 DEFINE_DEVICE_REMOVE(nic, destroy, 1)
 
+/* vscsi */
+DEFINE_DEVICE_REMOVE(vscsi, destroy, 1)
+
 /* vkb */
 DEFINE_DEVICE_REMOVE(vkb, remove, 0)
 DEFINE_DEVICE_REMOVE(vkb, destroy, 1)
@@ -4227,6 +4662,7 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
 /* The following functions are defined:
  * libxl_device_disk_add
  * libxl_device_nic_add
+ * libxl_device_vscsi_add
  * libxl_device_vtpm_add
  */
 
@@ -4256,6 +4692,9 @@ DEFINE_DEVICE_ADD(disk)
 /* nic */
 DEFINE_DEVICE_ADD(nic)
 
+/* vscsi */
+DEFINE_DEVICE_ADD(vscsi)
+
 /* vtpm */
 DEFINE_DEVICE_ADD(vtpm)
 
@@ -6740,6 +7179,8 @@ int libxl_retrieve_domain_configuration(libxl_ctx *ctx, uint32_t domid,
 
     MERGE(nic, nics, COMPARE_DEVID, {});
 
+    MERGE(vscsi, vscsis, COMPARE_VSCSI, {});
+
     MERGE(vtpm, vtpms, COMPARE_DEVID, {});
 
     MERGE(pci, pcidevs, COMPARE_PCI, {});
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6bc75c5..c594402 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -736,6 +736,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
 #define LIBXL_HAVE_PSR_MBM 1
 #endif
 
+/*
+ * LIBXL_HAVE_VSCSI
+ *
+ * If this is defined, the PV SCSI feature is supported.
+ */
+#define LIBXL_HAVE_VSCSI 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
@@ -1252,6 +1259,26 @@ int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
                                  libxl_device_channel *channel,
                                  libxl_channelinfo *channelinfo);
 
+/* Virtual SCSI */
+int libxl_device_vscsi_add(libxl_ctx *ctx, uint32_t domid,
+                           libxl_device_vscsi *vscsi,
+                           const libxl_asyncop_how *ao_how)
+                           LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vscsi_remove(libxl_ctx *ctx, uint32_t domid,
+                              libxl_device_vscsi *vscsi,
+                              const libxl_asyncop_how *ao_how)
+                              LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vscsi_destroy(libxl_ctx *ctx, uint32_t domid,
+                               libxl_device_vscsi *vscsi,
+                               const libxl_asyncop_how *ao_how)
+                               LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_vscsi *libxl_device_vscsi_list(libxl_ctx *ctx, uint32_t domid, int *num);
+int libxl_device_vscsi_getinfo(libxl_ctx *ctx, uint32_t domid,
+                               libxl_device_vscsi *vscsi_host,
+                               libxl_vscsi_dev *vscsi_dev,
+                               libxl_vscsiinfo *vscsiinfo);
+
 /* 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 e5a343f..f67b4ce 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1157,6 +1157,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_vscsis(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 0c06dc4..111e348 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_vscsis
  * 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(vscsi)
 DEFINE_DEVICES_ADD(vtpm)
 
 #undef DEFINE_DEVICES_ADD
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9c22309..d27936a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2413,6 +2413,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_vscsi_add(libxl__egc *egc, uint32_t domid,
+                                     libxl_device_vscsi *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);
@@ -3037,6 +3041,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_vscsis(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);
@@ -3608,6 +3616,13 @@ static inline void libxl__update_config_nic(libxl__gc *gc,
     libxl_mac_copy(CTX, &dst->mac, &src->mac);
 }
 
+static inline void libxl__update_config_vscsi(libxl__gc *gc,
+                                             libxl_device_vscsi *dst,
+                                             libxl_device_vscsi *src)
+{
+    dst->devid = src->devid;
+}
+
 static inline void libxl__update_config_vtpm(libxl__gc *gc,
                                              libxl_device_vtpm *dst,
                                              libxl_device_vtpm *src)
@@ -3620,6 +3635,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)->v_hst == (b)->v_hst)
 #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 0866433..aae8961 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -552,6 +552,45 @@ 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", uint32),
+    ])
+
+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_vscsi_dev = Struct("vscsi_dev", [
+    ("vscsi_dev_id",     libxl_devid),
+    ("remove",           bool),
+    ("pdev",             libxl_vscsi_pdev),
+    ("vdev",             libxl_vscsi_hctl),
+    ])
+
+libxl_device_vscsi = Struct("device_vscsi", [
+    ("backend_domid",    libxl_domid),
+    ("devid",            libxl_devid),
+    ("v_hst",            uint32),
+    ("vscsi_devs",       Array(libxl_vscsi_dev, "num_vscsi_devs")),
+    ("next_vscsi_dev_id", libxl_devid),
+    ("feature_host",     libxl_defbool),
+    ])
+
 libxl_domain_config = Struct("domain_config", [
     ("c_info", libxl_domain_create_info),
     ("b_info", libxl_domain_build_info),
@@ -561,6 +600,7 @@ libxl_domain_config = Struct("domain_config", [
     ("pcidevs", Array(libxl_device_pci, "num_pcidevs")),
     ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
     ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
+    ("vscsis", Array(libxl_device_vscsi, "num_vscsis")),
     ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
     # a channel manifests as a console with a name,
     # see docs/misc/channels.txt
@@ -595,6 +635,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),
+    ("vscsi_dev_id", libxl_devid),
+    ("feature_host", bool),
+    ("vscsi_host_state", integer),
+    ("vscsi_dev_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..c301689
--- /dev/null
+++ b/tools/libxl/libxl_vscsi.c
@@ -0,0 +1,271 @@
+/*
+ * Copyright (C) 2015      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"
+
+static int vscsi_parse_hctl(char *str, libxl_vscsi_hctl *hctl)
+{
+    unsigned int hst, chn, tgt, lun;
+
+    if (sscanf(str, "%u:%u:%u:%u", &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 < 16; 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_ctx *ctx, libxl_vscsi_dev *v_dev,
+                             char *c, char *p, char *v)
+{
+    GC_INIT(ctx);
+    libxl_vscsi_hctl hctl;
+    unsigned int lun;
+    char wwn[16 + 1];
+    bool parsed_ok = false;
+
+    libxl_vscsi_hctl_init(&hctl);
+
+    v_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:%u", wwn, &lun) == 2 && vscsi_wwn_valid(wwn)) {
+            libxl_vscsi_pdev_init_type(&v_dev->pdev, LIBXL_VSCSI_PDEV_TYPE_WWN);
+            v_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(&v_dev->pdev, LIBXL_VSCSI_PDEV_TYPE_HCTL);
+        libxl_vscsi_hctl_copy(ctx, &v_dev->pdev.u.hctl.m, &hctl);
+        parsed_ok = true;
+    }
+
+    if (parsed_ok && vscsi_parse_hctl(v, &v_dev->vdev) != 0)
+        parsed_ok = false;
+
+    libxl_vscsi_hctl_dispose(&hctl);
+
+    GC_FREE;
+    return parsed_ok;
+}
+
+libxl_device_vscsi *libxl_device_vscsi_list(libxl_ctx *ctx, uint32_t domid,
+                                            int *num)
+{
+    GC_INIT(ctx);
+    libxl_vscsi_dev *v_dev;
+    libxl_device_vscsi *v_hst, *vscsi_hosts = NULL;
+    char *fe_path, *tmp, *c, *p, *v, *s;
+    char **dir, **devs_dir;
+    const char *devs_path, *be_path;
+    int r;
+    bool parsed_ok;
+    unsigned int ndirs = 0, ndevs_dirs = 0, i;
+    unsigned int vscsi_dev_id;
+
+    fe_path = libxl__sprintf(gc, "%s/device/vscsi",
+                             libxl__xs_get_dompath(gc, domid));
+    dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs);
+    /* Nothing to do */
+    if (!(dir && ndirs))
+        goto out;
+
+    /* List of hosts to be returned to the caller */
+    vscsi_hosts = libxl__malloc(NOGC, ndirs * sizeof(*vscsi_hosts));
+
+    /* Fill each host */
+    for (v_hst = vscsi_hosts; v_hst < vscsi_hosts + ndirs; ++v_hst, ++dir) {
+        libxl_device_vscsi_init(v_hst);
+
+        v_hst->devid = atoi(*dir);
+
+        tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/%s/backend-id",
+                             fe_path, *dir));
+        /* FIXME what if xenstore is broken? */
+        if (tmp)
+            v_hst->backend_domid = atoi(tmp);
+
+        be_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/%s/backend",
+                                 fe_path, *dir));
+        /* FIXME what if xenstore is broken? */
+        if (be_path) {
+            parsed_ok = false;
+            tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/feature-host", be_path));
+            if (tmp)
+                parsed_ok = atoi(tmp) != 0;
+            libxl_defbool_set(&v_hst->feature_host, parsed_ok);
+
+            devs_path = libxl__sprintf(gc, "%s/vscsi-devs", be_path);
+            devs_dir = libxl__xs_directory(gc, XBT_NULL, devs_path, &ndevs_dirs);
+        } else {
+            devs_dir = NULL;
+            libxl_defbool_set(&v_hst->feature_host, false);
+        }
+
+        if (devs_dir && ndevs_dirs) {
+            v_hst->vscsi_devs = libxl__malloc(NOGC, ndevs_dirs * sizeof(*v_dev));
+            v_hst->num_vscsi_devs = ndevs_dirs;
+            /* Fill each device connected to the host */
+            for (i = 0; i < ndevs_dirs; i++, devs_dir++) {
+                v_dev = &v_hst->vscsi_devs[i];
+                libxl_vscsi_dev_init(v_dev);
+                parsed_ok = false;
+                r = sscanf(*devs_dir, "dev-%u", &vscsi_dev_id);
+                if (r == 1) {
+                    c = libxl__xs_read(gc, XBT_NULL,
+                                       GCSPRINTF("%s/vscsi-devs/dev-%u/p-devname",
+                                       be_path, vscsi_dev_id));
+                    p = libxl__xs_read(gc, XBT_NULL,
+                                       GCSPRINTF("%s/vscsi-devs/dev-%u/p-dev",
+                                       be_path, vscsi_dev_id));
+                    v = libxl__xs_read(gc, XBT_NULL,
+                                       GCSPRINTF("%s/vscsi-devs/dev-%u/v-dev",
+                                       be_path, vscsi_dev_id));
+                    s = libxl__xs_read(gc, XBT_NULL,
+                                       GCSPRINTF("%s/vscsi-devs/dev-%u/state",
+                                       be_path, vscsi_dev_id));
+                    if (c && p && v && s) {
+                        parsed_ok = vscsi_parse_pdev(ctx, v_dev, c, p, v);
+                        LOG(DEBUG, "%s dev-%u state is %s", be_path, vscsi_dev_id, s);
+                        switch (atoi(s)) {
+                            case XenbusStateUnknown:
+                            case XenbusStateInitialising:
+                            case XenbusStateInitWait:
+                            case XenbusStateInitialised:
+                            case XenbusStateConnected:
+                            case XenbusStateReconfiguring:
+                            case XenbusStateReconfigured:
+                                break;
+                            case XenbusStateClosing:
+                            case XenbusStateClosed:
+                                v_dev->remove = true;
+                                break;
+                        }
+                    }
+
+                    /* Indication for caller that this v_dev is usable */
+                    if (parsed_ok) {
+                        v_dev->vscsi_dev_id = vscsi_dev_id;
+                        if (vscsi_dev_id >= v_hst->next_vscsi_dev_id ||
+                            v_hst->next_vscsi_dev_id == -1)
+                            v_hst->next_vscsi_dev_id = vscsi_dev_id + 1;
+                        v_hst->v_hst = v_dev->vdev.hst;
+                    }
+                }
+
+                if (!parsed_ok) {
+                    /* FIXME what if xenstore is broken? */
+                    LOG(ERROR, "%s/vscsi-devs/%s failed to parse",
+                               be_path, *devs_dir);
+                    continue;
+                }
+            }
+        }
+    }
+
+out:
+    *num = ndirs;
+
+    GC_FREE;
+    return vscsi_hosts;
+}
+
+int libxl_device_vscsi_getinfo(libxl_ctx *ctx, uint32_t domid,
+                               libxl_device_vscsi *vscsi_host,
+                               libxl_vscsi_dev *vscsi_dev,
+                               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 = vscsi_host->devid;
+    libxl_vscsi_pdev_copy(ctx, &vscsiinfo->pdev, &vscsi_dev->pdev);
+    libxl_vscsi_hctl_copy(ctx, &vscsiinfo->vdev, &vscsi_dev->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->vscsi_host_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, vscsi_dev->vscsi_dev_id));
+    vscsiinfo->vscsi_dev_state = val ? strtoul(val, NULL, 10) : -1;
+
+    rc = 0;
+out:
+    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..f76af3b
--- /dev/null
+++ b/tools/libxl/libxlu_vscsi.c
@@ -0,0 +1,750 @@
+/*
+ * libxlu_vscsi.c - xl configuration file parsing: setup and helper functions
+ *
+ * Copyright (C) 2015      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 int lun;
+};
+
+static int xlu__vscsi_parse_hctl(char *str, libxl_vscsi_hctl *hctl)
+{
+    unsigned int hst, chn, tgt, lun;
+
+    if (sscanf(str, "%u:%u:%u:%u", &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)
+{
+    unsigned int 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_%u", &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:%u", 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 int lun;
+    char wwn[XLU_WWN_LEN + 1];
+
+    memset(wwn, 0, sizeof(wwn));
+    if (sscanf(str, "naa.%16c:%u", 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_vscsi *new_host,
+                             libxl_vscsi_dev *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_host->v_hst = new_dev->vdev.hst;
+
+    if (fhost) {
+        fhost = xlu__vscsi_trim_string(fhost);
+        if (strcmp(fhost, "feature-host") == 0) {
+            libxl_defbool_set(&new_host->feature_host, true);
+        } else {
+            LOG(cfg, "invalid option '%s', expecting %s", fhost, "feature-host");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+    } else
+        libxl_defbool_set(&new_host->feature_host, false);
+    rc = 0;
+
+out:
+    free(tmp);
+    return rc;
+}
+
+
+int xlu_vscsi_append_dev(libxl_ctx *ctx, libxl_device_vscsi *hst,
+                                   libxl_vscsi_dev *dev)
+{
+    int rc;
+    libxl_vscsi_dev *devs;
+
+    devs = realloc(hst->vscsi_devs, sizeof(*dev) * (hst->num_vscsi_devs + 1));
+    if (!devs) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+
+    hst->vscsi_devs = devs;
+    libxl_vscsi_dev_init(hst->vscsi_devs + hst->num_vscsi_devs);
+    dev->vscsi_dev_id = hst->next_vscsi_dev_id;
+    libxl_vscsi_dev_copy(ctx, hst->vscsi_devs + hst->num_vscsi_devs, dev);
+    hst->num_vscsi_devs++;
+    hst->next_vscsi_dev_id++;
+    rc = 0;
+out:
+    return rc;
+}
+
+int xlu_vscsi_get_host(XLU_Config *cfg, libxl_ctx *ctx, uint32_t domid,
+                       const char *str, libxl_device_vscsi *vscsi_host)
+{
+    libxl_vscsi_dev *new_dev = NULL;
+    libxl_device_vscsi *new_host, *vscsi_hosts = NULL, *tmp;
+    int rc, found_host = -1, i;
+    int num_hosts;
+
+    new_host = malloc(sizeof(*new_host));
+    new_dev = malloc(sizeof(*new_dev));
+    if (!(new_host && new_dev)) {
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+    libxl_device_vscsi_init(new_host);
+    libxl_vscsi_dev_init(new_dev);
+
+    rc = xlu_vscsi_parse(cfg, ctx, str, new_host, new_dev);
+    if (rc)
+        goto out;
+
+    /* Look for existing vscsi_host for given domain */
+    vscsi_hosts = libxl_device_vscsi_list(ctx, domid, &num_hosts);
+    if (vscsi_hosts) {
+        for (i = 0; i < num_hosts; ++i) {
+            if (vscsi_hosts[i].v_hst == new_host->v_hst) {
+                found_host = i;
+                break;
+            }
+        }
+    }
+
+    if (found_host == -1) {
+        /* Not found, create new host */
+        new_host->next_vscsi_dev_id = 0;
+        tmp = new_host;
+    } else {
+        tmp = vscsi_hosts + found_host;
+
+        /* Check if the vdev address is already taken */
+        for (i = 0; i < tmp->num_vscsi_devs; ++i) {
+            if (tmp->vscsi_devs[i].vdev.chn == new_dev->vdev.chn &&
+                tmp->vscsi_devs[i].vdev.tgt == new_dev->vdev.tgt &&
+                tmp->vscsi_devs[i].vdev.lun == new_dev->vdev.lun) {
+                LOG(cfg, "vdev '%u:%u:%u:%u' is already used.\n",
+                    new_dev->vdev.hst, new_dev->vdev.chn, new_dev->vdev.tgt, new_dev->vdev.lun);
+                rc = ERROR_INVAL;
+                goto out;
+            }
+        }
+
+        if (libxl_defbool_val(new_host->feature_host) !=
+            libxl_defbool_val(tmp->feature_host)) {
+            LOG(cfg, "different feature-host setting: "
+                      "existing host has it %s, new host has it %s\n",
+                libxl_defbool_val(new_host->feature_host) ? "set" : "unset",
+                libxl_defbool_val(tmp->feature_host) ? "set" : "unset");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+    }
+
+    libxl_device_vscsi_copy(ctx, vscsi_host, tmp);
+    rc = xlu_vscsi_append_dev(ctx, vscsi_host, new_dev);
+    if (rc)
+        goto out;
+
+    rc = 0;
+
+out:
+    if (vscsi_hosts) {
+        for (i = 0; i < num_hosts; ++i)
+            libxl_device_vscsi_dispose(&vscsi_hosts[i]);
+        free(vscsi_hosts);
+    }
+    libxl_vscsi_dev_dispose(new_dev);
+    libxl_device_vscsi_dispose(new_host);
+    free(new_dev);
+    free(new_host);
+    return rc;
+}
+
+int xlu_vscsi_detach(XLU_Config *cfg, libxl_ctx *ctx, uint32_t domid, char *str)
+{
+    libxl_vscsi_dev v_dev = { }, *vd;
+    libxl_device_vscsi v_hst = { }, *vh, *vscsi_hosts;
+    int num_hosts, h, d, found = 0;
+    char *tmp = NULL;
+
+    libxl_device_vscsi_init(&v_hst);
+    libxl_vscsi_dev_init(&v_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);
+        goto out;
+    }
+
+    if (xlu_vscsi_parse(cfg, ctx, tmp, &v_hst, &v_dev))
+        goto out;
+
+    vscsi_hosts = libxl_device_vscsi_list(ctx, domid, &num_hosts);
+    if (!vscsi_hosts)
+        goto out;
+
+    for (h = 0; h < num_hosts; ++h) {
+        vh = vscsi_hosts + h;
+        for (d = 0; d < vh->num_vscsi_devs; d++) {
+            vd = vh->vscsi_devs + d;
+#define CMP(member) (vd->vdev.member == v_dev.vdev.member)
+            if (!found && CMP(hst) && CMP(chn) && CMP(tgt) && CMP(lun)) {
+                vd->remove = true;
+                libxl_device_vscsi_remove(ctx, domid, vh, NULL);
+                found = 1;
+            }
+#undef CMP
+            libxl_vscsi_dev_dispose(vd);
+        }
+        libxl_device_vscsi_dispose(vh);
+    }
+    free(vscsi_hosts);
+
+out:
+    free(tmp);
+    libxl_vscsi_dev_dispose(&v_dev);
+    libxl_device_vscsi_dispose(&v_hst);
+    return found;
+}
+
+int xlu_vscsi_config_add(XLU_Config *cfg,
+                         libxl_ctx *ctx,
+                         const char *str,
+                         int *num_vscsis,
+                         libxl_device_vscsi **vscsis)
+{
+    int rc, i;
+    libxl_vscsi_dev v_dev = { };
+    libxl_device_vscsi *tmp, v_hst = { };
+    bool hst_found = false;
+
+    /*
+     * #1: parse the devspec and place it in temporary host+dev part
+     * #2: find existing vscsi_host with number v_hst
+     *     if found, append the vscsi_dev to this vscsi_host
+     * #3: otherwise, create new vscsi_host and append vscsi_dev
+     * Note: v_hst does not represent the index named "num_vscsis",
+     *       it is a private index used just in the config file
+     */
+    libxl_device_vscsi_init(&v_hst);
+    libxl_vscsi_dev_init(&v_dev);
+
+    rc = xlu_vscsi_parse(cfg, ctx, str, &v_hst, &v_dev);
+    if (rc)
+        goto out;
+
+    if (*num_vscsis) {
+        for (i = 0; i < *num_vscsis; i++) {
+            tmp = *vscsis + i;
+            if (tmp->v_hst == v_hst.v_hst) {
+                rc = xlu_vscsi_append_dev(ctx, tmp, &v_dev);
+                if (rc) {
+                    LOG(cfg, "xlu_vscsi_append_dev failed: %d\n", rc);
+                    goto out;
+                }
+                hst_found = true;
+                break;
+	           }
+        }
+    }
+
+    if (!hst_found || !*num_vscsis) {
+        tmp = realloc(*vscsis, sizeof(v_hst) * (*num_vscsis + 1));
+        if (!tmp) {
+            LOG(cfg, "realloc #%d failed", *num_vscsis + 1);
+            rc = ERROR_NOMEM;
+            goto out;
+        }
+        *vscsis = tmp;
+        tmp = *vscsis + *num_vscsis;
+        libxl_device_vscsi_init(tmp);
+
+        v_hst.devid = *num_vscsis;
+        v_hst.next_vscsi_dev_id = 0;
+        libxl_device_vscsi_copy(ctx, tmp, &v_hst);
+
+        rc = xlu_vscsi_append_dev(ctx, tmp, &v_dev);
+        if (rc) {
+            LOG(cfg, "xlu_vscsi_append_dev failed: %d\n", rc);
+            goto out;
+        }
+
+        (*num_vscsis)++;
+    }
+
+    rc = 0;
+out:
+    libxl_vscsi_dev_dispose(&v_dev);
+    libxl_device_vscsi_dispose(&v_hst);
+    return rc;
+}
+#else /* ! __linux__ */
+int xlu_vscsi_append_dev(libxl_ctx *ctx,
+                         libxl_device_vscsi *hst,
+                         libxl_vscsi_dev *dev)
+{
+    return ERROR_INVAL;
+}
+
+int xlu_vscsi_get_host(XLU_Config *config,
+                       libxl_ctx *ctx,
+                       uint32_t domid,
+                       const char *str,
+                       libxl_device_vscsi *vscsi_host)
+{
+    return ERROR_INVAL;
+}
+
+int xlu_vscsi_parse(XLU_Config *cfg,
+                    libxl_ctx *ctx,
+                    const char *str,
+                    libxl_device_vscsi *new_host,
+                    libxl_vscsi_dev *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_vscsi **vscsis)
+{
+    return ERROR_INVAL;
+}
+#endif
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
index 989605a..f5928fb 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -114,6 +114,27 @@ int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str
 int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate,
                        libxl_device_nic *nic);
 
+/* Append a vscsi device to a vscsi host */
+int xlu_vscsi_append_dev(libxl_ctx *ctx, libxl_device_vscsi *hst,
+                         libxl_vscsi_dev *dev);
+/* Fill vscsi_host with device described in str (pdev,vdev[,options]) */
+int xlu_vscsi_get_host(XLU_Config *config,
+                       libxl_ctx *ctx,
+                       uint32_t domid,
+                       const char *str,
+                       libxl_device_vscsi *vscsi_host);
+/* Parse config string and fill provided vscsi host and vscsi device */
+int xlu_vscsi_parse(XLU_Config *cfg, libxl_ctx *ctx, const char *str,
+                    libxl_device_vscsi *new_host,
+                    libxl_vscsi_dev *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_vscsi **vscsis);
 #endif /* LIBXLUTIL_H */
 
 /*
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 5bc138c..5c82688 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -83,6 +83,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 394b55d..418ed6c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1168,7 +1168,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, *vscsis;
     XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian;
     int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian;
     int pci_power_mgmt = 0;
@@ -1683,6 +1683,17 @@ static void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_list(config, "vscsi", &vscsis, 0, 0)) {
+        int num_vscsi_items = 0;
+        d_config->num_vscsis = 0;
+        d_config->vscsis = NULL;
+        while ((buf = xlu_cfg_get_listitem (vscsis, num_vscsi_items)) != NULL) {
+            if (xlu_vscsi_config_add(config, ctx, buf, &d_config->num_vscsis, &d_config->vscsis))
+                exit(1);
+            num_vscsi_items++;
+        }
+    }
+
     if (!xlu_cfg_get_list(config, "vtpm", &vtpms, 0, 0)) {
         d_config->num_vtpms = 0;
         d_config->vtpms = NULL;
@@ -6630,6 +6641,177 @@ 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_vscsi *vscsi_host = 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;;
+    }
+
+    vscsi_host = xmalloc(sizeof(*vscsi_host));
+    libxl_device_vscsi_init(vscsi_host);
+
+    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_host(config, ctx, domid, str, vscsi_host);
+    if (rc < 0)
+        goto out;
+
+    if (dryrun_only) {
+        char *json = libxl_device_vscsi_to_json(ctx, vscsi_host);
+        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_vscsi_add(ctx, domid, vscsi_host, NULL)) {
+        fprintf(stderr, "libxl_device_vscsi_add failed.\n");
+        rc = 1;
+        goto out;
+    }
+
+    rc = 0;
+out:
+    if (config)
+        xlu_cfg_destroy(config);
+    libxl_device_vscsi_dispose(vscsi_host);
+    free(vscsi_host);
+    free(str);
+    free(feat_buf);
+    return rc;
+}
+
+int main_vscsilist(int argc, char **argv)
+{
+    int opt;
+    uint32_t domid;
+    libxl_device_vscsi *vscsi_hosts;
+    libxl_vscsiinfo vscsiinfo;
+    int num_hosts, h, d;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "scsi-list", 1) {
+        /* No options */
+    }
+    if (argc < 2) {
+        help("scsi-list");
+        return 1;
+    }
+
+    /*      Idx  BE  state host p_hst v_hst state */
+    printf("%-3s %-3s %-5s %-5s %-10s %-10s %-5s\n",
+           "Idx", "BE", "state", "host", "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 (!(vscsi_hosts = libxl_device_vscsi_list(ctx, domid, &num_hosts))) {
+            continue;
+        }
+        for (h = 0; h < num_hosts; ++h) {
+            for (d = 0; d < vscsi_hosts[h].num_vscsi_devs; d++) {
+                if (!libxl_device_vscsi_getinfo(ctx, domid, &vscsi_hosts[h], &vscsi_hosts[h].vscsi_devs[d], &vscsiinfo)) {
+                    char vdev[64];
+                    snprintf(vdev, sizeof(vdev), "%u:%u:%u:%u",
+                             vscsiinfo.vdev.hst, vscsiinfo.vdev.chn, vscsiinfo.vdev.tgt, vscsiinfo.vdev.lun);
+                    /*      Idx  BE  state Sta */
+                    printf("%-3d %-3d %-5d %-5d %-10s %-10s %d\n",
+                           vscsiinfo.devid,
+                           vscsiinfo.backend_id,
+                           vscsiinfo.vscsi_host_state,
+                           vscsiinfo.backend_id,
+                           vscsiinfo.pdev.p_devname, vdev,
+                           vscsiinfo.vscsi_dev_state);
+
+                    libxl_vscsiinfo_dispose(&vscsiinfo);
+                }
+            }
+            libxl_device_vscsi_dispose(&vscsi_hosts[h]);
+        }
+        free(vscsi_hosts);
+
+    }
+
+    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 found = 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;
+    }
+
+    found = xlu_vscsi_detach(config, ctx, domid, str);
+    if (!found)
+        fprintf(stderr, "%s(%u) vdev %s does not exist in domain %s\n", __func__, __LINE__, str, dom);
+
+out:
+    if (config)
+        xlu_cfg_destroy(config);
+    return !found;
+}
+
 int main_vtpmattach(int argc, char **argv)
 {
     int opt;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 9284887..80a7f53 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -366,6 +366,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",

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Konrad Rzeszutek Wilk]

On Fri, Apr 17, 2015 at 08:30:58AM +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                  | 441 ++++++++++++++++++++
>  tools/libxl/libxl.h                  |  27 ++
>  tools/libxl/libxl_create.c           |   1 +
>  tools/libxl/libxl_device.c           |   2 +
>  tools/libxl/libxl_internal.h         |  16 +
>  tools/libxl/libxl_types.idl          |  56 +++
>  tools/libxl/libxl_types_internal.idl |   1 +
>  tools/libxl/libxl_vscsi.c            | 271 +++++++++++++
>  tools/libxl/libxlu_vscsi.c           | 750 +++++++++++++++++++++++++++++++++++
>  tools/libxl/libxlutil.h              |  21 +
>  tools/libxl/xl.h                     |   3 +
>  tools/libxl/xl_cmdimpl.c             | 184 ++++++++-
>  tools/libxl/xl_cmdtable.c            |  15 +
>  16 files changed, 1862 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index f936dfc..d395e56 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -510,6 +510,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
> +dom0 SCSI devices as-is to the guest.

s/dom0/backend/

As in theory you could run an HVM guest with an SCSI device passed in - and
use said backed to pass the SCSI device to another guest.

'as-is' ? I thought there were some filtering done in the backend?

> +
> +Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
> +'pdev' describes the physical device, preferable in a persistant format.

What is 'persistent' format?

> +'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
> +'option' lists additional flags which a backend may recognize.
> +
> +The supported values for "pdev" and "option" depends on the used backend driver:
> +
> +=over 4
> +
> +=item B<Linux pvops>

s/pvops//

> +
> +=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. Ususally this is a WWN in the form of "na.WWN:LUN".

Usually.

> +
> +=item C<option>
> +
> +No options recognized.
> +
> +=back
> +
> +=item B<Linux xenlinux>

xenlinux? Classic Xen?

> +
> +=over 4
> +
> +=item C<pdev>
> +
> +The dom0 device in either /dev/scsidev or pHOST:CHANNEL:TARGET:LUN notation.
> +
> +Its recommended to use persistant 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<option>
> +
> +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 16783c8..19bdbfa 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1328,6 +1328,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 1b16598..79b3867 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -91,6 +91,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 \
> @@ -122,6 +123,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 511eef1..abe7d75 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2014,6 +2014,437 @@ static int libxl__resolve_domid(libxl__gc *gc, const char *name,
>  }
>  
>  /******************************************************************************/
> +
> +static void libxl__device_vscsi_dev_backend_rm(libxl__gc *gc,
> +                                              libxl_vscsi_dev *v,
> +                                              xs_transaction_t t,
> +                                              char *be_path,
> +                                              int dev_wait)
> +{
> +    char *path, *val;
> +    libxl_ctx *ctx = libxl__gc_owner(gc);
> +
> +    path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> +    val = libxl__xs_read(gc, t, path);
> +    LOG(DEBUG, "%s is %s", path, val);
> +    if (val && strcmp(val, GCSPRINTF("%d", dev_wait)) == 0) {
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-devname", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-dev", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/v-dev", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +    } else {
> +        LOG(ERROR, "%s has %s, expected %d", path, val, dev_wait);
> +    }
> +}
> +
> +static int libxl__device_vscsi_dev_backend_set(libxl__gc *gc,
> +                                               libxl_vscsi_dev *v,
> +                                               flexarray_t *back)
> +{
> +    int rc;
> +    libxl_vscsi_hctl *hctl;
> +
> +    switch (v->pdev.type) {
> +        case LIBXL_VSCSI_PDEV_TYPE_WWN:
> +            flexarray_append_pair(back,
> +                                  GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
> +                                  v->pdev.u.wwn.m);
> +            break;
> +        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
> +            hctl = &v->pdev.u.hctl.m;
> +            flexarray_append_pair(back,
> +                                  GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
> +                                  GCSPRINTF("%u:%u:%u:%u", hctl->hst, hctl->chn, hctl->tgt, hctl->lun));
> +            break;
> +        case LIBXL_VSCSI_PDEV_TYPE_INVALID:
> +        default:
> +            rc = ERROR_FAIL;
> +            goto out;
> +    }
> +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/p-devname", v->vscsi_dev_id),
> +                          v->pdev.p_devname);
> +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/v-dev", v->vscsi_dev_id),
> +                          GCSPRINTF("%u:%u:%u:%u", v->vdev.hst, v->vdev.chn, v->vdev.tgt, v->vdev.lun));
> +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id),
> +                          GCSPRINTF("%d", XenbusStateInitialising));
> +    rc = 0;
> +out:
> +    return rc;
> +}
> +
> +static int libxl__device_vscsi_new_backend(libxl__egc *egc,
> +                                           libxl__ao_device *aodev,
> +                                           libxl_device_vscsi *vscsi,
> +                                           libxl_domain_config *d_config)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    int rc, i;
> +    flexarray_t *back;
> +    flexarray_t *front;
> +    libxl_vscsi_dev *v;
> +    xs_transaction_t t = XBT_NULL;
> +
> +    /* Prealloc key+value: 4 toplevel + 4 per device */
> +    i = 2 * (4 + (4 * vscsi->num_vscsi_devs));
> +    back = flexarray_make(gc, i, 1);
> +    front = flexarray_make(gc, 2 * 2, 1);
> +
> +    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->feature_host) ?
> +                          "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_vscsi_devs; i++) {
> +        v = vscsi->vscsi_devs + i;
> +        if (v->remove)
> +            continue;
> +        rc = libxl__device_vscsi_dev_backend_set(gc, v, back);
> +        if (rc) goto out;

Don't you want 'return rc' as the out will try to call libxl__xs_transaction_abort?

> +    }
> +
> +    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;
> +}

Stopped here as I had to run to a meeting.

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

[Olaf Hering]

On Tue, Apr 21, Konrad Rzeszutek Wilk wrote:

> On Fri, Apr 17, 2015 at 08:30:58AM +0000, Olaf Hering wrote:
> > +=item B<vscsi=[ "VSCSI_SPEC_STRING", "VSCSI_SPEC_STRING", ...]>
> > +
> > +Specifies the PVSCSI devices to be provided to the guest. PVSCSI passes
> > +dom0 SCSI devices as-is to the guest.
> s/dom0/backend/

I will make that change.

> As in theory you could run an HVM guest with an SCSI device passed in - and
> use said backed to pass the SCSI device to another guest.

Sounds interesting. I assume you meant "backend"? How will the domU talk
to the other domU? Not sure if my libxl change covers that use case, and
how I would be able to verify such functionality. Hopefully merging my
series will not depend on such use case.

> 'as-is' ? I thought there were some filtering done in the backend?

Perhaps 'passthrough with filtering'? I will remove it.

> > +Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
> > +'pdev' describes the physical device, preferable in a persistant format.
> What is 'persistent' format?

Anything that uses identifiers provided by the hardware, like serial
numbers. /dev/disk/by-*/* for example. The h:c:t:l notation is not
persistant because the "h" part will change across reboots.

> > +
> > +=item C<option>
> > +
> > +No options recognized.
> > +
> > +=back
> > +
> > +=item B<Linux xenlinux>
> xenlinux? Classic Xen?

Anything based on linux-2.6.18-xen.hg. I will adjust this to "Linux
based on classic xen kernel"

> > +static int libxl__device_vscsi_new_backend(libxl__egc *egc,
> > +                                           libxl__ao_device *aodev,
> > +                                           libxl_device_vscsi *vscsi,
> > +                                           libxl_domain_config *d_config)
> > +{

> > +        rc = libxl__device_vscsi_dev_backend_set(gc, v, back);
> > +        if (rc) goto out;
> Don't you want 'return rc' as the out will try to call libxl__xs_transaction_abort?

The current code works because t is initialized to XBT_NULL, and
libxl__xs_transaction_abort will do nothing in that case. I will adjust
this as you suggested.

Olaf

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

[Konrad Rzeszutek Wilk]

On Wed, Apr 22, 2015 at 10:08:52AM +0200, Olaf Hering wrote:
> On Tue, Apr 21, Konrad Rzeszutek Wilk wrote:
> 
> > On Fri, Apr 17, 2015 at 08:30:58AM +0000, Olaf Hering wrote:
> > > +=item B<vscsi=[ "VSCSI_SPEC_STRING", "VSCSI_SPEC_STRING", ...]>
> > > +
> > > +Specifies the PVSCSI devices to be provided to the guest. PVSCSI passes
> > > +dom0 SCSI devices as-is to the guest.
> > s/dom0/backend/
> 
> I will make that change.
> 
> > As in theory you could run an HVM guest with an SCSI device passed in - and
> > use said backed to pass the SCSI device to another guest.
> 
> Sounds interesting. I assume you meant "backend"? How will the domU talk

Yes, backend.
> to the other domU? Not sure if my libxl change covers that use case, and

Using the ring-buffer? Communicate via the Xenstore?

> how I would be able to verify such functionality. Hopefully merging my
> series will not depend on such use case.

Right, not dependent on it.
> 
> > 'as-is' ? I thought there were some filtering done in the backend?
> 
> Perhaps 'passthrough with filtering'? I will remove it.
> 
> > > +Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
> > > +'pdev' describes the physical device, preferable in a persistant format.
> > What is 'persistent' format?
> 
> Anything that uses identifiers provided by the hardware, like serial
> numbers. /dev/disk/by-*/* for example. The h:c:t:l notation is not
> persistant because the "h" part will change across reboots.

You might want to define what you mean by that. Or include the
example (/dev/disk/by-*/*).
> 
> > > +
> > > +=item C<option>
> > > +
> > > +No options recognized.
> > > +
> > > +=back
> > > +
> > > +=item B<Linux xenlinux>
> > xenlinux? Classic Xen?
> 
> Anything based on linux-2.6.18-xen.hg. I will adjust this to "Linux
> based on classic xen kernel"

Excellent!
> 
> > > +static int libxl__device_vscsi_new_backend(libxl__egc *egc,
> > > +                                           libxl__ao_device *aodev,
> > > +                                           libxl_device_vscsi *vscsi,
> > > +                                           libxl_domain_config *d_config)
> > > +{
> 
> > > +        rc = libxl__device_vscsi_dev_backend_set(gc, v, back);
> > > +        if (rc) goto out;
> > Don't you want 'return rc' as the out will try to call libxl__xs_transaction_abort?
> 
> The current code works because t is initialized to XBT_NULL, and
> libxl__xs_transaction_abort will do nothing in that case. I will adjust
> this as you suggested.

Thanks.
> 
> Olaf

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

[Wei Liu]

On Wed, Apr 22, 2015 at 10:08:52AM +0200, Olaf Hering wrote:
> On Tue, Apr 21, Konrad Rzeszutek Wilk wrote:
> 
> > On Fri, Apr 17, 2015 at 08:30:58AM +0000, Olaf Hering wrote:
> > > +=item B<vscsi=[ "VSCSI_SPEC_STRING", "VSCSI_SPEC_STRING", ...]>
> > > +
> > > +Specifies the PVSCSI devices to be provided to the guest. PVSCSI passes
> > > +dom0 SCSI devices as-is to the guest.
> > s/dom0/backend/
> 
> I will make that change.
> 
> > As in theory you could run an HVM guest with an SCSI device passed in - and
> > use said backed to pass the SCSI device to another guest.
> 
> Sounds interesting. I assume you meant "backend"? How will the domU talk
> to the other domU? Not sure if my libxl change covers that use case, and
> how I would be able to verify such functionality. Hopefully merging my
> series will not depend on such use case.

We can already do that with network driver domain. DomUs communicate via
xenstore.

The first step is to not use anything hardcoded domid in you xenstore
paths, i.e. do not make assumption on the domain that runs the backend.

Wei.

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

[Olaf Hering]

On Tue, May 05, Wei Liu wrote:

> The first step is to not use anything hardcoded domid in you xenstore
> paths, i.e. do not make assumption on the domain that runs the backend.

Once someone wants that for pvscsi the vscsi=[] syntax has to be
adjusted to also recognize another string.

Olaf

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

[Wei Liu]

On Wed, May 06, 2015 at 08:58:29AM +0200, Olaf Hering wrote:
> On Tue, May 05, Wei Liu wrote:
> 
> > The first step is to not use anything hardcoded domid in you xenstore
> > paths, i.e. do not make assumption on the domain that runs the backend.
> 
> Once someone wants that for pvscsi the vscsi=[] syntax has to be
> adjusted to also recognize another string.
> 

Normally this is done in the spce string with backend= option. The
default backend is Dom0 (0).

I don't think you need to implement this feature at this point, just
keep in mind to not implement a protocol that's prevents us from using
another domain as backend.

Wei.

> Olaf

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

[Wei Liu]

On Fri, Apr 17, 2015 at 08:30:58AM +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                  | 441 ++++++++++++++++++++
>  tools/libxl/libxl.h                  |  27 ++
>  tools/libxl/libxl_create.c           |   1 +
>  tools/libxl/libxl_device.c           |   2 +
>  tools/libxl/libxl_internal.h         |  16 +
>  tools/libxl/libxl_types.idl          |  56 +++
>  tools/libxl/libxl_types_internal.idl |   1 +
>  tools/libxl/libxl_vscsi.c            | 271 +++++++++++++
>  tools/libxl/libxlu_vscsi.c           | 750 +++++++++++++++++++++++++++++++++++
>  tools/libxl/libxlutil.h              |  21 +
>  tools/libxl/xl.h                     |   3 +
>  tools/libxl/xl_cmdimpl.c             | 184 ++++++++-
>  tools/libxl/xl_cmdtable.c            |  15 +
>  16 files changed, 1862 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index f936dfc..d395e56 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -510,6 +510,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
> +dom0 SCSI devices as-is to the guest.
> +
> +Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
> +'pdev' describes the physical device, preferable in a persistant format.

"persistent", and please explain when "persistent format" is.

> +'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
> +'option' lists additional flags which a backend may recognize.
> +
> +The supported values for "pdev" and "option" depends on the used backend driver:
> +
> +=over 4
> +
> +=item B<Linux pvops>
> +
> +=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"

What is "targetcli"?

> +in domU.cfg has to refer to a config item in that framework instead of the raw
> +device. Ususally this is a WWN in the form of "na.WWN:LUN".
> +
> +=item C<option>
> +
> +No options recognized.
> +
> +=back
> +
> +=item B<Linux xenlinux>
> +
> +=over 4
> +
> +=item C<pdev>
> +
> +The dom0 device in either /dev/scsidev or pHOST:CHANNEL:TARGET:LUN notation.
> +

What is vHOST? What is pHOST?

> +Its recommended to use persistant names "/dev/disk/by-*/*" to refer to a "pdev".

"It's"  "persistent"

> +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<option>
> +
> +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 16783c8..19bdbfa 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1328,6 +1328,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 1b16598..79b3867 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -91,6 +91,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 \
> @@ -122,6 +123,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 511eef1..abe7d75 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2014,6 +2014,437 @@ static int libxl__resolve_domid(libxl__gc *gc, const char *name,
>  }
>  
>  /******************************************************************************/
> +
> +static void libxl__device_vscsi_dev_backend_rm(libxl__gc *gc,
> +                                              libxl_vscsi_dev *v,
> +                                              xs_transaction_t t,
> +                                              char *be_path,

const char *be_path

> +                                              int dev_wait)
> +{
> +    char *path, *val;
> +    libxl_ctx *ctx = libxl__gc_owner(gc);

You can just use CTX macro.

> +
> +    path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> +    val = libxl__xs_read(gc, t, path);
> +    LOG(DEBUG, "%s is %s", path, val);
> +    if (val && strcmp(val, GCSPRINTF("%d", dev_wait)) == 0) {
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-devname", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-dev", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/v-dev", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);
> +        path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
> +        xs_rm(ctx->xsh, t, path);

Some lines are too long. Please wrap them properly.

But can you not simplify this by removing be_path all together?

Also this function should return error number -- you probably don't want
caller to commence should things go wrong here.

> +    } else {
> +        LOG(ERROR, "%s has %s, expected %d", path, val, dev_wait);
> +    }
> +}
> +
> +static int libxl__device_vscsi_dev_backend_set(libxl__gc *gc,
> +                                               libxl_vscsi_dev *v,
> +                                               flexarray_t *back)
> +{
> +    int rc;
> +    libxl_vscsi_hctl *hctl;
> +
> +    switch (v->pdev.type) {
> +        case LIBXL_VSCSI_PDEV_TYPE_WWN:
> +            flexarray_append_pair(back,
> +                                  GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
> +                                  v->pdev.u.wwn.m);
> +            break;
> +        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
> +            hctl = &v->pdev.u.hctl.m;
> +            flexarray_append_pair(back,
> +                                  GCSPRINTF("vscsi-devs/dev-%u/p-dev", v->vscsi_dev_id),
> +                                  GCSPRINTF("%u:%u:%u:%u", hctl->hst, hctl->chn, hctl->tgt, hctl->lun));
> +            break;
> +        case LIBXL_VSCSI_PDEV_TYPE_INVALID:
> +        default:
> +            rc = ERROR_FAIL;
> +            goto out;
> +    }
> +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/p-devname", v->vscsi_dev_id),
> +                          v->pdev.p_devname);
> +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/v-dev", v->vscsi_dev_id),
> +                          GCSPRINTF("%u:%u:%u:%u", v->vdev.hst, v->vdev.chn, v->vdev.tgt, v->vdev.lun));
> +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id),
> +                          GCSPRINTF("%d", XenbusStateInitialising));

Please fix the lines that are too long.

> +    rc = 0;
> +out:
> +    return rc;
> +}
> +
> +static int libxl__device_vscsi_new_backend(libxl__egc *egc,
> +                                           libxl__ao_device *aodev,
> +                                           libxl_device_vscsi *vscsi,
> +                                           libxl_domain_config *d_config)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    int rc, i;
> +    flexarray_t *back;
> +    flexarray_t *front;
> +    libxl_vscsi_dev *v;
> +    xs_transaction_t t = XBT_NULL;
> +
> +    /* Prealloc key+value: 4 toplevel + 4 per device */
> +    i = 2 * (4 + (4 * vscsi->num_vscsi_devs));
> +    back = flexarray_make(gc, i, 1);
> +    front = flexarray_make(gc, 2 * 2, 1);
> +
> +    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->feature_host) ?
> +                          "1" : "0");
> +
> +    flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", vscsi->backend_domid));
> +    flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
> +

Lines too long.

> +    for (i = 0; i < vscsi->num_vscsi_devs; i++) {
> +        v = vscsi->vscsi_devs + i;
> +        if (v->remove)
> +            continue;
> +        rc = libxl__device_vscsi_dev_backend_set(gc, v, back);
> +        if (rc) goto out;
> +    }
> +
> +    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(libxl__egc *egc,
> +                                           libxl__ao_device *aodev,
> +                                           libxl_device_vscsi *vscsi,
> +                                           libxl_domain_config *d_config,
> +                                           char *be_path,

const char *

> +                                           int *dev_wait)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    int rc, i, be_state, be_wait;
> +    char *dev_path, *state_path, *state_val;
> +    flexarray_t *back;
> +    libxl_vscsi_dev *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_vscsi_devs));
> +    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;
> +                if (dev_wait)
> +                    *dev_wait = XenbusStateInitialising;
> +                do_reconfigure = false;
> +                break;
> +            case XenbusStateConnected:
> +                /* The backend can handle reconfigure */
> +                be_wait = XenbusStateConnected;
> +                if (dev_wait)
> +                    *dev_wait = XenbusStateClosed;
> +                flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateReconfiguring));
> +                do_reconfigure = true;
> +                break;
> +        }
> +
> +        /* Append new device or trigger removal */
> +        for (i = 0; i < vscsi->num_vscsi_devs; i++) {
> +            v = vscsi->vscsi_devs + i;
> +            unsigned int nb = 0;

Move this line at the beginning of this block. I think you will hit a
gcc warning here, won't you?

> +            dev_path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
> +            /* Preserve existing device */
> +            if (libxl__xs_directory(gc, XBT_NULL, dev_path, &nb) && nb) {
> +                /* Trigger device removal by forwarding state to XenbusStateClosing */
> +                if (do_reconfigure && v->remove)
> +                    flexarray_append_pair(back,
> +                                          GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id),
> +                                          GCSPRINTF("%d", XenbusStateClosing));
> +                continue;
> +            }
> +            rc = libxl__device_vscsi_dev_backend_set(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:
> +    LOG(ERROR, "%u: rc %d", __LINE__, rc);

Remove this line.

> +    libxl__xs_transaction_abort(gc, &t);
> +    return rc;
> +}
> +
> +static int libxl__device_from_vscsi(libxl__gc *gc, uint32_t domid,
> +                                    libxl_device_vscsi *vscsi,
> +                                    libxl__device *device)
> +{
> +    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_vscsi_add(libxl__egc *egc, uint32_t domid,
> +                             libxl_device_vscsi *vscsi,
> +                             libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    libxl__device *device;
> +    char *be_path;
> +    unsigned int be_dirs = 0;
> +    int rc;
> +    libxl_domain_config d_config;
> +    libxl_device_vscsi vscsi_saved;
> +    libxl__domain_userdata_lock *lock = NULL;
> +
> +    libxl_domain_config_init(&d_config);
> +
> +    /* In case of vscsi the copy remains identical to the provided input */

This comment is confusing.

> +    libxl_device_vscsi_init(&vscsi_saved);
> +    libxl_device_vscsi_copy(CTX, &vscsi_saved, vscsi);
> +
> +    if (vscsi->devid == -1) {
> +        if ((vscsi->devid = libxl__device_nextid(gc, domid, "vscsi")) < 0) {
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
> +    }
> +
> +    /* Adjust copy */
> +    libxl__update_config_vscsi(gc, &vscsi_saved, vscsi);
> +
> +    GCNEW(device);
> +    rc = libxl__device_from_vscsi(gc, domid, vscsi, device);
> +    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(vscsi, vscsis, domid, &vscsi_saved, COMPARE_VSCSI, &d_config);
> +    }
> +
> +    aodev->dev = device;
> +
> +    be_path = libxl__device_backend_path(gc, aodev->dev);
> +    if (libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs)) {

This looks bogus. Shouldn't you use a real transaction here and in the
following two functions?

> +        rc = libxl__device_vscsi_reconfigure(egc, aodev, vscsi, &d_config, be_path, NULL);
> +        if (rc)
> +            goto out;
> +        /* Notify that this is done */
> +        aodev->callback(egc, aodev);

And this is for? You can just do this in out path, right?

> +    } else {
> +        rc = libxl__device_vscsi_new_backend(egc, aodev, vscsi, &d_config);
> +        if (rc)
> +            goto out;
> +    }
> +
> +    rc = 0;
> +out:
> +    if (lock) libxl__unlock_domain_userdata(lock);
> +    libxl_device_vscsi_dispose(&vscsi_saved);
> +    libxl_domain_config_dispose(&d_config);
> +    aodev->rc = rc;
> +    if (rc) aodev->callback(egc, aodev);
> +    return;
> +}
> +
> +static void libxl__device_vscsi_dev_rm(libxl__egc *egc,
> +                                       libxl_device_vscsi *vscsi,
> +                                       libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    char *be_path;
> +    int rc, i, dev_wait;
> +    libxl_domain_config d_config;
> +    libxl__domain_userdata_lock *lock = NULL;
> +    libxl_vscsi_dev *v;
> +    xs_transaction_t t = XBT_NULL;
> +
> +    libxl_domain_config_init(&d_config);
> +
> +    if (vscsi->devid == -1) {
> +        rc = ERROR_INVAL;
> +        goto out;
> +    }
> +
> +    /* No other code will traverse device list, update json with removal info */
> +    if (aodev->update_json) {

When is update_json set to true?

Note that in DEFINE_DEVICE_ADD update_json is set to true. But in you
patch there doesn't seem to be code that does this.

> +        lock = libxl__lock_domain_userdata(gc, aodev->dev->domid);
> +        if (!lock) {
> +            rc = ERROR_LOCK_FAIL;
> +            goto out;
> +        }
> +
> +        rc = libxl__get_domain_configuration(gc, aodev->dev->domid, &d_config);
> +        if (rc) goto out;
> +
> +        /* Replace the item in the domain config */
> +        DEVICE_ADD(vscsi, vscsis, aodev->dev->domid, vscsi, COMPARE_VSCSI, &d_config);
> +    }
> +

Actually, what fields inside vscsi structure are updated in this case?

> +    be_path = libxl__device_backend_path(gc, aodev->dev);
> +    rc = libxl__device_vscsi_reconfigure(egc, aodev, vscsi, &d_config, be_path, &dev_wait);
> +    if (rc) goto out;
> +
> +    for (;;) {
> +        rc = libxl__xs_transaction_start(gc, &t);
> +        if (rc) goto out;
> +
> +        for (i = 0; i < vscsi->num_vscsi_devs; i++) {
> +            v = vscsi->vscsi_devs + i;
> +            if (v->remove)
> +                libxl__device_vscsi_dev_backend_rm(gc, v, t, be_path, dev_wait);
> +        }
> +
> +        rc = libxl__xs_transaction_commit(gc, &t);
> +        if (!rc) break;
> +        if (rc < 0) goto out;
> +    }
> +
> +    rc = 0;
> +
> +out:
> +    if (lock) libxl__unlock_domain_userdata(lock);
> +    libxl_domain_config_dispose(&d_config);
> +    aodev->rc = rc;
> +    /* Notify that this is done */
> +    aodev->callback(egc, aodev);
> +}
> +
> +/* Extended variant of DEFINE_DEVICE_REMOVE to handle reconfigure */

I have a very dumb question, what is "reconfigure"? Is this property of
vscsi device? What does this suppose to do?


> +                              libxl_device_vscsi *vscsi,
> +                              const libxl_asyncop_how *ao_how)
> +{
> +    AO_CREATE(ctx, domid, ao_how);
> +    libxl__device *device;
> +    libxl__ao_device *aodev;
> +    int rc, i;
> +    unsigned int remaining = vscsi->num_vscsi_devs;
> +
> +    GCNEW(device);
> +    rc = libxl__device_from_vscsi(gc, domid, vscsi, 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 = 0;
> +
> +    for (i = 0; i < vscsi->num_vscsi_devs; i++) {
> +        if (vscsi->vscsi_devs[i].remove)
> +            remaining--;
> +    }
> +
> +    LOG(DEBUG, "%u: v_hst %u, %u of %u remaining", domid, vscsi->v_hst, remaining, vscsi->num_vscsi_devs);
> +    if (remaining)
> +        libxl__device_vscsi_dev_rm(egc, vscsi, aodev);
> +    else
> +        libxl__initiate_device_remove(egc, aodev);
> +
> +out:
> +    if (rc) return AO_ABORT(rc);
> +    return AO_INPROGRESS;
> +}
> +

The rest of this patch is mostly parser, declarations and xl stuff. I've
omitted the reset for now...

Wei.

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

[Olaf Hering]

On Tue, May 05, Wei Liu wrote:

> On Fri, Apr 17, 2015 at 08:30:58AM +0000, Olaf Hering wrote:
> > +'pdev' describes the physical device, preferable in a persistant format.
> "persistent", and please explain when "persistent format" is.

Done.

> > +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"
> What is "targetcli"?

A tool to setup the various backend/frontend drivers of the Linux target
IO framework. http://linux-iscsi.org/wiki/Targetcli
In fact the underlying python-rtslib has to be aware of xen-scsiback.

> > +The dom0 device in either /dev/scsidev or pHOST:CHANNEL:TARGET:LUN notation.
> What is vHOST? What is pHOST?

This refers to the "physical" SCSI host in the backend domain, and vHOST
is the "index number" of a SCSI host presented to the guest.

> > +Its recommended to use persistant names "/dev/disk/by-*/*" to refer to a "pdev".
> "It's"  "persistent"

Done.

> > +    path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> > +    val = libxl__xs_read(gc, t, path);
> > +    LOG(DEBUG, "%s is %s", path, val);
> > +    if (val && strcmp(val, GCSPRINTF("%d", dev_wait)) == 0) {
> > +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/state", be_path, v->vscsi_dev_id);
> > +        xs_rm(ctx->xsh, t, path);
> > +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-devname", be_path, v->vscsi_dev_id);
> > +        xs_rm(ctx->xsh, t, path);
> > +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/p-dev", be_path, v->vscsi_dev_id);
> > +        xs_rm(ctx->xsh, t, path);
> > +        path = GCSPRINTF("%s/vscsi-devs/dev-%u/v-dev", be_path, v->vscsi_dev_id);
> > +        xs_rm(ctx->xsh, t, path);
> > +        path = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
> > +        xs_rm(ctx->xsh, t, path);
> Some lines are too long. Please wrap them properly.
> But can you not simplify this by removing be_path all together?

Done.

> Also this function should return error number -- you probably don't want
> caller to commence should things go wrong here.

Why would anyone care about failed xs_rm? The state was forwarded and
the domU is in unknown state if anything happens here. Also that
vscsi-devs/dev-N path will not be used again.

> > +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/p-devname", v->vscsi_dev_id),
> > +                          v->pdev.p_devname);
> > +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/v-dev", v->vscsi_dev_id),
> > +                          GCSPRINTF("%u:%u:%u:%u", v->vdev.hst, v->vdev.chn, v->vdev.tgt, v->vdev.lun));
> > +    flexarray_append_pair(back, GCSPRINTF("vscsi-devs/dev-%u/state", v->vscsi_dev_id),
> > +                          GCSPRINTF("%d", XenbusStateInitialising));
> Please fix the lines that are too long.

I did adjustments, but getting below 80 makes it look broken.
I'm willing to donate wider editor windows.

> > +    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->feature_host) ?
> > +                          "1" : "0");
> > +
> > +    flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", vscsi->backend_domid));
> > +    flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
> Lines too long.

Getting below 80 makes it look broken.

> > +        /* Append new device or trigger removal */
> > +        for (i = 0; i < vscsi->num_vscsi_devs; i++) {
> > +            v = vscsi->vscsi_devs + i;
> > +            unsigned int nb = 0;
> Move this line at the beginning of this block. I think you will hit a
> gcc warning here, won't you?

No warning, -Werror would have caught this. I think -std=gnu99 allows
this. I have flipped these two lines.

> > +    /* In case of vscsi the copy remains identical to the provided input */
> This comment is confusing.

What is confusing about it? "*vscsi == vscsi_saved". I will remove the
comment.

> > +    be_path = libxl__device_backend_path(gc, aodev->dev);
> > +    if (libxl__xs_directory(gc, XBT_NULL, be_path, &be_dirs)) {
> This looks bogus. Shouldn't you use a real transaction here and in the
> following two functions?

Not sure if that can be racy. Should the for-loop to retry the
transaction be in this function? After all that thing is protected by
the userdata lock.

> > +        rc = libxl__device_vscsi_reconfigure(egc, aodev, vscsi, &d_config, be_path, NULL);
> > +        if (rc)
> > +            goto out;
> > +        /* Notify that this is done */
> > +        aodev->callback(egc, aodev);
> And this is for? You can just do this in out path, right?

This has to be called unconditionally, the outpath does it only in case
of failure.

> > +    } else {
> > +        rc = libxl__device_vscsi_new_backend(egc, aodev, vscsi, &d_config);
> > +        if (rc)
> > +            goto out;
> > +    }
> > +
> > +    rc = 0;
> > +out:
> > +    if (lock) libxl__unlock_domain_userdata(lock);
> > +    libxl_device_vscsi_dispose(&vscsi_saved);
> > +    libxl_domain_config_dispose(&d_config);
> > +    aodev->rc = rc;
> > +    if (rc) aodev->callback(egc, aodev);
> > +    return;
> > +}


> > +    /* No other code will traverse device list, update json with removal info */
> > +    if (aodev->update_json) {
> When is update_json set to true?
> 
> Note that in DEFINE_DEVICE_ADD update_json is set to true. But in you
> patch there doesn't seem to be code that does this.

Right, somehow this got lost.

> > +        /* Replace the item in the domain config */
> > +        DEVICE_ADD(vscsi, vscsis, aodev->dev->domid, vscsi, COMPARE_VSCSI, &d_config);
> Actually, what fields inside vscsi structure are updated in this case?

The essential part is the ->vscsi_devs array, and the ->remove flag in
each subdevice.

> > +/* Extended variant of DEFINE_DEVICE_REMOVE to handle reconfigure */
> 
> I have a very dumb question, what is "reconfigure"? Is this property of
> vscsi device? What does this suppose to do?

Its XenbusStateReconfiguring, which tells the backend to rescan the
vscsi-devs directory. vscsi and pci do this because they have the
single_host-many_devices concept.

Thanks for the review. I will resend the current state later today.

Olaf

[PATCH v4 4/5] vscsiif.h: add some notes about xenstore layout

[Olaf Hering]

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/include/public/io/vscsiif.h | 68 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h
index e8e38a9..6aaba22 100644
--- a/xen/include/public/io/vscsiif.h
+++ b/xen/include/public/io/vscsiif.h
@@ -104,6 +104,74 @@
  *      response structures.
  */
 
+/*
+ * Xenstore format in practice
+ * ===========================
+ * 
+ * The backend driver uses a single_host:many_devices notation to manage domU
+ * devices. Everything is stored in /local/domain/0/backend/vscsi/. The
+ * xenstore layout looks like this:
+ * 
+ *     <domid>/<vhost>/feature-host = "0"
+ *     <domid>/<vhost>/frontend = "/local/domain/<domid>/device/vscsi/0"
+ *     <domid>/<vhost>/frontend-id = "<domid>"
+ *     <domid>/<vhost>/online = "1"
+ *     <domid>/<vhost>/state = "4"
+ *     <domid>/<vhost>/vscsi-devs/dev-0/p-dev = "8:0:2:1" or "naa.wwn:lun"
+ *     <domid>/<vhost>/vscsi-devs/dev-0/state = "4"
+ *     <domid>/<vhost>/vscsi-devs/dev-0/v-dev = "0:0:0:0"
+ *     <domid>/<vhost>/vscsi-devs/dev-1/p-dev = "8:0:2:2"
+ *     <domid>/<vhost>/vscsi-devs/dev-1/state = "4"
+ *     <domid>/<vhost>/vscsi-devs/dev-1/v-dev = "0:0:1:0"
+ * 
+ * The frontend driver maintains its state in
+ * /local/domain/<domid>/device/vscsi/.
+ * 
+ *     <vhost>/backend = "/local/domain/0/backend/vscsi/<domid>/<vhost>"
+ *     <vhost>/backend-id = "0"
+ *     <vhost>/event-channel = "20"
+ *     <vhost>/ring-ref = "43"
+ *     <vhost>/state = "4"
+ *     <vhost>/vscsi-devs/dev-0/state = "4"
+ *     <vhost>/vscsi-devs/dev-1/state = "4"
+ * 
+ * In addition to the entries for backend and frontend these flags are stored
+ * for the toolstack:
+ * 
+ *     <domid>/<vhost>/vscsi-devs/dev-1/p-devname = "/dev/$device"
+ * 
+ * 
+ * Backend/frontend protocol
+ * =========================
+ * 
+ * To create a vhost along with a device:
+ *     <domid>/<vhost>/feature-host = "0"
+ *     <domid>/<vhost>/frontend = "/local/domain/<domid>/device/vscsi/0"
+ *     <domid>/<vhost>/frontend-id = "<domid>"
+ *     <domid>/<vhost>/online = "1"
+ *     <domid>/<vhost>/state = "1"
+ *     <domid>/<vhost>/vscsi-devs/dev-0/p-dev = "8:0:2:1"
+ *     <domid>/<vhost>/vscsi-devs/dev-0/state = "1"
+ *     <domid>/<vhost>/vscsi-devs/dev-0/v-dev = "0:0:0:0"
+ * Wait for <domid>/<vhost>/state + <domid>/<vhost>/vscsi-devs/dev-0/state become 4
+ * 
+ * To add another device to a vhost:
+ *     <domid>/<vhost>/state = "7"
+ *     <domid>/<vhost>/vscsi-devs/dev-1/p-dev = "8:0:2:2"
+ *     <domid>/<vhost>/vscsi-devs/dev-1/state = "1"
+ *     <domid>/<vhost>/vscsi-devs/dev-1/v-dev = "0:0:1:0"
+ * Wait for <domid>/<vhost>/state + <domid>/<vhost>/vscsi-devs/dev-1/state become 4
+ * 
+ * To remove a device from a vhost:
+ *     <domid>/<vhost>/state = "7"
+ *     <domid>/<vhost>/vscsi-devs/dev-1/state = "5"
+ * Wait for <domid>/<vhost>/state to become 4
+ * Wait for <domid>/<vhost>/vscsi-devs/dev-1/state become 6
+ * Remove <domid>/<vhost>/vscsi-devs/dev-1/{state,p-dev,v-dev,p-devname}
+ * Remove <domid>/<vhost>/vscsi-devs/dev-1/
+ *
+ */
+
 /* Requests from the frontend to the backend */
 
 /*

Re: [PATCH v4 4/5] vscsiif.h: add some notes about xenstore layout

[Wei Liu]

On Fri, Apr 17, 2015 at 08:30:59AM +0000, Olaf Hering wrote:
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> ---
>  xen/include/public/io/vscsiif.h | 68 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h
> index e8e38a9..6aaba22 100644
> --- a/xen/include/public/io/vscsiif.h
> +++ b/xen/include/public/io/vscsiif.h
> @@ -104,6 +104,74 @@
>   *      response structures.
>   */
>  
> +/*
> + * Xenstore format in practice
> + * ===========================
> + * 
> + * The backend driver uses a single_host:many_devices notation to manage domU
> + * devices. Everything is stored in /local/domain/0/backend/vscsi/. The

Don't assume backend domid to be 0.

Wei.

Re: [PATCH v4 4/5] vscsiif.h: add some notes about xenstore layout

[Olaf Hering]

On Tue, May 05, Wei Liu wrote:

> On Fri, Apr 17, 2015 at 08:30:59AM +0000, Olaf Hering wrote:
> > + * The backend driver uses a single_host:many_devices notation to manage domU
> > + * devices. Everything is stored in /local/domain/0/backend/vscsi/. The
> Don't assume backend domid to be 0.

Once vscsi=[] understands such setup that number will change. But I will
will adjust this variable to refer to <backend_domid>.

Olaf

[PATCH v4 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework

[Olaf Hering]

Just to make them public, not meant for merging:
The scripts used during development to create a bunch of SCSI devices in
dom0 using the Linux target framework. targetcli3 and rtslib3 is used.

A patch is required for python-rtslib:
http://article.gmane.org/gmane.linux.scsi.target.devel/8146

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/misc/Makefile                      |  4 ++
 tools/misc/target-create-xen-scsiback.sh | 94 ++++++++++++++++++++++++++++++++
 tools/misc/target-delete-xen-scsiback.sh | 39 +++++++++++++
 3 files changed, 137 insertions(+)

diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index ccd36af..be101d8 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -32,6 +32,8 @@ INSTALL_SBIN += $(INSTALL_SBIN-y)
 
 # Everything to be installed in a private bin/
 INSTALL_PRIVBIN                += xenpvnetboot
+INSTALL_PRIVBIN                += target-create-xen-scsiback.sh
+INSTALL_PRIVBIN                += target-delete-xen-scsiback.sh
 
 # Everything to be installed
 TARGETS_ALL := $(INSTALL_BIN) $(INSTALL_SBIN) $(INSTALL_PRIVBIN)
@@ -42,6 +44,8 @@ TARGETS_COPY += xen-ringwatch
 TARGETS_COPY += xencons
 TARGETS_COPY += xencov_split
 TARGETS_COPY += xenpvnetboot
+TARGETS_COPY += target-create-xen-scsiback.sh
+TARGETS_COPY += target-delete-xen-scsiback.sh
 
 # Everything which needs to be built
 TARGETS_BUILD := $(filter-out $(TARGETS_COPY),$(TARGETS_ALL))
diff --git a/tools/misc/target-create-xen-scsiback.sh b/tools/misc/target-create-xen-scsiback.sh
new file mode 100644
index 0000000..fc49c2e
--- /dev/null
+++ b/tools/misc/target-create-xen-scsiback.sh
@@ -0,0 +1,94 @@
+#!/usr/bin/env bash
+unset LANG
+unset ${!LC_*}
+set -x
+set -e
+
+modprobe --version
+targetcli --version
+udevadm --version
+blockdev --version
+parted --version
+sfdisk --version
+mkswap --version
+
+configfs=/sys/kernel/config
+target_path=$configfs/target
+
+num_luns=4
+num_hosts=4
+
+get_wwn() {
+	sed '
+	s@-@@g
+	s@^\(.\{16\}\)\(.*\)@\1@
+	' /proc/sys/kernel/random/uuid
+}
+
+if test ! -d "${target_path}"
+then
+	modprobe -v configfs
+	mount -vt configfs configfs $configfs
+	modprobe -v target_core_mod
+fi
+modprobe -v xen-scsiback
+
+host=0
+while test $host -lt $num_hosts
+do
+	host=$(( $host + 1 ))
+	lun=0
+	loopback_wwn="naa.`get_wwn`"
+	pvscsi_wwn="naa.`get_wwn`"
+	targetcli /loopback create ${loopback_wwn}
+	targetcli /xen-pvscsi create ${pvscsi_wwn}
+	while test $lun -lt $num_luns
+	do
+		: h $host l $lun
+		f_file=/dev/shm/Fileio.${host}.${lun}.file
+		f_uuid=/dev/shm/Fileio.${host}.${lun}.uuid
+		f_link=/dev/shm/Fileio.${host}.${lun}.link
+		fileio_name="fio_${host}.${lun}"
+		pscsi_name="ps_${host}.${lun}"
+
+		targetcli /backstores/fileio create name=${fileio_name} "file_or_dev=${f_file}" size=$((1024*1024 * 8 )) sparse=true
+		targetcli /loopback/${loopback_wwn}/luns create /backstores/fileio/${fileio_name} $lun
+
+		udevadm settle --timeout=4
+
+		vpd_uuid="`sed -n '/^T10 VPD Unit Serial Number:/s@^[^:]\+:[[:blank:]]\+@@p' /sys/kernel/config/target/core/fileio_*/${fileio_name}/wwn/vpd_unit_serial`"
+		if test -z "${vpd_uuid}"
+		then
+			exit 1
+		fi
+		echo "${vpd_uuid}" > "${f_uuid}"
+		by_id="`echo ${vpd_uuid} | sed 's@-@@g;s@^\(.\{25\}\)\(.*\)@scsi-36001405\1@'`"
+		ln -sfvbn "/dev/disk/by-id/${by_id}" "${f_link}"
+
+		blockdev --rereadpt "${f_link}"
+		udevadm settle --timeout=4
+		echo 1,12,S | sfdisk "${f_link}"
+		udevadm settle --timeout=4
+		blockdev --rereadpt "${f_link}"
+		udevadm settle --timeout=4
+		parted -s "${f_link}" unit s print
+
+		d_link="`readlink \"${f_link}\"`"
+		if test -n "${d_link}"
+		then
+			p_link="${d_link}-part1"
+			ls -l "${p_link}"
+			mkswap -L "swp_${fileio_name}" "${p_link}"
+			udevadm settle --timeout=4
+			blockdev --rereadpt "${f_link}"
+			udevadm settle --timeout=4
+			parted -s "${f_link}" unit s print
+		fi
+
+		targetcli /backstores/pscsi create "dev=${f_link}" "${pscsi_name}"
+		targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1/luns create "/backstores/pscsi/${pscsi_name}" $lun
+
+		lun=$(( $lun + 1 ))
+	done
+done
+
diff --git a/tools/misc/target-delete-xen-scsiback.sh b/tools/misc/target-delete-xen-scsiback.sh
new file mode 100644
index 0000000..a90bd82
--- /dev/null
+++ b/tools/misc/target-delete-xen-scsiback.sh
@@ -0,0 +1,39 @@
+#!/usr/bin/env bash
+unset LANG
+unset ${!LC_*}
+set -x
+set -e
+
+targetcli --version
+
+configfs=/sys/kernel/config
+target_path=$configfs/target
+
+cd "${target_path}"
+cd xen-pvscsi
+for wwn in `ls -d naa.*`
+do
+	targetcli /xen-pvscsi delete $wwn
+done
+cd "${target_path}"
+cd core
+for name in `ls -d pscsi_*/*/wwn`
+do
+	name=${name%/wwn}
+	name=${name##*/}
+	targetcli /backstores/pscsi delete $name
+done
+cd "${target_path}"
+cd loopback
+for wwn in `ls -d naa.*`
+do
+	targetcli /loopback delete $wwn
+done
+cd "${target_path}"
+cd core
+for name in `ls -d fileio_*/*/wwn`
+do
+	name=${name%/wwn}
+	name=${name##*/}
+	targetcli /backstores/fileio delete $name
+done

Re: [PATCH v4 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework

[Olaf Hering]

On Fri, Apr 17, Olaf Hering wrote:

> Just to make them public, not meant for merging:
> The scripts used during development to create a bunch of SCSI devices in
> dom0 using the Linux target framework. targetcli3 and rtslib3 is used.
> 
> A patch is required for python-rtslib:
> http://article.gmane.org/gmane.linux.scsi.target.devel/8146

Because libvirt understands just h:c:t:l it is required to configure
also an alias with the appropriate numbers. Otherwise xen-scsiback will
not find its "p-dev".


diff --git a/tools/misc/target-create-xen-scsiback.sh b/tools/misc/target-create-xen-scsiback.sh
index fc49c2e..e168a3e 100644
--- a/tools/misc/target-create-xen-scsiback.sh
+++ b/tools/misc/target-create-xen-scsiback.sh
@@ -65,6 +65,19 @@ do
 		by_id="`echo ${vpd_uuid} | sed 's@-@@g;s@^\(.\{25\}\)\(.*\)@scsi-36001405\1@'`"
 		ln -sfvbn "/dev/disk/by-id/${by_id}" "${f_link}"
 
+		f_major=$((`stat --dereference --format=0x%t "${f_link}"`))
+		f_minor=$((`stat --dereference --format=0x%T "${f_link}"`))
+		if test -z "${f_major}" || test -z "${f_minor}"
+		then
+			exit 1
+		fi
+		f_alias=`ls -d /sys/dev/block/${f_major}:${f_minor}/device/scsi_device/*:*:*:*`
+		if test -z "${f_alias}"
+		then
+			exit 1
+		fi
+		f_alias=${f_alias##*/}
+
 		blockdev --rereadpt "${f_link}"
 		udevadm settle --timeout=4
 		echo 1,12,S | sfdisk "${f_link}"
@@ -87,6 +100,7 @@ do
 
 		targetcli /backstores/pscsi create "dev=${f_link}" "${pscsi_name}"
 		targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1/luns create "/backstores/pscsi/${pscsi_name}" $lun
+		targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1  set parameter alias=${f_alias%:*}
 
 		lun=$(( $lun + 1 ))
 	done