On Wed, May 07, 2025 at 15:56:52 +0200, Martin Kletzander wrote: > On Sun, Apr 27, 2025 at 07:48:02PM +0800, honglei.w...@smartx.com wrote: > > From: hongleiwang <honglei.w...@smartx.com> > > > > QEMU has supported nvme disk emulation for a long time, > > see: https://qemu-project.gitlab.io/qemu/system/devices/nvme.html. > > > > The following patches introduce nvme-ns disk bus type: > > Thanks for the v2. I have some suggestions which I'd like some more > feedback on, which might be controversial, hence the added Cc's for > people who replied to any of the versions. > > > A disk with nvme-ns as bus is represented as an nvme namespace > > and needs to be attached to an nvme controller. In XML, it can be > > used like this: > > <devices> > > ... > > <disk type='file' device='disk'> > > <driver name='qemu' type='raw'/> > > <source file='/tmp/data.img'/> > > <target dev='nvmensa' bus='nvme-ns'/> > > Since we're not using the bus='nvme', what if we abandoned the qemu > naming and not specify the -ns; on top of that it would be nicer to have > the device naming same as it actually happens on Linux. That is kind of > biased, but we already do that for other disk types.
Keep in mind that we explicitly document that 'dev' may not match what the device becomes in the guest ... > > The above would then become: > > <target dev='nvme0n1' bus='nvme'/> ... in this case it could be more problematic because the disk target/dev doesn't force you to put nvme0n1 and nvme0n2 on the same controller nor does the namespace bit need to correspond to the namespace id. Yes it is true that with other buses you still can mix stuff up if you configure it explicitly. > Because that means a slightly bigger change to how we do things > currently (bunch of the code depends on the original numbering) I'd be > willing to submit the XML part of this series with that change. One > of the reasons is that I also need it for support in a different > driver. I guess that is fair. I still think that just doing 'nvmea', 'nvmeb', 'nvmec' etc should be sufficient here, but the code already has provisions for partition numbers so this shouldn't be much different.
