From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55808) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g9UVQ-0000DW-RS for qemu-devel@nongnu.org; Mon, 08 Oct 2018 08:19:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g9UVN-0006lc-0x for qemu-devel@nongnu.org; Mon, 08 Oct 2018 08:19:24 -0400 Date: Mon, 8 Oct 2018 14:19:04 +0200 From: Igor Mammedov Message-ID: <20181008141904.17b601d3@redhat.com> In-Reply-To: <85d59b49-25d7-6485-843c-838324cbc52e@redhat.com> References: <20180926094219.20322-1-david@redhat.com> <20180926094219.20322-19-david@redhat.com> <20180927150141.60a6488a@redhat.com> <20181001152443.2cca5c5e@redhat.com> <85d59b49-25d7-6485-843c-838324cbc52e@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 18/24] qdev: hotplug: provide do_unplug handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, "Dr . David Alan Gilbert" , "Michael S . Tsirkin" , Marcel Apfelbaum , Paolo Bonzini , Richard Henderson , Eduardo Habkost , Eric Blake , Markus Armbruster , Pankaj Gupta , Luiz Capitulino , Xiao Guangrong , David Gibson , Alexander Graf On Mon, 8 Oct 2018 13:47:53 +0200 David Hildenbrand wrote: > > That way using [2] and [1 - modulo it should match only concrete type] > > machine would be able to override hotplug handlers for TYPE_VIRTIO_PMEM_PCI > > and explicitly call machine + pci hotplug handlers in necessary order. > > > > flow would look like: > > [acpi|shcp|native pci-e eject]-> > > hotplug_ctrl = qdev_get_hotplug_handler(dev); > > hotplug_handler_unplug(hotplug_ctrl, dev, &local_err); -> > > machine_unplug() > > machine_virtio_pci_pmem_cb(): > > // we now that's device has 2 stage hotplug handlers, > > // so we can arrange hotplug sequence in necessary order > > hotplug_ctrl2 = qdev_get_bus_hotplug_handler(dev); > > > > //then do unplug in whatever order that's correct, > > // I'd assume tear down/stop PCI device first, flushing > > // command virtio command queues and that unplug memory itself. > > hotplug_handler_unplug(hotplug_ctrl2, dev, &local_err); > > memory_device_unplug() > > > > Looking into the details, this order is not possible. The unplug will > essentially do a device_unparent() leading to the whole hierarchy > getting destroyed. The memory_device part always has to come first. Question here is if there are anything that should be handled first on virtio level before memory_device/pmem part is called? If there isn't it might be fine to swap the order of unplug sequence. > > Similar logic applies to device_add/device_del paths, with a difference that > > origin point would be monitor/qmp. > > > > Point is to have a single explicit callback chain that applies to a concrete > > device type. That way if ever change an ordering of calling plug callbacks > > in qdev core, the expected for a device callback sequence would still > > remain in place ensuring that device (un)plugged as expected. > > > > Also it should be easier to trace for a reader, than 2 disjoint callbacks of > > composite device (which only know to author of that device and then only > > till he/she remembers how that thing works). > > > >