[Qemu-devel] [PATCH v3 0/3] tests: more test cases for virtio-9p

[Qemu-devel] [PATCH v3 0/3] tests: more test cases for virtio-9p

[Greg Kurz]

This series brings some basic functional testing to 9P (only the virtio
part actually).

As with other virtio-* qtests, PC platform is assumed.

This v3 fixes the endianness issue in patch 3/3.

---

Greg Kurz (3):
      tests: virtio-9p: introduce start/stop functions
      tests: virtio-9p: add basic configuration test
      tests: virtio-9p: add basic transaction test


 tests/Makefile.include |    2 -
 tests/virtio-9p-test.c |  182 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 167 insertions(+), 17 deletions(-)

--
Greg

[Qemu-devel] [PATCH v3 1/3] tests: virtio-9p: introduce start/stop functions

[Greg Kurz]

First step to be able to run several functional steps.

Signed-off-by: Greg Kurz <groug@kaod.org>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
v2: - moved "mount_tag" change from 2/3 to 1/3 (Cornelia)
    - added Cornelia's A-b tag
---
 tests/virtio-9p-test.c |   42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
index 1e39335a7945..45fc8041d7f3 100644
--- a/tests/virtio-9p-test.c
+++ b/tests/virtio-9p-test.c
@@ -11,33 +11,41 @@
 #include "libqtest.h"
 #include "qemu-common.h"
 
-/* Tests only initialization so far. TODO: Replace with functional tests */
-static void pci_nop(void)
-{
-}
+static const char mount_tag[] = "qtest";
+static char *test_share;
 
-static char test_share[] = "/tmp/qtest.XXXXXX";
-
-int main(int argc, char **argv)
+static void qvirtio_9p_start(void)
 {
     char *args;
-    int ret;
 
-    g_test_init(&argc, &argv, NULL);
-    qtest_add_func("/virtio/9p/pci/nop", pci_nop);
-
-    g_assert(mkdtemp(test_share));
+    test_share = g_strdup("/tmp/qtest.XXXXXX");
+    g_assert_nonnull(mkdtemp(test_share));
 
     args = g_strdup_printf("-fsdev local,id=fsdev0,security_model=none,path=%s "
-                           "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=qtest",
-                           test_share);
+                           "-device virtio-9p-pci,fsdev=fsdev0,mount_tag=%s",
+                           test_share, mount_tag);
+
     qtest_start(args);
     g_free(args);
+}
 
-    ret = g_test_run();
-
+static void qvirtio_9p_stop(void)
+{
     qtest_end();
     rmdir(test_share);
+    g_free(test_share);
+}
+
+static void pci_nop(void)
+{
+    qvirtio_9p_start();
+    qvirtio_9p_stop();
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/virtio/9p/pci/nop", pci_nop);
 
-    return ret;
+    return g_test_run();
 }

[Qemu-devel] [PATCH v3 2/3] tests: virtio-9p: add basic configuration test

[Greg Kurz]

This adds PCI init code and a basic test that checks the device config
matches what is passed on the command line.

Signed-off-by: Greg Kurz <groug@kaod.org>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
V2: - s/char* tag/char *tag/ (patchew)
    - moved "mount_tag" change from 2/3 to 1/3 (Cornelia)
    - added Cornelia's A-b tag
---
 tests/Makefile.include |    2 +
 tests/virtio-9p-test.c |   77 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 03382b5fe7b8..a9dce206fbf6 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -600,7 +600,7 @@ tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
 tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o $(libqos-pc-obj-y) $(libqos-virtio-obj-y)
 tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o $(libqos-pc-obj-y)
 tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o $(libqos-virtio-obj-y)
-tests/virtio-9p-test$(EXESUF): tests/virtio-9p-test.o
+tests/virtio-9p-test$(EXESUF): tests/virtio-9p-test.o $(libqos-virtio-obj-y)
 tests/virtio-serial-test$(EXESUF): tests/virtio-serial-test.o
 tests/virtio-console-test$(EXESUF): tests/virtio-console-test.o
 tests/tpci200-test$(EXESUF): tests/tpci200-test.o
diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
index 45fc8041d7f3..b8fb6cd869a9 100644
--- a/tests/virtio-9p-test.c
+++ b/tests/virtio-9p-test.c
@@ -10,6 +10,13 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "qemu-common.h"
+#include "libqos/pci-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;
@@ -42,10 +49,80 @@ static void pci_nop(void)
     qvirtio_9p_stop();
 }
 
+typedef struct {
+    QVirtioDevice *dev;
+    QGuestAllocator *alloc;
+    QPCIBus *bus;
+    QVirtQueue *vq;
+} QVirtIO9P;
+
+static QVirtIO9P *qvirtio_9p_pci_init(void)
+{
+    QVirtIO9P *v9p;
+    QVirtioPCIDevice *dev;
+
+    v9p = g_new0(QVirtIO9P, 1);
+    v9p->alloc = pc_alloc_init();
+    v9p->bus = qpci_init_pc();
+
+    dev = qvirtio_pci_device_find(v9p->bus, VIRTIO_ID_9P);
+    g_assert_nonnull(dev);
+    g_assert_cmphex(dev->vdev.device_type, ==, VIRTIO_ID_9P);
+    v9p->dev = (QVirtioDevice *) dev;
+
+    qvirtio_pci_device_enable(dev);
+    qvirtio_reset(&qvirtio_pci, v9p->dev);
+    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);
+    return v9p;
+}
+
+static void qvirtio_9p_pci_free(QVirtIO9P *v9p)
+{
+    qvirtqueue_cleanup(&qvirtio_pci, v9p->vq, v9p->alloc);
+    pc_alloc_uninit(v9p->alloc);
+    qvirtio_pci_device_disable(container_of(v9p->dev, QVirtioPCIDevice, vdev));
+    g_free(v9p->dev);
+    qpci_free_pc(v9p->bus);
+    g_free(v9p);
+}
+
+static void pci_basic_config(void)
+{
+    QVirtIO9P *v9p;
+    void *addr;
+    size_t tag_len;
+    char *tag;
+    int i;
+
+    qvirtio_9p_start();
+    v9p = qvirtio_9p_pci_init();
+
+    addr = ((QVirtioPCIDevice *) v9p->dev)->addr + VIRTIO_PCI_CONFIG_OFF(false);
+    tag_len = qvirtio_config_readw(&qvirtio_pci, v9p->dev,
+                                   (uint64_t)(uintptr_t)addr);
+    g_assert_cmpint(tag_len, ==, strlen(mount_tag));
+    addr += sizeof(uint16_t);
+
+    tag = g_malloc(tag_len);
+    for (i = 0; i < tag_len; i++) {
+        tag[i] = qvirtio_config_readb(&qvirtio_pci, v9p->dev,
+                                      (uint64_t)(uintptr_t)addr + i);
+    }
+    g_assert_cmpmem(tag, tag_len, mount_tag, tag_len);
+    g_free(tag);
+
+    qvirtio_9p_pci_free(v9p);
+    qvirtio_9p_stop();
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/virtio/9p/pci/nop", pci_nop);
+    qtest_add_func("/virtio/9p/pci/basic/configuration", pci_basic_config);
 
     return g_test_run();
 }

[Qemu-devel] [PATCH v3 3/3] tests: virtio-9p: add basic transaction test

[Greg Kurz]

This adds a simple test to validate the device is functional: it transmits
a request with type Terror, which is not used by the 9P protocol [1], and
expects QEMU to return a reply with type Rerror and the "Operation not
supported" error string.

[1] http://lxr.free-electrons.com/source/include/net/9p/9p.h#L121

Signed-off-by: Greg Kurz <groug@kaod.org>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
v3: - the 9P protocol uses little-endian ordering
---
---
 tests/virtio-9p-test.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
index b8fb6cd869a9..95cba3fd3e8e 100644
--- a/tests/virtio-9p-test.c
+++ b/tests/virtio-9p-test.c
@@ -8,6 +8,7 @@
  */
 
 #include "qemu/osdep.h"
+#include <glib/gprintf.h>
 #include "libqtest.h"
 #include "qemu-common.h"
 #include "libqos/pci-pc.h"
@@ -17,6 +18,9 @@
 #include "libqos/malloc-pc.h"
 #include "standard-headers/linux/virtio_ids.h"
 #include "standard-headers/linux/virtio_pci.h"
+#include "hw/9pfs/9p.h"
+
+#define QVIRTIO_9P_TIMEOUT_US (1 * 1000 * 1000)
 
 static const char mount_tag[] = "qtest";
 static char *test_share;
@@ -118,11 +122,72 @@ static void pci_basic_config(void)
     qvirtio_9p_stop();
 }
 
+typedef struct VirtIO9PHdr {
+    uint32_t size;
+    uint8_t id;
+    uint16_t tag;
+} QEMU_PACKED VirtIO9PHdr;
+
+typedef struct VirtIO9PMsgRError {
+    uint16_t error_len;
+    char error[0];
+} QEMU_PACKED VirtIO9PMsgRError;
+
+#define P9_MAX_SIZE 8192
+
+static void pci_basic_transaction(void)
+{
+    QVirtIO9P *v9p;
+    VirtIO9PHdr hdr;
+    VirtIO9PMsgRError *resp;
+    uint64_t req_addr, resp_addr;
+    uint32_t free_head;
+    char *expected_error = strerror(ENOTSUP);
+
+    qvirtio_9p_start();
+    v9p = qvirtio_9p_pci_init();
+
+    hdr.size = cpu_to_le32(sizeof(hdr));
+    hdr.id = P9_TERROR;
+    hdr.tag = cpu_to_le16(12345);
+
+    req_addr = guest_alloc(v9p->alloc, hdr.size);
+    memwrite(req_addr, &hdr, sizeof(hdr));
+    free_head = qvirtqueue_add(v9p->vq, req_addr, hdr.size, false, true);
+
+    resp_addr = guest_alloc(v9p->alloc, P9_MAX_SIZE);
+    qvirtqueue_add(v9p->vq, resp_addr, P9_MAX_SIZE, true, false);
+
+    qvirtqueue_kick(&qvirtio_pci, v9p->dev, v9p->vq, free_head);
+    guest_free(v9p->alloc, req_addr);
+    qvirtio_wait_queue_isr(&qvirtio_pci, v9p->dev, v9p->vq,
+                           QVIRTIO_9P_TIMEOUT_US);
+
+    memread(resp_addr, &hdr, sizeof(hdr));
+    le32_to_cpus(&hdr.size);
+    le16_to_cpus(&hdr.tag);
+    g_assert_cmpint(hdr.size, <, (uint32_t) P9_MAX_SIZE);
+    g_assert_cmpint(hdr.id, ==, (uint8_t) P9_RERROR);
+    g_assert_cmpint(hdr.tag, ==, (uint16_t) 12345);
+
+    resp = g_malloc(hdr.size);
+    memread(resp_addr + sizeof(hdr), resp, hdr.size - sizeof(hdr));
+    guest_free(v9p->alloc, resp_addr);
+    le16_to_cpus(&resp->error_len);
+    g_assert_cmpmem(resp->error, resp->error_len, expected_error,
+                    strlen(expected_error));
+    g_free(resp);
+
+    qvirtio_9p_pci_free(v9p);
+    qvirtio_9p_stop();
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
     qtest_add_func("/virtio/9p/pci/nop", pci_nop);
     qtest_add_func("/virtio/9p/pci/basic/configuration", pci_basic_config);
+    qtest_add_func("/virtio/9p/pci/basic/transaction", pci_basic_transaction);
 
     return g_test_run();
 }

Re: [Qemu-devel] [PATCH v3 3/3] tests: virtio-9p: add basic transaction test

[Greg Kurz]

On Fri, 16 Sep 2016 19:43:57 +0200
Greg Kurz <groug@kaod.org> wrote:

> This adds a simple test to validate the device is functional: it transmits
> a request with type Terror, which is not used by the 9P protocol [1], and
> expects QEMU to return a reply with type Rerror and the "Operation not
> supported" error string.
> 
> [1] http://lxr.free-electrons.com/source/include/net/9p/9p.h#L121
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
> v3: - the 9P protocol uses little-endian ordering
> ---
> ---
>  tests/virtio-9p-test.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/tests/virtio-9p-test.c b/tests/virtio-9p-test.c
> index b8fb6cd869a9..95cba3fd3e8e 100644
> --- a/tests/virtio-9p-test.c
> +++ b/tests/virtio-9p-test.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include <glib/gprintf.h>
>  #include "libqtest.h"
>  #include "qemu-common.h"
>  #include "libqos/pci-pc.h"
> @@ -17,6 +18,9 @@
>  #include "libqos/malloc-pc.h"
>  #include "standard-headers/linux/virtio_ids.h"
>  #include "standard-headers/linux/virtio_pci.h"
> +#include "hw/9pfs/9p.h"
> +
> +#define QVIRTIO_9P_TIMEOUT_US (1 * 1000 * 1000)
>  
>  static const char mount_tag[] = "qtest";
>  static char *test_share;
> @@ -118,11 +122,72 @@ static void pci_basic_config(void)
>      qvirtio_9p_stop();
>  }
>  
> +typedef struct VirtIO9PHdr {
> +    uint32_t size;
> +    uint8_t id;
> +    uint16_t tag;
> +} QEMU_PACKED VirtIO9PHdr;
> +
> +typedef struct VirtIO9PMsgRError {
> +    uint16_t error_len;
> +    char error[0];
> +} QEMU_PACKED VirtIO9PMsgRError;
> +
> +#define P9_MAX_SIZE 8192
> +
> +static void pci_basic_transaction(void)
> +{
> +    QVirtIO9P *v9p;
> +    VirtIO9PHdr hdr;
> +    VirtIO9PMsgRError *resp;
> +    uint64_t req_addr, resp_addr;
> +    uint32_t free_head;
> +    char *expected_error = strerror(ENOTSUP);
> +
> +    qvirtio_9p_start();
> +    v9p = qvirtio_9p_pci_init();
> +
> +    hdr.size = cpu_to_le32(sizeof(hdr));
> +    hdr.id = P9_TERROR;
> +    hdr.tag = cpu_to_le16(12345);
> +
> +    req_addr = guest_alloc(v9p->alloc, hdr.size);

Ugh... this is wrong since hdr.size isn't cpu endian. :-\

The code must be globally rearranged to handle endianness correctly.

Cheers.

--
Greg

> +    memwrite(req_addr, &hdr, sizeof(hdr));
> +    free_head = qvirtqueue_add(v9p->vq, req_addr, hdr.size, false, true);
> +
> +    resp_addr = guest_alloc(v9p->alloc, P9_MAX_SIZE);
> +    qvirtqueue_add(v9p->vq, resp_addr, P9_MAX_SIZE, true, false);
> +
> +    qvirtqueue_kick(&qvirtio_pci, v9p->dev, v9p->vq, free_head);
> +    guest_free(v9p->alloc, req_addr);
> +    qvirtio_wait_queue_isr(&qvirtio_pci, v9p->dev, v9p->vq,
> +                           QVIRTIO_9P_TIMEOUT_US);
> +
> +    memread(resp_addr, &hdr, sizeof(hdr));
> +    le32_to_cpus(&hdr.size);
> +    le16_to_cpus(&hdr.tag);
> +    g_assert_cmpint(hdr.size, <, (uint32_t) P9_MAX_SIZE);
> +    g_assert_cmpint(hdr.id, ==, (uint8_t) P9_RERROR);
> +    g_assert_cmpint(hdr.tag, ==, (uint16_t) 12345);
> +
> +    resp = g_malloc(hdr.size);
> +    memread(resp_addr + sizeof(hdr), resp, hdr.size - sizeof(hdr));
> +    guest_free(v9p->alloc, resp_addr);
> +    le16_to_cpus(&resp->error_len);
> +    g_assert_cmpmem(resp->error, resp->error_len, expected_error,
> +                    strlen(expected_error));
> +    g_free(resp);
> +
> +    qvirtio_9p_pci_free(v9p);
> +    qvirtio_9p_stop();
> +}
> +
>  int main(int argc, char **argv)
>  {
>      g_test_init(&argc, &argv, NULL);
>      qtest_add_func("/virtio/9p/pci/nop", pci_nop);
>      qtest_add_func("/virtio/9p/pci/basic/configuration", pci_basic_config);
> +    qtest_add_func("/virtio/9p/pci/basic/transaction", pci_basic_transaction);
>  
>      return g_test_run();
>  }
>