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

[Ian Campbell <ian.campbell@citrix.com>]

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.