Re: [Qemu-devel] [PATCH 15/30] virtio-pci: replace BusState.allow_hotplug with hotplug_handler

[Igor Mammedov]

On Wed, 24 Sep 2014 14:23:25 +0200
Paolo Bonzini <address@hidden> wrote:

> Il 24/09/2014 13:48, Igor Mammedov ha scritto:
> > Although virtio-pci-bus is internal object of composite
> > virtio-pci device and it doesn't participate in
> > -device/device_add hotplug flow, it's required by
> > bus_add_child() that bus must be hotpluggable to
> > be able to add child at runtime.
> > Set parent of virtio-pci-bus as NOP hotplug controller,
> > so that bus_add_child() would allow to add child
> > device during hotplug time when BusState.allow_hotplug
> > is dropped.
> > 
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/virtio/virtio-pci.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index f560814..0486b25 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -1070,6 +1070,10 @@ static const TypeInfo virtio_pci_info = {
> >      .class_init    = virtio_pci_class_init,
> >      .class_size    = sizeof(VirtioPCIClass),
> >      .abstract      = true,
> > +    .interfaces = (InterfaceInfo[]) {
> > +        { TYPE_HOTPLUG_HANDLER },
> > +        { }
> > +    }
> >  };
> >  
> >  /* virtio-blk-pci */
> > @@ -1543,13 +1547,11 @@ static void virtio_pci_bus_new(VirtioBusState *bus, 
> > size_t bus_size,
> >                                 VirtIOPCIProxy *dev)
> >  {
> >      DeviceState *qdev = DEVICE(dev);
> > -    BusState *qbus;
> >      char virtio_bus_name[] = "virtio-bus";
> >  
> >      qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_PCI_BUS, qdev,
> >                          virtio_bus_name);
> > -    qbus = BUS(bus);
> > -    qbus->allow_hotplug = 1;
> > +    qbus_set_hotplug_handler(BUS(bus), qdev, &error_abort);
> >  }
> >  
> >  static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
> > 
> 
> So this is the opposite of icc-bus; here you need to call the simple
> hot-unplug callback in the hotplug handler, right?
It's only to workaround bus_add_child() limitation, there is no need
to call unplug callback for child of this bus, since bus and child device
are manually managed by owner of bus without involving hotplug machinery
(via PCIDeviceClass.[init|exit] callbacks).

Perhaps instead we should just allow to add child to bus is bus_add_child()
regardless whether it's hotpluggable or not and drop this and other
similar patches.
> 
> Paolo