On Apr 23 14:21, Peter Maydell wrote:
>On Fri, 23 Apr 2021 at 06:21, Klaus Jensen <its@irrelevant.dk> wrote:
>>
>> From: Klaus Jensen <k.jensen@samsung.com>
>>
>> If a controller is linked to a subsystem, do not allow it to be
>> hotplugged since this will mess up the (possibly shared) namespaces.
>>
>> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
>> ---
>> hw/block/nvme.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>> index 5fe082ec34c5..7606b58a39b9 100644
>> --- a/hw/block/nvme.c
>> +++ b/hw/block/nvme.c
>> @@ -6140,12 +6140,16 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
*pci_dev)
>>
>> static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
>> {
>> + DeviceClass *dc;
>> int cntlid;
>>
>> if (!n->subsys) {
>> return 0;
>> }
>>
>> + dc = DEVICE_GET_CLASS(n);
>> + dc->hotpluggable = false;
>> +
>> cntlid = nvme_subsys_register_ctrl(n, errp);
>> if (cntlid < 0) {
>> return -1;
>
>I'm not sure this is right -- the DeviceClass is the
>class struct, which there's only one of for every instance
>of the device in the system. So this is saying "if this instance
>is linked to a subsystem, don't let any *future* instances ever
>be hotpluggable". I'm not even sure if it will do the right
>thing for the current device, because this function is called
>from the device's realize method, and the device_set_realized()
>function does the "forbid if dc->hotpluggable is false" check
>before calling the realize method.
>
>Possibly what you want to do here is to call the
>device_get_hotplugged() function and just make the realize
>method fail with a suitable error if the device is both (a) being
>hotplugged and (b) has a subsystem link; but I'm not an expert on
>hotplug, so I might be wrong.
>
Thanks Peter, this sounds exactly like what I want. I'll respin!
I have a "full" fix that actually makes the device hotpluggable in the
context of subsystems, but it is definitely not an -rc thing.
signature.asc