[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks gen
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic |
Date: |
Mon, 7 Apr 2014 15:07:15 +0300 |
On Mon, Apr 07, 2014 at 02:00:37PM +0200, Igor Mammedov wrote:
> On Mon, 7 Apr 2014 14:32:41 +0300
> "Michael S. Tsirkin" <address@hidden> wrote:
>
> > On Fri, Apr 04, 2014 at 03:36:48PM +0200, Igor Mammedov wrote:
> > > ... and report error if plugged in device is not supported.
> > > Later generic callbacks will be used by memory hotplug.
> > >
> > > Signed-off-by: Igor Mammedov <address@hidden>
> >
> >
> > OK in that case, how about teaching all hotplug callbacks about this?
> >
> > There are two ATM:
> > shpc_device_hotplug_cb
> > pcie_cap_slot_hotplug_cb
> >
> > Teach them both to fail gracefully if they get
> > an object that is not a pci device.
> >
> > Afterwards, simply iterate over all objects of type
> > TYPE_HOTPLUG_HANDLER
> > and look for one that will accept your object.
> Then you would never know if any hotplug handler has actually
> handled event.
Why not? Check the error.
If no one accepts your object, return error to user.
> I think hotplug handler should return error if unsupported
> device passed in rather than ignore it. It makes catching
> wiring errors easier.
Absolutely.
> Dropping error so that we could not care which hotplug handler
> should be notified, looks like a wrong direction and makes
> system more fragile.
That's not what I was suggesting.
> It shouldn't be up to consumer to determine that event should
> be routed to it, but rather by external routing that knows
> what and when should be notified.
Yes. So
for each hotplug handler (&err)
handler->plug(device, &err)
if (!err)
break;
if (err)
hotplug failed - destroy device
> >
> >
> > > ---
> > > hw/acpi/piix4.c | 31 ++++++++++++++++++++++---------
> > > 1 file changed, 22 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > index 67dc075..4341f82 100644
> > > --- a/hw/acpi/piix4.c
> > > +++ b/hw/acpi/piix4.c
> > > @@ -310,19 +310,32 @@ static void piix4_pm_powerdown_req(Notifier *n,
> > > void *opaque)
> > > acpi_pm1_evt_power_down(&s->ar);
> > > }
> > >
> > > -static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev,
> > > - DeviceState *dev, Error **errp)
> > > +static void piix4_device_plug_cb(HotplugHandler *hotplug_dev,
> > > + DeviceState *dev, Error **errp)
> > > {
> > > PIIX4PMState *s = PIIX4_PM(hotplug_dev);
> > > - acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug, dev,
> > > errp);
> > > +
> > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> > > + acpi_pcihp_device_plug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug,
> > > dev,
> > > + errp);
> > > + } else {
> > > + error_setg(errp, "acpi: device plug request for not supported
> > > device"
> > > + " type: %s", object_get_typename(OBJECT(dev)));
> > > + }
> > > }
> > >
> > > -static void piix4_pci_device_unplug_cb(HotplugHandler *hotplug_dev,
> > > - DeviceState *dev, Error **errp)
> > > +static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev,
> > > + DeviceState *dev, Error **errp)
> > > {
> > > PIIX4PMState *s = PIIX4_PM(hotplug_dev);
> > > - acpi_pcihp_device_unplug_cb(&s->ar, s->irq, &s->acpi_pci_hotplug,
> > > dev,
> > > - errp);
> > > +
> > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> > > + acpi_pcihp_device_unplug_cb(&s->ar, s->irq,
> > > &s->acpi_pci_hotplug, dev,
> > > + errp);
> > > + } else {
> > > + error_setg(errp, "acpi: device unplug request for not supported
> > > device"
> > > + " type: %s", object_get_typename(OBJECT(dev)));
> > > + }
> > > }
> > >
> > > static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque)
> > > @@ -553,8 +566,8 @@ static void piix4_pm_class_init(ObjectClass *klass,
> > > void *data)
> > > */
> > > dc->cannot_instantiate_with_device_add_yet = true;
> > > dc->hotpluggable = false;
> > > - hc->plug = piix4_pci_device_plug_cb;
> > > - hc->unplug = piix4_pci_device_unplug_cb;
> > > + hc->plug = piix4_device_plug_cb;
> > > + hc->unplug = piix4_device_unplug_cb;
> > > }
> > >
> > > static const TypeInfo piix4_pm_info = {
> > > --
> > > 1.9.0
- [Qemu-devel] [PATCH 15/35] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS, (continued)
- [Qemu-devel] [PATCH 15/35] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS, Igor Mammedov, 2014/04/04
- [Qemu-devel] [PATCH 17/35] dimm: add busy address check and address auto-allocation, Igor Mammedov, 2014/04/04
- [Qemu-devel] [PATCH 07/35] pc: create custom generic PC machine type, Igor Mammedov, 2014/04/04
- [Qemu-devel] [PATCH 18/35] dimm: add busy slot check and slot auto-allocation, Igor Mammedov, 2014/04/04
- [Qemu-devel] [PATCH 20/35] acpi: memory hotplug ACPI hardware implementation, Igor Mammedov, 2014/04/04
- [Qemu-devel] [PATCH 10/35] dimm: implement dimm device abstraction, Igor Mammedov, 2014/04/04
- [Qemu-devel] [PATCH 19/35] acpi: rename cpu_hotplug_defs.h to acpi_defs.h, Igor Mammedov, 2014/04/04
- [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Igor Mammedov, 2014/04/04
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Michael S. Tsirkin, 2014/04/07
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Igor Mammedov, 2014/04/07
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Igor Mammedov, 2014/04/07
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Michael S. Tsirkin, 2014/04/07
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Igor Mammedov, 2014/04/07
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Michael S. Tsirkin, 2014/04/07
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Igor Mammedov, 2014/04/11
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Paolo Bonzini, 2014/04/29
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Michael S. Tsirkin, 2014/04/29
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Paolo Bonzini, 2014/04/29
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Michael S. Tsirkin, 2014/04/29
- Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic, Paolo Bonzini, 2014/04/29