From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32781) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZHWt-0004vM-Bz for qemu-devel@nongnu.org; Wed, 01 Oct 2014 06:53:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XZHWk-00073V-37 for qemu-devel@nongnu.org; Wed, 01 Oct 2014 06:53:07 -0400 Received: from e23smtp06.au.ibm.com ([202.81.31.148]:58097) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZHWj-00072Y-G7 for qemu-devel@nongnu.org; Wed, 01 Oct 2014 06:52:58 -0400 Received: from /spool/local by e23smtp06.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 1 Oct 2014 20:52:51 +1000 Received: from d23relay06.au.ibm.com (d23relay06.au.ibm.com [9.185.63.219]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 10828357804E for ; Wed, 1 Oct 2014 20:52:47 +1000 (EST) Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay06.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id s91AsbAL30736572 for ; Wed, 1 Oct 2014 20:54:38 +1000 Received: from d23av02.au.ibm.com (localhost [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s91Aqj8j030586 for ; Wed, 1 Oct 2014 20:52:45 +1000 Date: Wed, 1 Oct 2014 16:22:23 +0530 From: Bharata B Rao Message-ID: <20141001105222.GC21207@in.ibm.com> References: <1411723721-20484-1-git-send-email-imammedo@redhat.com> <1411723721-20484-37-git-send-email-imammedo@redhat.com> <20141001085718.GB21207@in.ibm.com> <20141001115737.398d8804@nial.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20141001115737.398d8804@nial.usersys.redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: dmitry@daynix.com, borntraeger@de.ibm.com, mst@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, amit.shah@redhat.com, kraxel@redhat.com, cornelia.huck@de.ibm.com, pbonzini@redhat.com, rth@twiddle.net On Wed, Oct 01, 2014 at 11:57:37AM +0200, Igor Mammedov wrote: > On Wed, 1 Oct 2014 14:27:19 +0530 > Bharata B Rao wrote: > > > On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote: > > > Signed-off-by: Igor Mammedov > > > --- > > > v2: > > > merged in Paolo's suggestion to reduce indentation in patch > > > --- > > > hw/core/qdev.c | 61 ++++++++++++++++++++++++++++++++-------------------------- > > > 1 file changed, 34 insertions(+), 27 deletions(-) > > > > > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > > > index bc45a59..215effb 100644 > > > --- a/hw/core/qdev.c > > > +++ b/hw/core/qdev.c > > > @@ -223,9 +223,28 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, > > > dev->alias_required_for_version = required_for_version; > > > } > > > > > > +static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev) > > > +{ > > > + HotplugHandler *hotplug_ctrl = NULL; > > > + > > > + if (dev->parent_bus && dev->parent_bus->hotplug_handler) { > > > + hotplug_ctrl = dev->parent_bus->hotplug_handler; > > > + } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { > > > + MachineState *machine = MACHINE(qdev_get_machine()); > > > + MachineClass *mc = MACHINE_GET_CLASS(machine); > > > + > > > + if (mc->get_hotplug_handler) { > > > + hotplug_ctrl = mc->get_hotplug_handler(machine, dev); > > > + } > > > + } > > > + return hotplug_ctrl; > > > +} > > > + > > > void qdev_unplug(DeviceState *dev, Error **errp) > > > { > > > DeviceClass *dc = DEVICE_GET_CLASS(dev); > > > + HotplugHandler *hotplug_ctrl; > > > + HotplugHandlerClass *hdc; > > > > > > if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { > > > error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); > > > > Unlike x86 CPU that sits on ICC bus, PowerPC CPU doesn't sit on any bus. > > I was under the impression that this patch helps in unplugging such > > devices. However I do see some problems. > Indeed, this patch goes only half way for supporting unplug of bussless > devices (i.e. it only fetches unplug handler without addressing device > lookup issue) > > > > > While trying to unplug a PowerPC CPU that has earlier been added using > > device_add, qdev-monitor.c:qmp_device_del() fails to find/locate the > > device since the device isn't on any bus and hence qdev_unplug() won't > > be called for such a device. I thought I could make "main_system_bus" as > > the parent bus of this CPU device but that wouldn't help during unplugging. > > With that qmp_device_del() can find the device, but qdev_uplug() fails at the > > above if condition since "main_system_bus" isn't hotpluggable. > > > > So how should we deal with such devices which don't have any parent bus ? > There is no need to invent non existing BUS. We should fix lookup issue > instead of it. Doing something along this lines: > https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg04574.html > > Above of cause assumes that device is placed in 'peripherals' tree. > Does above patch fixes issue? Yes it does fix the issue. Thanks. However device_del command completion isn't showing my CPU device. I guess the patch you mentioned should take care of looking up in the peripherals tree in monitor.c:device_del_bus_completion() too ? Regards, Bharata.