qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events
@ 2025-11-07 17:43 Peter Maydell
  2025-11-14 13:21 ` Peter Maydell
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Peter Maydell @ 2025-11-07 17:43 UTC (permalink / raw)
  To: qemu-devel; +Cc: Fabiano Rosas, Laurent Vivier, Paolo Bonzini

In the qtest_event() QEMUChrEvent handler, we create a timer
and log OPENED on CHR_EVENT_OPENED, and we destroy the timer and
log CLOSED on CHR_EVENT_CLOSED. However, the chardev subsystem
can send us more than one CHR_EVENT_CLOSED if we're reading from
a file chardev:
 * the first one happens when we read the last data from the file
 * the second one happens when the user hits ^C to exit QEMU
   and the chardev is finalized: char_fd_finalize()

This causes us to call g_timer_elapsed() with a NULL timer
(which glib complains about) and print an extra CLOSED log line
with a zero timestamp:

[I +0.063829] CLOSED
qemu-system-aarch64: GLib: g_timer_elapsed: assertion 'timer != NULL' failed
[I +0.000000] CLOSED

Avoid this by ignoring a CHR_EVENT_CLOSED if we have already
processed one.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 system/qtest.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/system/qtest.c b/system/qtest.c
index baef06d4d1b..67e2385f4b0 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -815,6 +815,10 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
         }
         break;
     case CHR_EVENT_CLOSED:
+        if (!qtest_opened) {
+            /* Ignore CLOSED events if we have already closed the log */
+            break;
+        }
         qtest_opened = false;
         if (qtest_log_fp) {
             fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n", g_timer_elapsed(timer, NULL));
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events
  2025-11-07 17:43 [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events Peter Maydell
@ 2025-11-14 13:21 ` Peter Maydell
  2025-11-14 19:53   ` Fabiano Rosas
  2025-11-14 14:30 ` Laurent Vivier
  2025-11-18 18:32 ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2025-11-14 13:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: Fabiano Rosas, Laurent Vivier, Paolo Bonzini

Ping for code review?

thanks
-- PMM

On Fri, 7 Nov 2025 at 17:43, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the qtest_event() QEMUChrEvent handler, we create a timer
> and log OPENED on CHR_EVENT_OPENED, and we destroy the timer and
> log CLOSED on CHR_EVENT_CLOSED. However, the chardev subsystem
> can send us more than one CHR_EVENT_CLOSED if we're reading from
> a file chardev:
>  * the first one happens when we read the last data from the file
>  * the second one happens when the user hits ^C to exit QEMU
>    and the chardev is finalized: char_fd_finalize()
>
> This causes us to call g_timer_elapsed() with a NULL timer
> (which glib complains about) and print an extra CLOSED log line
> with a zero timestamp:
>
> [I +0.063829] CLOSED
> qemu-system-aarch64: GLib: g_timer_elapsed: assertion 'timer != NULL' failed
> [I +0.000000] CLOSED
>
> Avoid this by ignoring a CHR_EVENT_CLOSED if we have already
> processed one.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  system/qtest.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/system/qtest.c b/system/qtest.c
> index baef06d4d1b..67e2385f4b0 100644
> --- a/system/qtest.c
> +++ b/system/qtest.c
> @@ -815,6 +815,10 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
>          }
>          break;
>      case CHR_EVENT_CLOSED:
> +        if (!qtest_opened) {
> +            /* Ignore CLOSED events if we have already closed the log */
> +            break;
> +        }
>          qtest_opened = false;
>          if (qtest_log_fp) {
>              fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n", g_timer_elapsed(timer, NULL));
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events
  2025-11-07 17:43 [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events Peter Maydell
  2025-11-14 13:21 ` Peter Maydell
@ 2025-11-14 14:30 ` Laurent Vivier
  2025-11-18 18:32 ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2025-11-14 14:30 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: Fabiano Rosas, Paolo Bonzini

On 11/7/25 18:43, Peter Maydell wrote:
> In the qtest_event() QEMUChrEvent handler, we create a timer
> and log OPENED on CHR_EVENT_OPENED, and we destroy the timer and
> log CLOSED on CHR_EVENT_CLOSED. However, the chardev subsystem
> can send us more than one CHR_EVENT_CLOSED if we're reading from
> a file chardev:
>   * the first one happens when we read the last data from the file
>   * the second one happens when the user hits ^C to exit QEMU
>     and the chardev is finalized: char_fd_finalize()
> 
> This causes us to call g_timer_elapsed() with a NULL timer
> (which glib complains about) and print an extra CLOSED log line
> with a zero timestamp:
> 
> [I +0.063829] CLOSED
> qemu-system-aarch64: GLib: g_timer_elapsed: assertion 'timer != NULL' failed
> [I +0.000000] CLOSED
> 
> Avoid this by ignoring a CHR_EVENT_CLOSED if we have already
> processed one.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   system/qtest.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/system/qtest.c b/system/qtest.c
> index baef06d4d1b..67e2385f4b0 100644
> --- a/system/qtest.c
> +++ b/system/qtest.c
> @@ -815,6 +815,10 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
>           }
>           break;
>       case CHR_EVENT_CLOSED:
> +        if (!qtest_opened) {
> +            /* Ignore CLOSED events if we have already closed the log */
> +            break;
> +        }
>           qtest_opened = false;
>           if (qtest_log_fp) {
>               fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n", g_timer_elapsed(timer, NULL));

Reviewed-by: Laurent Vivier <lvivier@redhat.com>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events
  2025-11-14 13:21 ` Peter Maydell
@ 2025-11-14 19:53   ` Fabiano Rosas
  0 siblings, 0 replies; 5+ messages in thread
From: Fabiano Rosas @ 2025-11-14 19:53 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: Laurent Vivier, Paolo Bonzini

Peter Maydell <peter.maydell@linaro.org> writes:

> Ping for code review?
>
> thanks
> -- PMM
>
> On Fri, 7 Nov 2025 at 17:43, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> In the qtest_event() QEMUChrEvent handler, we create a timer
>> and log OPENED on CHR_EVENT_OPENED, and we destroy the timer and
>> log CLOSED on CHR_EVENT_CLOSED. However, the chardev subsystem
>> can send us more than one CHR_EVENT_CLOSED if we're reading from
>> a file chardev:
>>  * the first one happens when we read the last data from the file
>>  * the second one happens when the user hits ^C to exit QEMU
>>    and the chardev is finalized: char_fd_finalize()
>>
>> This causes us to call g_timer_elapsed() with a NULL timer
>> (which glib complains about) and print an extra CLOSED log line
>> with a zero timestamp:
>>
>> [I +0.063829] CLOSED
>> qemu-system-aarch64: GLib: g_timer_elapsed: assertion 'timer != NULL' failed
>> [I +0.000000] CLOSED
>>
>> Avoid this by ignoring a CHR_EVENT_CLOSED if we have already
>> processed one.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  system/qtest.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/system/qtest.c b/system/qtest.c
>> index baef06d4d1b..67e2385f4b0 100644
>> --- a/system/qtest.c
>> +++ b/system/qtest.c
>> @@ -815,6 +815,10 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
>>          }
>>          break;
>>      case CHR_EVENT_CLOSED:
>> +        if (!qtest_opened) {
>> +            /* Ignore CLOSED events if we have already closed the log */
>> +            break;
>> +        }
>>          qtest_opened = false;
>>          if (qtest_log_fp) {
>>              fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n", g_timer_elapsed(timer, NULL));
>> --
>> 2.43.0

I'm aware some qtest stuff is lingering on the list, I came back from
vacations yesterday and will give attention to it in the next few days.

Reviewed-by: Fabiano Rosas <farosas@suse.de>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events
  2025-11-07 17:43 [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events Peter Maydell
  2025-11-14 13:21 ` Peter Maydell
  2025-11-14 14:30 ` Laurent Vivier
@ 2025-11-18 18:32 ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-11-18 18:32 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel; +Cc: Fabiano Rosas, Laurent Vivier, Paolo Bonzini

On 7/11/25 18:43, Peter Maydell wrote:
> In the qtest_event() QEMUChrEvent handler, we create a timer
> and log OPENED on CHR_EVENT_OPENED, and we destroy the timer and
> log CLOSED on CHR_EVENT_CLOSED. However, the chardev subsystem
> can send us more than one CHR_EVENT_CLOSED if we're reading from
> a file chardev:
>   * the first one happens when we read the last data from the file
>   * the second one happens when the user hits ^C to exit QEMU
>     and the chardev is finalized: char_fd_finalize()
> 
> This causes us to call g_timer_elapsed() with a NULL timer
> (which glib complains about) and print an extra CLOSED log line
> with a zero timestamp:
> 
> [I +0.063829] CLOSED
> qemu-system-aarch64: GLib: g_timer_elapsed: assertion 'timer != NULL' failed
> [I +0.000000] CLOSED
> 
> Avoid this by ignoring a CHR_EVENT_CLOSED if we have already
> processed one.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   system/qtest.c | 4 ++++
>   1 file changed, 4 insertions(+)

Queued, thanks.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-11-18 18:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07 17:43 [PATCH] system/qtest.c: Allow for multiple CHR_EVENT_CLOSED events Peter Maydell
2025-11-14 13:21 ` Peter Maydell
2025-11-14 19:53   ` Fabiano Rosas
2025-11-14 14:30 ` Laurent Vivier
2025-11-18 18:32 ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).