From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53193) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZJIJ-0005nI-IG for qemu-devel@nongnu.org; Wed, 01 Oct 2014 08:46:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XZJID-0004ZA-AX for qemu-devel@nongnu.org; Wed, 01 Oct 2014 08:46:11 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44636 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XZJIC-0004Yk-W8 for qemu-devel@nongnu.org; Wed, 01 Oct 2014 08:46:05 -0400 Message-ID: <542BF78B.20008@suse.de> Date: Wed, 01 Oct 2014 14:46:03 +0200 From: =?windows-1252?Q?Andreas_F=E4rber?= MIME-Version: 1.0 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> <20141001105222.GC21207@in.ibm.com> <20141001144323.5dfa8a1d@igors-macbook-pro.local> In-Reply-To: <20141001144323.5dfa8a1d@igors-macbook-pro.local> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov , Bharata B Rao Cc: qemu-devel@nongnu.org Am 01.10.2014 um 14:43 schrieb Igor Mammedov: > On Wed, 1 Oct 2014 16:22:23 +0530 > Bharata B Rao wrote: >=20 >> 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 =3D required_for_version; } >>>>> >>>>> +static HotplugHandler *qdev_get_hotplug_handler(DeviceState >>>>> *dev) +{ >>>>> + HotplugHandler *hotplug_ctrl =3D NULL; >>>>> + >>>>> + if (dev->parent_bus && dev->parent_bus->hotplug_handler) { >>>>> + hotplug_ctrl =3D dev->parent_bus->hotplug_handler; >>>>> + } else if (object_dynamic_cast(qdev_get_machine(), >>>>> TYPE_MACHINE)) { >>>>> + MachineState *machine =3D MACHINE(qdev_get_machine()); >>>>> + MachineClass *mc =3D MACHINE_GET_CLASS(machine); >>>>> + >>>>> + if (mc->get_hotplug_handler) { >>>>> + hotplug_ctrl =3D mc->get_hotplug_handler(machine, >>>>> dev); >>>>> + } >>>>> + } >>>>> + return hotplug_ctrl; >>>>> +} >>>>> + >>>>> void qdev_unplug(DeviceState *dev, Error **errp) >>>>> { >>>>> DeviceClass *dc =3D 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.htm= l >>> >>> Above of cause assumes that device is placed in 'peripherals' tree. >>> Does above patch fixes issue? >> >> Yes it does fix the issue. Thanks. > Thanks for confirming, I'll try to rewrite above path to a more > acceptable form and post it as follow up to this series, hopefully > tomorrow. >=20 > Andreas, > is it acceptable or should I respin fixedup 36/36 instead? As I started looking into this series I'd appreciate not respinning the full series if not necessary. :) Whether you do a 37/36 v2 or a 36/36 v3 is up to you guys. Thanks, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg