On 31.01.19 21:40, Collin Walling wrote: > On 1/30/19 10:57 AM, David Hildenbrand wrote: >> PCI on s390x is really weird and how it was modeled in QEMU might not have >> been the right choice. Anyhow, right now it is the case that: >> - Hotplugging a PCI device will silently create a zPCI device >> (if none is provided) >> - Hotunplugging a zPCI device will unplug the PCI device (if any) >> - Hotunplugging a PCI device will unplug also the zPCI device >> As far as I can see, we can no longer change this behavior. But we >> should fix it. >> >> Both device types are handled via a single hotplug handler call. This >> is problematic for various reasons: >> 1. Unplugging via the zPCI device allows to unplug devices that are not >> hot removable. (check performed in qdev_unplug()) - bad. >> 2. Hotplug handler chains are not possible for the unplug case. In the >> future, the machine might want to override hotplug handlers, to >> process device specific stuff and to then branch off to the actual >> hotplug handler. We need separate hotplug handler calls for both the >> PCI and zPCI device to make this work reliably. All other PCI >> implementations are already prepared to handle this correctly, only >> s390x is missing. >> >> Therefore, introduce the unplug_request handler and properly perform >> unplug checks by redirecting to the separate unplug_request handlers. >> When finally unplugging, perform two separate hotplug_handler_unplug() >> calls, first for the PCI device, followed by the zPCI device. This now >> nicely splits unplugging paths for both devices. >> >> The redirect part is a little hairy, as the user is allowed to trigger >> unplug either via the PCI or the zPCI device. So redirect always to the >> PCI unplug request handler first and remember if that check has been >> performed in the zPCI device. Redirect then to the zPCI device unplug >> request handler to perform the magic. Remembering that we already >> checked the PCI device breaks the redirect loop. >> >> Signed-off-by: David Hildenbrand <da...@redhat.com> >> --- > > Thanks for clearing up some of the areas I was having troubles following > during the last post. I think this looks a lot better than what we had > before. Patches 4, 5, and 6 make the code a lot easier to understand > what is going on here. > > I assume you were able to squeeze in an easy test? At least making sure > we can still hot unplug a device from a guest successfully?
I only tested under TCG, but I did quite some tests - Plug/unplug a device - Disable slot in guest, unplug device - Plug bridges, try to unplug - Create hierarchy of bridges (statically and dynamically) - Plug devices to bridge hierarchies ... Thanks! > > From the code perspective: > > Reviewed-by: Collin Walling <wall...@linux.ibm.com> > -- Thanks, David / dhildenb
