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 v2 1/3] tests: use qtest_pc_boot()/qtest_shutdown() in virtio tests
Date: Sat, 1 Oct 2016 12:57:45 +0200 [thread overview]
Message-ID: <20161001125745.23f7d129@bahia> (raw)
In-Reply-To: <1475245152-31411-2-git-send-email-lvivier@redhat.com>
On Fri, 30 Sep 2016 16:19:10 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> This patch replaces calls to qtest_start() and qtest_end() by
> calls to qtest_pc_boot() and qtest_shutdown().
>
> This allows to initialize memory allocator and PCI interface
> functions. This will ease to enable virtio tests on other
> architectures by only adding a specific qtest_XXX_boot() (like
> qtest_spapr_boot()).
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
Hi Laurent,
Please find some remarks below.
> tests/libqos/libqos.c | 2 +
> tests/rtas-test.c | 1 -
> tests/virtio-9p-test.c | 51 +++++++---------
> tests/virtio-blk-test.c | 150 ++++++++++++++++++++---------------------------
> tests/virtio-net-test.c | 39 +++++-------
> tests/virtio-scsi-test.c | 67 +++++++++------------
> 6 files changed, 131 insertions(+), 179 deletions(-)
>
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index 7abb482..6bd4c19 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -17,6 +17,8 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap)
>
> struct QOSState *qs = g_malloc(sizeof(QOSState));
>
> + g_assert_nonnull(qs);
> +
g_malloc() does that already.
With this change dropped, you get:
Reviewed-by: Greg Kurz <groug@kaod.org>
The other remarks call for extra patches actually.
> cmdline = g_strdup_vprintf(cmdline_fmt, ap);
> qs->qts = qtest_start(cmdline);
> qs->ops = ops;
> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
> index ba0867a..276c87e 100644
> --- a/tests/rtas-test.c
> +++ b/tests/rtas-test.c
> @@ -14,7 +14,6 @@ static void test_rtas_get_time_of_day(void)
> time_t t1, t2;
>
> qs = qtest_spapr_boot("-machine pseries");
> - g_assert(qs != NULL);
>
Maybe worth to document in libqos headers that the qtest_*boot() routines
never return NULL ? This could be done in a followup patch.
> t1 = time(NULL);
> ret = qrtas_get_time_of_day(qs->alloc, &tm, &ns);
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index e8b2196..7698014 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -10,62 +10,56 @@
> #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_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();
> + 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 +69,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 +88,10 @@ 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();
> + 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 +108,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..f4eb66a 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;
> 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,34 @@ 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();
> + 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,
> - (uint64_t)(uintptr_t)addr);
> + 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 +312,9 @@ static void pci_indirect(void)
> uint8_t status;
> char *data;
>
> - bus = pci_test_start();
> + qs = pci_test_start();
>
> - 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 +330,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 +341,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 +357,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 +366,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 +387,26 @@ 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();
>
> - 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 +427,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 +446,12 @@ static void pci_msix(void)
> uint8_t status;
> char *data;
>
> - bus = pci_test_start();
> - alloc = pc_alloc_init();
> + qs = pci_test_start();
>
> - 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 +468,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 +489,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 +504,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 +512,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 +534,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 +558,12 @@ static void pci_idx(void)
> uint8_t status;
> char *data;
>
> - bus = pci_test_start();
> - alloc = pc_alloc_init();
> + qs = pci_test_start();
>
> - 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 +580,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 +592,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 +611,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 +628,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 +636,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 +657,35 @@ 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();
>
> /* 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 +724,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..13bb889 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,25 @@ 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]);
> + 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..55ff064 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,11 @@ 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);
> + qvirtio_scsi_stop(vs->qs);
As mentioned in another mail, vs is allocated in qvirtio_scsi_pci_init().
It should be freed here... but I guess this belongs to a preparatory patch.
Cheers.
--
Greg
> }
>
> static uint64_t qvirtio_scsi_alloc(QVirtIOSCSI *vs, size_t alloc_size,
> @@ -71,7 +62,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 +119,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 +136,11 @@ 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");
> + 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,18 @@ 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);
> + 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");
> response = qmp("{\"execute\": \"device_add\","
> " \"arguments\": {"
> " \"driver\": \"scsi-hd\","
> @@ -214,7 +209,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 +225,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 +234,6 @@ 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();
> }
>
> int main(int argc, char **argv)
next prev parent reply other threads:[~2016-10-01 10:58 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-30 14:19 [Qemu-devel] [PATCH v2 0/3] tests: enable virtio tests on SPAPR Laurent Vivier
2016-09-30 14:19 ` [Qemu-devel] [PATCH v2 1/3] tests: use qtest_pc_boot()/qtest_shutdown() in virtio tests Laurent Vivier
2016-10-01 10:57 ` Greg Kurz [this message]
2016-09-30 14:19 ` [Qemu-devel] [PATCH v2 2/3] qtest: evaluate endianness of the target in qtest_init() Laurent Vivier
2016-09-30 14:19 ` [Qemu-devel] [PATCH v2 3/3] tests: enable virtio tests on SPAPR Laurent Vivier
2016-10-01 12:29 ` Greg Kurz
2016-10-01 16:23 ` Thomas Huth
2016-10-01 18:23 ` Mark Cave-Ayland
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=20161001125745.23f7d129@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.