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

[Hannes Reinecke <hare@suse.de>]

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?

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: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman