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

[Klaus Jensen <its@irrelevant.dk>]

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

On Okt 10 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.
> > 
> > Cheers,
> > 
> > Hannes
> > > > >   -device nvme-subsys,...
> > > > >   -device nvme,...
> > > > >   -device nvme-ns,...
> > > > > 
> > > > > And the nvme-ns device will then reparent to the NvmeBus on nvme-subsys
> > > > > (which magically now IS available when nvme-ns is realized). This has
> > > > > the same end result, but I really would like that the namespaces could
> > > > > be specified as children of the subsys directly.
> > > > > 
> > > > Shudder.
> > > > Automatic reparenting.
> > > > To my understanding from qdev that shouldn't even be possible.
> > > > Please don't.
> > > > 
> > > 
> > > It's perfectly possible with the API and used to implement stuff like
> > > failover. We are not changing the parent object, we are changing the
> > > parent bus. hw/sd does something like this (but does mention that its a
> > > bit of a hack). In this case I'd say we could argue to get away with it
> > > as well.
> > > Allowing the nvme-ns device to be a child of the controller allows the
> > > initially attached controller of non-shared namespaces to be easily
> > > expressible. But I agree, the approach is a bit wacky, which is why I
> > > havnt posted anything yet.
> > 
> > Yep, I did try to implement multipathing for SCSI at one point, and that
> > became patently horrible as it would run against qdev where everything is
> > ordered within a tree.
> 
> 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.
> 

This is all upstream. Namespaces with 'shared=on' *should* all be
automatically attached to any hotplugged controller devices.

With what setup is this not working for you?

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