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

[Hannes Reinecke]

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.

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer