Hi, We wanted to check if it’s possible to specify a disk’s target as nvme (so that the disk shows up as a nvme disk to the guest VM). Per libvirt documentation it looks like (since Libvirt 6.0.0) we can specify the disk type as nvme and disks source as a nvme. But the documentation does not say anything about being specify the disk’s target as nvme. Is it possible to present the disk to the guest as a nvme disk, if so how?

-----Original Message----- From: Peter Krempa <pkrempa(a)redhat.com&gt; Sent: 09 November 2020 16:44 To: Suraj Kasi <suraj.kasi(a)nutanix.com&gt; Cc: libvirt-list(a)redhat.com; Thanos Makatos <thanos.makatos(a)nutanix.com>; John Levon <john.levon(a)nutanix.com&gt; Subject: Re: Libvirt NVME support On Mon, Nov 09, 2020 at 16:38:11 +0000, Suraj Kasi wrote: > Hi, > > We wanted to check if it’s possible to specify a disk’s target as nvme (so that the disk shows up as a nvme disk to the guest VM). > > Per libvirt documentation it looks like (since Libvirt 6.0.0) we can specify the disk type as nvme and disks source as a nvme. But the documentation does not say anything about being specify the disk’s target as nvme. Is it possible to present the disk to the guest as a nvme disk, if so how? > NVMe device emulation is not supported at this point. I'm not even sure what the state of the feature in qemu upstream is.

If you have a real NVMe device, you can obviously use PCI device assignment with it to pass it to the guest os.

As a separate question, is there any performance benefit of emulating a NVMe controller compared to e.g. virtio-scsi?

On Wed, Nov 18, 2020 at 09:57:14 +0000, Thanos Makatos wrote: >> As a separate question, is there any performance benefit of emulating a >> NVMe controller compared to e.g. virtio-scsi? > > We haven't measured that yet; I would expect it to be slight faster and/or more > CPU efficient but wouldn't be surprised if it isn't. The main benefit of using > NVMe is that we don't have to install VirtIO drivers in the guest. Okay, I'm not sold on the drivers bit but that is definitely not a problem in regards of adding support for emulating NVME controllers to libvirt. As a starting point a trivial way to model this in the XML will be: <controller type='nvme' index='1' model='nvme'> And then add the storage into it as: <disk type='file' device='disk'> <source dev='/Host/QEMUGuest1.qcow2'/> <target dev='sda' bus='nvme'/> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <source dev='/Host/QEMUGuest2.qcow2'/> <target dev='sdb' bus='nvme'/> <address type='drive' controller='1' bus='0' target='0' unit='1'/> </disk> The 'drive' address here maps the disk to the controller. This example uses unit= as the way to specify the namespace ID. Both 'bus' and 'target' must be 0. You can theoretically also add your own address type if 'drive' doesn't look right. This model will have problems with hotplug/unplug if the NVMe spec doesn't actually allow hotplug of a single namespace into a controller as libvirt's hotplug APIs only deal with one element at time. We theoretically could work this around by allowing hotplug of disks which correspond to the namespace used while the controller was not attached yet, and the attach of the controller then attaches both the backends and the controller. This is a bit hacky though. Another obvious solution is to disallow hotplug of the namespaces and thus also the controller.

As a starting point a trivial way to model this in the XML will be: <controller type='nvme' index='1' model='nvme'> And then add the storage into it as: <disk type='file' device='disk'> <source dev='/Host/QEMUGuest1.qcow2'/> <target dev='sda' bus='nvme'/>

<address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='disk'> <source dev='/Host/QEMUGuest2.qcow2'/> <target dev='sdb' bus='nvme'/> <address type='drive' controller='1' bus='0' target='0' unit='1'/> </disk> The 'drive' address here maps the disk to the controller. This example uses unit= as the way to specify the namespace ID. Both 'bus' and 'target' must be 0. You can theoretically also add your own address type if 'drive' doesn't look right. This model will have problems with hotplug/unplug if the NVMe spec doesn't actually allow hotplug of a single namespace into a controller as libvirt's hotplug APIs only deal with one element at time.

On Thu, Nov 19, 2020 at 10:17:56 +0000, Thanos Makatos wrote: > > As a starting point a trivial way to model this in the XML will be: > > > > <controller type='nvme' index='1' model='nvme'> > > > > And then add the storage into it as: > > > > <disk type='file' device='disk'> > > <source dev='/Host/QEMUGuest1.qcow2'/> > > <target dev='sda' bus='nvme'/> > > 'target dev' is how the device appears in the guest, right? It should be > something like 'nvme0n1'. I'm not sure though this is something that we can > put here anyway, I think the guest driver can number controllers arbitrarily. Well, it was supposed to be like that but really is not. Even with other buses the kernel can name the device arbitrarily, so it doesn't really matter. > Maybe we should specify something like BDF? Or is this something QEMU will > have to figure out how to do? > > > <address type='drive' controller='1' bus='0' target='0' unit='0'/> > > </disk> > > > > <disk type='file' device='disk'> > > <source dev='/Host/QEMUGuest2.qcow2'/> > > <target dev='sdb' bus='nvme'/> > > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > > </disk>

> > > > The 'drive' address here maps the disk to the controller. This example

> > uses unit= as the way to specify the namespace ID. Both 'bus' and 'target' > > must be 0.

On Mon, Nov 23, 2020 at 09:47:23 +0000, Thanos Makatos wrote: > > On Thu, Nov 19, 2020 at 10:17:56 +0000, Thanos Makatos wrote: > > > > As a starting point a trivial way to model this in the XML will be: > > > > > > > > <controller type='nvme' index='1' model='nvme'> > > > > > > > > And then add the storage into it as: > > > > > > > > <disk type='file' device='disk'> > > > > <source dev='/Host/QEMUGuest1.qcow2'/> > > > > <target dev='sda' bus='nvme'/> > > > > > > 'target dev' is how the device appears in the guest, right? It should be > > > something like 'nvme0n1'. I'm not sure though this is something that we > > can > > > put here anyway, I think the guest driver can number controllers arbitrarily. > > > > Well, it was supposed to be like that but really is not. Even with other > > buses the kernel can name the device arbitrarily, so it doesn't really > > matter. > > > > > Maybe we should specify something like BDF? Or is this something QEMU > > will > > > have to figure out how to do? > > > > > > > <address type='drive' controller='1' bus='0' target='0' unit='0'/> > > > > </disk> > > > > > > > > <disk type='file' device='disk'> > > > > <source dev='/Host/QEMUGuest2.qcow2'/> > > > > <target dev='sdb' bus='nvme'/> > > > > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > > > > </disk> > > Revistiting your initial suggestion, it should be something like this > (s/sdb/nvme0): > > <disk type='file' device='disk'> > <source dev='/Host/QEMUGuest2.qcow2'/> > <target dev='nvme0' bus='nvme'/> > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > </disk> Note that the parser for 'dev' is a bit quirky, old, and used in many places besides the qemu driver. It's also used with numbers in non-qemu cases. Extending that to parse numbers for nvme but not for sda might become ugly very quickly. Sticking with a letter at the end ('nvmea' might be a more straightforward approach.

> > > > > > > > > The 'drive' address here maps the disk to the controller. This example > > IIUC we need a way to associate storage (this XML snippet) with the controller > you defined earlier (<controller type='nvme' index='1' model='nvme'>). So > shouldn't we only require associating this piece of storage with the controller > based on the index? No. The common approach is to do it via what's specified as <address> > > > > > uses unit= as the way to specify the namespace ID. Both 'bus' and 'target' > > > > must be 0. > > I think 'namespace' or 'ns' would be more suitable instead of 'unit'. > What are 'bus' and 'target' here? And why do they have to be 0? > Do we really need dev='nvme0' in <target ...>? Specifying the controller index > should be enough, no? You certainly can add <address type='nvme' controller='1' ns='2'/> > Wouldn't this contain the minimum amount of information to unambiguously map > this piece of storage to the controller? > > <disk type='file' device='disk'> > <source dev='/Host/QEMUGuest2.qcow2'/> > <target dev='sdb' bus='nvme'/> > <address controller='1' ns='1'/> > </disk> That certainly is correct if you include the "type='nvme'" attribute.

On Mon, Nov 23, 2020 at 13:07:51 +0000, Thanos Makatos wrote: > > > On Mon, Nov 23, 2020 at 09:47:23 +0000, Thanos Makatos wrote: > > > > On Thu, Nov 19, 2020 at 10:17:56 +0000, Thanos Makatos wrote: > > > > > > As a starting point a trivial way to model this in the XML will be: > > > > > > > > > > > > <controller type='nvme' index='1' model='nvme'> > > > > > > > > > > > > And then add the storage into it as: > > > > > > > > > > > > <disk type='file' device='disk'> > > > > > > <source dev='/Host/QEMUGuest1.qcow2'/> > > > > > > <target dev='sda' bus='nvme'/> > > > > > > > > > > 'target dev' is how the device appears in the guest, right? It should be > > > > > something like 'nvme0n1'. I'm not sure though this is something that > > we > > > > can > > > > > put here anyway, I think the guest driver can number controllers > > arbitrarily. > > > > > > > > Well, it was supposed to be like that but really is not. Even with other > > > > buses the kernel can name the device arbitrarily, so it doesn't really > > > > matter. > > > > > > > > > Maybe we should specify something like BDF? Or is this something > > QEMU > > > > will > > > > > have to figure out how to do? > > > > > > > > > > > <address type='drive' controller='1' bus='0' target='0' unit='0'/> > > > > > > </disk> > > > > > > > > > > > > <disk type='file' device='disk'> > > > > > > <source dev='/Host/QEMUGuest2.qcow2'/> > > > > > > <target dev='sdb' bus='nvme'/> > > > > > > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > > > > > > </disk> > > > > > > Revistiting your initial suggestion, it should be something like this > > > (s/sdb/nvme0): > > > > > > <disk type='file' device='disk'> > > > <source dev='/Host/QEMUGuest2.qcow2'/> > > > <target dev='nvme0' bus='nvme'/> > > > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > > > </disk> > > > > Note that the parser for 'dev' is a bit quirky, old, and used in many > > places besides the qemu driver. It's also used with numbers in non-qemu > > cases. Extending that to parse numbers for nvme but not for sda might > > become ugly very quickly. Sticking with a letter at the end ('nvmea' > > might be a more straightforward approach. > > Then I think we should just stick with 'nvme'. You still need a way to "index" it somehow. The target must be unique for each disk.

-----Original Message----- From: Peter Krempa <pkrempa(a)redhat.com&gt; Sent: 23 November 2020 16:56 To: Thanos Makatos <thanos.makatos(a)nutanix.com&gt; Cc: Suraj Kasi <suraj.kasi(a)nutanix.com>; libvirt-list(a)redhat.com; John Levon <john.levon(a)nutanix.com&gt; Subject: Re: Libvirt NVME support On Mon, Nov 23, 2020 at 16:48:55 +0000, Thanos Makatos wrote: > > On Mon, Nov 23, 2020 at 13:07:51 +0000, Thanos Makatos wrote: > > > > > > > On Mon, Nov 23, 2020 at 09:47:23 +0000, Thanos Makatos wrote: > > > > > > On Thu, Nov 19, 2020 at 10:17:56 +0000, Thanos Makatos wrote: > > > > > > > > > > Revistiting your initial suggestion, it should be something like this > > > > > (s/sdb/nvme0): > > > > > > > > > > <disk type='file' device='disk'> > > > > > <source dev='/Host/QEMUGuest2.qcow2'/> > > > > > <target dev='nvme0' bus='nvme'/> > > > > > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > > > > > </disk> > > > > > > > > Note that the parser for 'dev' is a bit quirky, old, and used in many > > > > places besides the qemu driver. It's also used with numbers in non- qemu > > > > cases. Extending that to parse numbers for nvme but not for sda might > > > > become ugly very quickly. Sticking with a letter at the end ('nvmea' > > > > might be a more straightforward approach. > > > > > > Then I think we should just stick with 'nvme'. > > > > You still need a way to "index" it somehow. The target must be unique > > for each disk. > > I think I've misunderstood something, I thought controller='1' in <address ...> > refers to index='1' in <controller ...>. So <address ...> should be: > > <address type='nvme' index='1' controller='1' ns='2'/> > > What's controller='1' then? What I meant by the above is that the value of "<target dev='THIS'" must be unique for every <disk>. I also wanted to advice to not use numbers for making it unique. Numbers used for it have a legacy meaning. Your suggested <address type='nvme' design looks good.

-----Original Message----- From: Peter Krempa <pkrempa(a)redhat.com&gt; Sent: 23 November 2020 17:47 To: Thanos Makatos <thanos.makatos(a)nutanix.com&gt; Cc: Suraj Kasi <suraj.kasi(a)nutanix.com>; libvirt-list(a)redhat.com; John Levon <john.levon(a)nutanix.com&gt; Subject: Re: Libvirt NVME support On Mon, Nov 23, 2020 at 17:40:58 +0000, Thanos Makatos wrote: > > > > -----Original Message----- > > From: Peter Krempa <pkrempa(a)redhat.com&gt; > > Sent: 23 November 2020 16:56 > > To: Thanos Makatos <thanos.makatos(a)nutanix.com&gt; > > Cc: Suraj Kasi <suraj.kasi(a)nutanix.com>; libvirt-list(a)redhat.com; John Levon > > <john.levon(a)nutanix.com&gt; > > Subject: Re: Libvirt NVME support > > > > On Mon, Nov 23, 2020 at 16:48:55 +0000, Thanos Makatos wrote: > > > > On Mon, Nov 23, 2020 at 13:07:51 +0000, Thanos Makatos wrote: > > > > > > > > > > > On Mon, Nov 23, 2020 at 09:47:23 +0000, Thanos Makatos wrote: > > > > > > > > On Thu, Nov 19, 2020 at 10:17:56 +0000, Thanos Makatos wrote: > > > > > > > > > > > > > > > > Revistiting your initial suggestion, it should be something like this > > > > > > > (s/sdb/nvme0): > > > > > > > > > > > > > > <disk type='file' device='disk'> > > > > > > > <source dev='/Host/QEMUGuest2.qcow2'/> > > > > > > > <target dev='nvme0' bus='nvme'/> > > > > > > > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > > > > > > > </disk> > > > > > > > > > > > > Note that the parser for 'dev' is a bit quirky, old, and used in many > > > > > > places besides the qemu driver. It's also used with numbers in non- > > qemu > > > > > > cases. Extending that to parse numbers for nvme but not for sda > > might > > > > > > become ugly very quickly. Sticking with a letter at the end ('nvmea' > > > > > > might be a more straightforward approach. > > > > > > > > > > Then I think we should just stick with 'nvme'. > > > > > > > > You still need a way to "index" it somehow. The target must be unique > > > > for each disk. > > > > > > I think I've misunderstood something, I thought controller='1' in <address > > ...> > > > refers to index='1' in <controller ...>. So <address ...> should be: > > > > > > <address type='nvme' index='1' controller='1' ns='2'/> > > > > > > What's controller='1' then? > > > > What I meant by the above is that the value of "<target dev='THIS'" must > > be unique for every <disk>. I also wanted to advice to not use numbers > > for making it unique. Numbers used for it have a legacy meaning. > > > > Your suggested <address type='nvme' design looks good. > > OK, so we definitely need dev='...' in target which is something like [a-zA- Z]+ > and is unique. If this identifier is not controlled by the user, I think it > would be best not to prefix it with 'nvme' (thus resulting in strings like > 'nvmea' or 'nvmeabc'), as they can be rather confusing for people who don't > know the details. > > However, I still don't understand how index='1' and controller='1' in address > relate to index='1' in controller: > > <address type='nvme' index='1' controller='1' ns='2'/> index should not be here at all .. > > and > > <controller type='nvme' index='1' model='nvme'> > > ... then it makes sense.

On Wed, Nov 18, 2020 at 11:24:30AM +0100, Peter Krempa wrote: > On Wed, Nov 18, 2020 at 09:57:14 +0000, Thanos Makatos wrote: >>> As a separate question, is there any performance benefit of emulating a >>> NVMe controller compared to e.g. virtio-scsi? >> >> We haven't measured that yet; I would expect it to be slight faster and/or more >> CPU efficient but wouldn't be surprised if it isn't. The main benefit of using >> NVMe is that we don't have to install VirtIO drivers in the guest. > > Okay, I'm not sold on the drivers bit but that is definitely not a > problem in regards of adding support for emulating NVME controllers to > libvirt. > > As a starting point a trivial way to model this in the XML will be: > > <controller type='nvme' index='1' model='nvme'> > > And then add the storage into it as: > > <disk type='file' device='disk'> > <source dev='/Host/QEMUGuest1.qcow2'/> > <target dev='sda' bus='nvme'/> > <address type='drive' controller='1' bus='0' target='0' unit='0'/> > </disk> > > <disk type='file' device='disk'> > <source dev='/Host/QEMUGuest2.qcow2'/> > <target dev='sdb' bus='nvme'/> > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > </disk> > > The 'drive' address here maps the disk to the controller. This example > uses unit= as the way to specify the namespace ID. Both 'bus' and 'target' > must be 0. FWIW, I think that our overloeading of type=drive for FDC, IDE, and SCSI was a mistake in retrospect. We should have had type=fdc, type=ide, type=scsi, since each uses a different subset of the attributes. Lets not continue this mistake with NVME - create a type=nvme address type.

On 11/23/20 3:03 PM, Daniel P. Berrangé wrote: > On Wed, Nov 18, 2020 at 11:24:30AM +0100, Peter Krempa wrote: > > On Wed, Nov 18, 2020 at 09:57:14 +0000, Thanos Makatos wrote: > > > > As a separate question, is there any performance benefit of emulating a > > > > NVMe controller compared to e.g. virtio-scsi? > > > > > > We haven't measured that yet; I would expect it to be slight faster and/or more > > > CPU efficient but wouldn't be surprised if it isn't. The main benefit of using > > > NVMe is that we don't have to install VirtIO drivers in the guest. > > > > Okay, I'm not sold on the drivers bit but that is definitely not a > > problem in regards of adding support for emulating NVME controllers to > > libvirt. > > > > As a starting point a trivial way to model this in the XML will be: > > > > <controller type='nvme' index='1' model='nvme'> > > > > And then add the storage into it as: > > > > <disk type='file' device='disk'> > > <source dev='/Host/QEMUGuest1.qcow2'/> > > <target dev='sda' bus='nvme'/> > > <address type='drive' controller='1' bus='0' target='0' unit='0'/> > > </disk> > > > > <disk type='file' device='disk'> > > <source dev='/Host/QEMUGuest2.qcow2'/> > > <target dev='sdb' bus='nvme'/> > > <address type='drive' controller='1' bus='0' target='0' unit='1'/> > > </disk> > > > > The 'drive' address here maps the disk to the controller. This example > > uses unit= as the way to specify the namespace ID. Both 'bus' and 'target' > > must be 0. > > FWIW, I think that our overloeading of type=drive for FDC, IDE, and SCSI > was a mistake in retrospect. We should have had type=fdc, type=ide, type=scsi, > since each uses a different subset of the attributes. > > Lets not continue this mistake with NVME - create a type=nvme address > type. Don't NVMes live on a PCI(e) bus? Can't we just threat NVMes as PCI devices? Or are we targeting sata too? Bcause we also have that type of address.

On Mon, Nov 23, 2020 at 03:36:42PM +0100, Peter Krempa wrote: > On Mon, Nov 23, 2020 at 15:32:20 +0100, Michal Privoznik wrote:

> No, the NVMe controller lives on PCIe. Here we are trying to emulate a > NVMe controller (as <contoller> if you look elsewhere in the other > subthread. The <disk> element here maps to individual emulated > namespaces for the emulated NVMe controller. > > If we'd try to map one <disk> per PCIe device, you'd prevent us from > emulating multiple namespaces. The odd thing here is that we're trying expose different host backing store for each namespace, hence the need to expose multiple <disk>. Does it even make sense if you expose a namespace "2" without first exposing a namespace "1" ?

It makes me a little uneasy, as it feels like trying to export an regular disk, where we have a different host backing store for each partition. The difference I guess is that partition tables are a purely software construct, where as namespaces are a hardware construct.

Exposing individual partitions to a disk was done in Xen, but most people think it was kind of a mistake, as you could get a partition without any containing disk. At least in this case we do have a NVME controller present so the namespace isn't orphaned, like the old Xen partitons.

The alternative is to say only one host backing store, and then either let the guest dynamically carve it up into namespaces, or have some data format in the host backing store to represent the namespaces, or have an XML element to specify the regions of host backing that correspond to namespaces, eg <disk type="file" device="nvme"> <source file="/some/file.qcow"/> <target bus="nvme"/> <namespaces> <region offset="0" size="1024000"/> <region offset="1024000" size="2024000"/> <region offset="2024000" size="4024000"/> </namespaces> <address type="pci" .../> </disk> this is of course less flexible, and I'm not entirely serious about suggesting this, but its an option that exists none the less.