[Qemu-devel] [PATCH 0/3] tests: enable virtio tests on SPAPR

[Qemu-devel] [PATCH 0/3] tests: enable virtio tests on SPAPR

[Laurent Vivier]

This series enables virtio tests on SPAPR by starting
machines using qtest_pc_boot() or qtest_spapr_boot() to
use the good libqos PCI framework (pc or spapr).

It adds also some byte-swapping in virtio-pci.c as
PCI is always little-endian and the endianness of
the virtio device depends on the endianness of the
guest.

This series does not enable virtio PCI MSI-X tests on
SPAPR as this needs more work and will be the aim
of another series.

This series must be applied on top of my previous
series: "tests: enable ohci/uhci/xhci tests on PPC64"

Laurent Vivier (3):
  tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests
  qtest: evaluate endianness of the target in qtest_init()
  tests: enable virtio tests on SPAPR

 tests/Makefile.include    |   3 +-
 tests/libqos/virtio-pci.c |  24 ++++++-
 tests/libqtest.c          |  96 +++++++++++++------------
 tests/libqtest.h          |  16 ++++-
 tests/virtio-9p-test.c    |  60 ++++++++--------
 tests/virtio-blk-test.c   | 176 ++++++++++++++++++++++------------------------
 tests/virtio-net-test.c   |  57 ++++++++-------
 tests/virtio-rng-test.c   |   7 +-
 tests/virtio-scsi-test.c  |  78 ++++++++++----------
 9 files changed, 281 insertions(+), 236 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Laurent Vivier]

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(-)

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);
+    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)
-- 
2.5.5

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 28523 bytes --]

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)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Laurent Vivier]

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

Re: [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Greg Kurz]

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

Re: [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Laurent Vivier]

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

Re: [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Greg Kurz]

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

Re: [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Laurent Vivier]

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

Re: [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Laurent Vivier]

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

Re: [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests

[Greg Kurz]

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

[Qemu-devel] [PATCH 2/3] qtest: evaluate endianness of the target in qtest_init()

[Laurent Vivier]

This allows to store it and not have to rescan the list
each time we need it.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
 tests/libqos/virtio-pci.c |  2 +-
 tests/libqtest.c          | 96 +++++++++++++++++++++++++----------------------
 tests/libqtest.h          | 16 ++++++--
 tests/virtio-blk-test.c   |  2 +-
 4 files changed, 66 insertions(+), 50 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 18b92b9..6e005c1 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -86,7 +86,7 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
     int i;
     uint64_t u64 = 0;
 
-    if (qtest_big_endian()) {
+    if (target_big_endian()) {
         for (i = 0; i < 8; ++i) {
             u64 |= (uint64_t)qpci_io_readb(dev->pdev,
                                 (void *)(uintptr_t)addr + i) << (7 - i) * 8;
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 6f6bdf1..aa4bc9e 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -37,6 +37,7 @@ struct QTestState
     bool irq_level[MAX_IRQ];
     GString *rx;
     pid_t qemu_pid;  /* our child QEMU process */
+    bool big_endian;
 };
 
 static GHookList abrt_hooks;
@@ -146,6 +147,52 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
     g_hook_prepend(&abrt_hooks, hook);
 }
 
+static bool arch_is_big_endian(const char *arch)
+{
+    int i;
+    static const struct {
+        const char *arch;
+        bool big_endian;
+    } endianness[] = {
+        { "aarch64", false },
+        { "alpha", false },
+        { "arm", false },
+        { "cris", false },
+        { "i386", false },
+        { "lm32", true },
+        { "m68k", true },
+        { "microblaze", true },
+        { "microblazeel", false },
+        { "mips", true },
+        { "mips64", true },
+        { "mips64el", false },
+        { "mipsel", false },
+        { "moxie", true },
+        { "or32", true },
+        { "ppc", true },
+        { "ppc64", true },
+        { "ppcemb", true },
+        { "s390x", true },
+        { "sh4", false },
+        { "sh4eb", true },
+        { "sparc", true },
+        { "sparc64", true },
+        { "unicore32", false },
+        { "x86_64", false },
+        { "xtensa", false },
+        { "xtensaeb", true },
+        { "tricore", false },
+        {},
+    };
+
+    for (i = 0; endianness[i].arch; i++) {
+        if (strcmp(endianness[i].arch, arch) == 0) {
+            return endianness[i].big_endian;
+        }
+    }
+    g_assert_not_reached();
+}
+
 QTestState *qtest_init(const char *extra_args)
 {
     QTestState *s;
@@ -209,6 +256,8 @@ QTestState *qtest_init(const char *extra_args)
         kill(s->qemu_pid, SIGSTOP);
     }
 
+    s->big_endian = arch_is_big_endian(qtest_get_arch());
+
     return s;
 }
 
@@ -886,50 +935,7 @@ char *hmp(const char *fmt, ...)
     return ret;
 }
 
-bool qtest_big_endian(void)
+bool qtest_big_endian(QTestState *s)
 {
-    const char *arch = qtest_get_arch();
-    int i;
-
-    static const struct {
-        const char *arch;
-        bool big_endian;
-    } endianness[] = {
-        { "aarch64", false },
-        { "alpha", false },
-        { "arm", false },
-        { "cris", false },
-        { "i386", false },
-        { "lm32", true },
-        { "m68k", true },
-        { "microblaze", true },
-        { "microblazeel", false },
-        { "mips", true },
-        { "mips64", true },
-        { "mips64el", false },
-        { "mipsel", false },
-        { "moxie", true },
-        { "or32", true },
-        { "ppc", true },
-        { "ppc64", true },
-        { "ppcemb", true },
-        { "s390x", true },
-        { "sh4", false },
-        { "sh4eb", true },
-        { "sparc", true },
-        { "sparc64", true },
-        { "unicore32", false },
-        { "x86_64", false },
-        { "xtensa", false },
-        { "xtensaeb", true },
-        {},
-    };
-
-    for (i = 0; endianness[i].arch; i++) {
-        if (strcmp(endianness[i].arch, arch) == 0) {
-            return endianness[i].big_endian;
-        }
-    }
-
-    return false;
+    return s->big_endian;
 }
diff --git a/tests/libqtest.h b/tests/libqtest.h
index f7402e0..4be1f77 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -410,6 +410,14 @@ int64_t qtest_clock_step(QTestState *s, int64_t step);
 int64_t qtest_clock_set(QTestState *s, int64_t val);
 
 /**
+ * qtest_big_endian:
+ * @s: QTestState instance to operate on.
+ *
+ * Returns: True if the architecture under test has a big endian configuration.
+ */
+bool qtest_big_endian(QTestState *s);
+
+/**
  * qtest_get_arch:
  *
  * Returns: The architecture for the QEMU executable under test.
@@ -874,12 +882,14 @@ static inline int64_t clock_set(int64_t val)
 }
 
 /**
- * qtest_big_endian:
+ * target_big_endian:
  *
  * Returns: True if the architecture under test has a big endian configuration.
  */
-bool qtest_big_endian(void);
-
+static inline bool target_big_endian(void)
+{
+    return qtest_big_endian(global_qtest);
+}
 
 QDict *qmp_fd_receive(int fd);
 void qmp_fd_sendv(int fd, const char *fmt, va_list ap);
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 8cf62f6..a4de3e4 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -120,7 +120,7 @@ static inline void virtio_blk_fix_request(QVirtioBlkReq *req)
     bool host_endian = false;
 #endif
 
-    if (qtest_big_endian() != host_endian) {
+    if (target_big_endian() != host_endian) {
         req->type = bswap32(req->type);
         req->ioprio = bswap32(req->ioprio);
         req->sector = bswap64(req->sector);
-- 
2.5.5

Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/3] qtest: evaluate endianness of the target in qtest_init()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6886 bytes --]

On Thu, Sep 29, 2016 at 07:15:06PM +0200, Laurent Vivier wrote:
> This allows to store it and not have to rescan the list
> each time we need it.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

In that it looks technically correct.  The whole notion of guest
endianness is kind of bogus, doubly so when we're essentially
replacing the cpu with the qtest proxy.  It'd be better to make all
the io operations have explicit endianness.

Still, it looks reasonable in the short term.

> ---
>  tests/libqos/virtio-pci.c |  2 +-
>  tests/libqtest.c          | 96 +++++++++++++++++++++++++----------------------
>  tests/libqtest.h          | 16 ++++++--
>  tests/virtio-blk-test.c   |  2 +-
>  4 files changed, 66 insertions(+), 50 deletions(-)
> 
> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> index 18b92b9..6e005c1 100644
> --- a/tests/libqos/virtio-pci.c
> +++ b/tests/libqos/virtio-pci.c
> @@ -86,7 +86,7 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
>      int i;
>      uint64_t u64 = 0;
>  
> -    if (qtest_big_endian()) {
> +    if (target_big_endian()) {
>          for (i = 0; i < 8; ++i) {
>              u64 |= (uint64_t)qpci_io_readb(dev->pdev,
>                                  (void *)(uintptr_t)addr + i) << (7 - i) * 8;
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 6f6bdf1..aa4bc9e 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -37,6 +37,7 @@ struct QTestState
>      bool irq_level[MAX_IRQ];
>      GString *rx;
>      pid_t qemu_pid;  /* our child QEMU process */
> +    bool big_endian;
>  };
>  
>  static GHookList abrt_hooks;
> @@ -146,6 +147,52 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
>      g_hook_prepend(&abrt_hooks, hook);
>  }
>  
> +static bool arch_is_big_endian(const char *arch)
> +{
> +    int i;
> +    static const struct {
> +        const char *arch;
> +        bool big_endian;
> +    } endianness[] = {
> +        { "aarch64", false },
> +        { "alpha", false },
> +        { "arm", false },
> +        { "cris", false },
> +        { "i386", false },
> +        { "lm32", true },
> +        { "m68k", true },
> +        { "microblaze", true },
> +        { "microblazeel", false },
> +        { "mips", true },
> +        { "mips64", true },
> +        { "mips64el", false },
> +        { "mipsel", false },
> +        { "moxie", true },
> +        { "or32", true },
> +        { "ppc", true },
> +        { "ppc64", true },
> +        { "ppcemb", true },
> +        { "s390x", true },
> +        { "sh4", false },
> +        { "sh4eb", true },
> +        { "sparc", true },
> +        { "sparc64", true },
> +        { "unicore32", false },
> +        { "x86_64", false },
> +        { "xtensa", false },
> +        { "xtensaeb", true },
> +        { "tricore", false },
> +        {},
> +    };
> +
> +    for (i = 0; endianness[i].arch; i++) {
> +        if (strcmp(endianness[i].arch, arch) == 0) {
> +            return endianness[i].big_endian;
> +        }
> +    }
> +    g_assert_not_reached();
> +}
> +
>  QTestState *qtest_init(const char *extra_args)
>  {
>      QTestState *s;
> @@ -209,6 +256,8 @@ QTestState *qtest_init(const char *extra_args)
>          kill(s->qemu_pid, SIGSTOP);
>      }
>  
> +    s->big_endian = arch_is_big_endian(qtest_get_arch());
> +
>      return s;
>  }
>  
> @@ -886,50 +935,7 @@ char *hmp(const char *fmt, ...)
>      return ret;
>  }
>  
> -bool qtest_big_endian(void)
> +bool qtest_big_endian(QTestState *s)
>  {
> -    const char *arch = qtest_get_arch();
> -    int i;
> -
> -    static const struct {
> -        const char *arch;
> -        bool big_endian;
> -    } endianness[] = {
> -        { "aarch64", false },
> -        { "alpha", false },
> -        { "arm", false },
> -        { "cris", false },
> -        { "i386", false },
> -        { "lm32", true },
> -        { "m68k", true },
> -        { "microblaze", true },
> -        { "microblazeel", false },
> -        { "mips", true },
> -        { "mips64", true },
> -        { "mips64el", false },
> -        { "mipsel", false },
> -        { "moxie", true },
> -        { "or32", true },
> -        { "ppc", true },
> -        { "ppc64", true },
> -        { "ppcemb", true },
> -        { "s390x", true },
> -        { "sh4", false },
> -        { "sh4eb", true },
> -        { "sparc", true },
> -        { "sparc64", true },
> -        { "unicore32", false },
> -        { "x86_64", false },
> -        { "xtensa", false },
> -        { "xtensaeb", true },
> -        {},
> -    };
> -
> -    for (i = 0; endianness[i].arch; i++) {
> -        if (strcmp(endianness[i].arch, arch) == 0) {
> -            return endianness[i].big_endian;
> -        }
> -    }
> -
> -    return false;
> +    return s->big_endian;
>  }
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index f7402e0..4be1f77 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -410,6 +410,14 @@ int64_t qtest_clock_step(QTestState *s, int64_t step);
>  int64_t qtest_clock_set(QTestState *s, int64_t val);
>  
>  /**
> + * qtest_big_endian:
> + * @s: QTestState instance to operate on.
> + *
> + * Returns: True if the architecture under test has a big endian configuration.
> + */
> +bool qtest_big_endian(QTestState *s);
> +
> +/**
>   * qtest_get_arch:
>   *
>   * Returns: The architecture for the QEMU executable under test.
> @@ -874,12 +882,14 @@ static inline int64_t clock_set(int64_t val)
>  }
>  
>  /**
> - * qtest_big_endian:
> + * target_big_endian:
>   *
>   * Returns: True if the architecture under test has a big endian configuration.
>   */
> -bool qtest_big_endian(void);
> -
> +static inline bool target_big_endian(void)
> +{
> +    return qtest_big_endian(global_qtest);
> +}
>  
>  QDict *qmp_fd_receive(int fd);
>  void qmp_fd_sendv(int fd, const char *fmt, va_list ap);
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 8cf62f6..a4de3e4 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -120,7 +120,7 @@ static inline void virtio_blk_fix_request(QVirtioBlkReq *req)
>      bool host_endian = false;
>  #endif
>  
> -    if (qtest_big_endian() != host_endian) {
> +    if (target_big_endian() != host_endian) {
>          req->type = bswap32(req->type);
>          req->ioprio = bswap32(req->ioprio);
>          req->sector = bswap64(req->sector);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Qemu-devel] [PATCH 3/3] tests: enable virtio tests on SPAPR

[Laurent Vivier]

but disable MSI-X tests on SPAPR as we can't check the result
(the memory region used on PC is not readable on SPAPR).

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 tests/Makefile.include    |  3 ++-
 tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
 tests/virtio-9p-test.c    | 11 ++++++++++-
 tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
 tests/virtio-net-test.c   | 17 +++++++++++++++--
 tests/virtio-rng-test.c   |  7 ++++++-
 tests/virtio-scsi-test.c  | 10 +++++++++-
 7 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index c46a32d..1e4a3d5 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF)
 gcov-files-ppc64-y += hw/usb/hcd-uhci.c
 check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF)
 gcov-files-ppc64-y += hw/usb/hcd-xhci.c
+check-qtest-ppc64-y += $(check-qtest-virtio-y)
 
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 
@@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
 libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
 libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
-libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
+libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
 
 tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
 tests/rtc-test$(EXESUF): tests/rtc-test.o
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 6e005c1..ed81df6 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
     return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr);
 }
 
+/* PCI is always read in little-endian order
+ * but virtio ( < 1.0) is in guest order
+ * so with a big-endian guest the order has been reversed
+ * reverse it again
+ */
+
 static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
+    uint16_t value;
+
+    value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
+    if (target_big_endian()) {
+        value = bswap16(value);
+    }
+    return value;
 }
 
 static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr)
 {
     QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-    return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
+    uint32_t value;
+
+    value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
+    if (target_big_endian()) {
+        value = bswap32(value);
+    }
+    return value;
 }
 
 static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
index 28d7f5b..a73bccb 100644
--- a/tests/virtio-9p-test.c
+++ b/tests/virtio-9p-test.c
@@ -11,6 +11,7 @@
 #include "libqtest.h"
 #include "qemu-common.h"
 #include "libqos/libqos-pc.h"
+#include "libqos/libqos-spapr.h"
 #include "libqos/virtio.h"
 #include "libqos/virtio-pci.h"
 #include "standard-headers/linux/virtio_ids.h"
@@ -22,12 +23,20 @@ static char *test_share;
 
 static QOSState *qvirtio_9p_start(void)
 {
+    const char *arch = qtest_get_arch();
+    QOSState *qs = NULL;
     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";
 
-    return qtest_pc_boot(cmd, test_share, mount_tag);
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qs = qtest_pc_boot(cmd, test_share, mount_tag);
+    } else if (strcmp(arch, "ppc64") == 0) {
+        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
+    }
+
+    return qs;
 }
 
 static void qvirtio_9p_stop(QOSState *qs)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index a4de3e4..a032f8e 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -11,6 +11,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "libqos/libqos-pc.h"
+#include "libqos/libqos-spapr.h"
 #include "libqos/virtio.h"
 #include "libqos/virtio-pci.h"
 #include "libqos/virtio-mmio.h"
@@ -58,6 +59,7 @@ static char *drive_create(void)
 
 static QOSState *pci_test_start(void)
 {
+    const char *arch = qtest_get_arch();
     QOSState *qs = NULL;
     char *tmp_path;
     const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
@@ -67,7 +69,11 @@ static QOSState *pci_test_start(void)
 
     tmp_path = drive_create();
 
-    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
+    } else if (strcmp(arch, "ppc64") == 0) {
+        qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
+    }
     unlink(tmp_path);
     g_free(tmp_path);
     return qs;
@@ -676,6 +682,7 @@ static void pci_hotplug(void)
 {
     QVirtioPCIDevice *dev;
     QOSState *qs;
+    const char *arch = qtest_get_arch();
 
     qs = pci_test_start();
     g_assert(qs);
@@ -690,7 +697,9 @@ static void pci_hotplug(void)
     g_free(dev);
 
     /* unplug secondary disk */
-    qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
+    }
     qtest_shutdown(qs);
 }
 
@@ -741,12 +750,15 @@ int main(int argc, char **argv)
 
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 ||
+        strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) {
         qtest_add_func("/virtio/blk/pci/basic", pci_basic);
         qtest_add_func("/virtio/blk/pci/indirect", pci_indirect);
         qtest_add_func("/virtio/blk/pci/config", pci_config);
-        qtest_add_func("/virtio/blk/pci/msix", pci_msix);
-        qtest_add_func("/virtio/blk/pci/idx", pci_idx);
+        if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+            qtest_add_func("/virtio/blk/pci/msix", pci_msix);
+            qtest_add_func("/virtio/blk/pci/idx", pci_idx);
+        }
         qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug);
     } else if (strcmp(arch, "arm") == 0) {
         qtest_add_func("/virtio/blk/mmio/basic", mmio_basic);
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 3e83685..14f2920 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -13,6 +13,7 @@
 #include "qemu/sockets.h"
 #include "qemu/iov.h"
 #include "libqos/libqos-pc.h"
+#include "libqos/libqos-spapr.h"
 #include "libqos/virtio.h"
 #include "libqos/virtio-pci.h"
 #include "qemu/bswap.h"
@@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
 
 static QOSState *pci_test_start(int socket)
 {
+    const char *arch = qtest_get_arch();
     const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
                       "virtio-net-pci,netdev=hs0";
 
-    return qtest_pc_boot(cmd, socket);
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        return qtest_pc_boot(cmd, socket);
+    }
+    if (strcmp(arch, "ppc64") == 0) {
+        return qtest_spapr_boot(cmd, socket);
+    }
+    return NULL;
 }
 
 static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
@@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data)
 
 static void hotplug(void)
 {
+    const char *arch = qtest_get_arch();
+
     qtest_start("-device virtio-net-pci");
 
     qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL);
-    qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
+    }
 
     test_end();
 }
diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c
index e1b2640..dcecf77 100644
--- a/tests/virtio-rng-test.c
+++ b/tests/virtio-rng-test.c
@@ -20,8 +20,13 @@ static void pci_nop(void)
 
 static void hotplug(void)
 {
+    const char *arch = qtest_get_arch();
+
     qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL);
-    qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
+    }
 }
 
 int main(int argc, char **argv)
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 721ae1f..dd5457b 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -12,6 +12,7 @@
 #include "libqtest.h"
 #include "block/scsi.h"
 #include "libqos/libqos-pc.h"
+#include "libqos/libqos-spapr.h"
 #include "libqos/virtio.h"
 #include "libqos/virtio-pci.h"
 #include "standard-headers/linux/virtio_ids.h"
@@ -33,11 +34,18 @@ typedef struct {
 
 static QOSState *qvirtio_scsi_start(const char *extra_opts)
 {
+    const char *arch = qtest_get_arch();
     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 ? : "");
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        return qtest_pc_boot(cmd, extra_opts ? : "");
+    }
+    if (strcmp(arch, "ppc64") == 0) {
+        return qtest_spapr_boot(cmd, extra_opts ? : "");
+    }
+    return NULL;
 }
 
 static void qvirtio_scsi_stop(QOSState *qs)
-- 
2.5.5

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] tests: enable virtio tests on SPAPR

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 11139 bytes --]

On Thu, Sep 29, 2016 at 07:15:07PM +0200, Laurent Vivier wrote:
> but disable MSI-X tests on SPAPR as we can't check the result
> (the memory region used on PC is not readable on SPAPR).
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tests/Makefile.include    |  3 ++-
>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
>  tests/virtio-9p-test.c    | 11 ++++++++++-
>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
>  tests/virtio-net-test.c   | 17 +++++++++++++++--
>  tests/virtio-rng-test.c   |  7 ++++++-
>  tests/virtio-scsi-test.c  | 10 +++++++++-
>  7 files changed, 79 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index c46a32d..1e4a3d5 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF)
>  gcov-files-ppc64-y += hw/usb/hcd-uhci.c
>  check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF)
>  gcov-files-ppc64-y += hw/usb/hcd-xhci.c
> +check-qtest-ppc64-y += $(check-qtest-virtio-y)
>  
>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>  
> @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o
>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
>  libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
> -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
> +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
>  
>  tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
>  tests/rtc-test$(EXESUF): tests/rtc-test.o
> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> index 6e005c1..ed81df6 100644
> --- a/tests/libqos/virtio-pci.c
> +++ b/tests/libqos/virtio-pci.c
> @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
>      return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr);
>  }
>  
> +/* PCI is always read in little-endian order
> + * but virtio ( < 1.0) is in guest order
> + * so with a big-endian guest the order has been reversed
> + * reverse it again
> + */
> +
>  static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
> +    uint16_t value;
> +
> +    value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
> +    if (target_big_endian()) {
> +        value = bswap16(value);
> +    }

Don't you need some sort of test to distinguish the virtio < 1.0 and
virtio-1.0 cases?

> +    return value;
>  }
>  
>  static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
> +    uint32_t value;
> +
> +    value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
> +    if (target_big_endian()) {
> +        value = bswap32(value);
> +    }
> +    return value;
>  }
>  
>  static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index 28d7f5b..a73bccb 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -11,6 +11,7 @@
>  #include "libqtest.h"
>  #include "qemu-common.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -22,12 +23,20 @@ static char *test_share;
>  
>  static QOSState *qvirtio_9p_start(void)
>  {
> +    const char *arch = qtest_get_arch();
> +    QOSState *qs = NULL;
>      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";
>  
> -    return qtest_pc_boot(cmd, test_share, mount_tag);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qs = qtest_pc_boot(cmd, test_share, mount_tag);
> +    } else if (strcmp(arch, "ppc64") == 0) {
> +        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
> +    }
> +
> +    return qs;
>  }
>  
>  static void qvirtio_9p_stop(QOSState *qs)
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index a4de3e4..a032f8e 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -11,6 +11,7 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "libqos/virtio-mmio.h"
> @@ -58,6 +59,7 @@ static char *drive_create(void)
>  
>  static QOSState *pci_test_start(void)
>  {
> +    const char *arch = qtest_get_arch();
>      QOSState *qs = NULL;
>      char *tmp_path;
>      const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
> @@ -67,7 +69,11 @@ static QOSState *pci_test_start(void)
>  
>      tmp_path = drive_create();
>  
> -    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
> +    } else if (strcmp(arch, "ppc64") == 0) {
> +        qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
> +    }
>      unlink(tmp_path);
>      g_free(tmp_path);
>      return qs;
> @@ -676,6 +682,7 @@ static void pci_hotplug(void)
>  {
>      QVirtioPCIDevice *dev;
>      QOSState *qs;
> +    const char *arch = qtest_get_arch();
>  
>      qs = pci_test_start();
>      g_assert(qs);
> @@ -690,7 +697,9 @@ static void pci_hotplug(void)
>      g_free(dev);
>  
>      /* unplug secondary disk */
> -    qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
> +    }
>      qtest_shutdown(qs);
>  }
>  
> @@ -741,12 +750,15 @@ int main(int argc, char **argv)
>  
>      g_test_init(&argc, &argv, NULL);
>  
> -    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 ||
> +        strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) {
>          qtest_add_func("/virtio/blk/pci/basic", pci_basic);
>          qtest_add_func("/virtio/blk/pci/indirect", pci_indirect);
>          qtest_add_func("/virtio/blk/pci/config", pci_config);
> -        qtest_add_func("/virtio/blk/pci/msix", pci_msix);
> -        qtest_add_func("/virtio/blk/pci/idx", pci_idx);
> +        if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +            qtest_add_func("/virtio/blk/pci/msix", pci_msix);
> +            qtest_add_func("/virtio/blk/pci/idx", pci_idx);
> +        }
>          qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug);
>      } else if (strcmp(arch, "arm") == 0) {
>          qtest_add_func("/virtio/blk/mmio/basic", mmio_basic);
> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index 3e83685..14f2920 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -13,6 +13,7 @@
>  #include "qemu/sockets.h"
>  #include "qemu/iov.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "qemu/bswap.h"
> @@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>  
>  static QOSState *pci_test_start(int socket)
>  {
> +    const char *arch = qtest_get_arch();
>      const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
>                        "virtio-net-pci,netdev=hs0";
>  
> -    return qtest_pc_boot(cmd, socket);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return qtest_pc_boot(cmd, socket);
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return qtest_spapr_boot(cmd, socket);
> +    }
> +    return NULL;
>  }
>  
>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
> @@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data)
>  
>  static void hotplug(void)
>  {
> +    const char *arch = qtest_get_arch();
> +
>      qtest_start("-device virtio-net-pci");
>  
>      qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL);
> -    qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
> +    }
>  
>      test_end();
>  }
> diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c
> index e1b2640..dcecf77 100644
> --- a/tests/virtio-rng-test.c
> +++ b/tests/virtio-rng-test.c
> @@ -20,8 +20,13 @@ static void pci_nop(void)
>  
>  static void hotplug(void)
>  {
> +    const char *arch = qtest_get_arch();
> +
>      qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL);
> -    qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
> +    }
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
> index 721ae1f..dd5457b 100644
> --- a/tests/virtio-scsi-test.c
> +++ b/tests/virtio-scsi-test.c
> @@ -12,6 +12,7 @@
>  #include "libqtest.h"
>  #include "block/scsi.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -33,11 +34,18 @@ typedef struct {
>  
>  static QOSState *qvirtio_scsi_start(const char *extra_opts)
>  {
> +    const char *arch = qtest_get_arch();
>      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 ? : "");
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return qtest_pc_boot(cmd, extra_opts ? : "");
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return qtest_spapr_boot(cmd, extra_opts ? : "");
> +    }
> +    return NULL;
>  }
>  
>  static void qvirtio_scsi_stop(QOSState *qs)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] tests: enable virtio tests on SPAPR

[Laurent Vivier]

On 30/09/2016 03:30, David Gibson wrote:
> On Thu, Sep 29, 2016 at 07:15:07PM +0200, Laurent Vivier wrote:
>> but disable MSI-X tests on SPAPR as we can't check the result
>> (the memory region used on PC is not readable on SPAPR).
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  tests/Makefile.include    |  3 ++-
>>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
>>  tests/virtio-9p-test.c    | 11 ++++++++++-
>>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
>>  tests/virtio-net-test.c   | 17 +++++++++++++++--
>>  tests/virtio-rng-test.c   |  7 ++++++-
>>  tests/virtio-scsi-test.c  | 10 +++++++++-
>>  7 files changed, 79 insertions(+), 13 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index c46a32d..1e4a3d5 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF)
>>  gcov-files-ppc64-y += hw/usb/hcd-uhci.c
>>  check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF)
>>  gcov-files-ppc64-y += hw/usb/hcd-xhci.c
>> +check-qtest-ppc64-y += $(check-qtest-virtio-y)
>>  
>>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>>  
>> @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o
>>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
>>  libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
>> -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
>> +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
>>  
>>  tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
>>  tests/rtc-test$(EXESUF): tests/rtc-test.o
>> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
>> index 6e005c1..ed81df6 100644
>> --- a/tests/libqos/virtio-pci.c
>> +++ b/tests/libqos/virtio-pci.c
>> @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
>>      return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr);
>>  }
>>  
>> +/* PCI is always read in little-endian order
>> + * but virtio ( < 1.0) is in guest order
>> + * so with a big-endian guest the order has been reversed
>> + * reverse it again
>> + */
>> +
>>  static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
>>  {
>>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
>> -    return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
>> +    uint16_t value;
>> +
>> +    value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
>> +    if (target_big_endian()) {
>> +        value = bswap16(value);
>> +    }
> 
> Don't you need some sort of test to distinguish the virtio < 1.0 and
> virtio-1.0 cases?

yes, but for the moment we don't test virtio-1.0, we will add the test
when will support it.

http://wiki.qemu.org/Outreachy_2016_DecemberMarch#VIRTIO_1.0_support_in_libqos

> 
>> +    return value;
>>  }
>>  
>>  static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr)
>>  {
>>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
>> -    return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
>> +    uint32_t value;
>> +
>> +    value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
>> +    if (target_big_endian()) {
>> +        value = bswap32(value);
>> +    }
>> +    return value;
>>  }
>>  
>>  static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
>> index 28d7f5b..a73bccb 100644
>> --- a/tests/virtio-9p-test.c
>> +++ b/tests/virtio-9p-test.c
>> @@ -11,6 +11,7 @@
>>  #include "libqtest.h"
>>  #include "qemu-common.h"
>>  #include "libqos/libqos-pc.h"
>> +#include "libqos/libqos-spapr.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>> @@ -22,12 +23,20 @@ static char *test_share;
>>  
>>  static QOSState *qvirtio_9p_start(void)
>>  {
>> +    const char *arch = qtest_get_arch();
>> +    QOSState *qs = NULL;
>>      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";
>>  
>> -    return qtest_pc_boot(cmd, test_share, mount_tag);
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        qs = qtest_pc_boot(cmd, test_share, mount_tag);
>> +    } else if (strcmp(arch, "ppc64") == 0) {
>> +        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
>> +    }
>> +
>> +    return qs;
>>  }
>>  
>>  static void qvirtio_9p_stop(QOSState *qs)
>> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
>> index a4de3e4..a032f8e 100644
>> --- a/tests/virtio-blk-test.c
>> +++ b/tests/virtio-blk-test.c
>> @@ -11,6 +11,7 @@
>>  #include "qemu/osdep.h"
>>  #include "libqtest.h"
>>  #include "libqos/libqos-pc.h"
>> +#include "libqos/libqos-spapr.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>>  #include "libqos/virtio-mmio.h"
>> @@ -58,6 +59,7 @@ static char *drive_create(void)
>>  
>>  static QOSState *pci_test_start(void)
>>  {
>> +    const char *arch = qtest_get_arch();
>>      QOSState *qs = NULL;
>>      char *tmp_path;
>>      const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
>> @@ -67,7 +69,11 @@ static QOSState *pci_test_start(void)
>>  
>>      tmp_path = drive_create();
>>  
>> -    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
>> +    } else if (strcmp(arch, "ppc64") == 0) {
>> +        qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
>> +    }
>>      unlink(tmp_path);
>>      g_free(tmp_path);
>>      return qs;
>> @@ -676,6 +682,7 @@ static void pci_hotplug(void)
>>  {
>>      QVirtioPCIDevice *dev;
>>      QOSState *qs;
>> +    const char *arch = qtest_get_arch();
>>  
>>      qs = pci_test_start();
>>      g_assert(qs);
>> @@ -690,7 +697,9 @@ static void pci_hotplug(void)
>>      g_free(dev);
>>  
>>      /* unplug secondary disk */
>> -    qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
>> +    }
>>      qtest_shutdown(qs);
>>  }
>>  
>> @@ -741,12 +750,15 @@ int main(int argc, char **argv)
>>  
>>      g_test_init(&argc, &argv, NULL);
>>  
>> -    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 ||
>> +        strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) {
>>          qtest_add_func("/virtio/blk/pci/basic", pci_basic);
>>          qtest_add_func("/virtio/blk/pci/indirect", pci_indirect);
>>          qtest_add_func("/virtio/blk/pci/config", pci_config);
>> -        qtest_add_func("/virtio/blk/pci/msix", pci_msix);
>> -        qtest_add_func("/virtio/blk/pci/idx", pci_idx);
>> +        if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +            qtest_add_func("/virtio/blk/pci/msix", pci_msix);
>> +            qtest_add_func("/virtio/blk/pci/idx", pci_idx);
>> +        }
>>          qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug);
>>      } else if (strcmp(arch, "arm") == 0) {
>>          qtest_add_func("/virtio/blk/mmio/basic", mmio_basic);
>> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
>> index 3e83685..14f2920 100644
>> --- a/tests/virtio-net-test.c
>> +++ b/tests/virtio-net-test.c
>> @@ -13,6 +13,7 @@
>>  #include "qemu/sockets.h"
>>  #include "qemu/iov.h"
>>  #include "libqos/libqos-pc.h"
>> +#include "libqos/libqos-spapr.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>>  #include "qemu/bswap.h"
>> @@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>>  
>>  static QOSState *pci_test_start(int socket)
>>  {
>> +    const char *arch = qtest_get_arch();
>>      const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
>>                        "virtio-net-pci,netdev=hs0";
>>  
>> -    return qtest_pc_boot(cmd, socket);
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        return qtest_pc_boot(cmd, socket);
>> +    }
>> +    if (strcmp(arch, "ppc64") == 0) {
>> +        return qtest_spapr_boot(cmd, socket);
>> +    }
>> +    return NULL;
>>  }
>>  
>>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
>> @@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data)
>>  
>>  static void hotplug(void)
>>  {
>> +    const char *arch = qtest_get_arch();
>> +
>>      qtest_start("-device virtio-net-pci");
>>  
>>      qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL);
>> -    qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
>> +
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
>> +    }
>>  
>>      test_end();
>>  }
>> diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c
>> index e1b2640..dcecf77 100644
>> --- a/tests/virtio-rng-test.c
>> +++ b/tests/virtio-rng-test.c
>> @@ -20,8 +20,13 @@ static void pci_nop(void)
>>  
>>  static void hotplug(void)
>>  {
>> +    const char *arch = qtest_get_arch();
>> +
>>      qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL);
>> -    qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
>> +
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
>> +    }
>>  }
>>  
>>  int main(int argc, char **argv)
>> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
>> index 721ae1f..dd5457b 100644
>> --- a/tests/virtio-scsi-test.c
>> +++ b/tests/virtio-scsi-test.c
>> @@ -12,6 +12,7 @@
>>  #include "libqtest.h"
>>  #include "block/scsi.h"
>>  #include "libqos/libqos-pc.h"
>> +#include "libqos/libqos-spapr.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>> @@ -33,11 +34,18 @@ typedef struct {
>>  
>>  static QOSState *qvirtio_scsi_start(const char *extra_opts)
>>  {
>> +    const char *arch = qtest_get_arch();
>>      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 ? : "");
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        return qtest_pc_boot(cmd, extra_opts ? : "");
>> +    }
>> +    if (strcmp(arch, "ppc64") == 0) {
>> +        return qtest_spapr_boot(cmd, extra_opts ? : "");
>> +    }
>> +    return NULL;
>>  }
>>  
>>  static void qvirtio_scsi_stop(QOSState *qs)
> 

Thanks,
Laurent

Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] tests: enable virtio tests on SPAPR

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3707 bytes --]

On Fri, Sep 30, 2016 at 08:59:39AM +0200, Laurent Vivier wrote:
> 
> 
> On 30/09/2016 03:30, David Gibson wrote:
> > On Thu, Sep 29, 2016 at 07:15:07PM +0200, Laurent Vivier wrote:
> >> but disable MSI-X tests on SPAPR as we can't check the result
> >> (the memory region used on PC is not readable on SPAPR).
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>  tests/Makefile.include    |  3 ++-
> >>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
> >>  tests/virtio-9p-test.c    | 11 ++++++++++-
> >>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
> >>  tests/virtio-net-test.c   | 17 +++++++++++++++--
> >>  tests/virtio-rng-test.c   |  7 ++++++-
> >>  tests/virtio-scsi-test.c  | 10 +++++++++-
> >>  7 files changed, 79 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/tests/Makefile.include b/tests/Makefile.include
> >> index c46a32d..1e4a3d5 100644
> >> --- a/tests/Makefile.include
> >> +++ b/tests/Makefile.include
> >> @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF)
> >>  gcov-files-ppc64-y += hw/usb/hcd-uhci.c
> >>  check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF)
> >>  gcov-files-ppc64-y += hw/usb/hcd-xhci.c
> >> +check-qtest-ppc64-y += $(check-qtest-virtio-y)
> >>  
> >>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
> >>  
> >> @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o
> >>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
> >>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
> >>  libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
> >> -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
> >> +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
> >>  
> >>  tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
> >>  tests/rtc-test$(EXESUF): tests/rtc-test.o
> >> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> >> index 6e005c1..ed81df6 100644
> >> --- a/tests/libqos/virtio-pci.c
> >> +++ b/tests/libqos/virtio-pci.c
> >> @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
> >>      return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr);
> >>  }
> >>  
> >> +/* PCI is always read in little-endian order
> >> + * but virtio ( < 1.0) is in guest order
> >> + * so with a big-endian guest the order has been reversed
> >> + * reverse it again
> >> + */
> >> +
> >>  static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
> >>  {
> >>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> >> -    return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
> >> +    uint16_t value;
> >> +
> >> +    value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
> >> +    if (target_big_endian()) {
> >> +        value = bswap16(value);
> >> +    }
> > 
> > Don't you need some sort of test to distinguish the virtio < 1.0 and
> > virtio-1.0 cases?
> 
> yes, but for the moment we don't test virtio-1.0, we will add the test
> when will support it.
> 
> http://wiki.qemu.org/Outreachy_2016_DecemberMarch#VIRTIO_1.0_support_in_libqos

Ok.  Please drop a comment in though, to make one less mystery for
whoever does end up adding the virtio 1.0 support.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 3/3] tests: enable virtio tests on SPAPR

[Greg Kurz]

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

> but disable MSI-X tests on SPAPR as we can't check the result
> (the memory region used on PC is not readable on SPAPR).
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tests/Makefile.include    |  3 ++-
>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
>  tests/virtio-9p-test.c    | 11 ++++++++++-
>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
>  tests/virtio-net-test.c   | 17 +++++++++++++++--
>  tests/virtio-rng-test.c   |  7 ++++++-
>  tests/virtio-scsi-test.c  | 10 +++++++++-
>  7 files changed, 79 insertions(+), 13 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index c46a32d..1e4a3d5 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -278,6 +278,7 @@ check-qtest-ppc64-y += tests/usb-hcd-uhci-test$(EXESUF)
>  gcov-files-ppc64-y += hw/usb/hcd-uhci.c
>  check-qtest-ppc64-y += tests/usb-hcd-xhci-test$(EXESUF)
>  gcov-files-ppc64-y += hw/usb/hcd-xhci.c
> +check-qtest-ppc64-y += $(check-qtest-virtio-y)
>  
>  check-qtest-sh4-y = tests/endianness-test$(EXESUF)
>  
> @@ -604,7 +605,7 @@ libqos-pc-obj-y += tests/libqos/ahci.o
>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
>  libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
> -libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
> +libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
>  
>  tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
>  tests/rtc-test$(EXESUF): tests/rtc-test.o
> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> index 6e005c1..ed81df6 100644
> --- a/tests/libqos/virtio-pci.c
> +++ b/tests/libqos/virtio-pci.c
> @@ -68,16 +68,34 @@ static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
>      return qpci_io_readb(dev->pdev, (void *)(uintptr_t)addr);
>  }
>  
> +/* PCI is always read in little-endian order
> + * but virtio ( < 1.0) is in guest order
> + * so with a big-endian guest the order has been reversed
> + * reverse it again
> + */
> +
>  static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, uint64_t addr)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    return qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
> +    uint16_t value;
> +
> +    value = qpci_io_readw(dev->pdev, (void *)(uintptr_t)addr);
> +    if (target_big_endian()) {
> +        value = bswap16(value);
> +    }
> +    return value;
>  }
>  
>  static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, uint64_t addr)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    return qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
> +    uint32_t value;
> +
> +    value = qpci_io_readl(dev->pdev, (void *)(uintptr_t)addr);
> +    if (target_big_endian()) {
> +        value = bswap32(value);
> +    }
> +    return value;
>  }
>  
>  static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t addr)
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index 28d7f5b..a73bccb 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -11,6 +11,7 @@
>  #include "libqtest.h"
>  #include "qemu-common.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -22,12 +23,20 @@ static char *test_share;
>  
>  static QOSState *qvirtio_9p_start(void)
>  {
> +    const char *arch = qtest_get_arch();
> +    QOSState *qs = NULL;
>      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";
>  
> -    return qtest_pc_boot(cmd, test_share, mount_tag);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qs = qtest_pc_boot(cmd, test_share, mount_tag);
> +    } else if (strcmp(arch, "ppc64") == 0) {
> +        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
> +    }
> +

What about introducing a qtest_arch_boot() helper that does ^^ and

} else {
    g_printerr("qtest_arch_boot() not supported for arch %s\n",
               qtest_get_arch());
    exit(EXIT_FAILURE);
}

> +    return qs;
>  }
>  
>  static void qvirtio_9p_stop(QOSState *qs)
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index a4de3e4..a032f8e 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -11,6 +11,7 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "libqos/virtio-mmio.h"
> @@ -58,6 +59,7 @@ static char *drive_create(void)
>  
>  static QOSState *pci_test_start(void)
>  {
> +    const char *arch = qtest_get_arch();
>      QOSState *qs = NULL;
>      char *tmp_path;
>      const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
> @@ -67,7 +69,11 @@ static QOSState *pci_test_start(void)
>  
>      tmp_path = drive_create();
>  
> -    qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qs = qtest_pc_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
> +    } else if (strcmp(arch, "ppc64") == 0) {
> +        qs = qtest_spapr_boot(cmd, tmp_path, PCI_SLOT, PCI_FN);
> +    }
>      unlink(tmp_path);
>      g_free(tmp_path);
>      return qs;
> @@ -676,6 +682,7 @@ static void pci_hotplug(void)
>  {
>      QVirtioPCIDevice *dev;
>      QOSState *qs;
> +    const char *arch = qtest_get_arch();
>  
>      qs = pci_test_start();
>      g_assert(qs);
> @@ -690,7 +697,9 @@ static void pci_hotplug(void)
>      g_free(dev);
>  
>      /* unplug secondary disk */
> -    qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP);
> +    }
>      qtest_shutdown(qs);
>  }
>  
> @@ -741,12 +750,15 @@ int main(int argc, char **argv)
>  
>      g_test_init(&argc, &argv, NULL);
>  
> -    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0 ||
> +        strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) {
>          qtest_add_func("/virtio/blk/pci/basic", pci_basic);
>          qtest_add_func("/virtio/blk/pci/indirect", pci_indirect);
>          qtest_add_func("/virtio/blk/pci/config", pci_config);
> -        qtest_add_func("/virtio/blk/pci/msix", pci_msix);
> -        qtest_add_func("/virtio/blk/pci/idx", pci_idx);
> +        if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +            qtest_add_func("/virtio/blk/pci/msix", pci_msix);
> +            qtest_add_func("/virtio/blk/pci/idx", pci_idx);
> +        }
>          qtest_add_func("/virtio/blk/pci/hotplug", pci_hotplug);
>      } else if (strcmp(arch, "arm") == 0) {
>          qtest_add_func("/virtio/blk/mmio/basic", mmio_basic);
> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index 3e83685..14f2920 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -13,6 +13,7 @@
>  #include "qemu/sockets.h"
>  #include "qemu/iov.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "qemu/bswap.h"
> @@ -52,10 +53,17 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
>  
>  static QOSState *pci_test_start(int socket)
>  {
> +    const char *arch = qtest_get_arch();
>      const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
>                        "virtio-net-pci,netdev=hs0";
>  
> -    return qtest_pc_boot(cmd, socket);
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return qtest_pc_boot(cmd, socket);
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return qtest_spapr_boot(cmd, socket);
> +    }
> +    return NULL;
>  }
>  
>  static void driver_init(const QVirtioBus *bus, QVirtioDevice *dev)
> @@ -236,10 +244,15 @@ static void pci_basic(gconstpointer data)
>  
>  static void hotplug(void)
>  {
> +    const char *arch = qtest_get_arch();
> +
>      qtest_start("-device virtio-net-pci");
>  
>      qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL);
> -    qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
> +    }
>  
>      test_end();
>  }
> diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c
> index e1b2640..dcecf77 100644
> --- a/tests/virtio-rng-test.c
> +++ b/tests/virtio-rng-test.c
> @@ -20,8 +20,13 @@ static void pci_nop(void)
>  
>  static void hotplug(void)
>  {
> +    const char *arch = qtest_get_arch();
> +
>      qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL);
> -    qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
> +    }
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
> index 721ae1f..dd5457b 100644
> --- a/tests/virtio-scsi-test.c
> +++ b/tests/virtio-scsi-test.c
> @@ -12,6 +12,7 @@
>  #include "libqtest.h"
>  #include "block/scsi.h"
>  #include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
>  #include "libqos/virtio.h"
>  #include "libqos/virtio-pci.h"
>  #include "standard-headers/linux/virtio_ids.h"
> @@ -33,11 +34,18 @@ typedef struct {
>  
>  static QOSState *qvirtio_scsi_start(const char *extra_opts)
>  {
> +    const char *arch = qtest_get_arch();
>      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 ? : "");
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return qtest_pc_boot(cmd, extra_opts ? : "");
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return qtest_spapr_boot(cmd, extra_opts ? : "");
> +    }
> +    return NULL;
>  }
>  
>  static void qvirtio_scsi_stop(QOSState *qs)

Re: [Qemu-devel] [PATCH 3/3] tests: enable virtio tests on SPAPR

[Laurent Vivier]

On 30/09/2016 12:18, Greg Kurz wrote:
> On Thu, 29 Sep 2016 19:15:07 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> but disable MSI-X tests on SPAPR as we can't check the result
>> (the memory region used on PC is not readable on SPAPR).
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  tests/Makefile.include    |  3 ++-
>>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
>>  tests/virtio-9p-test.c    | 11 ++++++++++-
>>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
>>  tests/virtio-net-test.c   | 17 +++++++++++++++--
>>  tests/virtio-rng-test.c   |  7 ++++++-
>>  tests/virtio-scsi-test.c  | 10 +++++++++-
>>  7 files changed, 79 insertions(+), 13 deletions(-)
...
>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
>> index 28d7f5b..a73bccb 100644
>> --- a/tests/virtio-9p-test.c
>> +++ b/tests/virtio-9p-test.c
>> @@ -11,6 +11,7 @@
>>  #include "libqtest.h"
>>  #include "qemu-common.h"
>>  #include "libqos/libqos-pc.h"
>> +#include "libqos/libqos-spapr.h"
>>  #include "libqos/virtio.h"
>>  #include "libqos/virtio-pci.h"
>>  #include "standard-headers/linux/virtio_ids.h"
>> @@ -22,12 +23,20 @@ static char *test_share;
>>  
>>  static QOSState *qvirtio_9p_start(void)
>>  {
>> +    const char *arch = qtest_get_arch();
>> +    QOSState *qs = NULL;
>>      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";
>>  
>> -    return qtest_pc_boot(cmd, test_share, mount_tag);
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        qs = qtest_pc_boot(cmd, test_share, mount_tag);
>> +    } else if (strcmp(arch, "ppc64") == 0) {
>> +        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
>> +    }
>> +
> 
> What about introducing a qtest_arch_boot() helper that does ^^ and
> 
> } else {
>     g_printerr("qtest_arch_boot() not supported for arch %s\n",
>                qtest_get_arch());
>     exit(EXIT_FAILURE);
> }
> 

The problem with adding a function like that is it will pull
$(libqos-pc-obj-y) and  $(libqos-spapr-obj-y) for every tests using it,
and for the moment we are pulling pc or spapr objects only if we need
them for the given test.

I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't
have a generic qtest_boot() calling the architecture specific function.

I cc: John Snow as he has written the initial code for this.
("90e5add libqos: add pc specific interface")

Laurent

Re: [Qemu-devel] [PATCH 3/3] tests: enable virtio tests on SPAPR

[Greg Kurz]

On Fri, 30 Sep 2016 12:30:08 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 30/09/2016 12:18, Greg Kurz wrote:
> > On Thu, 29 Sep 2016 19:15:07 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> but disable MSI-X tests on SPAPR as we can't check the result
> >> (the memory region used on PC is not readable on SPAPR).
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>  tests/Makefile.include    |  3 ++-
> >>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
> >>  tests/virtio-9p-test.c    | 11 ++++++++++-
> >>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
> >>  tests/virtio-net-test.c   | 17 +++++++++++++++--
> >>  tests/virtio-rng-test.c   |  7 ++++++-
> >>  tests/virtio-scsi-test.c  | 10 +++++++++-
> >>  7 files changed, 79 insertions(+), 13 deletions(-)  
> ...
> >> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> >> index 28d7f5b..a73bccb 100644
> >> --- a/tests/virtio-9p-test.c
> >> +++ b/tests/virtio-9p-test.c
> >> @@ -11,6 +11,7 @@
> >>  #include "libqtest.h"
> >>  #include "qemu-common.h"
> >>  #include "libqos/libqos-pc.h"
> >> +#include "libqos/libqos-spapr.h"
> >>  #include "libqos/virtio.h"
> >>  #include "libqos/virtio-pci.h"
> >>  #include "standard-headers/linux/virtio_ids.h"
> >> @@ -22,12 +23,20 @@ static char *test_share;
> >>  
> >>  static QOSState *qvirtio_9p_start(void)
> >>  {
> >> +    const char *arch = qtest_get_arch();
> >> +    QOSState *qs = NULL;
> >>      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";
> >>  
> >> -    return qtest_pc_boot(cmd, test_share, mount_tag);
> >> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >> +        qs = qtest_pc_boot(cmd, test_share, mount_tag);
> >> +    } else if (strcmp(arch, "ppc64") == 0) {
> >> +        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
> >> +    }
> >> +  
> > 
> > What about introducing a qtest_arch_boot() helper that does ^^ and
> > 
> > } else {
> >     g_printerr("qtest_arch_boot() not supported for arch %s\n",
> >                qtest_get_arch());
> >     exit(EXIT_FAILURE);
> > }
> >   
> 
> The problem with adding a function like that is it will pull
> $(libqos-pc-obj-y) and  $(libqos-spapr-obj-y) for every tests using it,
> and for the moment we are pulling pc or spapr objects only if we need
> them for the given test.
> 
> I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't
> have a generic qtest_boot() calling the architecture specific function.
> 

Ok I see the problem... this would call for a libqos-all-arch-obj-y variable
to be used by specific tests maybe.

And by the way, even if not adding such generic function, each test that can
only run on specific archs should have an g_printerr()+exit() path, instead
of calling g_assert(), for the same reason it was done for rtas-test.c.

> I cc: John Snow as he has written the initial code for this.
> ("90e5add libqos: add pc specific interface")
> 
> Laurent
> 
> 

Cheers.

--
Greg

Re: [Qemu-devel] [PATCH 3/3] tests: enable virtio tests on SPAPR

[John Snow]

On 09/30/2016 06:30 AM, Laurent Vivier wrote:
>
>
> On 30/09/2016 12:18, Greg Kurz wrote:
>> On Thu, 29 Sep 2016 19:15:07 +0200
>> Laurent Vivier <lvivier@redhat.com> wrote:
>>
>>> but disable MSI-X tests on SPAPR as we can't check the result
>>> (the memory region used on PC is not readable on SPAPR).
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>  tests/Makefile.include    |  3 ++-
>>>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
>>>  tests/virtio-9p-test.c    | 11 ++++++++++-
>>>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
>>>  tests/virtio-net-test.c   | 17 +++++++++++++++--
>>>  tests/virtio-rng-test.c   |  7 ++++++-
>>>  tests/virtio-scsi-test.c  | 10 +++++++++-
>>>  7 files changed, 79 insertions(+), 13 deletions(-)
> ...
>>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
>>> index 28d7f5b..a73bccb 100644
>>> --- a/tests/virtio-9p-test.c
>>> +++ b/tests/virtio-9p-test.c
>>> @@ -11,6 +11,7 @@
>>>  #include "libqtest.h"
>>>  #include "qemu-common.h"
>>>  #include "libqos/libqos-pc.h"
>>> +#include "libqos/libqos-spapr.h"
>>>  #include "libqos/virtio.h"
>>>  #include "libqos/virtio-pci.h"
>>>  #include "standard-headers/linux/virtio_ids.h"
>>> @@ -22,12 +23,20 @@ static char *test_share;
>>>
>>>  static QOSState *qvirtio_9p_start(void)
>>>  {
>>> +    const char *arch = qtest_get_arch();
>>> +    QOSState *qs = NULL;
>>>      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";
>>>
>>> -    return qtest_pc_boot(cmd, test_share, mount_tag);
>>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>> +        qs = qtest_pc_boot(cmd, test_share, mount_tag);
>>> +    } else if (strcmp(arch, "ppc64") == 0) {
>>> +        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
>>> +    }
>>> +
>>
>> What about introducing a qtest_arch_boot() helper that does ^^ and
>>
>> } else {
>>     g_printerr("qtest_arch_boot() not supported for arch %s\n",
>>                qtest_get_arch());
>>     exit(EXIT_FAILURE);
>> }
>>
>
> The problem with adding a function like that is it will pull
> $(libqos-pc-obj-y) and  $(libqos-spapr-obj-y) for every tests using it,
> and for the moment we are pulling pc or spapr objects only if we need
> them for the given test.
>
> I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't
> have a generic qtest_boot() calling the architecture specific function.
>
> I cc: John Snow as he has written the initial code for this.
> ("90e5add libqos: add pc specific interface")
>
> Laurent
>
>

This was a while ago for me (and I was brand new to QEMU!), but that 
sounds about right. I wasn't able to reason about requirements for other 
architectures, so we made the PC-specific frontend to do the 
configuration for us. libqos.o does not pull in any of the PC-specific 
requirements as a result. Neither does the allocator.

I didn't necessarily design it to be like this, just a path of least 
resistance type of thing.

You probably could make an ArchOps callback structure if you wanted and 
pass that along to a generic bootup function to avoid the linking issues 
if you wanted a one-size-fits-all initialization function.

--js

Re: [Qemu-devel] [PATCH 3/3] tests: enable virtio tests on SPAPR

[Greg Kurz]

On Fri, 30 Sep 2016 11:19:39 -0400
John Snow <jsnow@redhat.com> wrote:

> On 09/30/2016 06:30 AM, Laurent Vivier wrote:
> >
> >
> > On 30/09/2016 12:18, Greg Kurz wrote:  
> >> On Thu, 29 Sep 2016 19:15:07 +0200
> >> Laurent Vivier <lvivier@redhat.com> wrote:
> >>  
> >>> but disable MSI-X tests on SPAPR as we can't check the result
> >>> (the memory region used on PC is not readable on SPAPR).
> >>>
> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>> ---
> >>>  tests/Makefile.include    |  3 ++-
> >>>  tests/libqos/virtio-pci.c | 22 ++++++++++++++++++++--
> >>>  tests/virtio-9p-test.c    | 11 ++++++++++-
> >>>  tests/virtio-blk-test.c   | 22 +++++++++++++++++-----
> >>>  tests/virtio-net-test.c   | 17 +++++++++++++++--
> >>>  tests/virtio-rng-test.c   |  7 ++++++-
> >>>  tests/virtio-scsi-test.c  | 10 +++++++++-
> >>>  7 files changed, 79 insertions(+), 13 deletions(-)  
> > ...  
> >>> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> >>> index 28d7f5b..a73bccb 100644
> >>> --- a/tests/virtio-9p-test.c
> >>> +++ b/tests/virtio-9p-test.c
> >>> @@ -11,6 +11,7 @@
> >>>  #include "libqtest.h"
> >>>  #include "qemu-common.h"
> >>>  #include "libqos/libqos-pc.h"
> >>> +#include "libqos/libqos-spapr.h"
> >>>  #include "libqos/virtio.h"
> >>>  #include "libqos/virtio-pci.h"
> >>>  #include "standard-headers/linux/virtio_ids.h"
> >>> @@ -22,12 +23,20 @@ static char *test_share;
> >>>
> >>>  static QOSState *qvirtio_9p_start(void)
> >>>  {
> >>> +    const char *arch = qtest_get_arch();
> >>> +    QOSState *qs = NULL;
> >>>      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";
> >>>
> >>> -    return qtest_pc_boot(cmd, test_share, mount_tag);
> >>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >>> +        qs = qtest_pc_boot(cmd, test_share, mount_tag);
> >>> +    } else if (strcmp(arch, "ppc64") == 0) {
> >>> +        qs = qtest_spapr_boot(cmd, test_share, mount_tag);
> >>> +    }
> >>> +  
> >>
> >> What about introducing a qtest_arch_boot() helper that does ^^ and
> >>
> >> } else {
> >>     g_printerr("qtest_arch_boot() not supported for arch %s\n",
> >>                qtest_get_arch());
> >>     exit(EXIT_FAILURE);
> >> }
> >>  
> >
> > The problem with adding a function like that is it will pull
> > $(libqos-pc-obj-y) and  $(libqos-spapr-obj-y) for every tests using it,
> > and for the moment we are pulling pc or spapr objects only if we need
> > them for the given test.
> >
> > I think it explains why qtest_pc_boot() calls qtest_vboot() and we don't
> > have a generic qtest_boot() calling the architecture specific function.
> >
> > I cc: John Snow as he has written the initial code for this.
> > ("90e5add libqos: add pc specific interface")
> >
> > Laurent
> >
> >  
> 
> This was a while ago for me (and I was brand new to QEMU!), but that 
> sounds about right. I wasn't able to reason about requirements for other 
> architectures, so we made the PC-specific frontend to do the 
> configuration for us. libqos.o does not pull in any of the PC-specific 
> requirements as a result. Neither does the allocator.
> 
> I didn't necessarily design it to be like this, just a path of least 
> resistance type of thing.
> 
> You probably could make an ArchOps callback structure if you wanted and 
> pass that along to a generic bootup function to avoid the linking issues 
> if you wanted a one-size-fits-all initialization function.
> 
> --js

It looks like libqos may be divided in 3 families:
- base libqos for tests that don't need platform specific support (basically
  what we currently have in libqos-y)
- platfrom specific libqos for platform specific tests (libqos-pc, libqos-spapr)
- one-size-fits-all libqos for tests that should run on several platforms

Makes sense ?

--
Greg