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 <lviv...@redhat.com>
> >> ---
> >> 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
