Re: [PATCH] hw/block/nvme: re-enable NVMe PCI hotplug

[Klaus Jensen <its@irrelevant.dk>]

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

On Okt 12 08:24, Hannes Reinecke wrote:
> On 10/10/22 19:01, Daniel Wagner wrote:
> > On Tue, May 11, 2021 at 06:12:47PM +0200, Hannes Reinecke wrote:
> > > On 5/11/21 6:03 PM, Klaus Jensen wrote:
> > > > On May 11 16:54, Hannes Reinecke wrote:
> > > > > On 5/11/21 3:37 PM, Klaus Jensen wrote:
> > > > > > On May 11 15:12, Hannes Reinecke wrote:
> > > > > > > On 5/11/21 2:22 PM, Klaus Jensen wrote:
> > > > > [ .. ]
> > > > > > > > The hotplug fix looks good - I'll post a series that
> > > > > > > > tries to integrate
> > > > > > > > both.
> > > > > > > > 
> > > > > > > Ta.
> > > > > > > 
> > > > > > > The more I think about it, the more I think we should be looking into
> > > > > > > reparenting the namespaces to the subsystem.
> > > > > > > That would have the _immediate_ benefit that 'device_del' and
> > > > > > > 'device_add' becomes symmetric (ie one doesn't have to do a separate
> > > > > > > 'device_add nvme-ns'), as the nvme namespace is not affected by the
> > > > > > > hotplug event.
> > > > > > > 
> > > > > > 
> > > > > > I have that working, but I'm struggling with a QEMU API technicality in
> > > > > > that I apparently cannot simply move the NvmeBus creation to the
> > > > > > nvme-subsys device. For some reason the bus is not available for the
> > > > > > nvme-ns devices. That is, if one does something like this:
> > > > > > 
> > > > > >    -device nvme-subsys,...
> > > > > >    -device nvme-ns,...
> > > > > > 
> > > > > > Then I get an error that "no 'nvme-bus' bus found for device 'nvme'ns".
> > > > > > This is probably just me not grok'ing the qdev well enough, so I'll keep
> > > > > > trying to fix that. What works now is to have the regular setup:
> > > > > > 
> > > > > _Normally_ the 'id' of the parent device spans a bus, so the syntax
> > > > > should be
> > > > > 
> > > > > -device nvme-subsys,id=subsys1,...
> > > > > -device nvme-ns,bus=subsys1,...
> > > > 
> > > > Yeah, I know, I just oversimplified the example. This *is* how I wanted
> > > > it to work ;)
> > > > 
> > > > > 
> > > > > As for the nvme device I would initially expose any namespace from the
> > > > > subsystem to the controller; the nvme spec has some concept of 'active'
> > > > > or 'inactive' namespaces which would allow us to blank out individual
> > > > > namespaces on a per-controller basis, but I fear that's not easy to
> > > > > model with qdev and the structure above.
> > > > > 
> > > > 
> > > > The nvme-ns device already supports the boolean 'detached' parameter to
> > > > support the concept of an inactive namespace.
> > > > 
> > > Yeah, but that doesn't really work if we move the namespace to the
> > > subsystem; the 'detached' parameter is for the controller<->namespace
> > > relationship.
> > > That's why I meant we have to have some sort of NSID map for the controller
> > > such that the controller knows with NSID to access.
> > > I guess we can copy the trick with the NSID array, and reverse the operation
> > > we have now wrt subsystem; keep the main NSID array in the subsystem, and
> > > per-controller NSID arrays holding those which can be accessed.
> > > 
> > > And ignore the commandline for now; figure that one out later.
> > > 
> [..]
> > 
> > Sorry to ask but has there been any progress on this topic? Just run
> > into the same issue that adding nvme device during runtime is not
> > showing any namespace.
> > 
> I _thought_ that the pci hotplug fixes have now been merged with qemu
> upstream. Klaus?
> 

Yup. It's all upstream.

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