Re: [Qemu-devel] [PATCH v4 40/47] tests: add ivshmem qtest

[Marc-André Lureau]

----- Original Message -----
> On 24.09.2015 13:37, address@hidden wrote:
> > From: Marc-André Lureau <address@hidden>
> > 
> > Adds 4 ivshmemtests:
> > - single qemu instance and basic IO
> > - pair of instances, check memory sharing
> > - pair of instances with server, and MSIX
> > - hot plug/unplug
> > 
> > A temporary shm is created as well as a directory to place server
> > socket, both should be clear on exit and abort.
> 
> Just one comment below, but in short, what about supporting TMPDIR?

I probably followed the trend in existing code.

Imho this should be a seperate commit for the whole tests/, do you volunteer? :)


> Also, have you considered SIGPIPE for your peers reading and writing to/from
> pipes?
> Is the default behavior the one you want?
> 

What do you mean? if one ends disconnect I should test this case?

Imho this is a seperate test I didn't consider, it could be added in a 
different patch.

> Ciao,
> 
> Claudio
> 
> 
> > 
> > Cc: Cam Macdonell <address@hidden>
> > CC: Andreas Färber <address@hidden>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >  tests/Makefile       |   3 +
> >  tests/ivshmem-test.c | 481
> >  +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 484 insertions(+)
> >  create mode 100644 tests/ivshmem-test.c
> > 
> > diff --git a/tests/Makefile b/tests/Makefile
> > index 4063639..7e6ac43 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -146,6 +146,8 @@ gcov-files-pci-y += hw/display/virtio-gpu-pci.c
> >  gcov-files-pci-$(CONFIG_VIRTIO_VGA) += hw/display/virtio-vga.c
> >  check-qtest-pci-y += tests/intel-hda-test$(EXESUF)
> >  gcov-files-pci-y += hw/audio/intel-hda.c hw/audio/hda-codec.c
> > +check-qtest-pci-$(CONFIG_LINUX) += tests/ivshmem-test$(EXESUF)
> > +gcov-files-pci-y += hw/misc/ivshmem.c
> >  
> >  check-qtest-i386-y = tests/endianness-test$(EXESUF)
> >  check-qtest-i386-y += tests/fdc-test$(EXESUF)
> > @@ -435,6 +437,7 @@ tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o
> > qemu-char.o qemu-timer.o
> >  tests/qemu-iotests/socket_scm_helper$(EXESUF):
> >  tests/qemu-iotests/socket_scm_helper.o
> >  tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o $(test-util-obj-y)
> >  tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o
> >  $(test-block-obj-y)
> > +tests/ivshmem-test$(EXESUF): tests/ivshmem-test.o
> > contrib/ivshmem-server/ivshmem-server.o $(libqos-pc-obj-y)
> >  
> >  ifeq ($(CONFIG_POSIX),y)
> >  LIBS += -lutil
> > diff --git a/tests/ivshmem-test.c b/tests/ivshmem-test.c
> > new file mode 100644
> > index 0000000..097de15
> > --- /dev/null
> > +++ b/tests/ivshmem-test.c
> > @@ -0,0 +1,481 @@
> > +/*
> > + * QTest testcase for ivshmem
> > + *
> > + * Copyright (c) 2015 Red Hat, Inc.
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <glib.h>
> > +#include <glib/gstdio.h>
> > +#include <string.h>
> > +#include <sys/mman.h>
> > +#include <unistd.h>
> > +#include "contrib/ivshmem-server/ivshmem-server.h"
> > +#include "libqos/pci-pc.h"
> > +#include "libqtest.h"
> > +#include "qemu/osdep.h"
> > +#include <stdlib.h>
> > +
> > +#if GLIB_CHECK_VERSION(2, 32, 0)
> > +#define HAVE_THREAD_NEW
> > +#endif
> > +
> > +#define TMPSHMSIZE (1 << 20)
> > +static char *tmpshm;
> > +static void *tmpshmem;
> > +static char *tmpdir;
> > +static char *tmpserver;
> > +
> > +static void save_fn(QPCIDevice *dev, int devfn, void *data)
> > +{
> > +    QPCIDevice **pdev = (QPCIDevice **) data;
> > +
> > +    *pdev = dev;
> > +}
> > +
> > +static QPCIDevice *get_device(void)
> > +{
> > +    QPCIDevice *dev;
> > +    QPCIBus *pcibus;
> > +
> > +    pcibus = qpci_init_pc();
> > +    qpci_device_foreach(pcibus, 0x1af4, 0x1110, save_fn, &dev);
> > +    g_assert(dev != NULL);
> > +
> > +    return dev;
> > +}
> > +
> > +typedef struct _IVState {
> > +    QTestState *qtest;
> > +    void *reg_base, *mem_base;
> > +    QPCIDevice *dev;
> > +} IVState;
> > +
> > +enum Reg {
> > +    INTRMASK = 0,
> > +    INTRSTATUS = 4,
> > +    IVPOSITION = 8,
> > +    DOORBELL = 12,
> > +};
> > +
> > +static const char* reg2str(enum Reg reg) {
> > +    switch (reg) {
> > +    case INTRMASK:
> > +        return "IntrMask";
> > +    case INTRSTATUS:
> > +        return "IntrStatus";
> > +    case IVPOSITION:
> > +        return "IVPosition";
> > +    case DOORBELL:
> > +        return "DoorBell";
> > +    default:
> > +        return NULL;
> > +    }
> > +}
> > +
> > +static inline unsigned in_reg(IVState *s, enum Reg reg)
> > +{
> > +    const char *name = reg2str(reg);
> > +    QTestState *qtest = global_qtest;
> > +    unsigned res;
> > +
> > +    global_qtest = s->qtest;
> > +    res = qpci_io_readl(s->dev, s->reg_base + reg);
> > +    g_test_message("*%s -> %x\n", name, res);
> > +    global_qtest = qtest;
> > +
> > +    return res;
> > +}
> > +
> > +static inline void out_reg(IVState *s, enum Reg reg, unsigned v)
> > +{
> > +    const char *name = reg2str(reg);
> > +    QTestState *qtest = global_qtest;
> > +
> > +    global_qtest = s->qtest;
> > +    g_test_message("%x -> *%s\n", v, name);
> > +    qpci_io_writel(s->dev, s->reg_base + reg, v);
> > +    global_qtest = qtest;
> > +}
> > +
> > +static void setup_vm_cmd(IVState *s, const char *cmd, bool msix)
> > +{
> > +    uint64_t barsize;
> > +
> > +    s->qtest = qtest_start(cmd);
> > +
> > +    s->dev = get_device();
> > +
> > +    /* FIXME: other bar order fails, mappings changes */
> > +    s->mem_base = qpci_iomap(s->dev, 2, &barsize);
> > +    g_assert_nonnull(s->mem_base);
> > +    g_assert_cmpuint(barsize, ==, TMPSHMSIZE);
> > +
> > +    if (msix) {
> > +        qpci_msix_enable(s->dev);
> > +    }
> > +
> > +    s->reg_base = qpci_iomap(s->dev, 0, &barsize);
> > +    g_assert_nonnull(s->reg_base);
> > +    g_assert_cmpuint(barsize, ==, 256);
> > +
> > +    qpci_device_enable(s->dev);
> > +}
> > +
> > +static void setup_vm(IVState *s)
> > +{
> > +    char *cmd = g_strdup_printf("-device ivshmem,shm=%s,size=1M", tmpshm);
> > +
> > +    setup_vm_cmd(s, cmd, false);
> > +
> > +    g_free(cmd);
> > +}
> > +
> > +static void test_ivshmem_single(void)
> > +{
> > +    IVState state, *s;
> > +    uint32_t data[1024];
> > +    int i;
> > +
> > +    setup_vm(&state);
> > +    s = &state;
> > +
> > +    /* valid io */
> > +    out_reg(s, INTRMASK, 0);
> > +    in_reg(s, INTRSTATUS);
> > +    in_reg(s, IVPOSITION);
> > +
> > +    out_reg(s, INTRMASK, 0xffffffff);
> > +    g_assert_cmpuint(in_reg(s, INTRMASK), ==, 0xffffffff);
> > +    out_reg(s, INTRSTATUS, 1);
> > +    /* XXX: intercept IRQ, not seen in resp */
> > +    g_assert_cmpuint(in_reg(s, INTRSTATUS), ==, 1);
> > +
> > +    /* invalid io */
> > +    out_reg(s, IVPOSITION, 1);
> > +    out_reg(s, DOORBELL, 8 << 16);
> > +
> > +    for (i = 0; i < G_N_ELEMENTS(data); i++) {
> > +        data[i] = i;
> > +    }
> > +    qtest_memwrite(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
> > +
> > +    for (i = 0; i < G_N_ELEMENTS(data); i++) {
> > +        g_assert_cmpuint(((uint32_t *)tmpshmem)[i], ==, i);
> > +    }
> > +
> > +    memset(data, 0, sizeof(data));
> > +
> > +    qtest_memread(s->qtest, (uintptr_t)s->mem_base, data, sizeof(data));
> > +    for (i = 0; i < G_N_ELEMENTS(data); i++) {
> > +        g_assert_cmpuint(data[i], ==, i);
> > +    }
> > +
> > +    qtest_quit(s->qtest);
> > +}
> > +
> > +static void test_ivshmem_pair(void)
> > +{
> > +    IVState state1, state2, *s1, *s2;
> > +    char *data;
> > +    int i;
> > +
> > +    setup_vm(&state1);
> > +    s1 = &state1;
> > +    setup_vm(&state2);
> > +    s2 = &state2;
> > +
> > +    data = g_malloc0(TMPSHMSIZE);
> > +
> > +    /* host write, guest 1 & 2 read */
> > +    memset(tmpshmem, 0x42, TMPSHMSIZE);
> > +    qtest_memread(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
> > +    for (i = 0; i < TMPSHMSIZE; i++) {
> > +        g_assert_cmpuint(data[i], ==, 0x42);
> > +    }
> > +    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    for (i = 0; i < TMPSHMSIZE; i++) {
> > +        g_assert_cmpuint(data[i], ==, 0x42);
> > +    }
> > +
> > +    /* guest 1 write, guest 2 read */
> > +    memset(data, 0x43, TMPSHMSIZE);
> > +    qtest_memwrite(s1->qtest, (uintptr_t)s1->mem_base, data, TMPSHMSIZE);
> > +    memset(data, 0, TMPSHMSIZE);
> > +    qtest_memread(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    for (i = 0; i < TMPSHMSIZE; i++) {
> > +        g_assert_cmpuint(data[i], ==, 0x43);
> > +    }
> > +
> > +    /* guest 2 write, guest 1 read */
> > +    memset(data, 0x44, TMPSHMSIZE);
> > +    qtest_memwrite(s2->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    memset(data, 0, TMPSHMSIZE);
> > +    qtest_memread(s1->qtest, (uintptr_t)s2->mem_base, data, TMPSHMSIZE);
> > +    for (i = 0; i < TMPSHMSIZE; i++) {
> > +        g_assert_cmpuint(data[i], ==, 0x44);
> > +    }
> > +
> > +    qtest_quit(s1->qtest);
> > +    qtest_quit(s2->qtest);
> > +    g_free(data);
> > +}
> > +
> > +typedef struct ServerThread {
> > +    GThread *thread;
> > +    IvshmemServer *server;
> > +    int pipe[2]; /* to handle quit */
> > +} ServerThread;
> > +
> > +static void *server_thread(void *data)
> > +{
> > +    ServerThread *t = data;
> > +    IvshmemServer *server = t->server;
> > +
> > +    while (true) {
> > +        fd_set fds;
> > +        int maxfd, ret;
> > +
> > +        FD_ZERO(&fds);
> > +        FD_SET(t->pipe[0], &fds);
> > +        maxfd = t->pipe[0] + 1;
> > +
> > +        ivshmem_server_get_fds(server, &fds, &maxfd);
> > +
> > +        ret = select(maxfd, &fds, NULL, NULL, NULL);
> > +
> > +        if (ret < 0) {
> > +            if (errno == EINTR) {
> > +                continue;
> > +            }
> > +
> > +            g_critical("select error: %s\n", strerror(errno));
> > +            break;
> > +        }
> > +        if (ret == 0) {
> > +            continue;
> > +        }
> > +
> > +        if (FD_ISSET(t->pipe[0], &fds)) {
> > +            break;
> > +        }
> > +
> > +        if (ivshmem_server_handle_fds(server, &fds, maxfd) < 0) {
> > +            g_critical("ivshmem_server_handle_fds() failed\n");
> > +            break;
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +static void setup_vm_with_server(IVState *s, int nvectors)
> > +{
> > +    char *cmd = g_strdup_printf("-chardev socket,id=chr0,path=%s,nowait "
> > +                                "-device
> > ivshmem,size=1M,chardev=chr0,vectors=%d",
> > +                                tmpserver, nvectors);
> > +
> > +    setup_vm_cmd(s, cmd, true);
> > +
> > +    g_free(cmd);
> > +}
> > +
> > +static GThread *thread_new(const gchar *name, GThreadFunc func, gpointer
> > data)
> > +{
> > +    GThread *thread = NULL;
> > +    GError *error = NULL;
> > +#ifdef HAVE_THREAD_NEW
> > +    thread = g_thread_try_new(name, func, data, &error);
> > +#else
> > +    thread = g_thread_create(func, data, TRUE, &error);
> > +#endif
> > +    g_assert_no_error(error);
> > +    return thread;
> > +}
> > +
> > +static void test_ivshmem_server(void)
> > +{
> > +    IVState state1, state2, *s1, *s2;
> > +    ServerThread thread;
> > +    IvshmemServer server;
> > +    int ret, vm1, vm2;
> > +    int nvectors = 2;
> > +
> > +    memset(tmpshmem, 0x42, TMPSHMSIZE);
> > +    ret = ivshmem_server_init(&server, tmpserver, tmpshm,
> > +                              TMPSHMSIZE, nvectors,
> > +                              getenv("QTEST_LOG") != NULL);
> > +    g_assert_cmpint(ret, ==, 0);
> > +
> > +    ret = ivshmem_server_start(&server);
> > +    g_assert_cmpint(ret, ==, 0);
> > +
> > +    setup_vm_with_server(&state1, nvectors);
> > +    s1 = &state1;
> > +    setup_vm_with_server(&state2, nvectors);
> > +    s2 = &state2;
> > +
> > +    g_assert_cmpuint(in_reg(s1, IVPOSITION), ==, 0xffffffff);
> > +    g_assert_cmpuint(in_reg(s2, IVPOSITION), ==, 0xffffffff);
> > +
> > +    g_assert_cmpuint(qtest_readb(s1->qtest, (uintptr_t)s1->mem_base), ==,
> > 0x00);
> > +
> > +    thread.server = &server;
> > +    ret = pipe(thread.pipe);
> > +    g_assert_cmpint(ret, ==, 0);
> > +    thread.thread = thread_new("ivshmem-server", server_thread, &thread);
> > +
> > +    /* waiting until mapping is done */
> > +    while (true) {
> > +        g_usleep(1000);
> > +
> > +        if (qtest_readb(s1->qtest, (uintptr_t)s1->mem_base) == 0x42 &&
> > +            qtest_readb(s2->qtest, (uintptr_t)s2->mem_base) == 0x42) {
> > +            break;
> > +        }
> > +    }
> > +
> > +    /* check got different VM ids */
> > +    vm1 = in_reg(s1, IVPOSITION);
> > +    vm2 = in_reg(s2, IVPOSITION);
> > +    g_assert_cmpuint(vm1, !=, vm2);
> > +
> > +    global_qtest = s1->qtest;
> > +    ret = qpci_msix_table_size(s1->dev);
> > +    g_assert_cmpuint(ret, ==, nvectors);
> > +
> > +    /* ping vm2 -> vm1 */
> > +    ret = qpci_msix_pending(s1->dev, 0);
> > +    g_assert_cmpuint(ret, ==, 0);
> > +    out_reg(s2, DOORBELL, vm1 << 16);
> > +    g_usleep(10000);
> > +    ret = qpci_msix_pending(s1->dev, 0);
> > +    g_assert_cmpuint(ret, !=, 0);
> > +
> > +    /* ping vm1 -> vm2 */
> > +    global_qtest = s2->qtest;
> > +    ret = qpci_msix_pending(s2->dev, 0);
> > +    g_assert_cmpuint(ret, ==, 0);
> > +    out_reg(s1, DOORBELL, vm2 << 16);
> > +    g_usleep(10000);
> > +    ret = qpci_msix_pending(s2->dev, 0);
> > +    g_assert_cmpuint(ret, !=, 0);
> > +
> > +    /* remove vm2 */
> > +    qtest_quit(s2->qtest);
> > +    /* XXX wait enough time for vm1 to be notified */
> > +    g_usleep(1000);
> > +
> > +    qtest_quit(s1->qtest);
> > +
> > +    write(thread.pipe[1], "q", 1);
> > +    g_thread_join(thread.thread);
> > +
> > +    ivshmem_server_close(&server);
> > +    close(thread.pipe[1]);
> > +    close(thread.pipe[0]);
> > +}
> > +
> > +#define PCI_SLOT_HP             0x06
> > +
> > +static void test_ivshmem_hotplug(void)
> > +{
> > +    gchar *opts;
> > +
> > +    qtest_start("");
> > +
> > +    opts = g_strdup_printf("'shm': '%s', 'size': '1M'", tmpshm);
> > +
> > +    qpci_plug_device_test("ivshmem", "iv1", PCI_SLOT_HP, opts);
> > +    qpci_unplug_acpi_device_test("iv1", PCI_SLOT_HP);
> > +
> > +    qtest_end();
> > +    g_free(opts);
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +    if (tmpshmem) {
> > +        munmap(tmpshmem, TMPSHMSIZE);
> > +        tmpshmem = NULL;
> > +    }
> > +
> > +    if (tmpshm) {
> > +        shm_unlink(tmpshm);
> > +        g_free(tmpshm);
> > +        tmpshm = NULL;
> > +    }
> > +
> > +    if (tmpserver) {
> > +        g_unlink(tmpserver);
> > +        g_free(tmpserver);
> > +        tmpserver = NULL;
> > +    }
> > +
> > +    if (tmpdir) {
> > +        g_rmdir(tmpdir);
> > +        tmpdir = NULL;
> > +    }
> > +}
> > +
> > +static void abrt_handler(void *data)
> > +{
> > +    cleanup();
> > +}
> > +
> > +static gchar *mktempshm(int size, int *fd)
> > +{
> > +    while (true) {
> > +        gchar *name;
> > +
> > +        name = g_strdup_printf("/qtest-%u-%u", getpid(), g_random_int());
> > +        *fd = shm_open(name, O_CREAT|O_RDWR|O_EXCL,
> > +                       S_IRWXU|S_IRWXG|S_IRWXO);
> > +        if (*fd > 0) {
> > +            g_assert(ftruncate(*fd, size) == 0);
> > +            return name;
> > +        }
> > +
> > +        g_free(name);
> > +    }
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +    int ret, fd;
> > +    static gchar dir[] = "/tmp/ivshmem-test.XXXXXX";
> 
> It forces creation of files in /tmp though, what about respecting user's
> desires via TMPDIR?
> Granted, the amount of offenders in tests/ is large.
> 
> > +
> > +#if !GLIB_CHECK_VERSION(2, 31, 0)
> > +    if (!g_thread_supported()) {
> > +        g_thread_init(NULL);
> > +    }
> > +#endif
> > +
> > +    g_test_init(&argc, &argv, NULL);
> > +
> > +    qtest_add_abrt_handler(abrt_handler, NULL);
> > +    /* shm */
> > +    tmpshm = mktempshm(TMPSHMSIZE, &fd);
> > +    tmpshmem = mmap(0, TMPSHMSIZE, PROT_READ|PROT_WRITE, MAP_SHARED, fd,
> > 0);
> > +    g_assert(tmpshmem != MAP_FAILED);
> > +    /* server */
> > +    if (g_mkdtemp_full(dir, 0700) == NULL) {
> > +        g_error("g_mkdtemp_full: %s", g_strerror(errno));
> > +    }
> > +    tmpdir = dir;
> > +    tmpserver = g_strconcat(tmpdir, "/server", NULL);
> > +
> > +    qtest_add_func("/ivshmem/single", test_ivshmem_single);
> > +    qtest_add_func("/ivshmem/pair", test_ivshmem_pair);
> > +    qtest_add_func("/ivshmem/server", test_ivshmem_server);
> > +    qtest_add_func("/ivshmem/hotplug", test_ivshmem_hotplug);
> > +
> > +    ret = g_test_run();
> > +
> > +    cleanup();
> > +    return ret;
> > +}
> > 
> 
> 
> 
>