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>