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

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Jagannathan Raman <jag.raman@oracle.com>
Cc: eduardo@habkost.net, elena.ufimtseva@oracle.com,
	john.g.johnson@oracle.com, berrange@redhat.com, bleal@redhat.com,
	john.levon@nutanix.com, mst@redhat.com, armbru@redhat.com,
	quintela@redhat.com, qemu-devel@nongnu.org, f4bug@amsat.org,
	marcandre.lureau@gmail.com, thanos.makatos@nutanix.com,
	pbonzini@redhat.com, eblake@redhat.com, dgilbert@redhat.com
Subject: Re: [PATCH v5 06/18] vfio-user: add HotplugHandler for remote machine
Date: Tue, 25 Jan 2022 10:32:47 +0000	[thread overview]
Message-ID: <Ye/Rz1bHJN2m9vCo@stefanha-x1.localdomain> (raw)
In-Reply-To: <fa3282607f7fed7736bfdf3c1ae9f7fce466ed44.1642626515.git.jag.raman@oracle.com>

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

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
> 

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