Re: [PATCH for-6.0 v2 2/2] hw/block/nvme: disable hotplugging for subsystem-linked controllers

[Klaus Jensen <its@irrelevant.dk>]

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

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.

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