[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH v2 36/36] qdev: HotplugHandler: add support for unplugging BUS-less devices |
Date: |
Wed, 1 Oct 2014 11:57:37 +0200 |
On Wed, 1 Oct 2014 14:27:19 +0530
Bharata B Rao <address@hidden> wrote:
> On Fri, Sep 26, 2014 at 09:28:41AM +0000, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > 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?
>
> Regards,
> Bharata.
>