On Wed, May 07, 2025 at 04:43:29PM +0200, Peter Krempa wrote:
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 ...
Yes, I meant "naming it the same way" but specifically not "guaranteeing it will match" =)
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.
It does not, but I see our dev= naming as a shortcut for assigning addresses. Of course, that might be a very biased view due to knowing how the code handles that, but still, the expectation should not be any different to the expectation of what happens when you use dev='vdzz'.
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.
The part of the code that needs changing to accommodate the nvme handling would basically be special-cased with a is_this_nvme() condition, but I, personally, see "nvme3n3" as an improvement compared to "nvmeadr". OTOH I'm fine with both because how often will I have to type it manually...
signature.asc
Description: PGP signature
