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

[Jag Raman]

> On Jan 26, 2022, at 4:35 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> On Tue, Jan 25, 2022 at 06:12:48PM +0000, Jag Raman wrote:
>> 
>> 
>>> On Jan 25, 2022, at 5:32 AM, Stefan Hajnoczi <stefanha@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.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
>> 
>> 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.
> 
> Hotplug is definitely possible with q35. I'm not familiar with the
> hotplug code though so I don't know how exactly that works for q35.

I was referring to the root PCI bus, other buses in Q35 probably support
hotplug. Please see error message below:

QEMU 6.2.50 monitor - type 'help' for more information
(qemu) device_add lsi53c895a,id=lsi2
Error: Bus 'pcie.0' does not support hotplugging

--
Jag

> 
>> 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.
> 
> Great!
> 
> Stefan