Re: [PATCH RESEND] tools/libxl: add support for emulated NVMe drives

[Wei Liu <wei.liu2@citrix.com>]

On Fri, Jan 13, 2017 at 02:00:41PM +0000, Paul Durrant wrote:
> Upstream QEMU supports emulation of NVM Express a.k.a. NVMe drives.
> 
> This patch adds a new vdev type into libxl to allow such drives to be
> presented to HVM guests. Because the purpose of the new vdev is purely
> to configure emulation, the syntax only supports specification of
> whole disks. Also there is no need to introduce a new concrete VBD
> encoding for NVMe drives.

This seems to be in contradiction with the code below?

> 
> NOTE: QEMU's emulation only supports a single NVMe namespace, so the
>       vdev syntax does not include specification of a namespace.
>       Also, current versions of SeaBIOS do not support booting from
>       NVMe devices, so the vdev should only be used for secondary
>       drives.
> 

I don't know much about NVMe, but I presume we could just extend the
proposed syntax to support namespaces should the need arise?

> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  docs/man/xen-vbd-interface.markdown.7 | 15 ++++++++-------
>  docs/man/xl-disk-configuration.pod.5  |  4 ++--
>  tools/libxl/libxl_device.c            |  8 ++++++++
>  tools/libxl/libxl_dm.c                |  6 ++++++
>  4 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/man/xen-vbd-interface.markdown.7 b/docs/man/xen-vbd-interface.markdown.7
> index 1c996bf..8fd378c 100644
> --- a/docs/man/xen-vbd-interface.markdown.7
> +++ b/docs/man/xen-vbd-interface.markdown.7
> @@ -8,12 +8,12 @@ emulated IDE, AHCI or SCSI disks.
>  The abstract interface involves specifying, for each block device:
>  
>   * Nominal disk type: Xen virtual disk (aka xvd*, the default); SCSI
> -   (sd*); IDE or AHCI (hd*).
> +   (sd*); IDE or AHCI (hd*); NVMe.

NVMe (nvme*) ?

>  
> -   For HVM guests, each whole-disk hd* and and sd* device is made
> -   available _both_ via emulated IDE resp. SCSI controller, _and_ as a
> -   Xen VBD.  The HVM guest is entitled to assume that the IDE or SCSI
> -   disks available via the emulated IDE controller target the same
> +   For HVM guests, each whole-disk hd*, sd* or nvme* device is made
> +   available _both_ via emulated IDE, SCSI controller or NVMe drive
> +   respectively _and_ as a Xen VBD.  The HVM guest is entitled to
> +   assume that the disks available via the emulation target the same

How do you expect the guest to deal with multipath NVMe devices? Maybe
we need to add unplug support for NVMe devices in QEMU?

>     underlying devices as the corresponding Xen VBD (ie, multipath).
>     In hd* case with hdtype=ahci, disk will be AHCI via emulated
>     ich9 disk controller.
> @@ -42,8 +42,7 @@ The abstract interface involves specifying, for each block device:
>     treat each vbd as it would a partition or slice or LVM volume (for
>     example by putting or expecting a filesystem on it).
>  
> -   Non-whole disk devices cannot be passed through to HVM guests via
> -   the emulated IDE or SCSI controllers.
> +   Only whole disk devices can be emulated for HVM guests.
>  
>  
>  Configuration file syntax
> @@ -56,6 +55,7 @@ The config file syntaxes are, for example
>         d536p37  xvdtq37  Xen virtual disk 536 partition 37
>         sdb3              SCSI disk 1 partition 3
>         hdc2              IDE disk 2 partition 2
> +       nvme0             NVMe disk 0 (whole disk only)
>  
>  The d*p* syntax is not supported by xm/xend.
>  
> @@ -78,6 +78,7 @@ encodes the information above as follows:
>       8 << 8 | disk << 4 | partition      sd, disks and partitions up to 15
>       3 << 8 | disk << 6 | partition      hd, disks 0..1, partitions 0..63
>      22 << 8 | (disk-2) << 6 | partition  hd, disks 2..3, partitions 0..63
> +    1 << 28 | disk << 8                  nvme, all disks, whole disk only

^ Here, concrete encoding is introduced.

>      2 << 28 onwards                      reserved for future use
>     other values less than 1 << 28        deprecated / reserved
>  
> diff --git a/docs/man/xl-disk-configuration.pod.5 b/docs/man/xl-disk-configuration.pod.5
> index d3eedc1..c40418e 100644
> --- a/docs/man/xl-disk-configuration.pod.5
> +++ b/docs/man/xl-disk-configuration.pod.5
> @@ -127,8 +127,8 @@ designation in some specifications).  L<xen-vbd-interface(7)>
>  
>  =item Supported values
>  
> -hd[x], xvd[x], sd[x] etc.  Please refer to the above specification for
> -further details.
> +hd[x], xvd[x], sd[x], nvme[x] etc.  Please refer to the above specification
> +for further details.
>  
>  =item Deprecated values
>  
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index b2aeefc..63a738c 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -532,6 +532,14 @@ int libxl__device_disk_dev_number(const char *virtpath, int *pdisk,
>          if (ppartition) *ppartition = partition;
>          return (8 << 8) | (disk << 4) | partition;
>      }
> +    if (!memcmp(virtpath, "nvme", 4)) {
> +        disk = strtoul(virtpath + 4, &ep, 10);
> +        if (*ep)
> +            return -1;
> +        if (pdisk) *pdisk = disk;
> +        if (ppartition) *ppartition = 0;
> +        return (1 << 28) | (disk << 8);
> +    }
>      return -1;
>  }
>  
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 281058d..980dad1 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -1430,6 +1430,12 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
>                                                          format,
>                                                          &disks[i],
>                                                          colo_mode);
> +                } else if (strncmp(disks[i].vdev, "nvme", 4) == 0) {
> +                    flexarray_vappend(dm_args,
> +                        "-drive",  GCSPRINTF("file=%s,if=none,id=nvmedisk-%d,format=%s,cache=writeback", target_path, disk, format),
> +                        "-device", GCSPRINTF("nvme,drive=nvmedisk-%d,serial=%d", disk, disk),
> +                        NULL);
> +                    continue;
>                  } else if (disk < 6 && b_info->u.hvm.hdtype == LIBXL_HDTYPE_AHCI) {
>                      if (!disks[i].readwrite) {
>                          LOGD(ERROR, guest_domid,
> -- 
> 2.1.4
> 

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