Re: [PULL 32/56] hw/block/nvme: split setup and register for namespace

From: Klaus Jensen <its@irrelevant.dk>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Fam Zheng <fam@euphon.net>,
	Peter Maydell <peter.maydell@linaro.org>,
	Willian Rampazzo <wrampazz@redhat.com>,
	qemu-block@nongnu.org, Klaus Jensen <k.jensen@samsung.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Alexander Graf <agraf@csgraf.de>,
	Minwoo Im <minwoo.im.dev@gmail.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Keith Busch <kbusch@kernel.org>, Kevin Wolf <kwolf@redhat.com>
Subject: Re: [PULL 32/56] hw/block/nvme: split setup and register for namespace
Date: Thu, 11 Feb 2021 12:46:09 +0100	[thread overview]
Message-ID: <YCUZAfXP/YAYbuM4@apples.localdomain> (raw)
In-Reply-To: <eff5e309-c932-d954-c2fe-da8cf62f98e2@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2755 bytes --]

On Feb 11 12:40, Philippe Mathieu-Daudé wrote:
> On 2/11/21 10:53 AM, Alexander Graf wrote:
> > Hi Klaus,
> > 
> > On 09.02.21 08:30, Klaus Jensen wrote:
> >> From: Minwoo Im <minwoo.im.dev@gmail.com>
> >>
> >> In NVMe, namespace is being attached to process I/O.  We register NVMe
> >> namespace to a controller via nvme_register_namespace() during
> >> nvme_ns_setup().  This is main reason of receiving NvmeCtrl object
> >> instance to this function to map the namespace to a controller.
> >>
> >> To make namespace instance more independent, it should be split into two
> >> parts: setup and register.  This patch split them into two differnt
> >> parts, and finally nvme_ns_setup() does not have nothing to do with
> >> NvmeCtrl instance at all.
> >>
> >> This patch is a former patch to introduce NVMe subsystem scheme to the
> >> existing design especially for multi-path.  In that case, it should be
> >> split into two to make namespace independent from a controller.
> >>
> >> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> >> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> > 
> > 
> > In latest master, I can no longer access NVMe devices from edk2. Git
> > bisect pointed me to this commit.
> > 
> > To reproduce:
> > 
> >   $ ./build/qemu-system-x86_64 -enable-kvm -pflash
> > build/pc-bios/edk2-x86_64-code.fd -drive
> > file=image.raw,if=none,id=d,snapshot=on -device nvme,serial=1234,drive=d
> > -nographic -net none
> > 
> > You will see that before this commit, edk2 is able to enumerate the
> > block device, while after this commit it does not find it.
> > 
> > 
> > good:
> > 
> > Mapping table
> >       FS0: Alias(s):HD1b:;BLK2:
> >         �
> > PciRoot(0x0)/Pci(0x3,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)/HD(1,GPT,7A866FF6-0A12-4911-A4ED-D0565BEB41A2,0x80,0x64000)
> > 
> >      BLK0: Alias(s):
> >           PciRoot(0x0)/Pci(0x1,0x1)/Ata(0x0)
> >      BLK1: Alias(s):
> >         � PciRoot(0x0)/Pci(0x3,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)
> >      BLK3: Alias(s):
> >         �
> > PciRoot(0x0)/Pci(0x3,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)/HD(2,GPT,F070566B-C58D-4F13-9B92-64F1794385E7,0x64080,0x40000)
> > 
> >      BLK4: Alias(s):
> >         �
> > PciRoot(0x0)/Pci(0x3,0x0)/NVMe(0x1,00-00-00-00-00-00-00-00)/HD(3,GPT,EDE86BE3-C64F-4ACB-B4AA-5E6C0135D335,0xA4080,0x315BF1B)
> 
> Good integration test for the emulated NVMe subsystem!
> 
> tests/acceptance/boot_linux_console.py should provide trivial template.
> 

Yes. This is something we really need. I will priorities this together
with documentation :)

Thanks for pointing me in the direction of this Philippe.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]