On Tue, Nov 19, 2013 at 01:51:58PM +0100, Andreas Färber wrote: > Am 06.11.2013 23:52, schrieb Bandan Das: > > > > PCIBus registers a vmstate during init. Unregister it upon > > removal/unplug. > > > > Signed-off-by: Bandan Das <b...@redhat.com> > > Michael, this patch looks good for 1.7 to me, are you planning to still > pick it up? Only one small comment below. > > Cc: qemu-sta...@nongnu.org > > --- > > Note that I didn't add a instance_init to register vmstate (yet) > > due to concerns expressed by Andreas that we shouldn't be registering > > global state there. > > What's happening here is the following: instance_init does in fact not > register anything, but vmstate_unregister() becomes a no-op loop if the > vmsd+opaque combo is not registered, so it is safe. The registration > happens in pci_bus_new() / pci_bus_new_inplace(), which I believe all > PCI buses to date inside QEMU use, i.e. after instance_init, so in > practice unregistering will not be no-op. > > > hw/pci/pci.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index a98c8a0..63ef7ce 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -47,6 +47,7 @@ static void pcibus_dev_print(Monitor *mon, DeviceState > > *dev, int indent); > > static char *pcibus_get_dev_path(DeviceState *dev); > > static char *pcibus_get_fw_dev_path(DeviceState *dev); > > static int pcibus_reset(BusState *qbus); > > +static void pci_bus_finalize(Object *obj); > > It may be nicer to avoid the prototype by moving the new > pci_bus_finalize() above pci_bus_info. But since what counts is the fix > to avoid segfaults during migration on access to a dangling opaque > pointer after hot-unplug of a PCI-PCI bridge, > > Reviewed-by: Andreas Färber <afaer...@suse.de> > > Thanks, > Andreas >
Thanks for the review, I'll review and hopefully merge later today. > > > > static Property pci_props[] = { > > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), > > @@ -73,6 +74,7 @@ static const TypeInfo pci_bus_info = { > > .name = TYPE_PCI_BUS, > > .parent = TYPE_BUS, > > .instance_size = sizeof(PCIBus), > > + .instance_finalize = pci_bus_finalize, > > .class_init = pci_bus_class_init, > > }; > > > > @@ -401,6 +403,12 @@ int pci_bus_num(PCIBus *s) > > return s->parent_dev->config[PCI_SECONDARY_BUS]; > > } > > > > +static void pci_bus_finalize(Object *obj) > > +{ > > + PCIBus *bus = PCI_BUS(obj); > > + vmstate_unregister(NULL, &vmstate_pcibus, bus); > > +} > > + > > static int get_pci_config_device(QEMUFile *f, void *pv, size_t size) > > { > > PCIDevice *s = container_of(pv, PCIDevice, config); > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg