diff mbox

[2/4] libqos: add PCI management in qtest_vboot()/qtest_shutdown()

Message ID 1474899049-12506-3-git-send-email-lvivier@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laurent Vivier Sept. 26, 2016, 2:10 p.m. UTC 
Signed-off-by: Laurent Vivier <lvivier@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(-)

Comments

David Gibson Sept. 27, 2016, 3:48 a.m. UTC | #1 
On Mon, Sep 26, 2016 at 04:10:47PM +0200, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@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.

> 
> diff --git a/tests/e1000e-test.c b/tests/e1000e-test.c
> index d497b08..3979b20 100644
> --- a/tests/e1000e-test.c
> +++ b/tests/e1000e-test.c
> @@ -390,7 +390,7 @@ static void data_test_init(e1000e_device *d)
>      qtest_start(cmdline);
>      g_free(cmdline);
>  
> -    test_bus = qpci_init_pc();
> +    test_bus = qpci_init_pc(NULL);
>      g_assert_nonnull(test_bus);
>  
>      test_alloc = pc_alloc_init();
> diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
> index 3542ad1..da2d5a5 100644
> --- a/tests/i440fx-test.c
> +++ b/tests/i440fx-test.c
> @@ -38,7 +38,7 @@ static QPCIBus *test_start_get_bus(const TestData *s)
>      cmdline = g_strdup_printf("-smp %d", s->num_cpus);
>      qtest_start(cmdline);
>      g_free(cmdline);
> -    return qpci_init_pc();
> +    return qpci_init_pc(NULL);
>  }
>  
>  static void test_i440fx_defaults(gconstpointer opaque)
> diff --git a/tests/ide-test.c b/tests/ide-test.c
> index 1e51af2..a8a4081 100644
> --- a/tests/ide-test.c
> +++ b/tests/ide-test.c
> @@ -143,7 +143,7 @@ static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
>      uint16_t vendor_id, device_id;
>  
>      if (!pcibus) {
> -        pcibus = qpci_init_pc();
> +        pcibus = qpci_init_pc(NULL);
>      }
>  
>      /* Find PCI device and verify it's the right one */
> diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
> index 0957ee7..f36bfe7 100644
> --- a/tests/ivshmem-test.c
> +++ b/tests/ivshmem-test.c
> @@ -105,7 +105,7 @@ static void setup_vm_cmd(IVState *s, const char *cmd, bool msix)
>      uint64_t barsize;
>  
>      s->qtest = qtest_start(cmd);
> -    s->pcibus = qpci_init_pc();
> +    s->pcibus = qpci_init_pc(NULL);
>      s->dev = get_device(s->pcibus);
>  
>      s->reg_base = qpci_iomap(s->dev, 0, &barsize);
> diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
> index f3be550..716ab79 100644
> --- a/tests/libqos/ahci.c
> +++ b/tests/libqos/ahci.c
> @@ -128,7 +128,7 @@ QPCIDevice *get_ahci_device(uint32_t *fingerprint)
>      uint32_t ahci_fingerprint;
>      QPCIBus *pcibus;
>  
> -    pcibus = qpci_init_pc();
> +    pcibus = qpci_init_pc(NULL);
>  
>      /* Find the AHCI PCI device and verify it's the right one. */
>      ahci = qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02));
> diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
> index df34092..aa17c98 100644
> --- a/tests/libqos/libqos-pc.c
> +++ b/tests/libqos/libqos-pc.c
> @@ -1,10 +1,13 @@
>  #include "qemu/osdep.h"
>  #include "libqos/libqos-pc.h"
>  #include "libqos/malloc-pc.h"
> +#include "libqos/pci-pc.h"
>  
>  static QOSOps qos_ops = {
>      .init_allocator = pc_alloc_init_flags,
> -    .uninit_allocator = pc_alloc_uninit
> +    .uninit_allocator = pc_alloc_uninit,
> +    .qpci_init = qpci_init_pc,
> +    .qpci_free = qpci_free_pc,
>  };
>  
>  QOSState *qtest_pc_vboot(const char *cmdline_fmt, va_list ap)
> diff --git a/tests/libqos/libqos-spapr.c b/tests/libqos/libqos-spapr.c
> index f19408b..125c6b3 100644
> --- a/tests/libqos/libqos-spapr.c
> +++ b/tests/libqos/libqos-spapr.c
> @@ -1,10 +1,13 @@
>  #include "qemu/osdep.h"
>  #include "libqos/libqos-spapr.h"
>  #include "libqos/malloc-spapr.h"
> +#include "libqos/pci-spapr.h"
>  
>  static QOSOps qos_ops = {
>      .init_allocator = spapr_alloc_init_flags,
> -    .uninit_allocator = spapr_alloc_uninit
> +    .uninit_allocator = spapr_alloc_uninit,
> +    .qpci_init = qpci_init_spapr,
> +    .qpci_free = qpci_free_spapr
>  };
>  
>  QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap)
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index a852dc5..332d60e 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -20,8 +20,13 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap)
>      cmdline = g_strdup_vprintf(cmdline_fmt, ap);
>      qs->qts = qtest_start(cmdline);
>      qs->ops = ops;
> -    if (ops && ops->init_allocator) {
> -        qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
> +    if (ops) {
> +        if (ops->init_allocator) {
> +            qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
> +        }
> +        if (ops->qpci_init && qs->alloc) {
> +            qs->pcibus = ops->qpci_init(qs->alloc);
> +        }
>      }
>  
>      g_free(cmdline);
> @@ -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.

>      }
>      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..

> diff --git a/tests/libqos/pci-pc.h b/tests/libqos/pci-pc.h
> index 2621179..9479b51 100644
> --- a/tests/libqos/pci-pc.h
> +++ b/tests/libqos/pci-pc.h
> @@ -14,8 +14,9 @@
>  #define LIBQOS_PCI_PC_H
>  
>  #include "libqos/pci.h"
> +#include "libqos/malloc.h"
>  
> -QPCIBus *qpci_init_pc(void);
> +QPCIBus *qpci_init_pc(QGuestAllocator *alloc);
>  void     qpci_free_pc(QPCIBus *bus);
>  
>  #endif
> diff --git a/tests/q35-test.c b/tests/q35-test.c
> index 71538fc..763fe3d 100644
> --- a/tests/q35-test.c
> +++ b/tests/q35-test.c
> @@ -42,7 +42,7 @@ static void test_smram_lock(void)
>      QPCIDevice *pcidev;
>      QDict *response;
>  
> -    pcibus = qpci_init_pc();
> +    pcibus = qpci_init_pc(NULL);
>      g_assert(pcibus != NULL);
>  
>      pcidev = qpci_device_find(pcibus, 0);
> diff --git a/tests/rtl8139-test.c b/tests/rtl8139-test.c
> index 13de7ee..c2f601a 100644
> --- a/tests/rtl8139-test.c
> +++ b/tests/rtl8139-test.c
> @@ -35,7 +35,7 @@ static QPCIDevice *get_device(void)
>  {
>      QPCIDevice *dev;
>  
> -    pcibus = qpci_init_pc();
> +    pcibus = qpci_init_pc(NULL);
>      qpci_device_foreach(pcibus, 0x10ec, 0x8139, save_fn, &dev);
>      g_assert(dev != NULL);
>  
> diff --git a/tests/tco-test.c b/tests/tco-test.c
> index 0d13aa8..0d201b1 100644
> --- a/tests/tco-test.c
> +++ b/tests/tco-test.c
> @@ -57,7 +57,7 @@ static void test_init(TestData *d)
>      qtest_irq_intercept_in(qs, "ioapic");
>      g_free(s);
>  
> -    bus = qpci_init_pc();
> +    bus = qpci_init_pc(NULL);
>      d->dev = qpci_device_find(bus, QPCI_DEVFN(0x1f, 0x00));
>      g_assert(d->dev != NULL);
>  
> diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c
> index eb247ad..a4ceeaa 100644
> --- a/tests/usb-hcd-ehci-test.c
> +++ b/tests/usb-hcd-ehci-test.c
> @@ -56,7 +56,7 @@ static void pci_init(void)
>      if (pcibus) {
>          return;
>      }
> -    pcibus = qpci_init_pc();
> +    pcibus = qpci_init_pc(NULL);
>      g_assert(pcibus != NULL);
>  
>      qusb_pci_init_one(pcibus, &uhci1, QPCI_DEVFN(0x1d, 0), 4);
> diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
> index 5cd59ad..c24063e 100644
> --- a/tests/usb-hcd-uhci-test.c
> +++ b/tests/usb-hcd-uhci-test.c
> @@ -23,7 +23,7 @@ static void test_port(int port)
>      struct qhc uhci;
>  
>      g_assert(port > 0);
> -    pcibus = qpci_init_pc();
> +    pcibus = qpci_init_pc(NULL);
>      g_assert(pcibus != NULL);
>      qusb_pci_init_one(pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4);
>      uhci_port_test(&uhci, port - 1, UHCI_PORT_CCS);
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index b89a551..300308c 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -146,7 +146,7 @@ static void init_virtio_dev(TestServer *s)
>      QVirtioPCIDevice *dev;
>      uint32_t features;
>  
> -    bus = qpci_init_pc();
> +    bus = qpci_init_pc(NULL);
>      g_assert_nonnull(bus);
>  
>      dev = qvirtio_pci_device_find(bus, VIRTIO_ID_NET);
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index b8fb6cd..e8b2196 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -63,7 +63,7 @@ static QVirtIO9P *qvirtio_9p_pci_init(void)
>  
>      v9p = g_new0(QVirtIO9P, 1);
>      v9p->alloc = pc_alloc_init();
> -    v9p->bus = qpci_init_pc();
> +    v9p->bus = qpci_init_pc(NULL);
>  
>      dev = qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P);
>      g_assert_nonnull(dev);
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 811cf75..3c4fecc 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -75,7 +75,7 @@ static QPCIBus *pci_test_start(void)
>      g_free(tmp_path);
>      g_free(cmdline);
>  
> -    return qpci_init_pc();
> +    return qpci_init_pc(NULL);
>  }
>  
>  static void arm_test_start(void)
> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index 361506f..a343a6b 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -62,7 +62,7 @@ static QPCIBus *pci_test_start(int socket)
>      qtest_start(cmdline);
>      g_free(cmdline);
>  
> -    return qpci_init_pc();
> +    return qpci_init_pc(NULL);
>  }
>  
>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
> index f1489e6..79088bb 100644
> --- a/tests/virtio-scsi-test.c
> +++ b/tests/virtio-scsi-test.c
> @@ -146,7 +146,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>  
>      vs = g_new0(QVirtIOSCSI, 1);
>      vs->alloc = pc_alloc_init();
> -    vs->bus = qpci_init_pc();
> +    vs->bus = qpci_init_pc(NULL);
>  
>      dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
>      vs->dev = (QVirtioDevice *)dev;
Laurent Vivier Sept. 27, 2016, 7:33 a.m. UTC | #2 
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 <lvivier@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.

>>      }
>>      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"?

Thanks,
Laurent
David Gibson Sept. 27, 2016, 8:29 a.m. UTC | #3 
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 <lvivier@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.
diff mbox

Patch

diff --git a/tests/e1000e-test.c b/tests/e1000e-test.c
index d497b08..3979b20 100644
--- a/tests/e1000e-test.c
+++ b/tests/e1000e-test.c
@@ -390,7 +390,7 @@  static void data_test_init(e1000e_device *d)
     qtest_start(cmdline);
     g_free(cmdline);
 
-    test_bus = qpci_init_pc();
+    test_bus = qpci_init_pc(NULL);
     g_assert_nonnull(test_bus);
 
     test_alloc = pc_alloc_init();
diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
index 3542ad1..da2d5a5 100644
--- a/tests/i440fx-test.c
+++ b/tests/i440fx-test.c
@@ -38,7 +38,7 @@  static QPCIBus *test_start_get_bus(const TestData *s)
     cmdline = g_strdup_printf("-smp %d", s->num_cpus);
     qtest_start(cmdline);
     g_free(cmdline);
-    return qpci_init_pc();
+    return qpci_init_pc(NULL);
 }
 
 static void test_i440fx_defaults(gconstpointer opaque)
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 1e51af2..a8a4081 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -143,7 +143,7 @@  static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
     uint16_t vendor_id, device_id;
 
     if (!pcibus) {
-        pcibus = qpci_init_pc();
+        pcibus = qpci_init_pc(NULL);
     }
 
     /* Find PCI device and verify it's the right one */
diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
index 0957ee7..f36bfe7 100644
--- a/tests/ivshmem-test.c
+++ b/tests/ivshmem-test.c
@@ -105,7 +105,7 @@  static void setup_vm_cmd(IVState *s, const char *cmd, bool msix)
     uint64_t barsize;
 
     s->qtest = qtest_start(cmd);
-    s->pcibus = qpci_init_pc();
+    s->pcibus = qpci_init_pc(NULL);
     s->dev = get_device(s->pcibus);
 
     s->reg_base = qpci_iomap(s->dev, 0, &barsize);
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index f3be550..716ab79 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -128,7 +128,7 @@  QPCIDevice *get_ahci_device(uint32_t *fingerprint)
     uint32_t ahci_fingerprint;
     QPCIBus *pcibus;
 
-    pcibus = qpci_init_pc();
+    pcibus = qpci_init_pc(NULL);
 
     /* Find the AHCI PCI device and verify it's the right one. */
     ahci = qpci_device_find(pcibus, QPCI_DEVFN(0x1F, 0x02));
diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
index df34092..aa17c98 100644
--- a/tests/libqos/libqos-pc.c
+++ b/tests/libqos/libqos-pc.c
@@ -1,10 +1,13 @@ 
 #include "qemu/osdep.h"
 #include "libqos/libqos-pc.h"
 #include "libqos/malloc-pc.h"
+#include "libqos/pci-pc.h"
 
 static QOSOps qos_ops = {
     .init_allocator = pc_alloc_init_flags,
-    .uninit_allocator = pc_alloc_uninit
+    .uninit_allocator = pc_alloc_uninit,
+    .qpci_init = qpci_init_pc,
+    .qpci_free = qpci_free_pc,
 };
 
 QOSState *qtest_pc_vboot(const char *cmdline_fmt, va_list ap)
diff --git a/tests/libqos/libqos-spapr.c b/tests/libqos/libqos-spapr.c
index f19408b..125c6b3 100644
--- a/tests/libqos/libqos-spapr.c
+++ b/tests/libqos/libqos-spapr.c
@@ -1,10 +1,13 @@ 
 #include "qemu/osdep.h"
 #include "libqos/libqos-spapr.h"
 #include "libqos/malloc-spapr.h"
+#include "libqos/pci-spapr.h"
 
 static QOSOps qos_ops = {
     .init_allocator = spapr_alloc_init_flags,
-    .uninit_allocator = spapr_alloc_uninit
+    .uninit_allocator = spapr_alloc_uninit,
+    .qpci_init = qpci_init_spapr,
+    .qpci_free = qpci_free_spapr
 };
 
 QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap)
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index a852dc5..332d60e 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -20,8 +20,13 @@  QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap)
     cmdline = g_strdup_vprintf(cmdline_fmt, ap);
     qs->qts = qtest_start(cmdline);
     qs->ops = ops;
-    if (ops && ops->init_allocator) {
-        qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
+    if (ops) {
+        if (ops->init_allocator) {
+            qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
+        }
+        if (ops->qpci_init && qs->alloc) {
+            qs->pcibus = ops->qpci_init(qs->alloc);
+        }
     }
 
     g_free(cmdline);
@@ -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;
+        }
     }
     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;
 
diff --git a/tests/libqos/pci-pc.h b/tests/libqos/pci-pc.h
index 2621179..9479b51 100644
--- a/tests/libqos/pci-pc.h
+++ b/tests/libqos/pci-pc.h
@@ -14,8 +14,9 @@ 
 #define LIBQOS_PCI_PC_H
 
 #include "libqos/pci.h"
+#include "libqos/malloc.h"
 
-QPCIBus *qpci_init_pc(void);
+QPCIBus *qpci_init_pc(QGuestAllocator *alloc);
 void     qpci_free_pc(QPCIBus *bus);
 
 #endif
diff --git a/tests/q35-test.c b/tests/q35-test.c
index 71538fc..763fe3d 100644
--- a/tests/q35-test.c
+++ b/tests/q35-test.c
@@ -42,7 +42,7 @@  static void test_smram_lock(void)
     QPCIDevice *pcidev;
     QDict *response;
 
-    pcibus = qpci_init_pc();
+    pcibus = qpci_init_pc(NULL);
     g_assert(pcibus != NULL);
 
     pcidev = qpci_device_find(pcibus, 0);
diff --git a/tests/rtl8139-test.c b/tests/rtl8139-test.c
index 13de7ee..c2f601a 100644
--- a/tests/rtl8139-test.c
+++ b/tests/rtl8139-test.c
@@ -35,7 +35,7 @@  static QPCIDevice *get_device(void)
 {
     QPCIDevice *dev;
 
-    pcibus = qpci_init_pc();
+    pcibus = qpci_init_pc(NULL);
     qpci_device_foreach(pcibus, 0x10ec, 0x8139, save_fn, &dev);
     g_assert(dev != NULL);
 
diff --git a/tests/tco-test.c b/tests/tco-test.c
index 0d13aa8..0d201b1 100644
--- a/tests/tco-test.c
+++ b/tests/tco-test.c
@@ -57,7 +57,7 @@  static void test_init(TestData *d)
     qtest_irq_intercept_in(qs, "ioapic");
     g_free(s);
 
-    bus = qpci_init_pc();
+    bus = qpci_init_pc(NULL);
     d->dev = qpci_device_find(bus, QPCI_DEVFN(0x1f, 0x00));
     g_assert(d->dev != NULL);
 
diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c
index eb247ad..a4ceeaa 100644
--- a/tests/usb-hcd-ehci-test.c
+++ b/tests/usb-hcd-ehci-test.c
@@ -56,7 +56,7 @@  static void pci_init(void)
     if (pcibus) {
         return;
     }
-    pcibus = qpci_init_pc();
+    pcibus = qpci_init_pc(NULL);
     g_assert(pcibus != NULL);
 
     qusb_pci_init_one(pcibus, &uhci1, QPCI_DEVFN(0x1d, 0), 4);
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index 5cd59ad..c24063e 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -23,7 +23,7 @@  static void test_port(int port)
     struct qhc uhci;
 
     g_assert(port > 0);
-    pcibus = qpci_init_pc();
+    pcibus = qpci_init_pc(NULL);
     g_assert(pcibus != NULL);
     qusb_pci_init_one(pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4);
     uhci_port_test(&uhci, port - 1, UHCI_PORT_CCS);
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index b89a551..300308c 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -146,7 +146,7 @@  static void init_virtio_dev(TestServer *s)
     QVirtioPCIDevice *dev;
     uint32_t features;
 
-    bus = qpci_init_pc();
+    bus = qpci_init_pc(NULL);
     g_assert_nonnull(bus);
 
     dev = qvirtio_pci_device_find(bus, VIRTIO_ID_NET);
diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
index b8fb6cd..e8b2196 100644
--- a/tests/virtio-9p-test.c
+++ b/tests/virtio-9p-test.c
@@ -63,7 +63,7 @@  static QVirtIO9P *qvirtio_9p_pci_init(void)
 
     v9p = g_new0(QVirtIO9P, 1);
     v9p->alloc = pc_alloc_init();
-    v9p->bus = qpci_init_pc();
+    v9p->bus = qpci_init_pc(NULL);
 
     dev = qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P);
     g_assert_nonnull(dev);
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 811cf75..3c4fecc 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -75,7 +75,7 @@  static QPCIBus *pci_test_start(void)
     g_free(tmp_path);
     g_free(cmdline);
 
-    return qpci_init_pc();
+    return qpci_init_pc(NULL);
 }
 
 static void arm_test_start(void)
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 361506f..a343a6b 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -62,7 +62,7 @@  static QPCIBus *pci_test_start(int socket)
     qtest_start(cmdline);
     g_free(cmdline);
 
-    return qpci_init_pc();
+    return qpci_init_pc(NULL);
 }
 
 static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index f1489e6..79088bb 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -146,7 +146,7 @@  static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
 
     vs = g_new0(QVirtIOSCSI, 1);
     vs->alloc = pc_alloc_init();
-    vs->bus = qpci_init_pc();
+    vs->bus = qpci_init_pc(NULL);
 
     dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
     vs->dev = (QVirtioDevice *)dev;