[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 18/24] qdev: hotplug: provide do_u

From: David Hildenbrand
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH v4 18/24] qdev: hotplug: provide do_unplug handler
Date: Fri, 12 Oct 2018 10:45:41 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0

> The correct order should be opposite to one that created a devices,
> i.e. unplug -> unrealize -> delete.
> Doing unplug stuff after device was unrealized looks outright wrong
> (essentially device doesn't exists anymore except memory where it's
> been located).

pre_plug -> realize -> plug

unplug -> unrealize -> post_unplug

doesn't look that wrong to me. But the problem seems to be that unplug
basically spans the whole unrealize phase (including the post_unplug
phase). So unplug should usually already contains the post_unplug part
as you noted below (when moving the object_unparent() part out).

>> As I already said that, the unplug/unplug_request handlers are very
>> different to the other handlers, as they actively delete(request to
>> delete) an object. In contrast to pre_plug/plug that don't create an
>> object but only wire it up while realizing the device.>
>> That is the reason why we can't do stuff after calling the bus hotunplug
>> handler but only before it. We cannot really modify the calling order.
> There is nothing special in unplug handlers wrt plug ones, they all are
> external to the being created device. Theoretically we can move pre_plug
> /plug from device_set_realize() to outer caller qdev_device_add() and
> nothing would change.

I guess at some point we should definitely move them out, this only
leads to confusion. (e.g. hotplug handlers getting called on device
within device hierarchies although we don't want this to be possible)

> The problem here is the lack of unplug handler for pci device so
> unplugging boils down to object_unparent() which will unrealize
> device (and in process unplug it) and then delete it.
> What we really need is to factor out unplug code from pci device
> unrealizefn(). Then ideally unplug controller could look like:
>  static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
>  {
> +    hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +    ... do some port specific unplug ...
> +    hotplug_handler_do_unplug(hotplug_ctrl, dev); // default pci device 
> unplug or pmem specific
>      object_unparent(OBJECT(dev));
>  }
> where tear down and unrealize/delete parts are separated from each other.

That makes sense, but we would then handle it for all PCI devices via
the hotplug chain I guess. (otherwise a object_unparent would be missing)

>> Do you have other ideas?
> I'd proceed with suggestions made earlier [1][2] on this thread.
> That should solve the issue at hand with out factoring out PCI unplug
> from old pci::unrealize(). One would have to introduce shim unplug
> handlers for pci/bridge/pcie that would call object_unparent(), but
> that's the extent of another shallow PCI re-factoring.
> Of cause that's assuming that sequence
>  1.  memory_device_unplug()
>  2.  pci_unplug()
> is correct in virtio-pmem-pci case.

That is indeed possible as long as the memory device part has to come
first. I'll give it a try.

I already started prototyping and found some other PCI hotplug handler
issues I have to solve first ....



David / dhildenb

reply via email to

[Prev in Thread] Current Thread [Next in Thread]