[Qemu-devel] [PATCH v2 0/8] Various qtest-related patches and a mc146818rtc fix

[Qemu-devel] [PATCH v2 0/8] Various qtest-related patches and a mc146818rtc fix

[Thomas Huth]

The basic idea for this patch series are the two patches that improve
the device-inrospection test: We now check that the qom-tree and the
qtree do not change during introspection, and the we also turn on the
device introspection test with all machines when running in SPEED=slow
mode. The other patches are a bug fix for the mc146818rtc device which
has been discovered with the improved device-introspection test, two
patches to shut up annoying messages during "make check", a patch to
speed up the tests that iterate over all machine types and an update
to the MAINTAINERS file.

v2:
 - Added Reviewed-bys from v1
 - Use g_test_message() instead of g_debug()
 - Improved the patch descriptions a little bit
 - Added patch to skip old versioned machine types in quick mode
 - Added Paolo's update to MAINTAINERS

Paolo Bonzini (1):
  MAINTAINERS: add maintainers for qtest

Thomas Huth (7):
  tests/migration-test: Silence the kvm_hv message by default
  net: Silence 'has no peer' messages in testing mode
  hw/timer/mc146818rtc: White space clean-up
  hw/timer/mc146818rtc: Fix introspection problem
  tests: Skip old versioned machine types in quick testing mode
  tests/device-introspection: Check that the qom-tree and qtree do not
    change
  tests/device-introspect: Test with all machines, not only with "none"

 MAINTAINERS                    | 10 ++++++++
 hw/ppc/prep.c                  |  3 +++
 hw/timer/mc146818rtc.c         | 20 ++++++---------
 tests/cpu-plug-test.c          |  6 ++---
 tests/device-introspect-test.c | 55 ++++++++++++++++++++++++++++++++++++------
 tests/libqtest.c               | 52 +++++++++++++++++++++++++++++++++++++--
 tests/libqtest.h               |  4 ++-
 tests/migration-test.c         | 20 ++++++++-------
 tests/qom-test.c               |  2 +-
 tests/test-hmp.c               |  2 +-
 vl.c                           |  3 +--
 11 files changed, 138 insertions(+), 39 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/8] tests/migration-test: Silence the kvm_hv message by default

[Thomas Huth]

When running "make check" on a non-POWER host, the output is quite
distorted like this:

  [...]
  GTESTER check-qtest-nios2
  GTESTER check-qtest-or1k
  GTESTER check-qtest-ppc64
Skipping test: kvm_hv not available Skipping test: kvm_hv not available Skipping test: kvm_hv not available Skipping test: kvm_hv not available   GTESTER check-qtest-ppcemb
  GTESTER check-qtest-ppc
  GTESTER check-qtest-riscv32
  GTESTER check-qtest-riscv64
  [...]

Move the check to the beginning of the main function instead, so that
we do not have to test the condition again and again for each test,
and better use g_test_message() instead of g_print() here, like it is
also done in ufd_version_check() already.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/migration-test.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index eb58d0a..0e687b7 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -438,15 +438,6 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                   " -incoming %s",
                                   accel, tmpfs, bootpath, uri);
     } else if (strcmp(arch, "ppc64") == 0) {
-
-        /* On ppc64, the test only works with kvm-hv, but not with kvm-pr
-         * and TCG is touchy due to race conditions on dirty bits
-         * (especially on PPC for some reason)
-         */
-        if (access("/sys/module/kvm_hv", F_OK)) {
-            g_print("Skipping test: kvm_hv not available ");
-            return -1;
-        }
         cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
                                   " -name source,debug-threads=on"
                                   " -serial file:%s/src_serial"
@@ -750,6 +741,17 @@ int main(int argc, char **argv)
         return 0;
     }
 
+    /*
+     * On ppc64, the test only works with kvm-hv, but not with kvm-pr and TCG
+     * is touchy due to race conditions on dirty bits (especially on PPC for
+     * some reason)
+     */
+    if (g_str_equal(qtest_get_arch(), "ppc64") &&
+        access("/sys/module/kvm_hv", F_OK)) {
+        g_test_message("Skipping test: kvm_hv not available");
+        return 0;
+    }
+
     tmpfs = mkdtemp(template);
     if (!tmpfs) {
         g_test_message("mkdtemp on path (%s): %s\n", template, strerror(errno));
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 1/8] tests/migration-test: Silence the kvm_hv message by default

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> When running "make check" on a non-POWER host, the output is quite
> distorted like this:
>
>   [...]
>   GTESTER check-qtest-nios2
>   GTESTER check-qtest-or1k
>   GTESTER check-qtest-ppc64
> Skipping test: kvm_hv not available Skipping test: kvm_hv not available Skipping test: kvm_hv not available Skipping test: kvm_hv not available   GTESTER check-qtest-ppcemb
>   GTESTER check-qtest-ppc
>   GTESTER check-qtest-riscv32
>   GTESTER check-qtest-riscv64
>   [...]
>
> Move the check to the beginning of the main function instead, so that
> we do not have to test the condition again and again for each test,
> and better use g_test_message() instead of g_print() here, like it is
> also done in ufd_version_check() already.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH v2 2/8] net: Silence 'has no peer' messages in testing mode

[Thomas Huth]

When running qtests with -nodefaults, we are not interested in
these 'XYZ has no peer' messages.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 vl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 16b913f..7055df3 100644
--- a/vl.c
+++ b/vl.c
@@ -4559,11 +4559,10 @@ int main(int argc, char **argv, char **envp)
      * (2) CONFIG_SLIRP not set, in which case the implicit "-net nic"
      * sets up a nic that isn't connected to anything.
      */
-    if (!default_net) {
+    if (!default_net && (!qtest_enabled() || has_defaults)) {
         net_check_clients();
     }
 
-
     if (boot_once) {
         qemu_boot_set(boot_once, &error_fatal);
         qemu_register_reset(restore_boot_order, g_strdup(boot_order));
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 3/8] hw/timer/mc146818rtc: White space clean-up

[Thomas Huth]

mc146818rtc.c still contains some TABs. Replace them with spaces.
And while we're at it, also delete trailing whitespace in this file.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/timer/mc146818rtc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 6f1f723..3a14075 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -120,7 +120,7 @@ static void rtc_coalesced_timer_update(RTCState *s)
         timer_del(s->coalesced_timer);
     } else {
         /* divide each RTC interval to 2 - 8 smaller intervals */
-        int c = MIN(s->irq_coalesced, 7) + 1; 
+        int c = MIN(s->irq_coalesced, 7) + 1;
         int64_t next_clock = qemu_clock_get_ns(rtc_clock) +
             periodic_clock_to_ns(s->period / c);
         timer_mod(s->coalesced_timer, next_clock);
@@ -485,7 +485,7 @@ static void cmos_ioport_write(void *opaque, hwaddr addr,
             s->cmos_data[s->cmos_index] = data;
             check_update_timer(s);
             break;
-	case RTC_IBM_PS2_CENTURY_BYTE:
+        case RTC_IBM_PS2_CENTURY_BYTE:
             s->cmos_index = RTC_CENTURY;
             /* fall through */
         case RTC_CENTURY:
@@ -713,7 +713,7 @@ static uint64_t cmos_ioport_read(void *opaque, hwaddr addr,
         return 0xff;
     } else {
         switch(s->cmos_index) {
-	case RTC_IBM_PS2_CENTURY_BYTE:
+        case RTC_IBM_PS2_CENTURY_BYTE:
             s->cmos_index = RTC_CENTURY;
             /* fall through */
         case RTC_CENTURY:
@@ -915,7 +915,7 @@ static void rtc_reset(void *opaque)
 
     if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
         s->irq_coalesced = 0;
-        s->irq_reinject_on_ack_count = 0;		
+        s->irq_reinject_on_ack_count = 0;
     }
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 3/8] hw/timer/mc146818rtc: White space clean-up

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> mc146818rtc.c still contains some TABs. Replace them with spaces.
> And while we're at it, also delete trailing whitespace in this file.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH v2 4/8] hw/timer/mc146818rtc: Fix introspection problem

[Thomas Huth]

There is currently a funny problem with the "mc146818rtc" device:
1) Start QEMU like this:
   qemu-system-ppc64 -M pseries -S
2) At the HMP monitor, enter "info qom-tree". Note that there is an
   entry for "/rtc (spapr-rtc)".
3) Introspect the mc146818rtc device like this:
   device_add mc146818rtc,help
4) Run "info qom-tree" again. The "/rtc" entry is gone now!

The rtc_finalize() function of the mc146818rtc device has two bugs: First,
it tries to remove a "rtc" property, while the rtc_realizefn() added a
"rtc-time" property instead. And second, it should have been done in an
unrealize function, not in a finalize function, to avoid that this causes
problems during introspection.

But since adding aliases to the global machine state should not be done
from a device's realize function anyway, let's rather fix this issue
by moving the creation of the alias to the code that creates the device
(and thus is run from the machine init functions instead), i.e. the
mc146818_rtc_init() function for most machines. The prep machines are
special, since the mc146818rtc device is created here in the realize
function of the i82378 device. Since we certainly don't want to add the
alias there, we add it to some code that is called from the ibm_40p_init()
machine init function instead.
Since the alias is now only created during the machine init, we can remove
the object_property_del() completely.

Fixes: 654a36d857ff949e0d1989904b76f53fded9dc83
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/prep.c          |  3 +++
 hw/timer/mc146818rtc.c | 12 +++---------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 3401570..91a8f42 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -696,6 +696,9 @@ static int prep_set_cmos_checksum(DeviceState *dev, void *opaque)
         rtc_set_memory(rtc, 0x3e, checksum & 0xff);
         rtc_set_memory(rtc, 0x2f, checksum >> 8);
         rtc_set_memory(rtc, 0x3f, checksum >> 8);
+
+        object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(rtc),
+                                  "date", NULL);
     }
     return 0;
 }
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 3a14075..a504f03 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -995,9 +995,6 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
 
     object_property_add_tm(OBJECT(s), "date", rtc_get_date, NULL);
 
-    object_property_add_alias(qdev_get_machine(), "rtc-time",
-                              OBJECT(s), "date", NULL);
-
     qdev_init_gpio_out(dev, &s->irq, 1);
 }
 
@@ -1019,6 +1016,9 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
     }
     QLIST_INSERT_HEAD(&rtc_devices, s, link);
 
+    object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(s),
+                              "date", NULL);
+
     return isadev;
 }
 
@@ -1052,17 +1052,11 @@ static void rtc_class_initfn(ObjectClass *klass, void *data)
     dc->user_creatable = false;
 }
 
-static void rtc_finalize(Object *obj)
-{
-    object_property_del(qdev_get_machine(), "rtc", NULL);
-}
-
 static const TypeInfo mc146818rtc_info = {
     .name          = TYPE_MC146818_RTC,
     .parent        = TYPE_ISA_DEVICE,
     .instance_size = sizeof(RTCState),
     .class_init    = rtc_class_initfn,
-    .instance_finalize = rtc_finalize,
 };
 
 static void mc146818rtc_register_types(void)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 4/8] hw/timer/mc146818rtc: Fix introspection problem

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> There is currently a funny problem with the "mc146818rtc" device:
> 1) Start QEMU like this:
>    qemu-system-ppc64 -M pseries -S
> 2) At the HMP monitor, enter "info qom-tree". Note that there is an
>    entry for "/rtc (spapr-rtc)".
> 3) Introspect the mc146818rtc device like this:
>    device_add mc146818rtc,help
> 4) Run "info qom-tree" again. The "/rtc" entry is gone now!
>
> The rtc_finalize() function of the mc146818rtc device has two bugs: First,
> it tries to remove a "rtc" property, while the rtc_realizefn() added a
> "rtc-time" property instead. And second, it should have been done in an
> unrealize function, not in a finalize function, to avoid that this causes
> problems during introspection.
>
> But since adding aliases to the global machine state should not be done
> from a device's realize function anyway, let's rather fix this issue
> by moving the creation of the alias to the code that creates the device
> (and thus is run from the machine init functions instead), i.e. the
> mc146818_rtc_init() function for most machines. The prep machines are
> special, since the mc146818rtc device is created here in the realize
> function of the i82378 device. Since we certainly don't want to add the
> alias there, we add it to some code that is called from the ibm_40p_init()
> machine init function instead.
> Since the alias is now only created during the machine init, we can remove
> the object_property_del() completely.
>
> Fixes: 654a36d857ff949e0d1989904b76f53fded9dc83
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH v2 5/8] tests: Skip old versioned machine types in quick testing mode

[Thomas Huth]

The tests that check something for all machine types currently spend
a lot of time checking old machine types (like "pc-i440fx-2.0" for
example). The chances that we find something new there in addition
to checking the latest version of a machine type are pretty low, so
we should not waste the time of the developers by testing this again
and again in the "quick" testing mode.
Thus let's add some code to determine whether we are testing a current
machine type or an old one, and only test the old types if we are
running in "SPEED=slow" mode.
This decreases the testing time quite a bit now, e.g. the qom-test
now finishes within 4 seconds for qemu-system-x86_64 instead of 30
seconds when testing all machines.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/cpu-plug-test.c |  6 +++---
 tests/libqtest.c      | 52 +++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/libqtest.h      |  4 +++-
 tests/qom-test.c      |  2 +-
 tests/test-hmp.c      |  2 +-
 5 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c
index f5d57da..3e93c8e 100644
--- a/tests/cpu-plug-test.c
+++ b/tests/cpu-plug-test.c
@@ -257,11 +257,11 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-        qtest_cb_for_every_machine(add_pc_test_case);
+        qtest_cb_for_every_machine(add_pc_test_case, g_test_quick());
     } else if (g_str_equal(arch, "ppc64")) {
-        qtest_cb_for_every_machine(add_pseries_test_case);
+        qtest_cb_for_every_machine(add_pseries_test_case, g_test_quick());
     } else if (g_str_equal(arch, "s390x")) {
-        qtest_cb_for_every_machine(add_s390x_test_case);
+        qtest_cb_for_every_machine(add_s390x_test_case, g_test_quick());
     }
 
     return g_test_run();
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 852ccff..1eefad4 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -991,7 +991,53 @@ bool qtest_big_endian(QTestState *s)
     return s->big_endian;
 }
 
-void qtest_cb_for_every_machine(void (*cb)(const char *machine))
+static bool qtest_check_machine_version(const char *mname, const char *basename,
+                                        int major, int minor)
+{
+    char *newname;
+    bool is_equal;
+
+    newname = g_strdup_printf("%s-%i.%i", basename, major, minor);
+    is_equal = g_str_equal(mname, newname);
+    g_free(newname);
+
+    return is_equal;
+}
+
+static bool qtest_is_old_versioned_machine(const char *mname)
+{
+    const char *dash = strrchr(mname, '-');
+    const char *dot = strrchr(mname, '.');
+    const char *chr;
+    char *bname;
+    const int major = QEMU_VERSION_MAJOR;
+    const int minor = QEMU_VERSION_MINOR;
+    bool res = false;
+
+    if (dash && dot && dot > dash) {
+        for (chr = dash + 1; *chr; chr++) {
+            if (!isdigit(*chr) && *chr != '.') {
+                return false;
+            }
+        }
+        /*
+         * Now check if it is one of the latest versions. Check major + 1
+         * and minor + 1 versions as well, since they might already exist
+         * in the development branch.
+         */
+        bname = g_strdup(mname);
+        bname[dash - mname] = 0;
+        res = !qtest_check_machine_version(mname, bname, major + 1, 0) &&
+              !qtest_check_machine_version(mname, bname, major, minor + 1) &&
+              !qtest_check_machine_version(mname, bname, major, minor);
+        g_free(bname);
+    }
+
+    return res;
+}
+
+void qtest_cb_for_every_machine(void (*cb)(const char *machine),
+                                bool skip_old_versioned)
 {
     QDict *response, *minfo;
     QList *list;
@@ -1014,7 +1060,9 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine))
         qstr = qobject_to(QString, qobj);
         g_assert(qstr);
         mname = qstring_get_str(qstr);
-        cb(mname);
+        if (!skip_old_versioned || !qtest_is_old_versioned_machine(mname)) {
+            cb(mname);
+        }
     }
 
     qtest_end();
diff --git a/tests/libqtest.h b/tests/libqtest.h
index def1eda..1159b73 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -954,10 +954,12 @@ QDict *qmp_fd(int fd, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 /**
  * qtest_cb_for_every_machine:
  * @cb: Pointer to the callback function
+ * @skip_old_versioned: true if versioned old machine types should be skipped
  *
  *  Call a callback function for every name of all available machines.
  */
-void qtest_cb_for_every_machine(void (*cb)(const char *machine));
+void qtest_cb_for_every_machine(void (*cb)(const char *machine),
+                                bool skip_old_versioned);
 
 /**
  * qtest_qmp_device_add:
diff --git a/tests/qom-test.c b/tests/qom-test.c
index e6f712c..73c52af 100644
--- a/tests/qom-test.c
+++ b/tests/qom-test.c
@@ -123,7 +123,7 @@ int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
-    qtest_cb_for_every_machine(add_machine_test_case);
+    qtest_cb_for_every_machine(add_machine_test_case, g_test_quick());
 
     return g_test_run();
 }
diff --git a/tests/test-hmp.c b/tests/test-hmp.c
index 5352c9c..1a3a9c5 100644
--- a/tests/test-hmp.c
+++ b/tests/test-hmp.c
@@ -158,7 +158,7 @@ int main(int argc, char **argv)
 
     g_test_init(&argc, &argv, NULL);
 
-    qtest_cb_for_every_machine(add_machine_test_case);
+    qtest_cb_for_every_machine(add_machine_test_case, g_test_quick());
 
     /* as none machine has no memory by default, add a test case with memory */
     qtest_add_data_func("hmp/none+2MB", g_strdup("none -m 2"), test_machine);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 5/8] tests: Skip old versioned machine types in quick testing mode

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> The tests that check something for all machine types currently spend
> a lot of time checking old machine types (like "pc-i440fx-2.0" for
> example). The chances that we find something new there in addition
> to checking the latest version of a machine type are pretty low, so
> we should not waste the time of the developers by testing this again
> and again in the "quick" testing mode.
> Thus let's add some code to determine whether we are testing a current
> machine type or an old one, and only test the old types if we are
> running in "SPEED=slow" mode.
> This decreases the testing time quite a bit now, e.g. the qom-test
> now finishes within 4 seconds for qemu-system-x86_64 instead of 30
> seconds when testing all machines.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Thanks!!!!!

[Qemu-devel] [PATCH v2 6/8] tests/device-introspection: Check that the qom-tree and qtree do not change

[Thomas Huth]

Introspection should not change the qom-tree / qtree, so we should check
this in the device-introspect-test, too. This patch helped to find lots
of instrospection bugs during the QEMU v3.0 soft/hard-freeze period in the
last two months.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/device-introspect-test.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index 0b4f221..a38193b 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -103,7 +103,14 @@ static QList *device_type_list(bool abstract)
 static void test_one_device(const char *type)
 {
     QDict *resp;
-    char *help, *qom_tree;
+    char *help;
+    char *qom_tree_start, *qom_tree_end;
+    char *qtree_start, *qtree_end;
+
+    g_test_message("Testing device '%s'", type);
+
+    qom_tree_start = hmp("info qom-tree");
+    qtree_start = hmp("info qtree");
 
     resp = qmp("{'execute': 'device-list-properties',"
                " 'arguments': {'typename': %s}}",
@@ -115,10 +122,18 @@ static void test_one_device(const char *type)
 
     /*
      * Some devices leave dangling pointers in QOM behind.
-     * "info qom-tree" has a good chance at crashing then
+     * "info qom-tree" or "info qtree" have a good chance at crashing then.
+     * Also make sure that the tree did not change.
      */
-    qom_tree = hmp("info qom-tree");
-    g_free(qom_tree);
+    qom_tree_end = hmp("info qom-tree");
+    g_assert_cmpstr(qom_tree_start, ==, qom_tree_end);
+    g_free(qom_tree_start);
+    g_free(qom_tree_end);
+
+    qtree_end = hmp("info qtree");
+    g_assert_cmpstr(qtree_start, ==, qtree_end);
+    g_free(qtree_start);
+    g_free(qtree_end);
 }
 
 static void test_device_intro_list(void)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 6/8] tests/device-introspection: Check that the qom-tree and qtree do not change

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> Introspection should not change the qom-tree / qtree, so we should check
> this in the device-introspect-test, too. This patch helped to find lots
> of instrospection bugs during the QEMU v3.0 soft/hard-freeze period in the
> last two months.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

[Qemu-devel] [PATCH v2 7/8] tests/device-introspect: Test with all machines, not only with "none"

[Thomas Huth]

Certain device introspection crashes used to only happen if you were
using a certain machine, e.g. if the machine was using serial_hd() or
nd_table[], and a device was trying to use these in its instance_init
function, too.

To be able to catch these problems, let's extend the device-introspect
test to check the devices on all machine types, with and without the
"-nodefaults" parameter (since this makes a difference sometimes, too).
Since this is a rather slow operation, and most of the problems are
already handled by testing with the "none" machine only, the test with
all machines is only run in the "make check SPEED=slow" mode.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/device-introspect-test.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
index a38193b..a25092d 100644
--- a/tests/device-introspect-test.c
+++ b/tests/device-introspect-test.c
@@ -221,13 +221,13 @@ static void test_device_intro_abstract(void)
     qtest_end();
 }
 
-static void test_device_intro_concrete(void)
+static void test_device_intro_concrete(const void *args)
 {
     QList *types;
     QListEntry *entry;
     const char *type;
 
-    qtest_start(common_args);
+    qtest_start(args);
     types = device_type_list(false);
 
     QLIST_FOREACH_ENTRY(types, entry) {
@@ -239,6 +239,7 @@ static void test_device_intro_concrete(void)
 
     qobject_unref(types);
     qtest_end();
+    g_free((void *)args);
 }
 
 static void test_abstract_interfaces(void)
@@ -275,6 +276,26 @@ static void test_abstract_interfaces(void)
     qtest_end();
 }
 
+static void add_machine_test_case(const char *mname)
+{
+    char *path, *args;
+
+    /* Ignore blacklisted machines */
+    if (g_str_equal("xenfv", mname) || g_str_equal("xenpv", mname)) {
+        return;
+    }
+
+    path = g_strdup_printf("device/introspect/concrete/defaults/%s", mname);
+    args = g_strdup_printf("-M %s", mname);
+    qtest_add_data_func(path, args, test_device_intro_concrete);
+    g_free(path);
+
+    path = g_strdup_printf("device/introspect/concrete/nodefaults/%s", mname);
+    args = g_strdup_printf("-nodefaults -M %s", mname);
+    qtest_add_data_func(path, args, test_device_intro_concrete);
+    g_free(path);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
@@ -283,8 +304,13 @@ int main(int argc, char **argv)
     qtest_add_func("device/introspect/list-fields", test_qom_list_fields);
     qtest_add_func("device/introspect/none", test_device_intro_none);
     qtest_add_func("device/introspect/abstract", test_device_intro_abstract);
-    qtest_add_func("device/introspect/concrete", test_device_intro_concrete);
     qtest_add_func("device/introspect/abstract-interfaces", test_abstract_interfaces);
+    if (g_test_quick()) {
+        qtest_add_data_func("device/introspect/concrete/defaults/none",
+                            g_strdup(common_args), test_device_intro_concrete);
+    } else {
+        qtest_cb_for_every_machine(add_machine_test_case, true);
+    }
 
     return g_test_run();
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 7/8] tests/device-introspect: Test with all machines, not only with "none"

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> Certain device introspection crashes used to only happen if you were
> using a certain machine, e.g. if the machine was using serial_hd() or
> nd_table[], and a device was trying to use these in its instance_init
> function, too.
>
> To be able to catch these problems, let's extend the device-introspect
> test to check the devices on all machine types, with and without the
> "-nodefaults" parameter (since this makes a difference sometimes, too).
> Since this is a rather slow operation, and most of the problems are
> already handled by testing with the "none" machine only, the test with
> all machines is only run in the "make check SPEED=slow" mode.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>


> ---
>  tests/device-introspect-test.c | 32 +++++++++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
> index a38193b..a25092d 100644
> --- a/tests/device-introspect-test.c
> +++ b/tests/device-introspect-test.c
> @@ -221,13 +221,13 @@ static void test_device_intro_abstract(void)
>      qtest_end();
>  }
>  
> -static void test_device_intro_concrete(void)
> +static void test_device_intro_concrete(const void *args)
                                          ^^^^^^^^^^^^^^^^

I hate this game of casts, but I have no good suggestions, sorry.

[Qemu-devel] [PATCH v2 8/8] MAINTAINERS: add maintainers for qtest

[Thomas Huth]

From: Paolo Bonzini <pbonzini@redhat.com>

Thomas has been doing a lot of work on qom-test and device-introspection-test,
and Laurent has ported libqos to sPAPR and co-mentored Emanuele on the
upcoming qtest device framework.  They deserve recognition. :)

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
[thuth: removed the hunk that deleted the tests/qom-test.c entry]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 MAINTAINERS | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 666e936..072824c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1697,6 +1697,16 @@ F: scripts/qmp/
 F: tests/qmp-test.c
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
 
+qtest
+M: Paolo Bonzini <pbonzini@redhat.com>
+M: Thomas Huth <thuth@redhat.com>
+M: Laurent Vivier <lvivier@redhat.com>
+S: Maintained
+F: qtest.c
+F: tests/libqtest.*
+F: tests/libqos/
+F: tests/*-test.c
+
 Register API
 M: Alistair Francis <alistair@alistair23.me>
 S: Maintained
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 8/8] MAINTAINERS: add maintainers for qtest

[Juan Quintela]

Thomas Huth <thuth@redhat.com> wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> Thomas has been doing a lot of work on qom-test and device-introspection-test,
> and Laurent has ported libqos to sPAPR and co-mentored Emanuele on the
> upcoming qtest device framework.  They deserve recognition. :)
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> [thuth: removed the hunk that deleted the tests/qom-test.c entry]
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/8] Various qtest-related patches and a mc146818rtc fix

[Paolo Bonzini]

On 16/08/2018 13:35, Thomas Huth wrote:
> The basic idea for this patch series are the two patches that improve
> the device-inrospection test: We now check that the qom-tree and the
> qtree do not change during introspection, and the we also turn on the
> device introspection test with all machines when running in SPEED=slow
> mode. The other patches are a bug fix for the mc146818rtc device which
> has been discovered with the improved device-introspection test, two
> patches to shut up annoying messages during "make check", a patch to
> speed up the tests that iterate over all machine types and an update
> to the MAINTAINERS file.
> 
> v2:
>  - Added Reviewed-bys from v1
>  - Use g_test_message() instead of g_debug()
>  - Improved the patch descriptions a little bit
>  - Added patch to skip old versioned machine types in quick mode
>  - Added Paolo's update to MAINTAINERS
> 
> Paolo Bonzini (1):
>   MAINTAINERS: add maintainers for qtest
> 
> Thomas Huth (7):
>   tests/migration-test: Silence the kvm_hv message by default
>   net: Silence 'has no peer' messages in testing mode
>   hw/timer/mc146818rtc: White space clean-up
>   hw/timer/mc146818rtc: Fix introspection problem
>   tests: Skip old versioned machine types in quick testing mode
>   tests/device-introspection: Check that the qom-tree and qtree do not
>     change
>   tests/device-introspect: Test with all machines, not only with "none"
> 
>  MAINTAINERS                    | 10 ++++++++
>  hw/ppc/prep.c                  |  3 +++
>  hw/timer/mc146818rtc.c         | 20 ++++++---------
>  tests/cpu-plug-test.c          |  6 ++---
>  tests/device-introspect-test.c | 55 ++++++++++++++++++++++++++++++++++++------
>  tests/libqtest.c               | 52 +++++++++++++++++++++++++++++++++++++--
>  tests/libqtest.h               |  4 ++-
>  tests/migration-test.c         | 20 ++++++++-------
>  tests/qom-test.c               |  2 +-
>  tests/test-hmp.c               |  2 +-
>  vl.c                           |  3 +--
>  11 files changed, 138 insertions(+), 39 deletions(-)
> 

Queued, thanks.

Paolo