From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35548) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq15w-0004n8-56 for qemu-devel@nongnu.org; Mon, 09 Dec 2013 08:42:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vq15q-0002zG-6Q for qemu-devel@nongnu.org; Mon, 09 Dec 2013 08:41:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24857) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vq15p-0002y9-Th for qemu-devel@nongnu.org; Mon, 09 Dec 2013 08:41:50 -0500 Date: Mon, 9 Dec 2013 14:41:39 +0100 From: Igor Mammedov Message-ID: <20131209144139.7f9ac562@nial.usersys.redhat.com> In-Reply-To: <52A20996.8020308@redhat.com> References: <1386349395-5710-1-git-send-email-imammedo@redhat.com> <1386349395-5710-8-git-send-email-imammedo@redhat.com> <52A20996.8020308@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 7/7] hw/pci: convert PCI bus to use "hotplug-device" interface. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: peter.maydell@linaro.org, ehabkost@redhat.com, marcel.a@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, alex.williamson@redhat.com, anthony@codemonkey.ws, afaerber@suse.de On Fri, 06 Dec 2013 18:29:58 +0100 Paolo Bonzini wrote: > Il 06/12/2013 18:03, Igor Mammedov ha scritto: > > + hotplug_dev = DEVICE(object_property_get_link(OBJECT(bus), "hotplug-device", > > + &local_err)); > > + if (error_is_set(&local_err)) { > > + goto error_exit; > > + } > > + if (hotplug_dev) { > > + HotplugDeviceClass *hdc = HOTPLUG_DEVICE_GET_CLASS(hotplug_dev); > > + > > + /* handler can differentiate between hotplug and when device is > > + * enabled during qemu machine creation by inspecting > > + * dev->hotplugged field. */ > > + if (hdc->hotplug) { > > + hdc->hotplug(hotplug_dev, qdev, &local_err); > > + if (error_is_set(&local_err)) { > > + int r = pci_unregister_device(&pci_dev->qdev); > > + assert(!r); > > + goto error_exit; > > + } > > } > > } > > > + > > + hotplug_dev = DEVICE(object_property_get_link(OBJECT(bus), "hotplug-device", > > + &local_err)); > > + if (error_is_set(&local_err)) { > > + goto error_exit; > > + } > > + if (hotplug_dev) { > > + HotplugDeviceClass *hdc = HOTPLUG_DEVICE_GET_CLASS(hotplug_dev); > > + > > + if (hdc->hot_unplug) { > > + hdc->hot_unplug(hotplug_dev, qdev, &local_err); > > + if (error_is_set(&local_err)) { > > + goto error_exit; > > + } > > + } > > + } > > Please move the parts under the "if" to hotplug.c (something like > hotplug_handler_plug and hotplug_handler_unplug). Also, should this be sure. > moved up to generic code (e.g. bus_add_child/bus_remove_child)? Current usage hints that it's more realize() related part. If handler fails device might want to do same device specific failure handling before returning from realize() with failure. It will work in bus_add_child/bus_remove_child as well but it would basically put handlers to nofail category. > > A NULL link can be a no-op for coldplugged devices, an error for > hotplugged devices, and an assertion failure for hot-unplug. > > Paolo >