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

[Peter Maydell]

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
-- PMM