Re: [Qemu-devel] [PATCH v2 00/34] Qtest driver framework

[Paolo Bonzini]

On 06/08/2018 16:33, Emanuele Giuseppe Esposito wrote:
> qgraph API for the qtest driver framework
> 
> This series of patches introduce a different qtest driver
> organization, viewing machines, drivers and tests as node in a
> graph, each having one or multiple edges relations.
> 
> The idea is to have a framework where each test asks for a specific
> driver, and the framework takes care of allocating the proper devices
> required and passing the correct command line arguments to QEMU.
> 
> A node can be of four types:
> - MACHINE:   for example "arm/raspi2"
> - DRIVER:    for example "generic-sdhci"
> - INTERFACE: for example "sdhci" (interface for all "-sdhci" drivers)
> - TEST:      for example "sdhci-test", consumes an interface and tests
>              the functions provided
> 
> An edge relation between two nodes (drivers or machines) X and Y can be:
> - X CONSUMES Y: Y can be plugged into X
> - X PRODUCES Y: X provides the interface Y
> - X CONTAINS Y: Y is part of X component
> 
> Basic framework steps are the following:
> - All nodes and edges are created in their respective machine/driver/test 
> files
> - The framework starts QEMU and asks for a list of available devices
>   and machines
> - The framework walks the graph starting from the available machines and
>   performs a Depth First Search for tests
> - Once a test is found, the path is walked again and all drivers are
>   allocated accordingly and the final interface is passed to the test
> - The test is executed
> - Unused objects are cleaned and the path discovery is continued
> 
> Depending on the QEMU binary used, only some drivers/machines will be 
> available
> and only test that are reached by them will be executed.
> 
> This work is being done as Google Summer of Code 2018 project for QEMU,
> my mentors are Paolo Bonzini and Laurent Vivier.
> Additional infos on the project can be found at:
> https://wiki.qemu.org/Features/qtest_driver_framework

Tests are not cleaning up properly.  There are two bugs, both related to
qtest_end not being called (from main in one case, after 
qos_invalidate_command_line in the other).  I tried this fix but then
the e1000e tests start failing:

diff --git a/tests/libqtest.h b/tests/libqtest.h
index ac52872..7a6fa6e 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -555,6 +555,9 @@ static inline QTestState *qtest_start(const char *args)
  */
 static inline void qtest_end(void)
 {
+    if (!global_qtest) {
+        return;
+    }
     qtest_quit(global_qtest);
     global_qtest = NULL;
 }
diff --git a/tests/qos-test.c b/tests/qos-test.c
index 1c3a7fc..a22d176 100644
--- a/tests/qos-test.c
+++ b/tests/qos-test.c
@@ -233,10 +233,7 @@ static void restart_qemu_or_continue(char *path)
     * new command line
     */
     if (g_strcmp0(old_path, path)) {
-        if (old_path) {
-            qtest_end();
-            g_free(old_path);
-        }
+        qtest_end();
         old_path = path;
         global_qtest = qtest_start(path);
     } else { /* if cmd line is the same, reset the guest */
@@ -247,7 +244,10 @@ static void restart_qemu_or_continue(char *path)
 
 void qos_invalidate_command_line(void)
 {
-    old_path = NULL;
+    if (old_path) {
+        g_free(old_path);
+        old_path = NULL;
+    }
 }
 
 /**
@@ -475,6 +475,7 @@ int main(int argc, char **argv)
 
     qos_graph_foreach_test_path(walk_path);
     g_test_run();
+    qtest_end();
     qos_graph_destroy();
     return 0;
 }


Also, qos-test can be added to check-qtest-generic-y, since it is not 
PCI-specific.

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14f19eb..6012f6e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -182,6 +182,7 @@ gcov-files-generic-y = monitor.c qapi/qmp-dispatch.c
 check-qtest-generic-y += tests/device-introspect-test$(EXESUF)
 gcov-files-generic-y = qdev-monitor.c qmp.c
 check-qtest-generic-y += tests/cdrom-test$(EXESUF)
+check-qtest-generic-y += tests/qos-test$(EXESUF)
 
 gcov-files-ipack-y += hw/ipack/ipack.c
 check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
@@ -790,9 +791,6 @@ libqgraph-tests-obj-y += tests/virtio-scsi-test.o
 check-unit-y += tests/test-qgraph$(EXESUF)
 tests/test-qgraph$(EXESUF): tests/test-qgraph.o $(libqgraph-obj-y)
 
-check-qtest-pci-y += tests/qos-test$(EXESUF)
-tests/qos-test$(EXESUF): tests/qos-test.o $(libqgraph-tests-obj-y)
-
 tests/qmp-test$(EXESUF): tests/qmp-test.o
 tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
 tests/rtc-test$(EXESUF): tests/rtc-test.o

Another small bug is that virtio-net-test should also call 
qos_invalidate_command_line, since it is closing the socket that
is opened in virtio_net_test_setup.

Paolo

> v2:
> - added command line arguments caching
> - converted all virtio tests
> - converted e1000e test
> - added two more machines (virt and ppc64)
> - introduced separation between edge-name (used by get_driver) and
>   node-name (matched with QMP query result)
> - edge carries also additional arguments, like the pci address of
>   destination device
> - test now can execute a function before and after the command line
>   is allocated, to allocate various file and sockets needed by command
>   line and test
> - better documentation with working example in qgraph.h
> - removed esplicit creation of interfaces
> 
> Signed-off-by: Emanuele Giuseppe Esposito <address@hidden>
> 
> Emanuele Giuseppe Esposito (33):
>   tests: qgraph API for the qtest driver framework
>   tests/qgraph: rename qpci_init_pc functions
>   tests/qgraph: pci-pc driver and interface nodes
>   tests/qgraph: x86_64/pc machine node
>   tests/qgraph: sdhci driver and interface nodes
>   tests/qgraph: sdhci test node
>   tests/qgraph: arm/raspi2 machine node
>   tests/qgraph: rename qpci_init_spapr functions
>   tests/qgraph: pci-spapr driver and interface nodes
>   tests/qgraph: ppc64/pseries machine node
>   test/qgraph: e1000e driver and interface nodes
>   test/qgraph: e1000e-test node
>   test/qgraph: virtio_start_device function
>   test/qgraph: virtio-pci driver and interface nodes
>   tests/qgraph: rename qvirtio_mmio_init_device functions
>   test/qgraph: virtio-mmio driver and interface nodes
>   test/qgraph: arm/virt machine node
>   test/qgraph: virtio-serial driver and interface nodes
>   test/qgraph: virtio-console test node
>   test/qgraph: virtio-serial test node
>   test/qgraph: virtio-9p driver and interface nodes
>   test/qgraph: virtio-9p test node
>   test/qgraph: virtio-balloon driver and interface nodes
>   test/qgraph: virtio-balloon test node
>   test/qgraph: virtio-rng driver and interface nodes
>   test/qgraph: virtio-rng test node
>   test/qgraph: virtio-blk driver and interface nodes
>   test/qgraph: virtio-blk test node
>   test/qgraph: virtio-net driver and interface nodes
>   test/qgraph: virtio-net test node
>   test/qgraph: virtio-scsi driver and interface nodes
>   test/qgraph: virtio-scsi test node
>   test/qgraph: temporarly commented vhost-user-test
> 
> Paolo Bonzini (1):
>   tests: virtio: separate ccw tests from libqos
> 
>  configure                            |   2 +-
>  include/qemu/module.h                |   2 +
>  tests/Makefile.include               |  79 +--
>  tests/e1000e-test.c                  | 354 +++----------
>  tests/i440fx-test.c                  |   2 +-
>  tests/ide-test.c                     |   2 +-
>  tests/libqos/ahci.c                  |   2 +-
>  tests/libqos/e1000e.c                | 262 ++++++++++
>  tests/libqos/e1000e.h                |  53 ++
>  tests/libqos/libqos-pc.c             |   2 +-
>  tests/libqos/libqos-spapr.c          |   2 +-
>  tests/libqos/pci-pc.c                |  41 +-
>  tests/libqos/pci-pc.h                |  22 +-
>  tests/libqos/pci-spapr.c             |  57 ++-
>  tests/libqos/pci-spapr.h             |  26 +-
>  tests/libqos/pci.c                   |  48 +-
>  tests/libqos/pci.h                   |  15 +
>  tests/libqos/ppc64_pseries-machine.c | 111 +++++
>  tests/libqos/qgraph.c                | 721 +++++++++++++++++++++++++++
>  tests/libqos/qgraph.h                | 514 +++++++++++++++++++
>  tests/libqos/qgraph_extra.h          | 263 ++++++++++
>  tests/libqos/raspi2-machine.c        |  82 +++
>  tests/libqos/sdhci.c                 | 163 ++++++
>  tests/libqos/sdhci.h                 |  69 +++
>  tests/libqos/virt-machine.c          |  90 ++++
>  tests/libqos/virtio-9p.c             | 164 ++++++
>  tests/libqos/virtio-9p.h             |  42 ++
>  tests/libqos/virtio-balloon.c        | 111 +++++
>  tests/libqos/virtio-balloon.h        |  39 ++
>  tests/libqos/virtio-blk.c            | 126 +++++
>  tests/libqos/virtio-blk.h            |  40 ++
>  tests/libqos/virtio-mmio.c           |  72 ++-
>  tests/libqos/virtio-mmio.h           |   6 +-
>  tests/libqos/virtio-net.c            | 177 +++++++
>  tests/libqos/virtio-net.h            |  41 ++
>  tests/libqos/virtio-pci.c            |  80 ++-
>  tests/libqos/virtio-pci.h            |  12 +
>  tests/libqos/virtio-rng.c            | 108 ++++
>  tests/libqos/virtio-rng.h            |  39 ++
>  tests/libqos/virtio-scsi.c           | 117 +++++
>  tests/libqos/virtio-scsi.h           |  39 ++
>  tests/libqos/virtio-serial.c         | 109 ++++
>  tests/libqos/virtio-serial.h         |  39 ++
>  tests/libqos/virtio.c                |   9 +
>  tests/libqos/virtio.h                |   1 +
>  tests/libqos/x86_64_pc-machine.c     | 110 ++++
>  tests/q35-test.c                     |   4 +-
>  tests/qos-test.c                     | 480 ++++++++++++++++++
>  tests/rtl8139-test.c                 |   2 +-
>  tests/sdhci-test.c                   | 222 +++------
>  tests/tco-test.c                     |   2 +-
>  tests/test-qgraph.c                  | 446 +++++++++++++++++
>  tests/usb-hcd-ehci-test.c            |   2 +-
>  tests/vhost-user-test.c              |   7 +-
>  tests/virtio-9p-test.c               | 221 +++-----
>  tests/virtio-balloon-test.c          |  22 +-
>  tests/virtio-blk-test.c              | 473 +++++++-----------
>  tests/virtio-ccw-test.c              | 121 +++++
>  tests/virtio-console-test.c          |  30 +-
>  tests/virtio-net-test.c              | 163 ++----
>  tests/virtio-rng-test.c              |  25 +-
>  tests/virtio-scsi-test.c             | 155 +++---
>  tests/virtio-serial-test.c           |  27 +-
>  63 files changed, 5624 insertions(+), 1243 deletions(-)
>  create mode 100644 tests/libqos/e1000e.c
>  create mode 100644 tests/libqos/e1000e.h
>  create mode 100644 tests/libqos/ppc64_pseries-machine.c
>  create mode 100644 tests/libqos/qgraph.c
>  create mode 100644 tests/libqos/qgraph.h
>  create mode 100644 tests/libqos/qgraph_extra.h
>  create mode 100644 tests/libqos/raspi2-machine.c
>  create mode 100644 tests/libqos/sdhci.c
>  create mode 100644 tests/libqos/sdhci.h
>  create mode 100644 tests/libqos/virt-machine.c
>  create mode 100644 tests/libqos/virtio-9p.c
>  create mode 100644 tests/libqos/virtio-9p.h
>  create mode 100644 tests/libqos/virtio-balloon.c
>  create mode 100644 tests/libqos/virtio-balloon.h
>  create mode 100644 tests/libqos/virtio-blk.c
>  create mode 100644 tests/libqos/virtio-blk.h
>  create mode 100644 tests/libqos/virtio-net.c
>  create mode 100644 tests/libqos/virtio-net.h
>  create mode 100644 tests/libqos/virtio-rng.c
>  create mode 100644 tests/libqos/virtio-rng.h
>  create mode 100644 tests/libqos/virtio-scsi.c
>  create mode 100644 tests/libqos/virtio-scsi.h
>  create mode 100644 tests/libqos/virtio-serial.c
>  create mode 100644 tests/libqos/virtio-serial.h
>  create mode 100644 tests/libqos/x86_64_pc-machine.c
>  create mode 100644 tests/qos-test.c
>  create mode 100644 tests/test-qgraph.c
>  create mode 100644 tests/virtio-ccw-test.c
>