From 07578b0ad6a6792fbe23bcd8477f3331b78f6027 Mon Sep 17 00:00:00 2001 From: David Hildenbrand Date: Thu, 28 Feb 2019 13:28:47 +0100 Subject: qdev: Let the hotplug_handler_unplug() caller delete the device When unplugging a device, at one point the device will be destroyed via object_unparent(). This will, one the one hand, unrealize the removed device hierarchy, and on the other hand, destroy/free the device hierarchy. When chaining hotplug handlers, we want to overwrite a bus hotplug handler by the machine hotplug handler, to be able to perform some part of the plug/unplug and to forward the calls to the bus hotplug handler. For now, the bus hotplug handler would trigger an object_unparent(), not allowing us to perform some unplug action on a device after we forwarded the call to the bus hotplug handler. The device would be gone at that point. machine_unplug_handler(dev) /* eventually do unplug stuff */ bus_unplug_handler(dev) /* dev is gone, we can't do more unplug stuff */ So move the object_unparent() to the original caller of the unplug. For now, keep the unrealize() at the original places of the object_unparent(). For implicitly chained hotplug handlers (e.g. pc code calling acpi hotplug handlers), the object_unparent() has to be done by the outermost caller. So when calling hotplug_handler_unplug() from inside an unplug handler, nothing is to be done. hotplug_handler_unplug(dev) -> calls machine_unplug_handler() machine_unplug_handler(dev) { /* eventually do unplug stuff */ bus_unplug_handler(dev) -> calls unrealize(dev) /* we can do more unplug stuff but device already unrealized */ } object_unparent(dev) In the long run, every unplug action should be factored out of the unrealize() function into the unplug handler (especially for PCI). Then we can get rid of the additonal unrealize() calls and object_unparent() will properly unrealize the device hierarchy after the device has been unplugged. hotplug_handler_unplug(dev) -> calls machine_unplug_handler() machine_unplug_handler(dev) { /* eventually do unplug stuff */ bus_unplug_handler(dev) -> only unplugs, does not unrealize /* we can do more unplug stuff */ } object_unparent(dev) -> will unrealize The original approach was suggested by Igor Mammedov for the PCI part, but I extended it to all hotplug handlers. I consider this one step into the right direction. To summarize: - object_unparent() on synchronous unplugs is done by common code -- "Caller of hotplug_handler_unplug" - object_unparent() on asynchronous unplugs ("unplug requests") has to be done manually -- "Caller of hotplug_handler_unplug" Reviewed-by: Igor Mammedov Acked-by: Cornelia Huck Signed-off-by: David Hildenbrand Message-Id: <20190228122849.4296-2-david@redhat.com> Reviewed-by: Greg Kurz Signed-off-by: Eduardo Habkost --- hw/pci/pcie.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'hw/pci/pcie.c') diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 3618d6ab2e..640f678773 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -450,7 +450,7 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, void pcie_cap_slot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { - object_unparent(OBJECT(dev)); + object_property_set_bool(OBJECT(dev), false, "realized", NULL); } static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque) @@ -458,6 +458,7 @@ static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque) HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(dev)); hotplug_handler_unplug(hotplug_ctrl, DEVICE(dev), &error_abort); + object_unparent(OBJECT(dev)); } void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev, -- cgit v1.2.3