All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] libbxl: add support for pvscsi, iteration 5
@ 2015-05-06 13:28 Olaf Hering
  2015-05-06 13:28 ` [PATCH v5 1/5] vscsiif.h: fix WWN notation for p-dev property Olaf Hering
                   ` (4 more replies)
  0 siblings, 5 replies; 24+ messages in thread
From: Olaf Hering @ 2015-05-06 13:28 UTC (permalink / raw)
  To: xen-devel; +Cc: Olaf Hering

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

libvirt uses its existing SCSI support:
http://lists.xenproject.org/archives/html/xen-devel/2015-04/msg02963.html

targetcli/rtslib has to be aware of xen-scsiback (upstream unresponsive):
http://article.gmane.org/gmane.linux.scsi.target.devel/8146

TODO:
 - check if a transaction should be used in libxl__device_vscsi_add
 - maybe use events instead of polling for "state" changes in reconfigure
   (libxl__wait_for_backend vs. libxl__ev_devstate_wait)

Changes between v4 and v5:
 - vscsiif.h: refer to backend_domid
 - Set update_json in libxl_device_vscsi_remove
 - Remove comment from libxl__device_vscsi_add
 - Remove debug LOG from libxl__device_vscsi_reconfigure
 - Move local nb variable in libxl__device_vscsi_reconfigure
 - Make be_path const in libxl__device_vscsi_reconfigure
 - Adjust libxl__device_vscsi_dev_backend_set to avoid long lines
 - Adjust libxl__device_vscsi_dev_backend_rm to avoid long lines
 - Use CTX in libxl__device_vscsi_dev_backend_rm
 - Make be_path const in libxl__device_vscsi_dev_backend_rm
 - xl.cfg: Its typo
 - xl.cfg: Use persistent instead of persistant
 - Rename feature_host to scsi_raw_cmds
 - target-create-xen-scsiback.sh: detect pvops and xenlinux
 - Wrap long lines in main_vscsilist
 - Call libxl_vscsiinfo_dispose unconditional
 - Let scsi-list print p-dev instead of p-devname
 - Handle broken vscsi device entry in xenstore
 - Split libxl__vscsi_fill_host from libxl_device_vscsi_list
 - Make xlu_vscsi_append_dev static
 - Remove reference to pvscsi.txt from xenstore-paths.markdown
 - xl.cfg: update Linux and xenlinux
 - xl.cfg: refer to backend domain instead of dom0
 - xl.cfg: be more verbose what persistant format is
 - return if libxl__device_vscsi_dev_backend_set fails in libxl__device_vscsi_new_backend
 - target-create-xen-scsiback.sh: set also alias for libvirt

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
http://lists.xenproject.org/archives/html/xen-devel/2015-04/msg01949.html

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                      | 440 ++++++++++++++++++
 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                | 274 ++++++++++++
 tools/libxl/libxlu_vscsi.c               | 745 +++++++++++++++++++++++++++++++
 tools/libxl/libxlutil.h                  |  18 +
 tools/libxl/xl.h                         |   3 +
 tools/libxl/xl_cmdimpl.c                 | 205 ++++++++-
 tools/libxl/xl_cmdtable.c                |  15 +
 tools/misc/Makefile                      |   4 +
 tools/misc/target-create-xen-scsiback.sh | 135 ++++++
 tools/misc/target-delete-xen-scsiback.sh |  41 ++
 xen/include/public/io/vscsiif.h          |  70 ++-
 21 files changed, 2136 insertions(+), 2 deletions(-)
 create mode 100644 tools/libxl/libxl_vscsi.c
 create mode 100644 tools/libxl/libxlu_vscsi.c
 create mode 100755 tools/misc/target-create-xen-scsiback.sh
 create mode 100755 tools/misc/target-delete-xen-scsiback.sh

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v5 1/5] vscsiif.h: fix WWN notation for p-dev property
  2015-05-06 13:28 [PATCH v5 0/5] libbxl: add support for pvscsi, iteration 5 Olaf Hering
@ 2015-05-06 13:28 ` Olaf Hering
  2015-05-13 14:11   ` Ian Campbell
  2015-05-06 13:28 ` [PATCH v5 2/5] docs: add vscsi to xenstore-paths.markdown Olaf Hering
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 24+ messages in thread
From: Olaf Hering @ 2015-05-06 13:28 UTC (permalink / raw)
  To: xen-devel
  Cc: Olaf Hering, Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson,
	Jan Beulich

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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 2/5] docs: add vscsi to xenstore-paths.markdown
  2015-05-06 13:28 [PATCH v5 0/5] libbxl: add support for pvscsi, iteration 5 Olaf Hering
  2015-05-06 13:28 ` [PATCH v5 1/5] vscsiif.h: fix WWN notation for p-dev property Olaf Hering
@ 2015-05-06 13:28 ` Olaf Hering
  2015-05-13 14:12   ` Ian Campbell
  2015-05-06 13:28 ` [PATCH v5 3/5] libxl: add support for vscsi Olaf Hering
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 24+ messages in thread
From: Olaf Hering @ 2015-05-06 13:28 UTC (permalink / raw)
  To: xen-devel
  Cc: Olaf Hering, Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson,
	Jan Beulich

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..b1c68ce 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.
+
 #### ~/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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-06 13:28 [PATCH v5 0/5] libbxl: add support for pvscsi, iteration 5 Olaf Hering
  2015-05-06 13:28 ` [PATCH v5 1/5] vscsiif.h: fix WWN notation for p-dev property Olaf Hering
  2015-05-06 13:28 ` [PATCH v5 2/5] docs: add vscsi to xenstore-paths.markdown Olaf Hering
@ 2015-05-06 13:28 ` Olaf Hering
  2015-05-13 14:23   ` Ian Campbell
  2015-05-13 14:44   ` Ian Campbell
  2015-05-06 13:28 ` [PATCH v5 4/5] vscsiif.h: add some notes about xenstore layout Olaf Hering
  2015-05-06 13:28 ` [PATCH v5 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework Olaf Hering
  4 siblings, 2 replies; 24+ messages in thread
From: Olaf Hering @ 2015-05-06 13:28 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Olaf Hering, Ian Jackson, Ian Campbell, Stefano Stabellini

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                  | 440 +++++++++++++++++++++
 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            | 274 +++++++++++++
 tools/libxl/libxlu_vscsi.c           | 745 +++++++++++++++++++++++++++++++++++
 tools/libxl/libxlutil.h              |  18 +
 tools/libxl/xl.h                     |   3 +
 tools/libxl/xl_cmdimpl.c             | 205 +++++++++-
 tools/libxl/xl_cmdtable.c            |  15 +
 16 files changed, 1877 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index f936dfc..6a7dc8c 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
+SCSI devices from the backend domain to the guest.
+
+Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
+'pdev' describes the physical device, preferable in a persistent format such as /dev/disk/by-*/*.
+'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
+'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>
+
+=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 based on classic Xen kernel>
+
+=over 4
+
+=item C<pdev>
+
+The dom0 device in either /dev/scsidev or pHOST:CHANNEL:TARGET:LUN notation.
+
+It's recommended to use persistent names "/dev/disk/by-*/*" to refer to a "pdev".
+The toolstack will translate this internally to "h:c:t:l" notation, which is how
+the backend driver will access the device. Using the "h:c:t:l" notation for
+"pdev" in domU.cfg is discouraged because this value will change across reboots,
+depending on the detection order in the OS.
+
+=item C<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 516713e..4c17d63 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2014,6 +2014,436 @@ 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,
+                                              const char *be_path,
+                                              int dev_wait)
+{
+    char *dir, *path, *val;
+
+    dir = GCSPRINTF("%s/vscsi-devs/dev-%u", be_path, v->vscsi_dev_id);
+    path = GCSPRINTF("%s/state", dir);
+    val = libxl__xs_read(gc, t, path);
+    LOG(DEBUG, "%s is %s", path, val);
+    if (val && strcmp(val, GCSPRINTF("%d", dev_wait)) == 0) {
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/state", dir));
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/p-devname", dir));
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/p-dev", dir));
+        xs_rm(CTX->xsh, t, GCSPRINTF("%s/v-dev", dir));
+        xs_rm(CTX->xsh, t, dir);
+    } else {
+        LOG(ERROR, "%s has %s, expected %d", path, val, dev_wait);
+    }
+}
+
+static int libxl__device_vscsi_dev_backend_set(libxl__gc *gc,
+                                               libxl_vscsi_dev *v,
+                                               flexarray_t *back)
+{
+    int rc;
+    char *dir;
+    libxl_vscsi_hctl *hctl;
+
+    dir = GCSPRINTF("vscsi-devs/dev-%u", v->vscsi_dev_id);
+    switch (v->pdev.type) {
+        case LIBXL_VSCSI_PDEV_TYPE_WWN:
+            flexarray_append_pair(back,
+                                  GCSPRINTF("%s/p-dev", dir),
+                                  v->pdev.u.wwn.m);
+            break;
+        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
+            hctl = &v->pdev.u.hctl.m;
+            flexarray_append_pair(back,
+                                  GCSPRINTF("%s/p-dev", dir),
+                                  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("%s/p-devname", dir),
+                          v->pdev.p_devname);
+    flexarray_append_pair(back,
+                          GCSPRINTF("%s/v-dev", dir),
+                          GCSPRINTF("%u:%u:%u:%u", v->vdev.hst, v->vdev.chn, v->vdev.tgt, v->vdev.lun));
+    flexarray_append_pair(back,
+                          GCSPRINTF("%s/state", dir),
+                          GCSPRINTF("%d", XenbusStateInitialising));
+    rc = 0;
+out:
+    return rc;
+}
+
+static int libxl__device_vscsi_new_backend(libxl__egc *egc,
+                                           libxl__ao_device *aodev,
+                                           libxl_device_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->scsi_raw_cmds) ?
+                          "1" : "0");
+
+    flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", vscsi->backend_domid));
+    flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
+
+    for (i = 0; i < vscsi->num_vscsi_devs; i++) {
+        v = vscsi->vscsi_devs + i;
+        if (v->remove)
+            continue;
+        rc = libxl__device_vscsi_dev_backend_set(gc, v, back);
+        if (rc) return rc;
+    }
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        rc = libxl__device_exists(gc, t, aodev->dev);
+        if (rc < 0) goto out;
+        if (rc == 1) {              /* already exists in xenstore */
+            LOG(ERROR, "device already exists in xenstore");
+            rc = ERROR_DEVICE_EXISTS;
+            goto out;
+        }
+
+        if (aodev->update_json) {
+            rc = libxl__set_domain_configuration(gc, aodev->dev->domid, d_config);
+            if (rc) goto out;
+        }
+
+        libxl__device_generic_add(gc, t, aodev->dev,
+                                  libxl__xs_kvs_of_flexarray(gc, back,
+                                                             back->count),
+                                  libxl__xs_kvs_of_flexarray(gc, front,
+                                                             front->count),
+                                  NULL);
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    libxl__wait_device_connection(egc, aodev);
+    rc = 0;
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
+static int libxl__device_vscsi_reconfigure(libxl__egc *egc,
+                                           libxl__ao_device *aodev,
+                                           libxl_device_vscsi *vscsi,
+                                           libxl_domain_config *d_config,
+                                           const 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++) {
+            unsigned int nb = 0;
+            v = vscsi->vscsi_devs + i;
+            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:
+    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);
+
+    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;
+    aodev->update_json = true;
+
+    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 +4588,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 +4633,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 +4661,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 +4691,9 @@ DEFINE_DEVICE_ADD(disk)
 /* nic */
 DEFINE_DEVICE_ADD(nic)
 
+/* vscsi */
+DEFINE_DEVICE_ADD(vscsi)
+
 /* vtpm */
 DEFINE_DEVICE_ADD(vtpm)
 
@@ -6740,6 +7178,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 44bd8e2..c3d5b74 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -750,6 +750,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);
@@ -1268,6 +1275,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 93bb41e..a1c63f8 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 8eb38aa..01b00e1 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2420,6 +2420,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);
@@ -3045,6 +3049,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);
@@ -3616,6 +3624,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)
@@ -3628,6 +3643,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 117b61d..7f0dcc6 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -553,6 +553,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),
+    ("scsi_raw_cmds",     libxl_defbool),
+    ])
+
 libxl_domain_config = Struct("domain_config", [
     ("c_info", libxl_domain_create_info),
     ("b_info", libxl_domain_build_info),
@@ -562,6 +601,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
@@ -596,6 +636,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),
+    ("scsi_raw_cmds", 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..a29117c
--- /dev/null
+++ b/tools/libxl/libxl_vscsi.c
@@ -0,0 +1,274 @@
+/*
+ * 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__gc *gc, libxl_vscsi_dev *v_dev,
+                             char *c, char *p, char *v)
+{
+    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);
+
+    return parsed_ok;
+}
+
+static void libxl__vscsi_fill_host(libxl__gc *gc,
+                                   const char *devs_path,
+                                   char **dev_dirs,
+                                   unsigned int ndev_dirs,
+                                   libxl_device_vscsi *v_hst)
+{
+    libxl_vscsi_dev *v_dev;
+    bool parsed_ok;
+    char *c, *p, *v, *s, *dev;
+    unsigned int vscsi_dev_id;
+    int i, r;
+
+    v_hst->vscsi_devs = libxl__malloc(NOGC, ndev_dirs * sizeof(*v_dev));
+    v_hst->num_vscsi_devs = ndev_dirs;
+    /* Fill each device connected to the host */
+    for (i = 0; i < ndev_dirs; i++, dev_dirs++) {
+        v_dev = v_hst->vscsi_devs + i;
+        libxl_vscsi_dev_init(v_dev);
+        parsed_ok = false;
+        r = sscanf(*dev_dirs, "dev-%u", &vscsi_dev_id);
+        if (r != 1) {
+            LOG(ERROR, "expected dev-N, got '%s'", *dev_dirs);
+            continue;
+        }
+
+        dev = GCSPRINTF("%s/%s", devs_path, *dev_dirs);
+        c = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/p-devname", dev));
+        p = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/p-dev", dev));
+        v = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/v-dev", dev));
+        s = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", dev));
+        LOG(DEBUG, "%s/state is %s", dev, s);
+        if (!(c && p && v && s)) {
+            LOG(ERROR, "p-devname '%s' p-dev '%s' v-dev '%s'", c, p, v);
+            continue;
+        }
+
+        parsed_ok = vscsi_parse_pdev(gc, v_dev, c, p, v);
+        switch (atoi(s)) {
+            case XenbusStateUnknown:
+            case XenbusStateInitialising:
+            case XenbusStateInitWait:
+            case XenbusStateInitialised:
+            case XenbusStateConnected:
+            case XenbusStateReconfiguring:
+            case XenbusStateReconfigured:
+                break;
+            case XenbusStateClosing:
+            case XenbusStateClosed:
+                v_dev->remove = true;
+                break;
+        }
+
+        /* 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;
+        }
+
+        /* FIXME what if xenstore is broken? */
+        if (!parsed_ok)
+            LOG(ERROR, "%s failed to parse", dev);
+    }
+}
+
+libxl_device_vscsi *libxl_device_vscsi_list(libxl_ctx *ctx, uint32_t domid,
+                                            int *num)
+{
+    GC_INIT(ctx);
+    libxl_device_vscsi *v_hst, *vscsi_hosts = NULL;
+    char *fe_path, *tmp;
+    char **dir, **dev_dirs;
+    const char *devs_path, *be_path;
+    bool parsed_ok;
+    unsigned int ndirs = 0, ndev_dirs;
+
+    fe_path = GCSPRINTF("%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) {
+            libxl_defbool_set(&v_hst->scsi_raw_cmds, false);
+            continue;
+        }
+
+        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->scsi_raw_cmds, parsed_ok);
+
+        devs_path = GCSPRINTF("%s/vscsi-devs", be_path);
+        dev_dirs = libxl__xs_directory(gc, XBT_NULL, devs_path, &ndev_dirs);
+        if (dev_dirs && ndev_dirs)
+            libxl__vscsi_fill_host(gc, devs_path, dev_dirs, ndev_dirs, v_hst);
+    }
+
+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..4131cfb
--- /dev/null
+++ b/tools/libxl/libxlu_vscsi.c
@@ -0,0 +1,745 @@
+/*
+ * 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)
+{
+    size_t len;
+
+    while (isspace(*s))
+        s++;
+    len = strlen(s);
+    while (len-- > 1 && isspace(s[len]))
+        s[len] = '\0';
+    return s;
+}
+
+
+static int xlu__vscsi_parse_dev(XLU_Config *cfg, char *pdev, libxl_vscsi_hctl *hctl)
+{
+    struct stat dentry;
+    char *sysfs = NULL;
+    const char *type;
+    int rc, found = 0;
+    DIR *dirp;
+    struct dirent *de;
+
+    /* stat pdev to get device's sysfs entry */
+    if (stat (pdev, &dentry) < 0) {
+        LOG(cfg, "%s, device node not found", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    if (S_ISBLK (dentry.st_mode)) {
+        type = "block";
+    } else if (S_ISCHR (dentry.st_mode)) {
+        type = "char";
+    } else {
+        LOG(cfg, "%s, device node not a block or char device", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    /* /sys/dev/type/major:minor symlink added in 2.6.27 */
+    if (asprintf(&sysfs, "/sys/dev/%s/%u:%u/device/scsi_device", type,
+                 major(dentry.st_rdev), minor(dentry.st_rdev)) < 0) {
+        sysfs = NULL;
+        rc = ERROR_NOMEM;
+        goto out;
+    }
+
+    dirp = opendir(sysfs);
+    if (!dirp) {
+        LOG(cfg, "%s, no major:minor link in sysfs", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        if (xlu__vscsi_parse_hctl(de->d_name, hctl))
+            continue;
+
+        found = 1;
+        break;
+    }
+    closedir(dirp);
+
+    if (!found) {
+        LOG(cfg, "%s, no h:c:t:l link in sysfs", pdev);
+        rc = ERROR_INVAL;
+        goto out;
+    }
+
+    rc = 0;
+out:
+    free(sysfs);
+    return rc;
+}
+
+static bool xlu__vscsi_wwn_valid(const char *p)
+{
+    bool ret = true;
+    int i = 0;
+
+    for (i = 0; i < XLU_WWN_LEN; i++, p++) {
+        if (*p >= '0' && *p <= '9')
+            continue;
+        if (*p >= 'a' && *p <= 'f')
+            continue;
+        if (*p >= 'A' && *p <= 'F')
+            continue;
+        ret = false;
+        break;
+    }
+    return ret;
+}
+
+static bool xlu__vscsi_compare_hctl(libxl_vscsi_hctl *a, libxl_vscsi_hctl *b)
+{
+    if (a->hst == b->hst &&
+        a->chn == b->chn &&
+        a->tgt == b->tgt &&
+        a->lun == b->lun)
+        return true;
+    return false;
+}
+
+/* Finally at
+ * /sys/kernel/config/target/xen-pvscsi/naa.<wwn>/tpgt_1/lun/lun_0/<X>/udev_path
+ */
+static bool xlu__vscsi_compare_udev(struct xlu__vscsi_target *tgt)
+{
+    bool ret;
+    int fd;
+    ssize_t read_sz;
+    libxl_vscsi_hctl udev_hctl;
+
+    libxl_vscsi_hctl_init(&udev_hctl);
+
+    fd = open(tgt->path, O_RDONLY);
+    if (fd < 0){
+        ret = false;
+        goto out;
+    }
+    read_sz = read(fd, &tgt->udev_path, sizeof(tgt->udev_path) - 1);
+    close(fd);
+
+    if (read_sz < 0 || read_sz > sizeof(tgt->udev_path) - 1) {
+        ret = false;
+        goto out;
+    }
+    tgt->udev_path[read_sz] = '\0';
+    read_sz--;
+    if (tgt->udev_path[read_sz] == '\n')
+        tgt->udev_path[read_sz] = '\0';
+
+    if (xlu__vscsi_parse_dev(tgt->cfg, tgt->udev_path, &udev_hctl)) {
+        ret = false;
+        goto out;
+    }
+    ret = xlu__vscsi_compare_hctl(tgt->pdev_hctl, &udev_hctl);
+
+out:
+    libxl_vscsi_hctl_dispose(&udev_hctl);
+    return ret;
+}
+
+/* /sys/kernel/config/target/xen-pvscsi/naa.<wwn>/tpgt_1/lun/lun_0/<X>/udev_path */
+static bool xlu__vscsi_walk_dir_lun(struct xlu__vscsi_target *tgt)
+{
+    bool found;
+    DIR *dirp;
+    struct dirent *de;
+    size_t path_len = strlen(tgt->path);
+    char *subdir = &tgt->path[path_len];
+
+    dirp = opendir(tgt->path);
+    if (!dirp)
+        return false;
+
+    found = false;
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        snprintf(subdir, sizeof(tgt->path) - path_len, "/%s/udev_path", de->d_name);
+
+        found = xlu__vscsi_compare_udev(tgt);
+        if (found)
+            break;
+
+        *subdir = '\0';
+    }
+    closedir(dirp);
+    return found;
+}
+
+/* /sys/kernel/config/target/xen-pvscsi/naa.<wwn>/tpgt_1/lun/lun_0 */
+static bool xlu__vscsi_walk_dir_luns(struct xlu__vscsi_target *tgt)
+{
+    bool found;
+    DIR *dirp;
+    struct dirent *de;
+    size_t path_len = strlen(tgt->path);
+    char *subdir = &tgt->path[path_len];
+
+    dirp = opendir(tgt->path);
+    if (!dirp)
+        return false;
+
+    found = false;
+    while ((de = readdir(dirp))) {
+        if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, ".."))
+            continue;
+
+        if (sscanf(de->d_name, "lun_%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->scsi_raw_cmds, true);
+        } else {
+            LOG(cfg, "invalid option '%s', expecting %s", fhost, "feature-host");
+            rc = ERROR_INVAL;
+            goto out;
+        }
+    } else
+        libxl_defbool_set(&new_host->scsi_raw_cmds, false);
+    rc = 0;
+
+out:
+    free(tmp);
+    return rc;
+}
+
+
+static 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].vscsi_dev_id != -1 &&
+                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->scsi_raw_cmds) !=
+            libxl_defbool_val(tmp->scsi_raw_cmds)) {
+            LOG(cfg, "different feature-host setting: "
+                      "existing host has it %s, new host has it %s\n",
+                libxl_defbool_val(new_host->scsi_raw_cmds) ? "set" : "unset",
+                libxl_defbool_val(tmp->scsi_raw_cmds) ? "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 && vd->vscsi_dev_id != -1 &&
+                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_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..1bd761c 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -114,6 +114,24 @@ 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);
 
+/* 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 648ca08..4d5d69a 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;
@@ -6643,6 +6654,198 @@ 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 pdev[64], vdev[64];
+                    switch (vscsiinfo.pdev.type) {
+                        case LIBXL_VSCSI_PDEV_TYPE_HCTL:
+                            snprintf(pdev, sizeof(pdev), "%u:%u:%u:%u",
+                                     vscsiinfo.pdev.u.hctl.m.hst,
+                                     vscsiinfo.pdev.u.hctl.m.chn,
+                                     vscsiinfo.pdev.u.hctl.m.tgt,
+                                     vscsiinfo.pdev.u.hctl.m.lun);
+                            break;
+                        case LIBXL_VSCSI_PDEV_TYPE_WWN:
+                            snprintf(pdev, sizeof(pdev), "%s",
+                                     vscsiinfo.pdev.u.wwn.m);
+                            break;
+                        default:
+                            pdev[0] = '\0';
+                            break;
+                    }
+                    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,
+                           pdev, 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 7f4759b..67fcce0 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -367,6 +367,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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 4/5] vscsiif.h: add some notes about xenstore layout
  2015-05-06 13:28 [PATCH v5 0/5] libbxl: add support for pvscsi, iteration 5 Olaf Hering
                   ` (2 preceding siblings ...)
  2015-05-06 13:28 ` [PATCH v5 3/5] libxl: add support for vscsi Olaf Hering
@ 2015-05-06 13:28 ` Olaf Hering
  2015-05-13 14:14   ` Ian Campbell
  2015-05-06 13:28 ` [PATCH v5 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework Olaf Hering
  4 siblings, 1 reply; 24+ messages in thread
From: Olaf Hering @ 2015-05-06 13:28 UTC (permalink / raw)
  To: xen-devel
  Cc: Olaf Hering, Keir Fraser, Ian Campbell, Tim Deegan, Ian Jackson,
	Jan Beulich

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..2c5f04a 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/<backend_domid>/backend/vscsi/.
+ * The xenstore layout looks like this (dom0 is assumed to be the backend_domid):
+ * 
+ *     <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 */
 
 /*

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v5 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework
  2015-05-06 13:28 [PATCH v5 0/5] libbxl: add support for pvscsi, iteration 5 Olaf Hering
                   ` (3 preceding siblings ...)
  2015-05-06 13:28 ` [PATCH v5 4/5] vscsiif.h: add some notes about xenstore layout Olaf Hering
@ 2015-05-06 13:28 ` Olaf Hering
  2015-05-13 14:18   ` Ian Campbell
  4 siblings, 1 reply; 24+ messages in thread
From: Olaf Hering @ 2015-05-06 13:28 UTC (permalink / raw)
  To: xen-devel; +Cc: 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 | 135 +++++++++++++++++++++++++++++++
 tools/misc/target-delete-xen-scsiback.sh |  41 ++++++++++
 3 files changed, 180 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 100755
index 0000000..96d4c39
--- /dev/null
+++ b/tools/misc/target-create-xen-scsiback.sh
@@ -0,0 +1,135 @@
+#!/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
+
+case "$1" in
+	-p)
+	backend="pvops"
+	;;
+	-x)
+	backend="xenlinux"
+	;;
+	*)
+	: "usage: $0 [-p|-x]"
+	if grep -qw xenfs$ /proc/filesystems
+	then
+		backend="pvops"
+	else
+		backend="xenlinux"
+	fi
+	;;
+esac
+
+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
+if test "${backend}" = "pvops"
+then
+	modprobe -v xen-scsiback
+fi
+
+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}
+	if test "${backend}" = "pvops"
+	then
+		targetcli /xen-pvscsi create ${pvscsi_wwn}
+	fi
+	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}"
+
+		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}"
+		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}"
+		if test "${backend}" = "pvops"
+		then
+			targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1/luns create "/backstores/pscsi/${pscsi_name}" $lun
+			targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1  set parameter alias=${f_alias%:*}
+		fi
+
+		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 100755
index 0000000..5b6dc54
--- /dev/null
+++ b/tools/misc/target-delete-xen-scsiback.sh
@@ -0,0 +1,41 @@
+#!/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}"
+if cd xen-pvscsi
+then
+	for wwn in `ls -d naa.*`
+	do
+		targetcli /xen-pvscsi delete $wwn
+	done
+fi
+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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 1/5] vscsiif.h: fix WWN notation for p-dev property
  2015-05-06 13:28 ` [PATCH v5 1/5] vscsiif.h: fix WWN notation for p-dev property Olaf Hering
@ 2015-05-13 14:11   ` Ian Campbell
  0 siblings, 0 replies; 24+ messages in thread
From: Ian Campbell @ 2015-05-13 14:11 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Keir Fraser, Tim Deegan, Ian Jackson, Jan Beulich, xen-devel

On Wed, 2015-05-06 at 13:28 +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.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 2/5] docs: add vscsi to xenstore-paths.markdown
  2015-05-06 13:28 ` [PATCH v5 2/5] docs: add vscsi to xenstore-paths.markdown Olaf Hering
@ 2015-05-13 14:12   ` Ian Campbell
  0 siblings, 0 replies; 24+ messages in thread
From: Ian Campbell @ 2015-05-13 14:12 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Keir Fraser, Tim Deegan, Ian Jackson, Jan Beulich, xen-devel

On Wed, 2015-05-06 at 13:28 +0000, Olaf Hering wrote:
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 4/5] vscsiif.h: add some notes about xenstore layout
  2015-05-06 13:28 ` [PATCH v5 4/5] vscsiif.h: add some notes about xenstore layout Olaf Hering
@ 2015-05-13 14:14   ` Ian Campbell
  0 siblings, 0 replies; 24+ messages in thread
From: Ian Campbell @ 2015-05-13 14:14 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Keir Fraser, Tim Deegan, Ian Jackson, Jan Beulich, xen-devel

On Wed, 2015-05-06 at 13:28 +0000, Olaf Hering wrote:
> Signed-off-by: Olaf Hering <olaf@aepfle.de>

I'm not sure how much of this really belongs in an ABI document, but I
don't think there's really anywhere else so

Acked-by: 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..2c5f04a 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/<backend_domid>/backend/vscsi/.
> + * The xenstore layout looks like this (dom0 is assumed to be the backend_domid):
> + * 
> + *     <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 */
>  
>  /*

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework
  2015-05-06 13:28 ` [PATCH v5 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework Olaf Hering
@ 2015-05-13 14:18   ` Ian Campbell
  2015-05-13 14:37     ` Olaf Hering
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2015-05-13 14:18 UTC (permalink / raw)
  To: Olaf Hering; +Cc: xen-devel

On Wed, 2015-05-06 at 13:28 +0000, 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.


I'm not sure what this is all about, but is there anything end-user-ish
which ought to be documented or put on the wiki or something?


> 
> 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 | 135 +++++++++++++++++++++++++++++++
>  tools/misc/target-delete-xen-scsiback.sh |  41 ++++++++++
>  3 files changed, 180 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 100755
> index 0000000..96d4c39
> --- /dev/null
> +++ b/tools/misc/target-create-xen-scsiback.sh
> @@ -0,0 +1,135 @@
> +#!/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
> +
> +case "$1" in
> +	-p)
> +	backend="pvops"
> +	;;
> +	-x)
> +	backend="xenlinux"
> +	;;
> +	*)
> +	: "usage: $0 [-p|-x]"
> +	if grep -qw xenfs$ /proc/filesystems
> +	then
> +		backend="pvops"
> +	else
> +		backend="xenlinux"
> +	fi
> +	;;
> +esac
> +
> +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
> +if test "${backend}" = "pvops"
> +then
> +	modprobe -v xen-scsiback
> +fi
> +
> +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}
> +	if test "${backend}" = "pvops"
> +	then
> +		targetcli /xen-pvscsi create ${pvscsi_wwn}
> +	fi
> +	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}"
> +
> +		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}"
> +		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}"
> +		if test "${backend}" = "pvops"
> +		then
> +			targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1/luns create "/backstores/pscsi/${pscsi_name}" $lun
> +			targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1  set parameter alias=${f_alias%:*}
> +		fi
> +
> +		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 100755
> index 0000000..5b6dc54
> --- /dev/null
> +++ b/tools/misc/target-delete-xen-scsiback.sh
> @@ -0,0 +1,41 @@
> +#!/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}"
> +if cd xen-pvscsi
> +then
> +	for wwn in `ls -d naa.*`
> +	do
> +		targetcli /xen-pvscsi delete $wwn
> +	done
> +fi
> +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
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-06 13:28 ` [PATCH v5 3/5] libxl: add support for vscsi Olaf Hering
@ 2015-05-13 14:23   ` Ian Campbell
  2015-05-15  6:29     ` Olaf Hering
  2015-05-13 14:44   ` Ian Campbell
  1 sibling, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2015-05-13 14:23 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel

On Wed, 2015-05-06 at 13:28 +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                  | 440 +++++++++++++++++++++
>  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            | 274 +++++++++++++
>  tools/libxl/libxlu_vscsi.c           | 745 +++++++++++++++++++++++++++++++++++
>  tools/libxl/libxlutil.h              |  18 +
>  tools/libxl/xl.h                     |   3 +
>  tools/libxl/xl_cmdimpl.c             | 205 +++++++++-
>  tools/libxl/xl_cmdtable.c            |  15 +
>  16 files changed, 1877 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index f936dfc..6a7dc8c 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
> +SCSI devices from the backend domain to the guest.
> +
> +Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
> +'pdev' describes the physical device, preferable in a persistent format such as /dev/disk/by-*/*.
> +'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
> +'option' lists additional flags which a backend may recognize.

It's options not option in the spec, I think options is more correct
(and below too).

> +The supported values for "pdev" and "option" depends on the used backend driver:

"... on the backend driver used"

> +
> +=over 4
> +
> +=item B<Linux>
> +
> +=over 4
> +
> +=item C<pdev>
> +
> +The backend driver in the pvops kernel is part of the Linux-IO Target framework


> +(LIO). As such the SCSI devices have to be configured first with the tools
> +provided by this framework, such as a xen-scsiback aware targetcli. The "pdev"
> +in domU.cfg has to refer to a config item in that framework instead of the raw
> +device. Ususally this is a WWN in the form of "na.WWN:LUN".

"Usually".

What sort configuration is needed? I assume it is not sufficient to just
point xl at /dev/scsi/a-thing. Is the requirement something like binding
a PCI device to pciback?

A quick example of the expected usage of targetcli would go a long way,
I think.

> [...]

> 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]>

Unlike in the xl.cfg disk spec the pdev and vdev are separated with
space rather than ",", is that deliberate? (I don't mind, just want to
check it's intended).
 
> +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>.

Does/could omitting the domid list them all?

I'm going to hit send now and then start again with the code portion of
this patch.

Ian.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework
  2015-05-13 14:18   ` Ian Campbell
@ 2015-05-13 14:37     ` Olaf Hering
  0 siblings, 0 replies; 24+ messages in thread
From: Olaf Hering @ 2015-05-13 14:37 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

On Wed, May 13, Ian Campbell wrote:

> On Wed, 2015-05-06 at 13:28 +0000, 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.
> 
> 
> I'm not sure what this is all about, but is there anything end-user-ish
> which ought to be documented or put on the wiki or something?

Essentially its just the two commands below, if an SCSI device should be
assigned to the guest.

As suggested in your other reply, I will add a simple example to the
xl.cfg man page.

Upstream has a wiki which should cover also the /xen-pvscsi backend. But
since they are unresponsive having it all in our own wiki is the best we
can do for now. I will start working on that part too.


Olaf

> > +		targetcli /backstores/pscsi create "dev=${f_link}" "${pscsi_name}"

> > +			targetcli /xen-pvscsi/${pvscsi_wwn}/tpg1/luns create "/backstores/pscsi/${pscsi_name}" $lun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-06 13:28 ` [PATCH v5 3/5] libxl: add support for vscsi Olaf Hering
  2015-05-13 14:23   ` Ian Campbell
@ 2015-05-13 14:44   ` Ian Campbell
  2015-05-13 15:12     ` Ian Campbell
  2015-05-13 15:24     ` Olaf Hering
  1 sibling, 2 replies; 24+ messages in thread
From: Ian Campbell @ 2015-05-13 14:44 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel

Just looking at the API functions on this pass.

General comment though: Please go through and rewrap any lines longer
than 75-80, I saw quite a few as I was scrolling.

> @@ -3628,6 +3643,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)

Does this end up doing the correct thing given that vscsi is a bit
different and the libxl_device_vscsi is actually a bus? Shouldn't
something somewhere be taking care that vscsi->vscsi_devs is up to date,
not just the list of the vscsi busses?


>  #define COMPARE_PCI(a, b) ((a)->func == (b)->func &&    \
>                             (a)->bus == (b)->bus &&      \
>                             (a)->dev == (b)->dev)
> libxl_vscsi_dev = Struct("vscsi_dev", [
> +    ("vscsi_dev_id",     libxl_devid),
> +    ("remove",           bool),

What is this remove field?

A libxl_vscsi_dev describes a device, not the actions which can be
performed on the device. Those are in the names of the functions.

> +    ("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),

This one also seems odd.

If you need to store state internally within libxl then you will have to
arrange to store it somewhere else, the public API structs are not
appropriate.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-13 14:44   ` Ian Campbell
@ 2015-05-13 15:12     ` Ian Campbell
  2015-05-13 17:31       ` Olaf Hering
  2015-05-13 17:56       ` George Dunlap
  2015-05-13 15:24     ` Olaf Hering
  1 sibling, 2 replies; 24+ messages in thread
From: Ian Campbell @ 2015-05-13 15:12 UTC (permalink / raw)
  To: Olaf Hering
  Cc: Wei Liu, Stefano Stabellini, George Dunlap, Ian Jackson,
	Chunyan Liu, xen-devel

On Wed, 2015-05-13 at 15:44 +0100, Ian Campbell wrote:
> >  #define COMPARE_PCI(a, b) ((a)->func == (b)->func &&    \
> >                             (a)->bus == (b)->bus &&      \
> >                             (a)->dev == (b)->dev)
> > libxl_vscsi_dev = Struct("vscsi_dev", [
> > +    ("vscsi_dev_id",     libxl_devid),
> > +    ("remove",           bool),
> 
> What is this remove field?
> 
> A libxl_vscsi_dev describes a device, not the actions which can be
> performed on the device. Those are in the names of the functions.

I started reviewing the code and came across the usage of this, and I'm
afraid we need to reconsider how this stuff fits together.

The underlying issue is that vscsi, like pci (and maybe pvusb?), is a
bit different to other devices, in that the xenstore front/back actually
represent a bus and not individual devices, which are represented as
subdevices.

You've represented this as a libxl_vscsi_dev which is actually a bus
containing an array of devices, which leads to an API where to remove a
device you have to provide libxl_device_scsi_remove with a
libxl_device_vscsi where the individual devices are tagged for remove or
not, using a field in the dev descriptor. Really the remove API should
be taking something like what you've called libxl_vscsi_dev.

There are also issues around updating the json state etc which I think
are made more complicated because of this (DEVICE_ADD called in dev_rm,
and a new version of DEVICE_REMOVE etc).

Is it important/useful that the user be able to configure/control the
number (and addresses) of the buses themselves and which devices are on
which, or can we get away with the pvpci model where the libxl user just
gives the individual devices and the library internally takes care of
what buses need to be created?

If that is not an option then we may need to follow the pvusb model
(Chunyan and George CC-d in case I've got it wrong) which is to have
explicit/separate host and device structs in the libxl API and
associated separate commands to add/remove buses vs add/remove devices
on those buses.

I don't think splitting the structs but not the API functions as you hae
here is something we want to go with.

Going the PCI route would almost certainly be less work at both the API
and implementation side.

Ian.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-13 14:44   ` Ian Campbell
  2015-05-13 15:12     ` Ian Campbell
@ 2015-05-13 15:24     ` Olaf Hering
  1 sibling, 0 replies; 24+ messages in thread
From: Olaf Hering @ 2015-05-13 15:24 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel

On Wed, May 13, Ian Campbell wrote:

> Just looking at the API functions on this pass.

> > @@ -3628,6 +3643,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)
> 
> Does this end up doing the correct thing given that vscsi is a bit
> different and the libxl_device_vscsi is actually a bus? Shouldn't
> something somewhere be taking care that vscsi->vscsi_devs is up to date,
> not just the list of the vscsi busses?

In the usecases I have seen comparing just the index number ->v_hst is
ok IMO. The tricky part is to keep xenstore and the json in sync. Given
your comment below, if ->remove will be dropped it will be required to
always keep xenstore and json in sync. I will see what needs to be done.

> >  #define COMPARE_PCI(a, b) ((a)->func == (b)->func &&    \
> >                             (a)->bus == (b)->bus &&      \
> >                             (a)->dev == (b)->dev)
> > libxl_vscsi_dev = Struct("vscsi_dev", [
> > +    ("vscsi_dev_id",     libxl_devid),
> > +    ("remove",           bool),
> 
> What is this remove field?
> 
> A libxl_vscsi_dev describes a device, not the actions which can be
> performed on the device. Those are in the names of the functions.

This is also used by the JSON to not readd an already removed device.

But perhaps the code should just use a dummy vhost->vdev and pass that
into libxl. Which in turn searches in the list of active vdevs for the
requested vdev and removes it. Sounds reasonable to do it that way.

> > +    ("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),
> 
> This one also seems odd.
> 
> If you need to store state internally within libxl then you will have to
> arrange to store it somewhere else, the public API structs are not
> appropriate.

Makes sense. I will rearrange the code which makes use of that member.

Olaf

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-13 15:12     ` Ian Campbell
@ 2015-05-13 17:31       ` Olaf Hering
  2015-05-15  4:11         ` Jürgen Groß
  2015-05-13 17:56       ` George Dunlap
  1 sibling, 1 reply; 24+ messages in thread
From: Olaf Hering @ 2015-05-13 17:31 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Wei Liu, Stefano Stabellini, George Dunlap, Ian Jackson,
	Chunyan Liu, xen-devel

On Wed, May 13, Ian Campbell wrote:

> You've represented this as a libxl_vscsi_dev which is actually a bus
> containing an array of devices, which leads to an API where to remove a
> device you have to provide libxl_device_scsi_remove with a
> libxl_device_vscsi where the individual devices are tagged for remove or
> not, using a field in the dev descriptor. Really the remove API should
> be taking something like what you've called libxl_vscsi_dev.

You are right: at least the remove function should just get a device
(libxl_vscsi_dev) instead of a bus (libxl_device_vscsi) with devices
tagged for remove. As things are implemented now libxl_device_vscsi_list
can be called by libxl itself, so the remove function can just search
the provided vdev and either just remove it or wipe the entire bus. That
means part of xlu_vscsi_detach goes into libxl_device_vscsi_remove.

> There are also issues around updating the json state etc which I think
> are made more complicated because of this (DEVICE_ADD called in dev_rm,
> and a new version of DEVICE_REMOVE etc).

Yes, libxl_device_vscsi_remove has to keep both xenstore and json in
sync.

> Is it important/useful that the user be able to configure/control the
> number (and addresses) of the buses themselves and which devices are on
> which, or can we get away with the pvpci model where the libxl user just
> gives the individual devices and the library internally takes care of
> what buses need to be created?

I do not know if its important for users of vscsi to build a bus with
several devices. Giving each device its own bus would work as well,
which would turn the whole thing into ordinary devices from libxl POV.

No idea where the initial idea came from. Even if the feature-host thing
is used, the raw passthrough of SCSI commands would continue to work.

> If that is not an option then we may need to follow the pvusb model
> (Chunyan and George CC-d in case I've got it wrong) which is to have
> explicit/separate host and device structs in the libxl API and
> associated separate commands to add/remove buses vs add/remove devices
> on those buses.

I think the backend could deal with an empty bus, which has an empty
vscsi-devs/ dir in xenstore. I will think about it.

> I don't think splitting the structs but not the API functions as you hae
> here is something we want to go with.

Not sure how pvusb or pci acutally looks like, but I will see how pvscsi
looks like if libxl gets just libxl_vscsi_dev as input. The bus handling
itself could be done within libxl, so that libxl_device_vscsi becomes
internal to libxl.

Olaf

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-13 15:12     ` Ian Campbell
  2015-05-13 17:31       ` Olaf Hering
@ 2015-05-13 17:56       ` George Dunlap
  1 sibling, 0 replies; 24+ messages in thread
From: George Dunlap @ 2015-05-13 17:56 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Olaf Hering, Wei Liu, Stefano Stabellini, Ian Jackson,
	Chunyan Liu, xen-devel

On Wed, May 13, 2015 at 4:12 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> If that is not an option then we may need to follow the pvusb model
> (Chunyan and George CC-d in case I've got it wrong) which is to have
> explicit/separate host and device structs in the libxl API and
> associated separate commands to add/remove buses vs add/remove devices
> on those buses.

I seem to recall that the end goal for pvusb was that you *could*
specify busses if you wanted to, but that if you didn't, libxl would
just do something sensible behind the scenes (as you are suggesting
with pvscsi).

One of the reasons for making this explicit was that libvirt (as I
understand it) wants to be able to make it explicit.  Another reason
is that in the USB case we're actually trying to share two different
pathways -- pvusb and dm-usb (i.e., a device passed through via qemu),
each of which are only available on some guests.  In the case of a PV
guest, it's pretty clear which one to use; but in the case of an HVM
guest, it may not be clear to the toolstack what pathway the guest OS
is capable of using.

 -George

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-13 17:31       ` Olaf Hering
@ 2015-05-15  4:11         ` Jürgen Groß
  2015-05-15  5:58           ` Olaf Hering
  0 siblings, 1 reply; 24+ messages in thread
From: Jürgen Groß @ 2015-05-15  4:11 UTC (permalink / raw)
  To: Olaf Hering, Ian Campbell
  Cc: Wei Liu, Stefano Stabellini, George Dunlap, Ian Jackson,
	Chunyan Liu, xen-devel

On 05/13/2015 07:31 PM, Olaf Hering wrote:
> On Wed, May 13, Ian Campbell wrote:
>> Is it important/useful that the user be able to configure/control the
>> number (and addresses) of the buses themselves and which devices are on
>> which, or can we get away with the pvpci model where the libxl user just
>> gives the individual devices and the library internally takes care of
>> what buses need to be created?
>
> I do not know if its important for users of vscsi to build a bus with
> several devices. Giving each device its own bus would work as well,
> which would turn the whole thing into ordinary devices from libxl POV.
>
> No idea where the initial idea came from. Even if the feature-host thing
> is used, the raw passthrough of SCSI commands would continue to work.

Are you talking about single LUNs or only targets?

Multi-LUN devices do exist and they are required to be presented as
those to the guest.


Juergen

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-15  4:11         ` Jürgen Groß
@ 2015-05-15  5:58           ` Olaf Hering
  2015-05-15  8:47             ` Ian Campbell
  0 siblings, 1 reply; 24+ messages in thread
From: Olaf Hering @ 2015-05-15  5:58 UTC (permalink / raw)
  To: Jürgen Groß
  Cc: Wei Liu, Ian Campbell, Stefano Stabellini, George Dunlap,
	Ian Jackson, Chunyan Liu, xen-devel

On Fri, May 15, Jürgen Groß wrote:

> Multi-LUN devices do exist and they are required to be presented as
> those to the guest.

Ok, this means we need the bus concept. I will see if the API for
add/remove can be changed that only devices get passed to the libxl.

Olaf

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-13 14:23   ` Ian Campbell
@ 2015-05-15  6:29     ` Olaf Hering
  0 siblings, 0 replies; 24+ messages in thread
From: Olaf Hering @ 2015-05-15  6:29 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel

On Wed, May 13, Ian Campbell wrote:

> On Wed, 2015-05-06 at 13:28 +0000, Olaf Hering wrote:
> > +++ 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]>
> 
> Unlike in the xl.cfg disk spec the pdev and vdev are separated with
> space rather than ",", is that deliberate? (I don't mind, just want to
> check it's intended).

Yes, pdev and vdev are space separated. I have to double check how xm
handled the additional feature-host option, it was most likely comma
separated.

> > +=item B<scsi-list> I<domain-id> I<[domain-id] ...>
> > +
> > +List vscsi devices for the domain specified by I<domain-id>.
> 
> Does/could omitting the domid list them all?

No, at least one domid is required. Not sure how desirable (and racy) it
is to walk every domain and look for vscsi devices.

Olaf

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-15  5:58           ` Olaf Hering
@ 2015-05-15  8:47             ` Ian Campbell
  2015-05-15  9:35               ` Juergen Gross
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2015-05-15  8:47 UTC (permalink / raw)
  To: Olaf Hering
  Cc: Jürgen Groß,
	Wei Liu, Stefano Stabellini, George Dunlap, Ian Jackson,
	Chunyan Liu, xen-devel

On Fri, 2015-05-15 at 07:58 +0200, Olaf Hering wrote:
> On Fri, May 15, Jürgen Groß wrote:
> 
> > Multi-LUN devices do exist and they are required to be presented as
> > those to the guest.
> 
> Ok, this means we need the bus concept. I will see if the API for
> add/remove can be changed that only devices get passed to the libxl.

Does a multi-LUN device require it's own bus or something?

Or do the LUN numbers need to be passed through to the guest because
things get upset otherwise?

IOW why does multi-LUN imply multi-bus, and even if so why does that
need to be exposed to the user?

Ian.


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-15  8:47             ` Ian Campbell
@ 2015-05-15  9:35               ` Juergen Gross
  2015-05-15  9:46                 ` Ian Campbell
  0 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2015-05-15  9:35 UTC (permalink / raw)
  To: Ian Campbell, Olaf Hering
  Cc: Wei Liu, Stefano Stabellini, George Dunlap, Ian Jackson,
	Chunyan Liu, xen-devel

On 05/15/2015 10:47 AM, Ian Campbell wrote:
> On Fri, 2015-05-15 at 07:58 +0200, Olaf Hering wrote:
>> On Fri, May 15, Jürgen Groß wrote:
>>
>>> Multi-LUN devices do exist and they are required to be presented as
>>> those to the guest.
>>
>> Ok, this means we need the bus concept. I will see if the API for
>> add/remove can be changed that only devices get passed to the libxl.
>
> Does a multi-LUN device require it's own bus or something?
>
> Or do the LUN numbers need to be passed through to the guest because
> things get upset otherwise?
>
> IOW why does multi-LUN imply multi-bus, and even if so why does that
> need to be exposed to the user?

These are e.g. RAID-systems or tape libraries. In most cases one LUN
is a control LUN via which the other LUNs can be configured and/or
administrated. This implies that all of those LUNs have to share the
same target and they need to have the same LUN numbers in the guest as
on the real device.


Juergen

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-15  9:35               ` Juergen Gross
@ 2015-05-15  9:46                 ` Ian Campbell
  2015-05-15  9:48                   ` Ian Campbell
  0 siblings, 1 reply; 24+ messages in thread
From: Ian Campbell @ 2015-05-15  9:46 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Olaf Hering, Wei Liu, Stefano Stabellini, George Dunlap,
	Ian Jackson, Chunyan Liu, xen-devel

On Fri, 2015-05-15 at 11:35 +0200, Juergen Gross wrote:
> On 05/15/2015 10:47 AM, Ian Campbell wrote:
> > On Fri, 2015-05-15 at 07:58 +0200, Olaf Hering wrote:
> >> On Fri, May 15, Jürgen Groß wrote:
> >>
> >>> Multi-LUN devices do exist and they are required to be presented as
> >>> those to the guest.
> >>
> >> Ok, this means we need the bus concept. I will see if the API for
> >> add/remove can be changed that only devices get passed to the libxl.
> >
> > Does a multi-LUN device require it's own bus or something?
> >
> > Or do the LUN numbers need to be passed through to the guest because
> > things get upset otherwise?
> >
> > IOW why does multi-LUN imply multi-bus, and even if so why does that
> > need to be exposed to the user?
> 
> These are e.g. RAID-systems or tape libraries. In most cases one LUN
> is a control LUN via which the other LUNs can be configured and/or
> administrated. This implies that all of those LUNs have to share the
> same target and they need to have the same LUN numbers in the guest as
> on the real device.

That makes sense, thanks.

So it seems we do need to expose some concept of the bus itself to
users.

My preference would be for the API for such devices (currently USB and
PVSCSI) to have broadly speaking the same shape, much like how the API
for the existing "busless" devices all look mostly the same.

(NB: Lets ignore the fact for now that PCI is in the wrong bucket...)

I'd like to start with seeing an expanded version of the comment in
libxl.h which begins:

/*
 * Devices
 * =======

which covers this new style API alongside the existing one and sets out
the expected pattern of naming (both struct and function) in the same
way.

Ian.


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v5 3/5] libxl: add support for vscsi
  2015-05-15  9:46                 ` Ian Campbell
@ 2015-05-15  9:48                   ` Ian Campbell
  0 siblings, 0 replies; 24+ messages in thread
From: Ian Campbell @ 2015-05-15  9:48 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Olaf Hering, Wei Liu, Stefano Stabellini, George Dunlap,
	Ian Jackson, Chunyan Liu, xen-devel

On Fri, 2015-05-15 at 10:46 +0100, Ian Campbell wrote:
> I'd like to start with seeing an expanded version of the comment in
> libxl.h which begins:
> 
> /*
>  * Devices
>  * =======
> 
> which covers this new style API alongside the existing one and sets out
> the expected pattern of naming (both struct and function) in the same
> way.

Which would need to be agreed with the PVUSB series author(s) too. In
fact I think they may already incorporate such an API proposal (even if
only implicitly). The first step should probably be to see if that API
can be generalised to apply to pvsci first, and then hopefully write
down the generalised form.

Ian

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2015-05-15  9:48 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-06 13:28 [PATCH v5 0/5] libbxl: add support for pvscsi, iteration 5 Olaf Hering
2015-05-06 13:28 ` [PATCH v5 1/5] vscsiif.h: fix WWN notation for p-dev property Olaf Hering
2015-05-13 14:11   ` Ian Campbell
2015-05-06 13:28 ` [PATCH v5 2/5] docs: add vscsi to xenstore-paths.markdown Olaf Hering
2015-05-13 14:12   ` Ian Campbell
2015-05-06 13:28 ` [PATCH v5 3/5] libxl: add support for vscsi Olaf Hering
2015-05-13 14:23   ` Ian Campbell
2015-05-15  6:29     ` Olaf Hering
2015-05-13 14:44   ` Ian Campbell
2015-05-13 15:12     ` Ian Campbell
2015-05-13 17:31       ` Olaf Hering
2015-05-15  4:11         ` Jürgen Groß
2015-05-15  5:58           ` Olaf Hering
2015-05-15  8:47             ` Ian Campbell
2015-05-15  9:35               ` Juergen Gross
2015-05-15  9:46                 ` Ian Campbell
2015-05-15  9:48                   ` Ian Campbell
2015-05-13 17:56       ` George Dunlap
2015-05-13 15:24     ` Olaf Hering
2015-05-06 13:28 ` [PATCH v5 4/5] vscsiif.h: add some notes about xenstore layout Olaf Hering
2015-05-13 14:14   ` Ian Campbell
2015-05-06 13:28 ` [PATCH v5 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework Olaf Hering
2015-05-13 14:18   ` Ian Campbell
2015-05-13 14:37     ` Olaf Hering

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.