Re: [PATCH v5 06/18] vfio-user: add HotplugHandler for remote machine

[Stefan Hajnoczi]

On Wed, Jan 19, 2022 at 04:41:55PM -0500, Jagannathan Raman wrote:
> Allow hotplugging of PCI(e) devices to remote machine
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  hw/remote/machine.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)

Why is this code necessary? I expected the default hotplug behavior to
pretty much handle this case - hotplugging device types that the bus
doesn't support should fail and unplug should already unparent/unrealize
the device.

> 
> diff --git a/hw/remote/machine.c b/hw/remote/machine.c
> index 952105eab5..220ff01aa9 100644
> --- a/hw/remote/machine.c
> +++ b/hw/remote/machine.c
> @@ -54,14 +54,39 @@ static void remote_machine_init(MachineState *machine)
>  
>      pci_bus_irqs(pci_host->bus, remote_iohub_set_irq, remote_iohub_map_irq,
>                   &s->iohub, REMOTE_IOHUB_NB_PIRQS);
> +
> +    qbus_set_hotplug_handler(BUS(pci_host->bus), OBJECT(s));
> +}
> +
> +static void remote_machine_pre_plug_cb(HotplugHandler *hotplug_dev,
> +                                       DeviceState *dev, Error **errp)
> +{
> +    if (!object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        error_setg(errp, "Only allowing PCI hotplug");
> +    }
> +}
> +
> +static void remote_machine_unplug_cb(HotplugHandler *hotplug_dev,
> +                                     DeviceState *dev, Error **errp)
> +{
> +    if (!object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> +        error_setg(errp, "Only allowing PCI hot-unplug");
> +        return;
> +    }
> +
> +    qdev_unrealize(dev);
>  }
>  
>  static void remote_machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> +    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>  
>      mc->init = remote_machine_init;
>      mc->desc = "Experimental remote machine";
> +
> +    hc->pre_plug = remote_machine_pre_plug_cb;
> +    hc->unplug = remote_machine_unplug_cb;
>  }
>  
>  static const TypeInfo remote_machine = {
> @@ -69,6 +94,10 @@ static const TypeInfo remote_machine = {
>      .parent = TYPE_MACHINE,
>      .instance_size = sizeof(RemoteMachineState),
>      .class_init = remote_machine_class_init,
> +    .interfaces = (InterfaceInfo[]) {
> +        { TYPE_HOTPLUG_HANDLER },
> +        { }
> +    }
>  };
>  
>  static void remote_machine_register_types(void)
> -- 
> 2.20.1
>

signature.asc
Description: PGP signature