[1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/virtio-9p-test.c   |  53 ++++++++--------
 tests/virtio-blk-test.c  | 154 +++++++++++++++++++++--------------------------
 tests/virtio-net-test.c  |  40 +++++-------
 tests/virtio-scsi-test.c |  70 ++++++++++-----------
 4 files changed, 140 insertions(+), 177 deletions(-)

On Thu, Sep 29, 2016 at 07:15:05PM +0200, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

This could do with a commit message, even if it's just to say that
this is supposed to be a refactor without behavioural change.

> ---
>  tests/virtio-9p-test.c   |  53 ++++++++--------
>  tests/virtio-blk-test.c  | 154 +++++++++++++++++++++--------------------------
>  tests/virtio-net-test.c  |  40 +++++-------
>  tests/virtio-scsi-test.c |  70 ++++++++++-----------
>  4 files changed, 140 insertions(+), 177 deletions(-)
> 
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index e8b2196..28d7f5b 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -10,62 +10,57 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "qemu-common.h"
> -#include "libqos/pci-pc.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
>  #include "standard-headers/linux/virtio_ids.h"
>  #include "standard-headers/linux/virtio_pci.h"
>  
>  static const char mount_tag[] = "qtest";
>  static char *test_share;
>  
> -static void qvirtio_9p_start(void)
> -{
> -    char *args;
>  
> +static QOSState *qvirtio_9p_start(void)
> +{
>      test_share = g_strdup("/tmp/qtest.XXXXXX");
>      g_assert_nonnull(mkdtemp(test_share));
> +    const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s "
> +                      "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s";
>  
> -    args = g_strdup_printf("-fsdev local,id=fsdev0,security_model=none,path=%s "
> -                           "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s",
> -                           test_share, mount_tag);
> -
> -    qtest_start(args);
> -    g_free(args);
> +    return qtest_pc_boot(cmd, test_share, mount_tag);
>  }
>  
> -static void qvirtio_9p_stop(void)
> +static void qvirtio_9p_stop(QOSState *qs)
>  {
> -    qtest_end();
> +    qtest_pc_shutdown(qs);

Shouldn't this be the generic shutdown call you added in the other series?

>      rmdir(test_share);
>      g_free(test_share);
>  }
>  
>  static void pci_nop(void)
>  {
> -    qvirtio_9p_start();
> -    qvirtio_9p_stop();
> +    QOSState *qs;
> +
> +    qs = qvirtio_9p_start();
> +    g_assert(qs);
> +    qvirtio_9p_stop(qs);
>  }
>  
>  typedef struct {
>      QVirtioDevice *dev;
> -    QGuestAllocator *alloc;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueue *vq;
>  } QVirtIO9P;
>  
> -static QVirtIO9P *qvirtio_9p_pci_init(void)
> +static QVirtIO9P *qvirtio_9p_pci_init(QOSState *qs)
>  {
>      QVirtIO9P *v9p;
>      QVirtioPCIDevice *dev;
>  
>      v9p = g_new0(QVirtIO9P, 1);
> -    v9p->alloc = pc_alloc_init();
> -    v9p->bus = qpci_init_pc(NULL);
>  
> -    dev = qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P);
> +    v9p->qs = qs;
> +    dev = qvirtio_pci_device_find(v9p->qs->pcibus, VIRTIO_ID_9P);
>      g_assert_nonnull(dev);
>      g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_9P);
>      v9p->dev = (QVirtioDevice *) dev;
> @@ -75,17 +70,15 @@ static QVirtIO9P *qvirtio_9p_pci_init(void)
>      qvirtio_set_acknowledge(&qvirtio_pci, v9p->dev);
>      qvirtio_set_driver(&qvirtio_pci, v9p->dev);
>  
> -    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->alloc, 0);
> +    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->qs->alloc, 0);
>      return v9p;
>  }
>  
>  static void qvirtio_9p_pci_free(QVirtIO9P *v9p)
>  {
> -    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->alloc);
> -    pc_alloc_uninit(v9p->alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->qs->alloc);
>      qvirtio_pci_device_disable(container_of(v9p->dev, QVirtioPCIDevice, vdev));
>      g_free(v9p->dev);
> -    qpci_free_pc(v9p->bus);
>      g_free(v9p);
>  }
>  
> @@ -96,9 +89,11 @@ static void pci_basic_config(void)
>      size_t tag_len;
>      char *tag;
>      int i;
> +    QOSState *qs;
>  
> -    qvirtio_9p_start();
> -    v9p = qvirtio_9p_pci_init();
> +    qs = qvirtio_9p_start();
> +    g_assert(qs);
> +    v9p = qvirtio_9p_pci_init(qs);
>  
>      addr = ((QVirtioPCIDevice *) v9p->dev)->addr + VIRTIO_PCI_CONFIG_OFF(false);
>      tag_len = qvirtio_config_readw(&qvirtio_pci, v9p->dev,
> @@ -115,7 +110,7 @@ static void pci_basic_config(void)
>      g_free(tag);
>  
>      qvirtio_9p_pci_free(v9p);
> -    qvirtio_9p_stop();
> +    qvirtio_9p_stop(qs);
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 3c4fecc..8cf62f6 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -10,12 +10,10 @@
>  
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "libqos/virtio-mmio.h"
> -#include "libqos/pci-pc.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
>  #include "libqos/malloc-generic.h"
>  #include "qemu/bswap.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -58,24 +56,21 @@ static char *drive_create(void)
>      return tmp_path;
>  }
>  
> -static QPCIBus *pci_test_start(void)
> +static QOSState *pci_test_start(void)
>  {
> -    char *cmdline;
> +    QOSState *qs = NULL;
>      char *tmp_path;
> +    const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
> +                      "-drive if=none,id=drive1,file=/dev/null,format=raw "
> +                      "-device virtio-blk-pci,id=drv0,drive=drive0,"
> +                      "addr=%x.%x";
>  
>      tmp_path = drive_create();
>  
> -    cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s,format=raw "
> -                        "-drive if=none,id=drive1,file=/dev/null,format=raw "
> -                        "-device virtio-blk-pci,id=drv0,drive=drive0,"
> -                        "addr=%x.%x",
> -                        tmp_path, PCI_SLOT, PCI_FN);
> -    qtest_start(cmdline);
> +    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
>      unlink(tmp_path);
>      g_free(tmp_path);
> -    g_free(cmdline);
> -
> -    return qpci_init_pc(NULL);
> +    return qs;
>  }
>  
>  static void arm_test_start(void)
> @@ -279,39 +274,35 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
>  static void pci_basic(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
>      void *addr;
>  
> -    bus = pci_test_start();
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    qs = pci_test_start();
> +    g_assert(qs);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>  
> -    alloc = pc_alloc_init();
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> +                                              qs->alloc, 0);
>  
>      /* MSI-X is not enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>  
> -    test_basic(&qvirtio_pci, &dev->vdev, alloc, &vqpci->vq,
> +    test_basic(&qvirtio_pci, &dev->vdev, qs->alloc, &vqpci->vq,
>                                                      (uint64_t)(uintptr_t)addr);
>  
>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);
>  }
>  
>  static void pci_indirect(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
> +    QOSState *qs;
>      QVirtioBlkReq req;
>      QVRingIndirectDesc *indirect;
>      void *addr;
> @@ -322,9 +313,10 @@ static void pci_indirect(void)
>      uint8_t status;
>      char *data;
>  
> -    bus = pci_test_start();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>  
>      /* MSI-X is not enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
> @@ -340,9 +332,8 @@ static void pci_indirect(void)
>                              (1u << VIRTIO_BLK_F_SCSI));
>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>  
> -    alloc = pc_alloc_init();
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> +                                              qs->alloc, 0);
>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>  
>      /* Write request */
> @@ -352,11 +343,11 @@ static void pci_indirect(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>      qvring_indirect_desc_add(indirect, req_addr, 528, false);
>      qvring_indirect_desc_add(indirect, req_addr + 528, 1, true);
>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
> @@ -368,7 +359,7 @@ static void pci_indirect(void)
>      g_assert_cmpint(status, ==, 0);
>  
>      g_free(indirect);
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* Read request */
>      req.type = VIRTIO_BLK_T_IN;
> @@ -377,11 +368,11 @@ static void pci_indirect(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>      qvring_indirect_desc_add(indirect, req_addr, 16, false);
>      qvring_indirect_desc_add(indirect, req_addr + 16, 513, true);
>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
> @@ -398,28 +389,27 @@ static void pci_indirect(void)
>      g_free(data);
>  
>      g_free(indirect);
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);
>  }
>  
>  static void pci_config(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      int n_size = TEST_IMAGE_SIZE / 2;
>      void *addr;
>      uint64_t capacity;
>  
> -    bus = pci_test_start();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>  
>      /* MSI-X is not enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
> @@ -440,16 +430,15 @@ static void pci_config(void)
>  
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +
> +    qtest_shutdown(qs);
>  }
>  
>  static void pci_msix(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
>      QVirtioBlkReq req;
>      int n_size = TEST_IMAGE_SIZE / 2;
>      void *addr;
> @@ -460,13 +449,13 @@ static void pci_msix(void)
>      uint8_t status;
>      char *data;
>  
> -    bus = pci_test_start();
> -    alloc = pc_alloc_init();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>      qpci_msix_enable(dev->pdev);
>  
> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>  
>      /* MSI-X is enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
> @@ -483,8 +472,8 @@ static void pci_msix(void)
>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>  
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
> +                                              qs->alloc, 0);
> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>  
>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>  
> @@ -504,7 +493,7 @@ static void pci_msix(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -519,7 +508,7 @@ static void pci_msix(void)
>      status = readb(req_addr + 528);
>      g_assert_cmpint(status, ==, 0);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* Read request */
>      req.type = VIRTIO_BLK_T_IN;
> @@ -527,7 +516,7 @@ static void pci_msix(void)
>      req.sector = 0;
>      req.data = g_malloc0(512);
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -549,24 +538,21 @@ static void pci_msix(void)
>      g_assert_cmpstr(data, ==, "TEST");
>      g_free(data);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qpci_msix_disable(dev->pdev);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);
>  }
>  
>  static void pci_idx(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
>      QVirtioBlkReq req;
>      void *addr;
>      uint64_t req_addr;
> @@ -576,13 +562,13 @@ static void pci_idx(void)
>      uint8_t status;
>      char *data;
>  
> -    bus = pci_test_start();
> -    alloc = pc_alloc_init();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>      qpci_msix_enable(dev->pdev);
>  
> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>  
>      /* MSI-X is enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
> @@ -599,8 +585,8 @@ static void pci_idx(void)
>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>  
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
> +                                              qs->alloc, 0);
> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>  
>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>  
> @@ -611,7 +597,7 @@ static void pci_idx(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -630,7 +616,7 @@ static void pci_idx(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -647,7 +633,7 @@ static void pci_idx(void)
>                                               QVIRTIO_BLK_TIMEOUT_US);
>      g_assert_cmpint(status, ==, 0);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* Read request */
>      req.type = VIRTIO_BLK_T_IN;
> @@ -655,7 +641,7 @@ static void pci_idx(void)
>      req.sector = 1;
>      req.data = g_malloc0(512);
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -676,38 +662,36 @@ static void pci_idx(void)
>      g_assert_cmpstr(data, ==, "TEST");
>      g_free(data);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qpci_msix_disable(dev->pdev);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);
>  }
>  
>  static void pci_hotplug(void)
>  {
> -    QPCIBus *bus;
>      QVirtioPCIDevice *dev;
> +    QOSState *qs;
>  
> -    bus = pci_test_start();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  
>      /* plug secondary disk */
>      qpci_plug_device_test("virtio-blk-pci", "drv1", PCI_SLOT_HP,
>                            "'drive': 'drive1'");
>  
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT_HP);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT_HP);
>      g_assert(dev);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
>  
>      /* unplug secondary disk */
>      qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);
>  }
>  
>  static void mmio_basic(void)
> @@ -746,8 +730,8 @@ static void mmio_basic(void)
>  
>      /* End test */
>      qvirtqueue_cleanup(&qvirtio_mmio, vq, alloc);
> -    generic_alloc_uninit(alloc);
>      g_free(dev);
> +    generic_alloc_uninit(alloc);
>      test_end();
>  }
>  
> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index a343a6b..3e83685 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -12,12 +12,9 @@
>  #include "qemu-common.h"
>  #include "qemu/sockets.h"
>  #include "qemu/iov.h"
> -#include "libqos/pci-pc.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
> -#include "libqos/malloc-generic.h"
>  #include "qemu/bswap.h"
>  #include "hw/virtio/virtio-net.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -53,16 +50,12 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>      return dev;
>  }
>  
> -static QPCIBus *pci_test_start(int socket)
> +static QOSState *pci_test_start(int socket)
>  {
> -    char *cmdline;
> +    const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
> +                      "virtio-net-pci,netdev=hs0";
>  
> -    cmdline = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
> -                              "virtio-net-pci,netdev=hs0", socket);
> -    qtest_start(cmdline);
> -    g_free(cmdline);
> -
> -    return qpci_init_pc(NULL);
> +    return qtest_pc_boot(cmd, socket);
>  }
>  
>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
> @@ -205,9 +198,8 @@ static void stop_cont_test(const QVirtioBus *bus, QVirtioDevice *dev,
>  static void pci_basic(gconstpointer data)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *tx, *rx;
> -    QGuestAllocator *alloc;
>      void (*func) (const QVirtioBus *bus,
>                    QVirtioDevice *dev,
>                    QGuestAllocator *alloc,
> @@ -219,28 +211,26 @@ static void pci_basic(gconstpointer data)
>      ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
>      g_assert_cmpint(ret, !=, -1);
>  
> -    bus = pci_test_start(sv[1]);
> -    dev = virtio_net_pci_init(bus, PCI_SLOT);
> +    qs = pci_test_start(sv[1]);
> +    g_assert(qs);
> +    dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
>  
> -    alloc = pc_alloc_init();
>      rx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                           alloc, 0);
> +                                           qs->alloc, 0);
>      tx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                           alloc, 1);
> +                                           qs->alloc, 1);
>  
>      driver_init(&qvirtio_pci, &dev->vdev);
> -    func(&qvirtio_pci, &dev->vdev, alloc, &rx->vq, &tx->vq, sv[0]);
> +    func(&qvirtio_pci, &dev->vdev, qs->alloc, &rx->vq, &tx->vq, sv[0]);
>  
>      /* End test */
>      close(sv[0]);
> -    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, alloc);
> -    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, qs->alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, qs->alloc);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev->pdev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);
>  }
>  #endif
>  
> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
> index 79088bb..721ae1f 100644
> --- a/tests/virtio-scsi-test.c
> +++ b/tests/virtio-scsi-test.c
> @@ -11,12 +11,9 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "block/scsi.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
> -#include "libqos/pci-pc.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
> -#include "libqos/malloc-generic.h"
>  #include "standard-headers/linux/virtio_ids.h"
>  #include "standard-headers/linux/virtio_pci.h"
>  #include "standard-headers/linux/virtio_scsi.h"
> @@ -29,28 +26,23 @@
>  
>  typedef struct {
>      QVirtioDevice *dev;
> -    QGuestAllocator *alloc;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      int num_queues;
>      QVirtQueue *vq[MAX_NUM_QUEUES + 2];
>  } QVirtIOSCSI;
>  
> -static void qvirtio_scsi_start(const char *extra_opts)
> +static QOSState *qvirtio_scsi_start(const char *extra_opts)
>  {
> -    char *cmdline;
> -
> -    cmdline = g_strdup_printf(
> -                "-drive id=drv0,if=none,file=/dev/null,format=raw "
> -                "-device virtio-scsi-pci,id=vs0 "
> -                "-device scsi-hd,bus=vs0.0,drive=drv0 %s",
> -                extra_opts ? : "");
> -    qtest_start(cmdline);
> -    g_free(cmdline);
> +    const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw "
> +                      "-device virtio-scsi-pci,id=vs0 "
> +                      "-device scsi-hd,bus=vs0.0,drive=drv0 %s";
> +
> +    return qtest_pc_boot(cmd, extra_opts ? : "");
>  }
>  
> -static void qvirtio_scsi_stop(void)
> +static void qvirtio_scsi_stop(QOSState *qs)
>  {
> -    qtest_end();
> +    qtest_shutdown(qs);
>  }
>  
>  static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
> @@ -58,12 +50,10 @@ static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
>      int i;
>  
>      for (i = 0; i < vs->num_queues + 2; i++) {
> -        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->alloc);
> +        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->qs->alloc);
>      }
> -    pc_alloc_uninit(vs->alloc);
>      qvirtio_pci_device_disable(container_of(vs->dev, QVirtioPCIDevice, vdev));
>      g_free(vs->dev);
> -    qpci_free_pc(vs->bus);
>  }
>  
>  static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
> @@ -71,7 +61,7 @@ static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
>  {
>      uint64_t addr;
>  
> -    addr = guest_alloc(vs->alloc, alloc_size);
> +    addr = guest_alloc(vs->qs->alloc, alloc_size);
>      if (data) {
>          memwrite(addr, data, alloc_size);
>      }
> @@ -128,10 +118,10 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
>          memread(resp_addr, resp_out, sizeof(*resp_out));
>      }
>  
> -    guest_free(vs->alloc, req_addr);
> -    guest_free(vs->alloc, resp_addr);
> -    guest_free(vs->alloc, data_in_addr);
> -    guest_free(vs->alloc, data_out_addr);
> +    guest_free(vs->qs->alloc, req_addr);
> +    guest_free(vs->qs->alloc, resp_addr);
> +    guest_free(vs->qs->alloc, data_in_addr);
> +    guest_free(vs->qs->alloc, data_out_addr);
>      return response;
>  }
>  
> @@ -145,10 +135,12 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>      int i;
>  
>      vs = g_new0(QVirtIOSCSI, 1);
> -    vs->alloc = pc_alloc_init();
> -    vs->bus = qpci_init_pc(NULL);
>  
> -    dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
> +    vs->qs = qvirtio_scsi_start("-drive file=blkdebug::null-co://,"
> +                                "if=none,id=dr1,format=raw,file.align=4k "
> +                                "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
> +    g_assert(vs->qs);
> +    dev = qvirtio_pci_device_find(vs->qs->pcibus, VIRTIO_ID_SCSI);
>      vs->dev = (QVirtioDevice *)dev;
>      g_assert(dev != NULL);
>      g_assert_cmphex(vs->dev->device_type, ==, VIRTIO_ID_SCSI);
> @@ -165,7 +157,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>      g_assert_cmpint(vs->num_queues, <, MAX_NUM_QUEUES);
>  
>      for (i = 0; i < vs->num_queues + 2; i++) {
> -        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->alloc, i);
> +        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->qs->alloc, i);
>      }
>  
>      /* Clear the POWER ON OCCURRED unit attention */
> @@ -184,15 +176,20 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>  /* Tests only initialization so far. TODO: Replace with functional tests */
>  static void pci_nop(void)
>  {
> -    qvirtio_scsi_start(NULL);
> -    qvirtio_scsi_stop();
> +    QOSState *qs;
> +
> +    qs = qvirtio_scsi_start(NULL);
> +    g_assert(qs);
> +    qvirtio_scsi_stop(qs);
>  }
>  
>  static void hotplug(void)
>  {
>      QDict *response;
> +    QOSState *qs;
>  
> -    qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
> +    qs = qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
> +    g_assert(qs);
>      response = qmp("{\"execute\": \"device_add\","
>                     " \"arguments\": {"
>                     "   \"driver\": \"scsi-hd\","
> @@ -214,7 +211,7 @@ static void hotplug(void)
>      g_assert(qdict_haskey(response, "event"));
>      g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
>      QDECREF(response);
> -    qvirtio_scsi_stop();
> +    qvirtio_scsi_stop(qs);
>  }
>  
>  /* Test WRITE SAME with the lba not aligned */
> @@ -230,9 +227,6 @@ static void test_unaligned_write_same(void)
>          0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
>      };
>  
> -    qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
> -                       ",format=raw,file.align=4k "
> -                       "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
>      vs = qvirtio_scsi_pci_init(PCI_SLOT);
>  
>      g_assert_cmphex(0, ==,
> @@ -242,7 +236,7 @@ static void test_unaligned_write_same(void)
>          virtio_scsi_do_command(vs, write_same_cdb_2, NULL, 0, buf2, 512, NULL));
>  
>      qvirtio_scsi_pci_free(vs);
> -    qvirtio_scsi_stop();
> +    qvirtio_scsi_stop(vs->qs);
>  }
>  
>  int main(int argc, char **argv)

On 30/09/2016 03:27, David Gibson wrote:
> On Thu, Sep 29, 2016 at 07:15:05PM +0200, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> This could do with a commit message, even if it's just to say that
> this is supposed to be a refactor without behavioural change.

OK

> 
>> ---
>>  tests/virtio-9p-test.c   |  53 ++++++++--------
>>  tests/virtio-blk-test.c  | 154 +++++++++++++++++++++--------------------------
>>  tests/virtio-net-test.c  |  40 +++++-------
>>  tests/virtio-scsi-test.c |  70 ++++++++++-----------
>>  4 files changed, 140 insertions(+), 177 deletions(-)
>>
>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
>> index e8b2196..28d7f5b 100644
>> --- a/tests/virtio-9p-test.c
>> +++ b/tests/virtio-9p-test.c
>> @@ -10,62 +10,57 @@
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>>  #include "qemu-common.h"
>> -#include "libqos/pci-pc.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>>  #include "standard-headers/linux/virtio_pci.h"
>>  
>>  static const char mount_tag[] = "qtest";
>>  static char *test_share;
>>  
>> -static void qvirtio_9p_start(void)
>> -{
>> -    char *args;
>>  
>> +static QOSState *qvirtio_9p_start(void)
>> +{
>>      test_share = g_strdup("/tmp/qtest.XXXXXX");
>>      g_assert_nonnull(mkdtemp(test_share));
>> +    const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s "
>> +                      "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s";
>>  
>> -    args = g_strdup_printf("-fsdev local,id=fsdev0,security_model=none,path=%s "
>> -                           "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s",
>> -                           test_share, mount_tag);
>> -
>> -    qtest_start(args);
>> -    g_free(args);
>> +    return qtest_pc_boot(cmd, test_share, mount_tag);
>>  }
>>  
>> -static void qvirtio_9p_stop(void)
>> +static void qvirtio_9p_stop(QOSState *qs)
>>  {
>> -    qtest_end();
>> +    qtest_pc_shutdown(qs);
> 
> Shouldn't this be the generic shutdown call you added in the other series?

Yes, I have reordered my series and I have missed that.

> 
>>      rmdir(test_share);
>>      g_free(test_share);
>>  }
>>  
>>  static void pci_nop(void)
>>  {
>> -    qvirtio_9p_start();
>> -    qvirtio_9p_stop();
>> +    QOSState *qs;
>> +
>> +    qs = qvirtio_9p_start();
>> +    g_assert(qs);
>> +    qvirtio_9p_stop(qs);
>>  }
>>  
>>  typedef struct {
>>      QVirtioDevice *dev;
>> -    QGuestAllocator *alloc;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueue *vq;
>>  } QVirtIO9P;
>>  
>> -static QVirtIO9P *qvirtio_9p_pci_init(void)
>> +static QVirtIO9P *qvirtio_9p_pci_init(QOSState *qs)
>>  {
>>      QVirtIO9P *v9p;
>>      QVirtioPCIDevice *dev;
>>  
>>      v9p = g_new0(QVirtIO9P, 1);
>> -    v9p->alloc = pc_alloc_init();
>> -    v9p->bus = qpci_init_pc(NULL);
>>  
>> -    dev = qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P);
>> +    v9p->qs = qs;
>> +    dev = qvirtio_pci_device_find(v9p->qs->pcibus, VIRTIO_ID_9P);
>>      g_assert_nonnull(dev);
>>      g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_9P);
>>      v9p->dev = (QVirtioDevice *) dev;
>> @@ -75,17 +70,15 @@ static QVirtIO9P *qvirtio_9p_pci_init(void)
>>      qvirtio_set_acknowledge(&qvirtio_pci, v9p->dev);
>>      qvirtio_set_driver(&qvirtio_pci, v9p->dev);
>>  
>> -    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->alloc, 0);
>> +    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->qs->alloc, 0);
>>      return v9p;
>>  }
>>  
>>  static void qvirtio_9p_pci_free(QVirtIO9P *v9p)
>>  {
>> -    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->alloc);
>> -    pc_alloc_uninit(v9p->alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->qs->alloc);
>>      qvirtio_pci_device_disable(container_of(v9p->dev, QVirtioPCIDevice, vdev));
>>      g_free(v9p->dev);
>> -    qpci_free_pc(v9p->bus);
>>      g_free(v9p);
>>  }
>>  
>> @@ -96,9 +89,11 @@ static void pci_basic_config(void)
>>      size_t tag_len;
>>      char *tag;
>>      int i;
>> +    QOSState *qs;
>>  
>> -    qvirtio_9p_start();
>> -    v9p = qvirtio_9p_pci_init();
>> +    qs = qvirtio_9p_start();
>> +    g_assert(qs);
>> +    v9p = qvirtio_9p_pci_init(qs);
>>  
>>      addr = ((QVirtioPCIDevice *) v9p->dev)->addr + VIRTIO_PCI_CONFIG_OFF(false);
>>      tag_len = qvirtio_config_readw(&qvirtio_pci, v9p->dev,
>> @@ -115,7 +110,7 @@ static void pci_basic_config(void)
>>      g_free(tag);
>>  
>>      qvirtio_9p_pci_free(v9p);
>> -    qvirtio_9p_stop();
>> +    qvirtio_9p_stop(qs);
>>  }
>>  
>>  int main(int argc, char **argv)
>> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
>> index 3c4fecc..8cf62f6 100644
>> --- a/tests/virtio-blk-test.c
>> +++ b/tests/virtio-blk-test.c
>> @@ -10,12 +10,10 @@
>>  
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>>  #include "libqos/virtio-mmio.h"
>> -#include "libqos/pci-pc.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>>  #include "libqos/malloc-generic.h"
>>  #include "qemu/bswap.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>> @@ -58,24 +56,21 @@ static char *drive_create(void)
>>      return tmp_path;
>>  }
>>  
>> -static QPCIBus *pci_test_start(void)
>> +static QOSState *pci_test_start(void)
>>  {
>> -    char *cmdline;
>> +    QOSState *qs = NULL;
>>      char *tmp_path;
>> +    const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
>> +                      "-drive if=none,id=drive1,file=/dev/null,format=raw "
>> +                      "-device virtio-blk-pci,id=drv0,drive=drive0,"
>> +                      "addr=%x.%x";
>>  
>>      tmp_path = drive_create();
>>  
>> -    cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s,format=raw "
>> -                        "-drive if=none,id=drive1,file=/dev/null,format=raw "
>> -                        "-device virtio-blk-pci,id=drv0,drive=drive0,"
>> -                        "addr=%x.%x",
>> -                        tmp_path, PCI_SLOT, PCI_FN);
>> -    qtest_start(cmdline);
>> +    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
>>      unlink(tmp_path);
>>      g_free(tmp_path);
>> -    g_free(cmdline);
>> -
>> -    return qpci_init_pc(NULL);
>> +    return qs;
>>  }
>>  
>>  static void arm_test_start(void)
>> @@ -279,39 +274,35 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
>>  static void pci_basic(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>>      void *addr;
>>  
>> -    bus = pci_test_start();
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>  
>> -    alloc = pc_alloc_init();
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> +                                              qs->alloc, 0);
>>  
>>      /* MSI-X is not enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>>  
>> -    test_basic(&qvirtio_pci, &dev->vdev, alloc, &vqpci->vq,
>> +    test_basic(&qvirtio_pci, &dev->vdev, qs->alloc, &vqpci->vq,
>>                                                      (uint64_t)(uintptr_t)addr);
>>  
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
>>  }
>>  
>>  static void pci_indirect(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>> +    QOSState *qs;
>>      QVirtioBlkReq req;
>>      QVRingIndirectDesc *indirect;
>>      void *addr;
>> @@ -322,9 +313,10 @@ static void pci_indirect(void)
>>      uint8_t status;
>>      char *data;
>>  
>> -    bus = pci_test_start();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>  
>>      /* MSI-X is not enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>> @@ -340,9 +332,8 @@ static void pci_indirect(void)
>>                              (1u << VIRTIO_BLK_F_SCSI));
>>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>>  
>> -    alloc = pc_alloc_init();
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> +                                              qs->alloc, 0);
>>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>>  
>>      /* Write request */
>> @@ -352,11 +343,11 @@ static void pci_indirect(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
>> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>>      qvring_indirect_desc_add(indirect, req_addr, 528, false);
>>      qvring_indirect_desc_add(indirect, req_addr + 528, 1, true);
>>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
>> @@ -368,7 +359,7 @@ static void pci_indirect(void)
>>      g_assert_cmpint(status, ==, 0);
>>  
>>      g_free(indirect);
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* Read request */
>>      req.type = VIRTIO_BLK_T_IN;
>> @@ -377,11 +368,11 @@ static void pci_indirect(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
>> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>>      qvring_indirect_desc_add(indirect, req_addr, 16, false);
>>      qvring_indirect_desc_add(indirect, req_addr + 16, 513, true);
>>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
>> @@ -398,28 +389,27 @@ static void pci_indirect(void)
>>      g_free(data);
>>  
>>      g_free(indirect);
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
>>  }
>>  
>>  static void pci_config(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      int n_size = TEST_IMAGE_SIZE / 2;
>>      void *addr;
>>      uint64_t capacity;
>>  
>> -    bus = pci_test_start();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>  
>>      /* MSI-X is not enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>> @@ -440,16 +430,15 @@ static void pci_config(void)
>>  
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +
>> +    qtest_shutdown(qs);
>>  }
>>  
>>  static void pci_msix(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>>      QVirtioBlkReq req;
>>      int n_size = TEST_IMAGE_SIZE / 2;
>>      void *addr;
>> @@ -460,13 +449,13 @@ static void pci_msix(void)
>>      uint8_t status;
>>      char *data;
>>  
>> -    bus = pci_test_start();
>> -    alloc = pc_alloc_init();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>      qpci_msix_enable(dev->pdev);
>>  
>> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
>> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>>  
>>      /* MSI-X is enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
>> @@ -483,8 +472,8 @@ static void pci_msix(void)
>>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>>  
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
>> +                                              qs->alloc, 0);
>> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>>  
>>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>>  
>> @@ -504,7 +493,7 @@ static void pci_msix(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -519,7 +508,7 @@ static void pci_msix(void)
>>      status = readb(req_addr + 528);
>>      g_assert_cmpint(status, ==, 0);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* Read request */
>>      req.type = VIRTIO_BLK_T_IN;
>> @@ -527,7 +516,7 @@ static void pci_msix(void)
>>      req.sector = 0;
>>      req.data = g_malloc0(512);
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -549,24 +538,21 @@ static void pci_msix(void)
>>      g_assert_cmpstr(data, ==, "TEST");
>>      g_free(data);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qpci_msix_disable(dev->pdev);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
>>  }
>>  
>>  static void pci_idx(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>>      QVirtioBlkReq req;
>>      void *addr;
>>      uint64_t req_addr;
>> @@ -576,13 +562,13 @@ static void pci_idx(void)
>>      uint8_t status;
>>      char *data;
>>  
>> -    bus = pci_test_start();
>> -    alloc = pc_alloc_init();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>      qpci_msix_enable(dev->pdev);
>>  
>> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
>> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>>  
>>      /* MSI-X is enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
>> @@ -599,8 +585,8 @@ static void pci_idx(void)
>>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>>  
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
>> +                                              qs->alloc, 0);
>> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>>  
>>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>>  
>> @@ -611,7 +597,7 @@ static void pci_idx(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -630,7 +616,7 @@ static void pci_idx(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -647,7 +633,7 @@ static void pci_idx(void)
>>                                               QVIRTIO_BLK_TIMEOUT_US);
>>      g_assert_cmpint(status, ==, 0);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* Read request */
>>      req.type = VIRTIO_BLK_T_IN;
>> @@ -655,7 +641,7 @@ static void pci_idx(void)
>>      req.sector = 1;
>>      req.data = g_malloc0(512);
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -676,38 +662,36 @@ static void pci_idx(void)
>>      g_assert_cmpstr(data, ==, "TEST");
>>      g_free(data);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qpci_msix_disable(dev->pdev);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
>>  }
>>  
>>  static void pci_hotplug(void)
>>  {
>> -    QPCIBus *bus;
>>      QVirtioPCIDevice *dev;
>> +    QOSState *qs;
>>  
>> -    bus = pci_test_start();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
>>      /* plug secondary disk */
>>      qpci_plug_device_test("virtio-blk-pci", "drv1", PCI_SLOT_HP,
>>                            "'drive': 'drive1'");
>>  
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT_HP);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT_HP);
>>      g_assert(dev);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>>  
>>      /* unplug secondary disk */
>>      qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
>>  }
>>  
>>  static void mmio_basic(void)
>> @@ -746,8 +730,8 @@ static void mmio_basic(void)
>>  
>>      /* End test */
>>      qvirtqueue_cleanup(&qvirtio_mmio, vq, alloc);
>> -    generic_alloc_uninit(alloc);
>>      g_free(dev);
>> +    generic_alloc_uninit(alloc);
>>      test_end();
>>  }
>>  
>> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
>> index a343a6b..3e83685 100644
>> --- a/tests/virtio-net-test.c
>> +++ b/tests/virtio-net-test.c
>> @@ -12,12 +12,9 @@
>>  #include "qemu-common.h"
>>  #include "qemu/sockets.h"
>>  #include "qemu/iov.h"
>> -#include "libqos/pci-pc.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>> -#include "libqos/malloc-generic.h"
>>  #include "qemu/bswap.h"
>>  #include "hw/virtio/virtio-net.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>> @@ -53,16 +50,12 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>>      return dev;
>>  }
>>  
>> -static QPCIBus *pci_test_start(int socket)
>> +static QOSState *pci_test_start(int socket)
>>  {
>> -    char *cmdline;
>> +    const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
>> +                      "virtio-net-pci,netdev=hs0";
>>  
>> -    cmdline = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
>> -                              "virtio-net-pci,netdev=hs0", socket);
>> -    qtest_start(cmdline);
>> -    g_free(cmdline);
>> -
>> -    return qpci_init_pc(NULL);
>> +    return qtest_pc_boot(cmd, socket);
>>  }
>>  
>>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
>> @@ -205,9 +198,8 @@ static void stop_cont_test(const QVirtioBus *bus, QVirtioDevice *dev,
>>  static void pci_basic(gconstpointer data)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *tx, *rx;
>> -    QGuestAllocator *alloc;
>>      void (*func) (const QVirtioBus *bus,
>>                    QVirtioDevice *dev,
>>                    QGuestAllocator *alloc,
>> @@ -219,28 +211,26 @@ static void pci_basic(gconstpointer data)
>>      ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
>>      g_assert_cmpint(ret, !=, -1);
>>  
>> -    bus = pci_test_start(sv[1]);
>> -    dev = virtio_net_pci_init(bus, PCI_SLOT);
>> +    qs = pci_test_start(sv[1]);
>> +    g_assert(qs);
>> +    dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
>>  
>> -    alloc = pc_alloc_init();
>>      rx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                           alloc, 0);
>> +                                           qs->alloc, 0);
>>      tx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                           alloc, 1);
>> +                                           qs->alloc, 1);
>>  
>>      driver_init(&qvirtio_pci, &dev->vdev);
>> -    func(&qvirtio_pci, &dev->vdev, alloc, &rx->vq, &tx->vq, sv[0]);
>> +    func(&qvirtio_pci, &dev->vdev, qs->alloc, &rx->vq, &tx->vq, sv[0]);
>>  
>>      /* End test */
>>      close(sv[0]);
>> -    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, alloc);
>> -    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, qs->alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, qs->alloc);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev->pdev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
>>  }
>>  #endif
>>  
>> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
>> index 79088bb..721ae1f 100644
>> --- a/tests/virtio-scsi-test.c
>> +++ b/tests/virtio-scsi-test.c
>> @@ -11,12 +11,9 @@
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>>  #include "block/scsi.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>> -#include "libqos/pci-pc.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>> -#include "libqos/malloc-generic.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>>  #include "standard-headers/linux/virtio_pci.h"
>>  #include "standard-headers/linux/virtio_scsi.h"
>> @@ -29,28 +26,23 @@
>>  
>>  typedef struct {
>>      QVirtioDevice *dev;
>> -    QGuestAllocator *alloc;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      int num_queues;
>>      QVirtQueue *vq[MAX_NUM_QUEUES + 2];
>>  } QVirtIOSCSI;
>>  
>> -static void qvirtio_scsi_start(const char *extra_opts)
>> +static QOSState *qvirtio_scsi_start(const char *extra_opts)
>>  {
>> -    char *cmdline;
>> -
>> -    cmdline = g_strdup_printf(
>> -                "-drive id=drv0,if=none,file=/dev/null,format=raw "
>> -                "-device virtio-scsi-pci,id=vs0 "
>> -                "-device scsi-hd,bus=vs0.0,drive=drv0 %s",
>> -                extra_opts ? : "");
>> -    qtest_start(cmdline);
>> -    g_free(cmdline);
>> +    const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw "
>> +                      "-device virtio-scsi-pci,id=vs0 "
>> +                      "-device scsi-hd,bus=vs0.0,drive=drv0 %s";
>> +
>> +    return qtest_pc_boot(cmd, extra_opts ? : "");
>>  }
>>  
>> -static void qvirtio_scsi_stop(void)
>> +static void qvirtio_scsi_stop(QOSState *qs)
>>  {
>> -    qtest_end();
>> +    qtest_shutdown(qs);
>>  }
>>  
>>  static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
>> @@ -58,12 +50,10 @@ static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
>>      int i;
>>  
>>      for (i = 0; i < vs->num_queues + 2; i++) {
>> -        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->alloc);
>> +        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->qs->alloc);
>>      }
>> -    pc_alloc_uninit(vs->alloc);
>>      qvirtio_pci_device_disable(container_of(vs->dev, QVirtioPCIDevice, vdev));
>>      g_free(vs->dev);
>> -    qpci_free_pc(vs->bus);
>>  }
>>  
>>  static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
>> @@ -71,7 +61,7 @@ static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
>>  {
>>      uint64_t addr;
>>  
>> -    addr = guest_alloc(vs->alloc, alloc_size);
>> +    addr = guest_alloc(vs->qs->alloc, alloc_size);
>>      if (data) {
>>          memwrite(addr, data, alloc_size);
>>      }
>> @@ -128,10 +118,10 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
>>          memread(resp_addr, resp_out, sizeof(*resp_out));
>>      }
>>  
>> -    guest_free(vs->alloc, req_addr);
>> -    guest_free(vs->alloc, resp_addr);
>> -    guest_free(vs->alloc, data_in_addr);
>> -    guest_free(vs->alloc, data_out_addr);
>> +    guest_free(vs->qs->alloc, req_addr);
>> +    guest_free(vs->qs->alloc, resp_addr);
>> +    guest_free(vs->qs->alloc, data_in_addr);
>> +    guest_free(vs->qs->alloc, data_out_addr);
>>      return response;
>>  }
>>  
>> @@ -145,10 +135,12 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>>      int i;
>>  
>>      vs = g_new0(QVirtIOSCSI, 1);
>> -    vs->alloc = pc_alloc_init();
>> -    vs->bus = qpci_init_pc(NULL);
>>  
>> -    dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
>> +    vs->qs = qvirtio_scsi_start("-drive file=blkdebug::null-co://,"
>> +                                "if=none,id=dr1,format=raw,file.align=4k "
>> +                                "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
>> +    g_assert(vs->qs);
>> +    dev = qvirtio_pci_device_find(vs->qs->pcibus, VIRTIO_ID_SCSI);
>>      vs->dev = (QVirtioDevice *)dev;
>>      g_assert(dev != NULL);
>>      g_assert_cmphex(vs->dev->device_type, ==, VIRTIO_ID_SCSI);
>> @@ -165,7 +157,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>>      g_assert_cmpint(vs->num_queues, <, MAX_NUM_QUEUES);
>>  
>>      for (i = 0; i < vs->num_queues + 2; i++) {
>> -        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->alloc, i);
>> +        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->qs->alloc, i);
>>      }
>>  
>>      /* Clear the POWER ON OCCURRED unit attention */
>> @@ -184,15 +176,20 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>>  /* Tests only initialization so far. TODO: Replace with functional tests */
>>  static void pci_nop(void)
>>  {
>> -    qvirtio_scsi_start(NULL);
>> -    qvirtio_scsi_stop();
>> +    QOSState *qs;
>> +
>> +    qs = qvirtio_scsi_start(NULL);
>> +    g_assert(qs);
>> +    qvirtio_scsi_stop(qs);
>>  }
>>  
>>  static void hotplug(void)
>>  {
>>      QDict *response;
>> +    QOSState *qs;
>>  
>> -    qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
>> +    qs = qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
>> +    g_assert(qs);
>>      response = qmp("{\"execute\": \"device_add\","
>>                     " \"arguments\": {"
>>                     "   \"driver\": \"scsi-hd\","
>> @@ -214,7 +211,7 @@ static void hotplug(void)
>>      g_assert(qdict_haskey(response, "event"));
>>      g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
>>      QDECREF(response);
>> -    qvirtio_scsi_stop();
>> +    qvirtio_scsi_stop(qs);
>>  }
>>  
>>  /* Test WRITE SAME with the lba not aligned */
>> @@ -230,9 +227,6 @@ static void test_unaligned_write_same(void)
>>          0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
>>      };
>>  
>> -    qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
>> -                       ",format=raw,file.align=4k "
>> -                       "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
>>      vs = qvirtio_scsi_pci_init(PCI_SLOT);
>>  
>>      g_assert_cmphex(0, ==,
>> @@ -242,7 +236,7 @@ static void test_unaligned_write_same(void)
>>          virtio_scsi_do_command(vs, write_same_cdb_2, NULL, 0, buf2, 512, NULL));
>>  
>>      qvirtio_scsi_pci_free(vs);
>> -    qvirtio_scsi_stop();
>> +    qvirtio_scsi_stop(vs->qs);
>>  }
>>  
>>  int main(int argc, char **argv)
> 

Thanks,
Laurent

On Thu, 29 Sep 2016 19:15:05 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tests/virtio-9p-test.c   |  53 ++++++++--------
>  tests/virtio-blk-test.c  | 154 +++++++++++++++++++++--------------------------
>  tests/virtio-net-test.c  |  40 +++++-------
>  tests/virtio-scsi-test.c |  70 ++++++++++-----------
>  4 files changed, 140 insertions(+), 177 deletions(-)
> 

Hi Laurent,

Please find my comments below.

> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index e8b2196..28d7f5b 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -10,62 +10,57 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "qemu-common.h"
> -#include "libqos/pci-pc.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
>  #include "standard-headers/linux/virtio_ids.h"
>  #include "standard-headers/linux/virtio_pci.h"
>  
>  static const char mount_tag[] = "qtest";
>  static char *test_share;
>  
> -static void qvirtio_9p_start(void)
> -{
> -    char *args;
>  
> +static QOSState *qvirtio_9p_start(void)
> +{
>      test_share = g_strdup("/tmp/qtest.XXXXXX");
>      g_assert_nonnull(mkdtemp(test_share));
> +    const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s "
> +                      "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s";
>  
> -    args = g_strdup_printf("-fsdev local,id=fsdev0,security_model=none,path=%s "
> -                           "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s",
> -                           test_share, mount_tag);
> -
> -    qtest_start(args);
> -    g_free(args);
> +    return qtest_pc_boot(cmd, test_share, mount_tag);
>  }
>  
> -static void qvirtio_9p_stop(void)
> +static void qvirtio_9p_stop(QOSState *qs)
>  {
> -    qtest_end();
> +    qtest_pc_shutdown(qs);
>      rmdir(test_share);
>      g_free(test_share);
>  }
>  
>  static void pci_nop(void)
>  {
> -    qvirtio_9p_start();
> -    qvirtio_9p_stop();
> +    QOSState *qs;
> +
> +    qs = qvirtio_9p_start();
> +    g_assert(qs);

The appropriate macro to use here is: g_assert_nonnull().

BTW, how can qs be NULL ?

> +    qvirtio_9p_stop(qs);
>  }
>  
>  typedef struct {
>      QVirtioDevice *dev;
> -    QGuestAllocator *alloc;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueue *vq;
>  } QVirtIO9P;
>  
> -static QVirtIO9P *qvirtio_9p_pci_init(void)
> +static QVirtIO9P *qvirtio_9p_pci_init(QOSState *qs)
>  {
>      QVirtIO9P *v9p;
>      QVirtioPCIDevice *dev;
>  
>      v9p = g_new0(QVirtIO9P, 1);
> -    v9p->alloc = pc_alloc_init();
> -    v9p->bus = qpci_init_pc(NULL);
>  
> -    dev = qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P);
> +    v9p->qs = qs;
> +    dev = qvirtio_pci_device_find(v9p->qs->pcibus, VIRTIO_ID_9P);
>      g_assert_nonnull(dev);
>      g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_9P);
>      v9p->dev = (QVirtioDevice *) dev;
> @@ -75,17 +70,15 @@ static QVirtIO9P *qvirtio_9p_pci_init(void)
>      qvirtio_set_acknowledge(&qvirtio_pci, v9p->dev);
>      qvirtio_set_driver(&qvirtio_pci, v9p->dev);
>  
> -    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->alloc, 0);
> +    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->qs->alloc, 0);
>      return v9p;
>  }
>  
>  static void qvirtio_9p_pci_free(QVirtIO9P *v9p)
>  {
> -    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->alloc);
> -    pc_alloc_uninit(v9p->alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->qs->alloc);
>      qvirtio_pci_device_disable(container_of(v9p->dev, QVirtioPCIDevice, vdev));
>      g_free(v9p->dev);
> -    qpci_free_pc(v9p->bus);
>      g_free(v9p);
>  }
>  
> @@ -96,9 +89,11 @@ static void pci_basic_config(void)
>      size_t tag_len;
>      char *tag;
>      int i;
> +    QOSState *qs;
>  
> -    qvirtio_9p_start();
> -    v9p = qvirtio_9p_pci_init();
> +    qs = qvirtio_9p_start();
> +    g_assert(qs);

Null qs ?

> +    v9p = qvirtio_9p_pci_init(qs);
>  
>      addr = ((QVirtioPCIDevice *) v9p->dev)->addr + VIRTIO_PCI_CONFIG_OFF(false);
>      tag_len = qvirtio_config_readw(&qvirtio_pci, v9p->dev,
> @@ -115,7 +110,7 @@ static void pci_basic_config(void)
>      g_free(tag);
>  
>      qvirtio_9p_pci_free(v9p);
> -    qvirtio_9p_stop();
> +    qvirtio_9p_stop(qs);
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 3c4fecc..8cf62f6 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -10,12 +10,10 @@
>  
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "libqos/virtio-mmio.h"
> -#include "libqos/pci-pc.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
>  #include "libqos/malloc-generic.h"
>  #include "qemu/bswap.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -58,24 +56,21 @@ static char *drive_create(void)
>      return tmp_path;
>  }
>  
> -static QPCIBus *pci_test_start(void)
> +static QOSState *pci_test_start(void)
>  {
> -    char *cmdline;
> +    QOSState *qs = NULL;

Why setting qs to NULL ? It is necessarily set...

>      char *tmp_path;
> +    const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
> +                      "-drive if=none,id=drive1,file=/dev/null,format=raw "
> +                      "-device virtio-blk-pci,id=drv0,drive=drive0,"
> +                      "addr=%x.%x";
>  
>      tmp_path = drive_create();
>  
> -    cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s,format=raw "
> -                        "-drive if=none,id=drive1,file=/dev/null,format=raw "
> -                        "-device virtio-blk-pci,id=drv0,drive=drive0,"
> -                        "addr=%x.%x",
> -                        tmp_path, PCI_SLOT, PCI_FN);
> -    qtest_start(cmdline);
> +    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);

... here.

>      unlink(tmp_path);
>      g_free(tmp_path);
> -    g_free(cmdline);
> -
> -    return qpci_init_pc(NULL);
> +    return qs;
>  }
>  
>  static void arm_test_start(void)
> @@ -279,39 +274,35 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
>  static void pci_basic(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
>      void *addr;
>  
> -    bus = pci_test_start();
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    qs = pci_test_start();
> +    g_assert(qs);

Null qs ?

> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>  
> -    alloc = pc_alloc_init();
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> +                                              qs->alloc, 0);
>  
>      /* MSI-X is not enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>  
> -    test_basic(&qvirtio_pci, &dev->vdev, alloc, &vqpci->vq,
> +    test_basic(&qvirtio_pci, &dev->vdev, qs->alloc, &vqpci->vq,
>                                                      (uint64_t)(uintptr_t)addr);

Maybe worth to fix the funky indentation... this can be done globally in a
followup patch.

>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);

The title is "tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests"

Even if qtest_shutdown() happens to be equivalent to qtest_pc_shutdown() when
qtest_pc_boot() was called, I would rather stick to the title, and convert
all qtest_pc_shutdown() to qtest_shutdown() in patch 3/3... 

No big deal though, you may s/qtest_pc_shutdown/qtest_shutdown in the title
as well :)

>  }
>  
>  static void pci_indirect(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
> +    QOSState *qs;
>      QVirtioBlkReq req;
>      QVRingIndirectDesc *indirect;
>      void *addr;
> @@ -322,9 +313,10 @@ static void pci_indirect(void)
>      uint8_t status;
>      char *data;
>  
> -    bus = pci_test_start();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  

Same remark about qs being NULL.

> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>  
>      /* MSI-X is not enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
> @@ -340,9 +332,8 @@ static void pci_indirect(void)
>                              (1u << VIRTIO_BLK_F_SCSI));
>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>  
> -    alloc = pc_alloc_init();
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> +                                              qs->alloc, 0);
>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>  
>      /* Write request */
> @@ -352,11 +343,11 @@ static void pci_indirect(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>      qvring_indirect_desc_add(indirect, req_addr, 528, false);
>      qvring_indirect_desc_add(indirect, req_addr + 528, 1, true);
>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
> @@ -368,7 +359,7 @@ static void pci_indirect(void)
>      g_assert_cmpint(status, ==, 0);
>  
>      g_free(indirect);
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* Read request */
>      req.type = VIRTIO_BLK_T_IN;
> @@ -377,11 +368,11 @@ static void pci_indirect(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>      qvring_indirect_desc_add(indirect, req_addr, 16, false);
>      qvring_indirect_desc_add(indirect, req_addr + 16, 513, true);
>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
> @@ -398,28 +389,27 @@ static void pci_indirect(void)
>      g_free(data);
>  
>      g_free(indirect);
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);

qtest_pc_shutdown() ?

>  }
>  
>  static void pci_config(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      int n_size = TEST_IMAGE_SIZE / 2;
>      void *addr;
>      uint64_t capacity;
>  
> -    bus = pci_test_start();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  

Same remark about qs being NULL.

> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>  
>      /* MSI-X is not enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
> @@ -440,16 +430,15 @@ static void pci_config(void)
>  
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +
> +    qtest_shutdown(qs);

qtest_pc_shutdown() ?

>  }
>  
>  static void pci_msix(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
>      QVirtioBlkReq req;
>      int n_size = TEST_IMAGE_SIZE / 2;
>      void *addr;
> @@ -460,13 +449,13 @@ static void pci_msix(void)
>      uint8_t status;
>      char *data;
>  
> -    bus = pci_test_start();
> -    alloc = pc_alloc_init();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  

Null qs ?

> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>      qpci_msix_enable(dev->pdev);
>  
> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>  
>      /* MSI-X is enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
> @@ -483,8 +472,8 @@ static void pci_msix(void)
>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>  
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
> +                                              qs->alloc, 0);
> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>  
>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>  
> @@ -504,7 +493,7 @@ static void pci_msix(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -519,7 +508,7 @@ static void pci_msix(void)
>      status = readb(req_addr + 528);
>      g_assert_cmpint(status, ==, 0);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* Read request */
>      req.type = VIRTIO_BLK_T_IN;
> @@ -527,7 +516,7 @@ static void pci_msix(void)
>      req.sector = 0;
>      req.data = g_malloc0(512);
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -549,24 +538,21 @@ static void pci_msix(void)
>      g_assert_cmpstr(data, ==, "TEST");
>      g_free(data);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qpci_msix_disable(dev->pdev);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);

qtest_pc_shutdown() ?

>  }
>  
>  static void pci_idx(void)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *vqpci;
> -    QGuestAllocator *alloc;
>      QVirtioBlkReq req;
>      void *addr;
>      uint64_t req_addr;
> @@ -576,13 +562,13 @@ static void pci_idx(void)
>      uint8_t status;
>      char *data;
>  
> -    bus = pci_test_start();
> -    alloc = pc_alloc_init();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  

Null qs ?

> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>      qpci_msix_enable(dev->pdev);
>  
> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>  
>      /* MSI-X is enabled */
>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
> @@ -599,8 +585,8 @@ static void pci_idx(void)
>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>  
>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                                                    alloc, 0);
> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
> +                                              qs->alloc, 0);
> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>  
>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>  
> @@ -611,7 +597,7 @@ static void pci_idx(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -630,7 +616,7 @@ static void pci_idx(void)
>      req.data = g_malloc0(512);
>      strcpy(req.data, "TEST");
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -647,7 +633,7 @@ static void pci_idx(void)
>                                               QVIRTIO_BLK_TIMEOUT_US);
>      g_assert_cmpint(status, ==, 0);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* Read request */
>      req.type = VIRTIO_BLK_T_IN;
> @@ -655,7 +641,7 @@ static void pci_idx(void)
>      req.sector = 1;
>      req.data = g_malloc0(512);
>  
> -    req_addr = virtio_blk_request(alloc, &req, 512);
> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>  
>      g_free(req.data);
>  
> @@ -676,38 +662,36 @@ static void pci_idx(void)
>      g_assert_cmpstr(data, ==, "TEST");
>      g_free(data);
>  
> -    guest_free(alloc, req_addr);
> +    guest_free(qs->alloc, req_addr);
>  
>      /* End test */
> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>      qpci_msix_disable(dev->pdev);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);

qtest_pc_shutdown() ?

>  }
>  
>  static void pci_hotplug(void)
>  {
> -    QPCIBus *bus;
>      QVirtioPCIDevice *dev;
> +    QOSState *qs;
>  
> -    bus = pci_test_start();
> +    qs = pci_test_start();
> +    g_assert(qs);
>  
>      /* plug secondary disk */
>      qpci_plug_device_test("virtio-blk-pci", "drv1", PCI_SLOT_HP,
>                            "'drive': 'drive1'");
>  
> -    dev = virtio_blk_pci_init(bus, PCI_SLOT_HP);
> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT_HP);
>      g_assert(dev);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev);
>  
>      /* unplug secondary disk */
>      qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);

qtest_pc_shutdown() ?

>  }
>  
>  static void mmio_basic(void)
> @@ -746,8 +730,8 @@ static void mmio_basic(void)
>  
>      /* End test */
>      qvirtqueue_cleanup(&qvirtio_mmio, vq, alloc);
> -    generic_alloc_uninit(alloc);
>      g_free(dev);
> +    generic_alloc_uninit(alloc);
>      test_end();
>  }
>  
> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index a343a6b..3e83685 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -12,12 +12,9 @@
>  #include "qemu-common.h"
>  #include "qemu/sockets.h"
>  #include "qemu/iov.h"
> -#include "libqos/pci-pc.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
> -#include "libqos/malloc-generic.h"
>  #include "qemu/bswap.h"
>  #include "hw/virtio/virtio-net.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -53,16 +50,12 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>      return dev;
>  }
>  
> -static QPCIBus *pci_test_start(int socket)
> +static QOSState *pci_test_start(int socket)
>  {
> -    char *cmdline;
> +    const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
> +                      "virtio-net-pci,netdev=hs0";
>  
> -    cmdline = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
> -                              "virtio-net-pci,netdev=hs0", socket);
> -    qtest_start(cmdline);
> -    g_free(cmdline);
> -
> -    return qpci_init_pc(NULL);
> +    return qtest_pc_boot(cmd, socket);
>  }
>  
>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
> @@ -205,9 +198,8 @@ static void stop_cont_test(const QVirtioBus *bus, QVirtioDevice *dev,
>  static void pci_basic(gconstpointer data)
>  {
>      QVirtioPCIDevice *dev;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      QVirtQueuePCI *tx, *rx;
> -    QGuestAllocator *alloc;
>      void (*func) (const QVirtioBus *bus,
>                    QVirtioDevice *dev,
>                    QGuestAllocator *alloc,
> @@ -219,28 +211,26 @@ static void pci_basic(gconstpointer data)
>      ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
>      g_assert_cmpint(ret, !=, -1);
>  
> -    bus = pci_test_start(sv[1]);
> -    dev = virtio_net_pci_init(bus, PCI_SLOT);
> +    qs = pci_test_start(sv[1]);
> +    g_assert(qs);

Null qs ?

> +    dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
>  
> -    alloc = pc_alloc_init();
>      rx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                           alloc, 0);
> +                                           qs->alloc, 0);
>      tx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
> -                                           alloc, 1);
> +                                           qs->alloc, 1);
>  
>      driver_init(&qvirtio_pci, &dev->vdev);
> -    func(&qvirtio_pci, &dev->vdev, alloc, &rx->vq, &tx->vq, sv[0]);
> +    func(&qvirtio_pci, &dev->vdev, qs->alloc, &rx->vq, &tx->vq, sv[0]);
>  
>      /* End test */
>      close(sv[0]);
> -    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, alloc);
> -    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, alloc);
> -    pc_alloc_uninit(alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, qs->alloc);
> +    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, qs->alloc);
>      qvirtio_pci_device_disable(dev);
>      g_free(dev->pdev);
>      g_free(dev);
> -    qpci_free_pc(bus);
> -    test_end();
> +    qtest_shutdown(qs);

qtest_pc_shutdown() ?

>  }
>  #endif
>  
> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
> index 79088bb..721ae1f 100644
> --- a/tests/virtio-scsi-test.c
> +++ b/tests/virtio-scsi-test.c
> @@ -11,12 +11,9 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "block/scsi.h"
> +#include "libqos/libqos-pc.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
> -#include "libqos/pci-pc.h"
> -#include "libqos/malloc.h"
> -#include "libqos/malloc-pc.h"
> -#include "libqos/malloc-generic.h"
>  #include "standard-headers/linux/virtio_ids.h"
>  #include "standard-headers/linux/virtio_pci.h"
>  #include "standard-headers/linux/virtio_scsi.h"
> @@ -29,28 +26,23 @@
>  
>  typedef struct {
>      QVirtioDevice *dev;
> -    QGuestAllocator *alloc;
> -    QPCIBus *bus;
> +    QOSState *qs;
>      int num_queues;
>      QVirtQueue *vq[MAX_NUM_QUEUES + 2];
>  } QVirtIOSCSI;
>  
> -static void qvirtio_scsi_start(const char *extra_opts)
> +static QOSState *qvirtio_scsi_start(const char *extra_opts)
>  {
> -    char *cmdline;
> -
> -    cmdline = g_strdup_printf(
> -                "-drive id=drv0,if=none,file=/dev/null,format=raw "
> -                "-device virtio-scsi-pci,id=vs0 "
> -                "-device scsi-hd,bus=vs0.0,drive=drv0 %s",
> -                extra_opts ? : "");
> -    qtest_start(cmdline);
> -    g_free(cmdline);
> +    const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw "
> +                      "-device virtio-scsi-pci,id=vs0 "
> +                      "-device scsi-hd,bus=vs0.0,drive=drv0 %s";
> +
> +    return qtest_pc_boot(cmd, extra_opts ? : "");
>  }
>  
> -static void qvirtio_scsi_stop(void)
> +static void qvirtio_scsi_stop(QOSState *qs)
>  {
> -    qtest_end();
> +    qtest_shutdown(qs);

qtest_pc_shutdown() ?

>  }
>  
>  static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
> @@ -58,12 +50,10 @@ static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
>      int i;
>  
>      for (i = 0; i < vs->num_queues + 2; i++) {
> -        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->alloc);
> +        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->qs->alloc);
>      }
> -    pc_alloc_uninit(vs->alloc);
>      qvirtio_pci_device_disable(container_of(vs->dev, QVirtioPCIDevice, vdev));
>      g_free(vs->dev);
> -    qpci_free_pc(vs->bus);
>  }
>  
>  static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
> @@ -71,7 +61,7 @@ static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
>  {
>      uint64_t addr;
>  
> -    addr = guest_alloc(vs->alloc, alloc_size);
> +    addr = guest_alloc(vs->qs->alloc, alloc_size);
>      if (data) {
>          memwrite(addr, data, alloc_size);
>      }
> @@ -128,10 +118,10 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
>          memread(resp_addr, resp_out, sizeof(*resp_out));
>      }
>  
> -    guest_free(vs->alloc, req_addr);
> -    guest_free(vs->alloc, resp_addr);
> -    guest_free(vs->alloc, data_in_addr);
> -    guest_free(vs->alloc, data_out_addr);
> +    guest_free(vs->qs->alloc, req_addr);
> +    guest_free(vs->qs->alloc, resp_addr);
> +    guest_free(vs->qs->alloc, data_in_addr);
> +    guest_free(vs->qs->alloc, data_out_addr);
>      return response;
>  }
>  
> @@ -145,10 +135,12 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>      int i;
>  
>      vs = g_new0(QVirtIOSCSI, 1);
> -    vs->alloc = pc_alloc_init();
> -    vs->bus = qpci_init_pc(NULL);
>  
> -    dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
> +    vs->qs = qvirtio_scsi_start("-drive file=blkdebug::null-co://,"
> +                                "if=none,id=dr1,format=raw,file.align=4k "
> +                                "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
> +    g_assert(vs->qs);

Null vs->qs ?

> +    dev = qvirtio_pci_device_find(vs->qs->pcibus, VIRTIO_ID_SCSI);
>      vs->dev = (QVirtioDevice *)dev;
>      g_assert(dev != NULL);
>      g_assert_cmphex(vs->dev->device_type, ==, VIRTIO_ID_SCSI);
> @@ -165,7 +157,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>      g_assert_cmpint(vs->num_queues, <, MAX_NUM_QUEUES);
>  
>      for (i = 0; i < vs->num_queues + 2; i++) {
> -        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->alloc, i);
> +        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->qs->alloc, i);
>      }
>  
>      /* Clear the POWER ON OCCURRED unit attention */
> @@ -184,15 +176,20 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>  /* Tests only initialization so far. TODO: Replace with functional tests */
>  static void pci_nop(void)
>  {
> -    qvirtio_scsi_start(NULL);
> -    qvirtio_scsi_stop();
> +    QOSState *qs;
> +
> +    qs = qvirtio_scsi_start(NULL);
> +    g_assert(qs);

Null qs ?

> +    qvirtio_scsi_stop(qs);
>  }
>  
>  static void hotplug(void)
>  {
>      QDict *response;
> +    QOSState *qs;
>  
> -    qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
> +    qs = qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
> +    g_assert(qs);

Null qs ?

>      response = qmp("{\"execute\": \"device_add\","
>                     " \"arguments\": {"
>                     "   \"driver\": \"scsi-hd\","
> @@ -214,7 +211,7 @@ static void hotplug(void)
>      g_assert(qdict_haskey(response, "event"));
>      g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
>      QDECREF(response);
> -    qvirtio_scsi_stop();
> +    qvirtio_scsi_stop(qs);
>  }
>  
>  /* Test WRITE SAME with the lba not aligned */
> @@ -230,9 +227,6 @@ static void test_unaligned_write_same(void)
>          0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
>      };
>  
> -    qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
> -                       ",format=raw,file.align=4k "
> -                       "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
>      vs = qvirtio_scsi_pci_init(PCI_SLOT);
>  
>      g_assert_cmphex(0, ==,
> @@ -242,7 +236,7 @@ static void test_unaligned_write_same(void)
>          virtio_scsi_do_command(vs, write_same_cdb_2, NULL, 0, buf2, 512, NULL));
>  
>      qvirtio_scsi_pci_free(vs);
> -    qvirtio_scsi_stop();
> +    qvirtio_scsi_stop(vs->qs);

Is still vs->qs still valid ? Also it looks wrong to call qvirtio_scsi_stop()
without any prior call to qvirtio_scsi_start()...

>  }
>  
>  int main(int argc, char **argv)

Cheers.

--
Greg

On 30/09/2016 10:33, Greg Kurz wrote:
> On Thu, 29 Sep 2016 19:15:05 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  tests/virtio-9p-test.c   |  53 ++++++++--------
>>  tests/virtio-blk-test.c  | 154 +++++++++++++++++++++--------------------------
>>  tests/virtio-net-test.c  |  40 +++++-------
>>  tests/virtio-scsi-test.c |  70 ++++++++++-----------
>>  4 files changed, 140 insertions(+), 177 deletions(-)
>>
> 
> Hi Laurent,
> 
> Please find my comments below.
> 
>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
>> index e8b2196..28d7f5b 100644
>> --- a/tests/virtio-9p-test.c
>> +++ b/tests/virtio-9p-test.c
>> @@ -10,62 +10,57 @@
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>>  #include "qemu-common.h"
>> -#include "libqos/pci-pc.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>>  #include "standard-headers/linux/virtio_pci.h"
>>  
>>  static const char mount_tag[] = "qtest";
>>  static char *test_share;
>>  
>> -static void qvirtio_9p_start(void)
>> -{
>> -    char *args;
>>  
>> +static QOSState *qvirtio_9p_start(void)
>> +{
>>      test_share = g_strdup("/tmp/qtest.XXXXXX");
>>      g_assert_nonnull(mkdtemp(test_share));
>> +    const char *cmd = "-fsdev local,id=fsdev0,security_model=none,path=%s "
>> +                      "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s";
>>  
>> -    args = g_strdup_printf("-fsdev local,id=fsdev0,security_model=none,path=%s "
>> -                           "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s",
>> -                           test_share, mount_tag);
>> -
>> -    qtest_start(args);
>> -    g_free(args);
>> +    return qtest_pc_boot(cmd, test_share, mount_tag);
>>  }
>>  
>> -static void qvirtio_9p_stop(void)
>> +static void qvirtio_9p_stop(QOSState *qs)
>>  {
>> -    qtest_end();
>> +    qtest_pc_shutdown(qs);
>>      rmdir(test_share);
>>      g_free(test_share);
>>  }
>>  
>>  static void pci_nop(void)
>>  {
>> -    qvirtio_9p_start();
>> -    qvirtio_9p_stop();
>> +    QOSState *qs;
>> +
>> +    qs = qvirtio_9p_start();
>> +    g_assert(qs);
> 
> The appropriate macro to use here is: g_assert_nonnull().

OK

> 
> BTW, how can qs be NULL ?

we should not know what happens in  qtest_pc_boot() (or
qtest_spapr_boot(), or qtest_XXX_boot())

So I think it i better to check it before to use it.

>> +    qvirtio_9p_stop(qs);
>>  }
>>  
>>  typedef struct {
>>      QVirtioDevice *dev;
>> -    QGuestAllocator *alloc;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueue *vq;
>>  } QVirtIO9P;
>>  
>> -static QVirtIO9P *qvirtio_9p_pci_init(void)
>> +static QVirtIO9P *qvirtio_9p_pci_init(QOSState *qs)
>>  {
>>      QVirtIO9P *v9p;
>>      QVirtioPCIDevice *dev;
>>  
>>      v9p = g_new0(QVirtIO9P, 1);
>> -    v9p->alloc = pc_alloc_init();
>> -    v9p->bus = qpci_init_pc(NULL);
>>  
>> -    dev = qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P);
>> +    v9p->qs = qs;
>> +    dev = qvirtio_pci_device_find(v9p->qs->pcibus, VIRTIO_ID_9P);
>>      g_assert_nonnull(dev);
>>      g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_9P);
>>      v9p->dev = (QVirtioDevice *) dev;
>> @@ -75,17 +70,15 @@ static QVirtIO9P *qvirtio_9p_pci_init(void)
>>      qvirtio_set_acknowledge(&qvirtio_pci, v9p->dev);
>>      qvirtio_set_driver(&qvirtio_pci, v9p->dev);
>>  
>> -    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->alloc, 0);
>> +    v9p->vq = qvirtqueue_setup(&qvirtio_pci, v9p->dev, v9p->qs->alloc, 0);
>>      return v9p;
>>  }
>>  
>>  static void qvirtio_9p_pci_free(QVirtIO9P *v9p)
>>  {
>> -    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->alloc);
>> -    pc_alloc_uninit(v9p->alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->qs->alloc);
>>      qvirtio_pci_device_disable(container_of(v9p->dev, QVirtioPCIDevice, vdev));
>>      g_free(v9p->dev);
>> -    qpci_free_pc(v9p->bus);
>>      g_free(v9p);
>>  }
>>  
>> @@ -96,9 +89,11 @@ static void pci_basic_config(void)
>>      size_t tag_len;
>>      char *tag;
>>      int i;
>> +    QOSState *qs;
>>  
>> -    qvirtio_9p_start();
>> -    v9p = qvirtio_9p_pci_init();
>> +    qs = qvirtio_9p_start();
>> +    g_assert(qs);
> 
> Null qs ?
> 
>> +    v9p = qvirtio_9p_pci_init(qs);
>>  
>>      addr = ((QVirtioPCIDevice *) v9p->dev)->addr + VIRTIO_PCI_CONFIG_OFF(false);
>>      tag_len = qvirtio_config_readw(&qvirtio_pci, v9p->dev,
>> @@ -115,7 +110,7 @@ static void pci_basic_config(void)
>>      g_free(tag);
>>  
>>      qvirtio_9p_pci_free(v9p);
>> -    qvirtio_9p_stop();
>> +    qvirtio_9p_stop(qs);
>>  }
>>  
>>  int main(int argc, char **argv)
>> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
>> index 3c4fecc..8cf62f6 100644
>> --- a/tests/virtio-blk-test.c
>> +++ b/tests/virtio-blk-test.c
>> @@ -10,12 +10,10 @@
>>  
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>>  #include "libqos/virtio-mmio.h"
>> -#include "libqos/pci-pc.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>>  #include "libqos/malloc-generic.h"
>>  #include "qemu/bswap.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>> @@ -58,24 +56,21 @@ static char *drive_create(void)
>>      return tmp_path;
>>  }
>>  
>> -static QPCIBus *pci_test_start(void)
>> +static QOSState *pci_test_start(void)
>>  {
>> -    char *cmdline;
>> +    QOSState *qs = NULL;
> 
> Why setting qs to NULL ? It is necessarily set...

Yes, I've mixed my patches: later we add a "if (arch == pc) { qs = }
else if (arch == spapr) { qs = }" and this case qs can be uninitialized.

>>      char *tmp_path;
>> +    const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
>> +                      "-drive if=none,id=drive1,file=/dev/null,format=raw "
>> +                      "-device virtio-blk-pci,id=drv0,drive=drive0,"
>> +                      "addr=%x.%x";
>>  
>>      tmp_path = drive_create();
>>  
>> -    cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s,format=raw "
>> -                        "-drive if=none,id=drive1,file=/dev/null,format=raw "
>> -                        "-device virtio-blk-pci,id=drv0,drive=drive0,"
>> -                        "addr=%x.%x",
>> -                        tmp_path, PCI_SLOT, PCI_FN);
>> -    qtest_start(cmdline);
>> +    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
> 
> ... here.
> 
>>      unlink(tmp_path);
>>      g_free(tmp_path);
>> -    g_free(cmdline);
>> -
>> -    return qpci_init_pc(NULL);
>> +    return qs;
>>  }
>>  
>>  static void arm_test_start(void)
>> @@ -279,39 +274,35 @@ static void test_basic(const QVirtioBus *bus, QVirtioDevice *dev,
>>  static void pci_basic(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>>      void *addr;
>>  
>> -    bus = pci_test_start();
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    qs = pci_test_start();
>> +    g_assert(qs);
> 
> Null qs ?
> 
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>  
>> -    alloc = pc_alloc_init();
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> +                                              qs->alloc, 0);
>>  
>>      /* MSI-X is not enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>>  
>> -    test_basic(&qvirtio_pci, &dev->vdev, alloc, &vqpci->vq,
>> +    test_basic(&qvirtio_pci, &dev->vdev, qs->alloc, &vqpci->vq,
>>                                                      (uint64_t)(uintptr_t)addr);
> 
> Maybe worth to fix the funky indentation... this can be done globally in a
> followup patch.

I will resend, so I will fix this

> 
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
> 
> The title is "tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests"
> 
> Even if qtest_shutdown() happens to be equivalent to qtest_pc_shutdown() when
> qtest_pc_boot() was called, I would rather stick to the title, and convert
> all qtest_pc_shutdown() to qtest_shutdown() in patch 3/3... 
> 
> No big deal though, you may s/qtest_pc_shutdown/qtest_shutdown in the title
> as well :)

I'm to change all qtest_pc_shutdown() to qtest_shutdown() here

>>  }
>>  
>>  static void pci_indirect(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>> +    QOSState *qs;
>>      QVirtioBlkReq req;
>>      QVRingIndirectDesc *indirect;
>>      void *addr;
>> @@ -322,9 +313,10 @@ static void pci_indirect(void)
>>      uint8_t status;
>>      char *data;
>>  
>> -    bus = pci_test_start();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
> 
> Same remark about qs being NULL.
> 
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>  
>>      /* MSI-X is not enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>> @@ -340,9 +332,8 @@ static void pci_indirect(void)
>>                              (1u << VIRTIO_BLK_F_SCSI));
>>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>>  
>> -    alloc = pc_alloc_init();
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> +                                              qs->alloc, 0);
>>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>>  
>>      /* Write request */
>> @@ -352,11 +343,11 @@ static void pci_indirect(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
>> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>>      qvring_indirect_desc_add(indirect, req_addr, 528, false);
>>      qvring_indirect_desc_add(indirect, req_addr + 528, 1, true);
>>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
>> @@ -368,7 +359,7 @@ static void pci_indirect(void)
>>      g_assert_cmpint(status, ==, 0);
>>  
>>      g_free(indirect);
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* Read request */
>>      req.type = VIRTIO_BLK_T_IN;
>> @@ -377,11 +368,11 @@ static void pci_indirect(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> -    indirect = qvring_indirect_desc_setup(&dev->vdev, alloc, 2);
>> +    indirect = qvring_indirect_desc_setup(&dev->vdev, qs->alloc, 2);
>>      qvring_indirect_desc_add(indirect, req_addr, 16, false);
>>      qvring_indirect_desc_add(indirect, req_addr + 16, 513, true);
>>      free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
>> @@ -398,28 +389,27 @@ static void pci_indirect(void)
>>      g_free(data);
>>  
>>      g_free(indirect);
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
> 
> qtest_pc_shutdown() ?

No, as we have qtest_shutdown() from a previous series, we can use it now.

> 
>>  }
>>  
>>  static void pci_config(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      int n_size = TEST_IMAGE_SIZE / 2;
>>      void *addr;
>>      uint64_t capacity;
>>  
>> -    bus = pci_test_start();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
> 
> Same remark about qs being NULL.
> 
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>  
>>      /* MSI-X is not enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(false);
>> @@ -440,16 +430,15 @@ static void pci_config(void)
>>  
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +
>> +    qtest_shutdown(qs);
> 
> qtest_pc_shutdown() ?
> 
>>  }
>>  
>>  static void pci_msix(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>>      QVirtioBlkReq req;
>>      int n_size = TEST_IMAGE_SIZE / 2;
>>      void *addr;
>> @@ -460,13 +449,13 @@ static void pci_msix(void)
>>      uint8_t status;
>>      char *data;
>>  
>> -    bus = pci_test_start();
>> -    alloc = pc_alloc_init();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
> 
> Null qs ?
> 
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>      qpci_msix_enable(dev->pdev);
>>  
>> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
>> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>>  
>>      /* MSI-X is enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
>> @@ -483,8 +472,8 @@ static void pci_msix(void)
>>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>>  
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
>> +                                              qs->alloc, 0);
>> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>>  
>>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>>  
>> @@ -504,7 +493,7 @@ static void pci_msix(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -519,7 +508,7 @@ static void pci_msix(void)
>>      status = readb(req_addr + 528);
>>      g_assert_cmpint(status, ==, 0);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* Read request */
>>      req.type = VIRTIO_BLK_T_IN;
>> @@ -527,7 +516,7 @@ static void pci_msix(void)
>>      req.sector = 0;
>>      req.data = g_malloc0(512);
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -549,24 +538,21 @@ static void pci_msix(void)
>>      g_assert_cmpstr(data, ==, "TEST");
>>      g_free(data);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qpci_msix_disable(dev->pdev);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
> 
> qtest_pc_shutdown() ?
> 
>>  }
>>  
>>  static void pci_idx(void)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *vqpci;
>> -    QGuestAllocator *alloc;
>>      QVirtioBlkReq req;
>>      void *addr;
>>      uint64_t req_addr;
>> @@ -576,13 +562,13 @@ static void pci_idx(void)
>>      uint8_t status;
>>      char *data;
>>  
>> -    bus = pci_test_start();
>> -    alloc = pc_alloc_init();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
> 
> Null qs ?
> 
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT);
>>      qpci_msix_enable(dev->pdev);
>>  
>> -    qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0);
>> +    qvirtio_pci_set_msix_configuration_vector(dev, qs->alloc, 0);
>>  
>>      /* MSI-X is enabled */
>>      addr = dev->addr + VIRTIO_PCI_CONFIG_OFF(true);
>> @@ -599,8 +585,8 @@ static void pci_idx(void)
>>      qvirtio_set_features(&qvirtio_pci, &dev->vdev, features);
>>  
>>      vqpci = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                                                    alloc, 0);
>> -    qvirtqueue_pci_msix_setup(dev, vqpci, alloc, 1);
>> +                                              qs->alloc, 0);
>> +    qvirtqueue_pci_msix_setup(dev, vqpci, qs->alloc, 1);
>>  
>>      qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
>>  
>> @@ -611,7 +597,7 @@ static void pci_idx(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -630,7 +616,7 @@ static void pci_idx(void)
>>      req.data = g_malloc0(512);
>>      strcpy(req.data, "TEST");
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -647,7 +633,7 @@ static void pci_idx(void)
>>                                               QVIRTIO_BLK_TIMEOUT_US);
>>      g_assert_cmpint(status, ==, 0);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* Read request */
>>      req.type = VIRTIO_BLK_T_IN;
>> @@ -655,7 +641,7 @@ static void pci_idx(void)
>>      req.sector = 1;
>>      req.data = g_malloc0(512);
>>  
>> -    req_addr = virtio_blk_request(alloc, &req, 512);
>> +    req_addr = virtio_blk_request(qs->alloc, &req, 512);
>>  
>>      g_free(req.data);
>>  
>> @@ -676,38 +662,36 @@ static void pci_idx(void)
>>      g_assert_cmpstr(data, ==, "TEST");
>>      g_free(data);
>>  
>> -    guest_free(alloc, req_addr);
>> +    guest_free(qs->alloc, req_addr);
>>  
>>      /* End test */
>> -    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &vqpci->vq, qs->alloc);
>>      qpci_msix_disable(dev->pdev);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
> 
> qtest_pc_shutdown() ?
> 
>>  }
>>  
>>  static void pci_hotplug(void)
>>  {
>> -    QPCIBus *bus;
>>      QVirtioPCIDevice *dev;
>> +    QOSState *qs;
>>  
>> -    bus = pci_test_start();
>> +    qs = pci_test_start();
>> +    g_assert(qs);
>>  
>>      /* plug secondary disk */
>>      qpci_plug_device_test("virtio-blk-pci", "drv1", PCI_SLOT_HP,
>>                            "'drive': 'drive1'");
>>  
>> -    dev = virtio_blk_pci_init(bus, PCI_SLOT_HP);
>> +    dev = virtio_blk_pci_init(qs->pcibus, PCI_SLOT_HP);
>>      g_assert(dev);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev);
>>  
>>      /* unplug secondary disk */
>>      qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
> 
> qtest_pc_shutdown() ?
> 
>>  }
>>  
>>  static void mmio_basic(void)
>> @@ -746,8 +730,8 @@ static void mmio_basic(void)
>>  
>>      /* End test */
>>      qvirtqueue_cleanup(&qvirtio_mmio, vq, alloc);
>> -    generic_alloc_uninit(alloc);
>>      g_free(dev);
>> +    generic_alloc_uninit(alloc);
>>      test_end();
>>  }
>>  
>> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
>> index a343a6b..3e83685 100644
>> --- a/tests/virtio-net-test.c
>> +++ b/tests/virtio-net-test.c
>> @@ -12,12 +12,9 @@
>>  #include "qemu-common.h"
>>  #include "qemu/sockets.h"
>>  #include "qemu/iov.h"
>> -#include "libqos/pci-pc.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>> -#include "libqos/malloc-generic.h"
>>  #include "qemu/bswap.h"
>>  #include "hw/virtio/virtio-net.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>> @@ -53,16 +50,12 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>>      return dev;
>>  }
>>  
>> -static QPCIBus *pci_test_start(int socket)
>> +static QOSState *pci_test_start(int socket)
>>  {
>> -    char *cmdline;
>> +    const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
>> +                      "virtio-net-pci,netdev=hs0";
>>  
>> -    cmdline = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
>> -                              "virtio-net-pci,netdev=hs0", socket);
>> -    qtest_start(cmdline);
>> -    g_free(cmdline);
>> -
>> -    return qpci_init_pc(NULL);
>> +    return qtest_pc_boot(cmd, socket);
>>  }
>>  
>>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
>> @@ -205,9 +198,8 @@ static void stop_cont_test(const QVirtioBus *bus, QVirtioDevice *dev,
>>  static void pci_basic(gconstpointer data)
>>  {
>>      QVirtioPCIDevice *dev;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      QVirtQueuePCI *tx, *rx;
>> -    QGuestAllocator *alloc;
>>      void (*func) (const QVirtioBus *bus,
>>                    QVirtioDevice *dev,
>>                    QGuestAllocator *alloc,
>> @@ -219,28 +211,26 @@ static void pci_basic(gconstpointer data)
>>      ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
>>      g_assert_cmpint(ret, !=, -1);
>>  
>> -    bus = pci_test_start(sv[1]);
>> -    dev = virtio_net_pci_init(bus, PCI_SLOT);
>> +    qs = pci_test_start(sv[1]);
>> +    g_assert(qs);
> 
> Null qs ?
> 
>> +    dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
>>  
>> -    alloc = pc_alloc_init();
>>      rx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                           alloc, 0);
>> +                                           qs->alloc, 0);
>>      tx = (QVirtQueuePCI *)qvirtqueue_setup(&qvirtio_pci, &dev->vdev,
>> -                                           alloc, 1);
>> +                                           qs->alloc, 1);
>>  
>>      driver_init(&qvirtio_pci, &dev->vdev);
>> -    func(&qvirtio_pci, &dev->vdev, alloc, &rx->vq, &tx->vq, sv[0]);
>> +    func(&qvirtio_pci, &dev->vdev, qs->alloc, &rx->vq, &tx->vq, sv[0]);
>>  
>>      /* End test */
>>      close(sv[0]);
>> -    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, alloc);
>> -    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, alloc);
>> -    pc_alloc_uninit(alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &tx->vq, qs->alloc);
>> +    qvirtqueue_cleanup(&qvirtio_pci, &rx->vq, qs->alloc);
>>      qvirtio_pci_device_disable(dev);
>>      g_free(dev->pdev);
>>      g_free(dev);
>> -    qpci_free_pc(bus);
>> -    test_end();
>> +    qtest_shutdown(qs);
> 
> qtest_pc_shutdown() ?
> 
>>  }
>>  #endif
>>  
>> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
>> index 79088bb..721ae1f 100644
>> --- a/tests/virtio-scsi-test.c
>> +++ b/tests/virtio-scsi-test.c
>> @@ -11,12 +11,9 @@
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>>  #include "block/scsi.h"
>> +#include "libqos/libqos-pc.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>> -#include "libqos/pci-pc.h"
>> -#include "libqos/malloc.h"
>> -#include "libqos/malloc-pc.h"
>> -#include "libqos/malloc-generic.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>>  #include "standard-headers/linux/virtio_pci.h"
>>  #include "standard-headers/linux/virtio_scsi.h"
>> @@ -29,28 +26,23 @@
>>  
>>  typedef struct {
>>      QVirtioDevice *dev;
>> -    QGuestAllocator *alloc;
>> -    QPCIBus *bus;
>> +    QOSState *qs;
>>      int num_queues;
>>      QVirtQueue *vq[MAX_NUM_QUEUES + 2];
>>  } QVirtIOSCSI;
>>  
>> -static void qvirtio_scsi_start(const char *extra_opts)
>> +static QOSState *qvirtio_scsi_start(const char *extra_opts)
>>  {
>> -    char *cmdline;
>> -
>> -    cmdline = g_strdup_printf(
>> -                "-drive id=drv0,if=none,file=/dev/null,format=raw "
>> -                "-device virtio-scsi-pci,id=vs0 "
>> -                "-device scsi-hd,bus=vs0.0,drive=drv0 %s",
>> -                extra_opts ? : "");
>> -    qtest_start(cmdline);
>> -    g_free(cmdline);
>> +    const char *cmd = "-drive id=drv0,if=none,file=/dev/null,format=raw "
>> +                      "-device virtio-scsi-pci,id=vs0 "
>> +                      "-device scsi-hd,bus=vs0.0,drive=drv0 %s";
>> +
>> +    return qtest_pc_boot(cmd, extra_opts ? : "");
>>  }
>>  
>> -static void qvirtio_scsi_stop(void)
>> +static void qvirtio_scsi_stop(QOSState *qs)
>>  {
>> -    qtest_end();
>> +    qtest_shutdown(qs);
> 
> qtest_pc_shutdown() ?
> 
>>  }
>>  
>>  static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
>> @@ -58,12 +50,10 @@ static void qvirtio_scsi_pci_free(QVirtIOSCSI *vs)
>>      int i;
>>  
>>      for (i = 0; i < vs->num_queues + 2; i++) {
>> -        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->alloc);
>> +        qvirtqueue_cleanup(&qvirtio_pci, vs->vq[i], vs->qs->alloc);
>>      }
>> -    pc_alloc_uninit(vs->alloc);
>>      qvirtio_pci_device_disable(container_of(vs->dev, QVirtioPCIDevice, vdev));
>>      g_free(vs->dev);
>> -    qpci_free_pc(vs->bus);
>>  }
>>  
>>  static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
>> @@ -71,7 +61,7 @@ static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
>>  {
>>      uint64_t addr;
>>  
>> -    addr = guest_alloc(vs->alloc, alloc_size);
>> +    addr = guest_alloc(vs->qs->alloc, alloc_size);
>>      if (data) {
>>          memwrite(addr, data, alloc_size);
>>      }
>> @@ -128,10 +118,10 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, const uint8_t *cdb,
>>          memread(resp_addr, resp_out, sizeof(*resp_out));
>>      }
>>  
>> -    guest_free(vs->alloc, req_addr);
>> -    guest_free(vs->alloc, resp_addr);
>> -    guest_free(vs->alloc, data_in_addr);
>> -    guest_free(vs->alloc, data_out_addr);
>> +    guest_free(vs->qs->alloc, req_addr);
>> +    guest_free(vs->qs->alloc, resp_addr);
>> +    guest_free(vs->qs->alloc, data_in_addr);
>> +    guest_free(vs->qs->alloc, data_out_addr);
>>      return response;
>>  }
>>  
>> @@ -145,10 +135,12 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>>      int i;
>>  
>>      vs = g_new0(QVirtIOSCSI, 1);
>> -    vs->alloc = pc_alloc_init();
>> -    vs->bus = qpci_init_pc(NULL);
>>  
>> -    dev = qvirtio_pci_device_find(vs->bus, VIRTIO_ID_SCSI);
>> +    vs->qs = qvirtio_scsi_start("-drive file=blkdebug::null-co://,"
>> +                                "if=none,id=dr1,format=raw,file.align=4k "
>> +                                "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
>> +    g_assert(vs->qs);
> 
> Null vs->qs ?
> 
>> +    dev = qvirtio_pci_device_find(vs->qs->pcibus, VIRTIO_ID_SCSI);
>>      vs->dev = (QVirtioDevice *)dev;
>>      g_assert(dev != NULL);
>>      g_assert_cmphex(vs->dev->device_type, ==, VIRTIO_ID_SCSI);
>> @@ -165,7 +157,7 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>>      g_assert_cmpint(vs->num_queues, <, MAX_NUM_QUEUES);
>>  
>>      for (i = 0; i < vs->num_queues + 2; i++) {
>> -        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->alloc, i);
>> +        vs->vq[i] = qvirtqueue_setup(&qvirtio_pci, vs->dev, vs->qs->alloc, i);
>>      }
>>  
>>      /* Clear the POWER ON OCCURRED unit attention */
>> @@ -184,15 +176,20 @@ static QVirtIOSCSI *qvirtio_scsi_pci_init(int slot)
>>  /* Tests only initialization so far. TODO: Replace with functional tests */
>>  static void pci_nop(void)
>>  {
>> -    qvirtio_scsi_start(NULL);
>> -    qvirtio_scsi_stop();
>> +    QOSState *qs;
>> +
>> +    qs = qvirtio_scsi_start(NULL);
>> +    g_assert(qs);
> 
> Null qs ?
> 
>> +    qvirtio_scsi_stop(qs);
>>  }
>>  
>>  static void hotplug(void)
>>  {
>>      QDict *response;
>> +    QOSState *qs;
>>  
>> -    qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
>> +    qs = qvirtio_scsi_start("-drive id=drv1,if=none,file=/dev/null,format=raw");
>> +    g_assert(qs);
> 
> Null qs ?
> 
>>      response = qmp("{\"execute\": \"device_add\","
>>                     " \"arguments\": {"
>>                     "   \"driver\": \"scsi-hd\","
>> @@ -214,7 +211,7 @@ static void hotplug(void)
>>      g_assert(qdict_haskey(response, "event"));
>>      g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
>>      QDECREF(response);
>> -    qvirtio_scsi_stop();
>> +    qvirtio_scsi_stop(qs);
>>  }
>>  
>>  /* Test WRITE SAME with the lba not aligned */
>> @@ -230,9 +227,6 @@ static void test_unaligned_write_same(void)
>>          0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
>>      };
>>  
>> -    qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
>> -                       ",format=raw,file.align=4k "
>> -                       "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
>>      vs = qvirtio_scsi_pci_init(PCI_SLOT);
>>  
>>      g_assert_cmphex(0, ==,
>> @@ -242,7 +236,7 @@ static void test_unaligned_write_same(void)
>>          virtio_scsi_do_command(vs, write_same_cdb_2, NULL, 0, buf2, 512, NULL));
>>  
>>      qvirtio_scsi_pci_free(vs);
>> -    qvirtio_scsi_stop();
>> +    qvirtio_scsi_stop(vs->qs);
> 
> Is still vs->qs still valid ? Also it looks wrong to call qvirtio_scsi_stop()
> without any prior call to qvirtio_scsi_start()...
> 
>>  }
>>  
>>  int main(int argc, char **argv)
> 
> Cheers.

Thanks,
Laurent

On 30/09/2016 10:33, Greg Kurz wrote:
> On Thu, 29 Sep 2016 19:15:05 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
...
>> @@ -230,9 +227,6 @@ static void test_unaligned_write_same(void)
>>          0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
>>      };
>>  
>> -    qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
>> -                       ",format=raw,file.align=4k "
>> -                       "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
>>      vs = qvirtio_scsi_pci_init(PCI_SLOT);
>>  
>>      g_assert_cmphex(0, ==,
>> @@ -242,7 +236,7 @@ static void test_unaligned_write_same(void)
>>          virtio_scsi_do_command(vs, write_same_cdb_2, NULL, 0, buf2, 512, NULL));
>>  
>>      qvirtio_scsi_pci_free(vs);
>> -    qvirtio_scsi_stop();
>> +    qvirtio_scsi_stop(vs->qs);
> 
> Is still vs->qs still valid ? Also it looks wrong to call qvirtio_scsi_stop()
> without any prior call to qvirtio_scsi_start()...

The qvirtio_scsi_start() is called by qvirtio_scsi_pci_init().

Laurent

On Fri, 30 Sep 2016 11:13:33 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 30/09/2016 10:33, Greg Kurz wrote:
> > On Thu, 29 Sep 2016 19:15:05 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> [...]
> >>  static void pci_nop(void)
> >>  {
> >> -    qvirtio_9p_start();
> >> -    qvirtio_9p_stop();
> >> +    QOSState *qs;
> >> +
> >> +    qs = qvirtio_9p_start();
> >> +    g_assert(qs);  
> > 
> > The appropriate macro to use here is: g_assert_nonnull().  
> 
> OK
> 
> > 
> > BTW, how can qs be NULL ?  
> 
> we should not know what happens in  qtest_pc_boot() (or
> qtest_spapr_boot(), or qtest_XXX_boot())
> 

What is the point in letting qtest_XXX_boot() return NULL then if
it is always followed by g_assert() in the test program code ?

I'd rather move the assertion there and document that it cannot
return NULL, since it is always unrecoverable in the test code.

> So I think it i better to check it before to use it.
> [...]
> >> +static QOSState *pci_test_start(void)
> >>  {
> >> -    char *cmdline;
> >> +    QOSState *qs = NULL;  
> > 
> > Why setting qs to NULL ? It is necessarily set...  
> 
> Yes, I've mixed my patches: later we add a "if (arch == pc) { qs = }
> else if (arch == spapr) { qs = }" and this case qs can be uninitialized.
> 

Ok, I've realized that when reading the other patch. :)

> [...]
> >> +    qtest_shutdown(qs);  
> > 
> > The title is "tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests"
> > 
> > Even if qtest_shutdown() happens to be equivalent to qtest_pc_shutdown() when
> > qtest_pc_boot() was called, I would rather stick to the title, and convert
> > all qtest_pc_shutdown() to qtest_shutdown() in patch 3/3... 
> > 
> > No big deal though, you may s/qtest_pc_shutdown/qtest_shutdown in the title
> > as well :)  
> 
> I'm to change all qtest_pc_shutdown() to qtest_shutdown() here
> 

Ok.

Cheers.

--
Greg

On 30/09/2016 12:29, Greg Kurz wrote:
> On Fri, 30 Sep 2016 11:13:33 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> On 30/09/2016 10:33, Greg Kurz wrote:
>>> On Thu, 29 Sep 2016 19:15:05 +0200
>>> Laurent Vivier <lvivier@redhat.com> wrote:
>>>   
>>>> [...]
>>>>  static void pci_nop(void)
>>>>  {
>>>> -    qvirtio_9p_start();
>>>> -    qvirtio_9p_stop();
>>>> +    QOSState *qs;
>>>> +
>>>> +    qs = qvirtio_9p_start();
>>>> +    g_assert(qs);  
>>>
>>> The appropriate macro to use here is: g_assert_nonnull().  
>>
>> OK
>>
>>>
>>> BTW, how can qs be NULL ?  
>>
>> we should not know what happens in  qtest_pc_boot() (or
>> qtest_spapr_boot(), or qtest_XXX_boot())
>>
> 
> What is the point in letting qtest_XXX_boot() return NULL then if
> it is always followed by g_assert() in the test program code ?
> 
> I'd rather move the assertion there and document that it cannot
> return NULL, since it is always unrecoverable in the test code.

You're right it looks better as you say. I'm going to change that.

Thanks,
Laurent

On Fri, 30 Sep 2016 11:56:50 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 30/09/2016 10:33, Greg Kurz wrote:
> > On Thu, 29 Sep 2016 19:15:05 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:  
> ...
> >> @@ -230,9 +227,6 @@ static void test_unaligned_write_same(void)
> >>          0x41, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x33, 0x00, 0x00
> >>      };
> >>  
> >> -    qvirtio_scsi_start("-drive file=blkdebug::null-co://,if=none,id=dr1"
> >> -                       ",format=raw,file.align=4k "
> >> -                       "-device scsi-disk,drive=dr1,lun=0,scsi-id=1");
> >>      vs = qvirtio_scsi_pci_init(PCI_SLOT);
> >>  
> >>      g_assert_cmphex(0, ==,
> >> @@ -242,7 +236,7 @@ static void test_unaligned_write_same(void)
> >>          virtio_scsi_do_command(vs, write_same_cdb_2, NULL, 0, buf2, 512, NULL));
> >>  
> >>      qvirtio_scsi_pci_free(vs);
> >> -    qvirtio_scsi_stop();
> >> +    qvirtio_scsi_stop(vs->qs);  
> > 
> > Is still vs->qs still valid ? Also it looks wrong to call qvirtio_scsi_stop()
> > without any prior call to qvirtio_scsi_start()...  
> 
> The qvirtio_scsi_start() is called by qvirtio_scsi_pci_init().
> 
> Laurent

I guess qvirtio_scsi_pci_free() should call qvirtio_scsi_stop() as well then.

Also, it looks like vs is leaked, but this is already the case with the
current code. Maybe worth fixing that first.

Cheers.

--
Greg