| Message ID | 20241210204349.723590-19-alex.bennee@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | testing/next: functional tests and qtest timers | expand |
On 10/12/2024 21.43, Alex Bennée wrote: > Time will not advance if the system is paused or there are no timer > events set for the future. In absence of pending timer events > advancing time would make no difference the system state. Attempting > to do so would be a bug and the test or device under test would need > fixing. > > Tighten up the result reporting to `FAIL` if time was not advanced. > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2687 > --- > system/qtest.c | 23 ++++++++++++++--------- > 1 file changed, 14 insertions(+), 9 deletions(-) > > diff --git a/system/qtest.c b/system/qtest.c > index 12703a2045..d9501153a4 100644 > --- a/system/qtest.c > +++ b/system/qtest.c > @@ -78,6 +78,11 @@ static void *qtest_server_send_opaque; > * let you adjust the value of the clock (monotonically). All the commands > * return the current value of the clock in nanoseconds. > * > + * If the commands FAIL then time wasn't advanced which is likely > + * because the machine was in a paused state or no timer events exist > + * in the future. This will cause qtest to abort and the test will > + * need to check its assumptions. > + * > * .. code-block:: none > * > * > clock_step > @@ -710,7 +715,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > qtest_sendf(chr, "OK little\n"); > } > } else if (qtest_enabled() && strcmp(words[0], "clock_step") == 0) { > - int64_t ns; > + int64_t old_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); > + int64_t ns, new_ns; > > if (words[1]) { > int ret = qemu_strtoi64(words[1], NULL, 0, &ns); > @@ -719,11 +725,10 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL, > QEMU_TIMER_ATTR_ALL); > } > - qemu_clock_advance_virtual_time( > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns); > + new_ns = qemu_clock_advance_virtual_time(old_ns + ns); > qtest_send_prefix(chr); > - qtest_sendf(chr, "OK %"PRIi64"\n", > - (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); > + qtest_sendf(chr, "%s %"PRIi64"\n", > + new_ns > old_ns ? "OK" : "FAIL", new_ns); > } else if (strcmp(words[0], "module_load") == 0) { > Error *local_err = NULL; > int rv; > @@ -740,16 +745,16 @@ static void qtest_process_command(CharBackend *chr, gchar **words) > qtest_sendf(chr, "FAIL\n"); > } > } else if (qtest_enabled() && strcmp(words[0], "clock_set") == 0) { > - int64_t ns; > + int64_t ns, new_ns; > int ret; > > g_assert(words[1]); > ret = qemu_strtoi64(words[1], NULL, 0, &ns); > g_assert(ret == 0); > - qemu_clock_advance_virtual_time(ns); > + new_ns = qemu_clock_advance_virtual_time(ns); > qtest_send_prefix(chr); > - qtest_sendf(chr, "OK %"PRIi64"\n", > - (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); > + qtest_sendf(chr, "%s %"PRIi64"\n", > + new_ns == ns ? "OK" : "FAIL", new_ns); > } else if (process_command_cb && process_command_cb(chr, words)) { > /* Command got consumed by the callback handler */ > } else { Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/system/qtest.c b/system/qtest.c index 12703a2045..d9501153a4 100644 --- a/system/qtest.c +++ b/system/qtest.c @@ -78,6 +78,11 @@ static void *qtest_server_send_opaque; * let you adjust the value of the clock (monotonically). All the commands * return the current value of the clock in nanoseconds. * + * If the commands FAIL then time wasn't advanced which is likely + * because the machine was in a paused state or no timer events exist + * in the future. This will cause qtest to abort and the test will + * need to check its assumptions. + * * .. code-block:: none * * > clock_step @@ -710,7 +715,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words) qtest_sendf(chr, "OK little\n"); } } else if (qtest_enabled() && strcmp(words[0], "clock_step") == 0) { - int64_t ns; + int64_t old_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); + int64_t ns, new_ns; if (words[1]) { int ret = qemu_strtoi64(words[1], NULL, 0, &ns); @@ -719,11 +725,10 @@ static void qtest_process_command(CharBackend *chr, gchar **words) ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL, QEMU_TIMER_ATTR_ALL); } - qemu_clock_advance_virtual_time( - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns); + new_ns = qemu_clock_advance_virtual_time(old_ns + ns); qtest_send_prefix(chr); - qtest_sendf(chr, "OK %"PRIi64"\n", - (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); + qtest_sendf(chr, "%s %"PRIi64"\n", + new_ns > old_ns ? "OK" : "FAIL", new_ns); } else if (strcmp(words[0], "module_load") == 0) { Error *local_err = NULL; int rv; @@ -740,16 +745,16 @@ static void qtest_process_command(CharBackend *chr, gchar **words) qtest_sendf(chr, "FAIL\n"); } } else if (qtest_enabled() && strcmp(words[0], "clock_set") == 0) { - int64_t ns; + int64_t ns, new_ns; int ret; g_assert(words[1]); ret = qemu_strtoi64(words[1], NULL, 0, &ns); g_assert(ret == 0); - qemu_clock_advance_virtual_time(ns); + new_ns = qemu_clock_advance_virtual_time(ns); qtest_send_prefix(chr); - qtest_sendf(chr, "OK %"PRIi64"\n", - (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)); + qtest_sendf(chr, "%s %"PRIi64"\n", + new_ns == ns ? "OK" : "FAIL", new_ns); } else if (process_command_cb && process_command_cb(chr, words)) { /* Command got consumed by the callback handler */ } else {
Time will not advance if the system is paused or there are no timer events set for the future. In absence of pending timer events advancing time would make no difference the system state. Attempting to do so would be a bug and the test or device under test would need fixing. Tighten up the result reporting to `FAIL` if time was not advanced. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2687 --- system/qtest.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)