[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown(
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-ppc] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests |
Date: |
Fri, 30 Sep 2016 11:13:33 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 30/09/2016 10:33, Greg Kurz wrote:
> On Thu, 29 Sep 2016 19:15:05 +0200
> Laurent Vivier <address@hidden> wrote:
>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>> 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