From: Greg Kurz <groug@kaod.org>
To: Laurent Vivier <lvivier@redhat.com>
Cc: qemu-devel@nongnu.org, dgibson@redhat.com, thuth@redhat.com,
qemu-ppc@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests
Date: Fri, 30 Sep 2016 10:33:41 +0200 [thread overview]
Message-ID: <20160930103341.3b2bee36@bahia> (raw)
In-Reply-To: <1475169307-1510-2-git-send-email-lvivier@redhat.com>
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
next prev parent reply other threads:[~2016-09-30 8:34 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-29 17:15 [Qemu-devel] [PATCH 0/3] tests: enable virtio tests on SPAPR Laurent Vivier
2016-09-29 17:15 ` [Qemu-devel] [PATCH 1/3] tests: use qtest_pc_boot()/qtest_pc_shutdown() in virtio tests Laurent Vivier
2016-09-30 1:27 ` [Qemu-devel] [Qemu-ppc] " David Gibson
2016-09-30 6:56 ` Laurent Vivier
2016-09-30 8:33 ` Greg Kurz [this message]
2016-09-30 9:13 ` [Qemu-devel] " Laurent Vivier
2016-09-30 10:29 ` Greg Kurz
2016-09-30 10:33 ` Laurent Vivier
2016-09-30 9:56 ` Laurent Vivier
2016-09-30 10:34 ` Greg Kurz
2016-09-29 17:15 ` [Qemu-devel] [PATCH 2/3] qtest: evaluate endianness of the target in qtest_init() Laurent Vivier
2016-09-30 1:29 ` [Qemu-devel] [Qemu-ppc] " David Gibson
2016-09-29 17:15 ` [Qemu-devel] [PATCH 3/3] tests: enable virtio tests on SPAPR Laurent Vivier
2016-09-30 1:30 ` [Qemu-devel] [Qemu-ppc] " David Gibson
2016-09-30 6:59 ` Laurent Vivier
2016-09-30 9:06 ` David Gibson
2016-09-30 10:18 ` [Qemu-devel] " Greg Kurz
2016-09-30 10:30 ` Laurent Vivier
2016-09-30 10:52 ` Greg Kurz
2016-09-30 15:19 ` John Snow
2016-10-01 12:14 ` Greg Kurz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160930103341.3b2bee36@bahia \
--to=groug@kaod.org \
--cc=dgibson@redhat.com \
--cc=lvivier@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.