> On Jan 25, 2022, at 5:32 AM, Stefan Hajnoczi <stefa...@redhat.com> wrote:
>
> 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.ufimts...@oracle.com>
>> Signed-off-by: John G Johnson <john.g.john...@oracle.com>
>> Signed-off-by: Jagannathan Raman <jag.ra...@oracle.com>
>> ---
>> hw/remote/machine.c | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>
> Why is this code necessary? I expected the default hotplug behavior to
I just discovered that TYPE_REMOTE_MACHINE wasn't setting up a hotplug
handler for the root PCI bus.
Looks like, some of the machines don’t support hotplugging PCI devices. I see
that the ‘pc’ machine does support hotplug, whereas ‘q35’ does not.
We didn’t check hotplug in multiprocess-qemu previously because it was limited
to one device per process, and the use cases attached the devices via
command line.
> pretty much handle this case - hotplugging device types that the bus
> doesn't support should fail and unplug should already unparent/unrealize
> the device.
OK, that makes sense. We don’t need to test the device type during
plug and unplug.
Therefore, I don’t think we need a callback for the plug operation. We
could set HotplugHandlerClass->unplug callback to the default
qdev_simple_device_unplug_cb() callback.
—
Jag
>
>>
>> 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
>>
