RE: Libvirt NVME support

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