On Tue, Sep 27, 2016 at 09:33:58AM +0200, Laurent Vivier wrote: > > > On 27/09/2016 05:48, David Gibson wrote: > > On Mon, Sep 26, 2016 at 04:10:47PM +0200, Laurent Vivier wrote: > >> Signed-off-by: Laurent Vivier <[email protected]> > >> --- > >> tests/e1000e-test.c | 2 +- > >> tests/i440fx-test.c | 2 +- > >> tests/ide-test.c | 2 +- > >> tests/ivshmem-test.c | 2 +- > >> tests/libqos/ahci.c | 2 +- > >> tests/libqos/libqos-pc.c | 5 ++++- > >> tests/libqos/libqos-spapr.c | 5 ++++- > >> tests/libqos/libqos.c | 21 ++++++++++++++++----- > >> tests/libqos/libqos.h | 3 +++ > >> tests/libqos/pci-pc.c | 2 +- > >> tests/libqos/pci-pc.h | 3 ++- > >> tests/q35-test.c | 2 +- > >> tests/rtl8139-test.c | 2 +- > >> tests/tco-test.c | 2 +- > >> tests/usb-hcd-ehci-test.c | 2 +- > >> tests/usb-hcd-uhci-test.c | 2 +- > >> tests/vhost-user-test.c | 2 +- > >> tests/virtio-9p-test.c | 2 +- > >> tests/virtio-blk-test.c | 2 +- > >> tests/virtio-net-test.c | 2 +- > >> tests/virtio-scsi-test.c | 2 +- > >> 21 files changed, 45 insertions(+), 24 deletions(-) > > > > Couple of queries below. > > > > ... > > >> @@ -49,9 +54,15 @@ QOSState *qtest_boot(QOSOps *ops, const char > >> *cmdline_fmt, ...) > >> */ > >> void qtest_shutdown(QOSState *qs) > >> { > >> - if (qs->alloc && qs->ops && qs->ops->uninit_allocator) { > >> - qs->ops->uninit_allocator(qs->alloc); > >> - qs->alloc = NULL; > >> + if (qs->ops) { > >> + if (qs->alloc && qs->ops->uninit_allocator) { > >> + qs->ops->uninit_allocator(qs->alloc); > >> + qs->alloc = NULL; > >> + } > >> + if (qs->pcibus && qs->ops->qpci_free) { > >> + qs->ops->qpci_free(qs->pcibus); > >> + qs->pcibus = NULL; > >> + } > > > > Is it safe to cleanup the allocator before the PCI stuff? Usually > > cleanups want to go in the opposite order to initialization. > > Yes, you're right. Im' going to fix that.
Ok.
> >> }
> >> qtest_quit(qs->qts);
> >> g_free(qs);
> >> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
> >> index 604980d..a9f6990 100644
> >> --- a/tests/libqos/libqos.h
> >> +++ b/tests/libqos/libqos.h
> >> @@ -8,11 +8,14 @@
> >> typedef struct QOSOps {
> >> QGuestAllocator *(*init_allocator)(QAllocOpts);
> >> void (*uninit_allocator)(QGuestAllocator *);
> >> + QPCIBus *(*qpci_init)(QGuestAllocator *alloc);
> >> + void (*qpci_free)(QPCIBus *bus);
> >> } QOSOps;
> >>
> >> typedef struct QOSState {
> >> QTestState *qts;
> >> QGuestAllocator *alloc;
> >> + QPCIBus *pcibus;
> >> QOSOps *ops;
> >> } QOSState;
> >>
> >> diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
> >> index 82066b8..9600ed6 100644
> >> --- a/tests/libqos/pci-pc.c
> >> +++ b/tests/libqos/pci-pc.c
> >> @@ -212,7 +212,7 @@ static void qpci_pc_iounmap(QPCIBus *bus, void *data)
> >> /* FIXME */
> >> }
> >>
> >> -QPCIBus *qpci_init_pc(void)
> >> +QPCIBus *qpci_init_pc(QGuestAllocator *alloc)
> >> {
> >> QPCIBusPC *ret;
> >>
> >
> > You've added the alloc parameter, but you don't actually appear to use it..
>
> It's normal: qpci_init_spapr() needs it and to have the same function
> signature we have to add it to qpci_init_pc() even if it is not used.
> (it's why I have added a lot of of qpci_init_pc(NULL)), so we can add
> init in a generic way in "struct QOSOps". Perhaps we can use "void
> *opaque" instead of "QGuestAllocator *alloc"?
Oh, of course. Sorry I missed that.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
