From: Hannes Reinecke <hare@suse.de>
To: Klaus Jensen <its@irrelevant.dk>
Cc: Kevin Wolf <kwolf@redhat.com>,
qemu-block@nongnu.org, qemu-devel@nongnu.org,
Keith Busch <keith.busch@wdc.com>
Subject: Re: [PATCH] hw/block/nvme: re-enable NVMe PCI hotplug
Date: Tue, 11 May 2021 18:12:47 +0200 [thread overview]
Message-ID: <7f4c0a64-582b-edc7-7362-2da45c137702@suse.de> (raw)
In-Reply-To: <YJqq6rTRTL3mxMK6@apples.localdomain>
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
next prev parent reply other threads:[~2021-05-11 16:19 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-11 7:35 [PATCH] hw/block/nvme: re-enable NVMe PCI hotplug Hannes Reinecke
2021-05-11 7:45 ` no-reply
2021-05-11 8:05 ` Philippe Mathieu-Daudé
2021-05-11 12:22 ` Klaus Jensen
2021-05-11 13:12 ` Hannes Reinecke
2021-05-11 13:37 ` Klaus Jensen
2021-05-11 14:54 ` Hannes Reinecke
2021-05-11 16:03 ` Klaus Jensen
2021-05-11 16:12 ` Hannes Reinecke [this message]
2022-10-10 17:01 ` Daniel Wagner
2022-10-10 17:15 ` Klaus Jensen
2022-10-18 8:15 ` Daniel Wagner
2022-10-21 12:59 ` Daniel Wagner
2022-10-31 7:30 ` Klaus Jensen
2022-10-12 6:24 ` Hannes Reinecke
2022-10-12 8:06 ` Klaus Jensen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f4c0a64-582b-edc7-7362-2da45c137702@suse.de \
--to=hare@suse.de \
--cc=its@irrelevant.dk \
--cc=keith.busch@wdc.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).