Re: [PATCH] monitor: flush messages on abort

[Steven Sistare]

On 11/15/2023 11:15 AM, Markus Armbruster wrote:
> Steven Sistare <steven.sistare@oracle.com> writes:
>> On 11/15/2023 3:41 AM, Markus Armbruster wrote:
>>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>>
>>>> On Fri, Nov 03, 2023 at 03:51:00PM -0400, Steven Sistare wrote:
>>>>> On 11/3/2023 1:33 PM, Daniel P. Berrangé wrote:
>>>>>> On Fri, Nov 03, 2023 at 09:01:29AM -0700, Steve Sistare wrote:
>>>>>>> Buffered monitor output is lost when abort() is called.  The pattern
>>>>>>> error_report() followed by abort() occurs about 60 times, so valuable
>>>>>>> information is being lost when the abort is called in the context of a
>>>>>>> monitor command.
>>>>>>
>>>>>> I'm curious, was there a particular abort() scenario that you hit ?
>>>>>
>>>>> Yes, while tweaking the suspended state, and forgetting to add 
>>>>> transitions:
>>>>>
>>>>>         error_report("invalid runstate transition: '%s' -> '%s'",
>>>>>         abort();
>>>>>
>>>>> But I have previously hit this for other errors.
>>>
>>> Can you provide a reproducer?
>>
>> I sometimes hit this when developing new code.  I do not have a reproducer 
>> for upstream
>> branches. The patch is aimed at helping developers, not users.
> 
> I'm asking because I can't see how the error message could be lost.  A
> reproducer would let me find out.  "Apply this set of broken patches,
> then do that" would serve.

$ patch -p1 << EOF
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index b0f948d..c9a3aee 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -47,8 +47,12 @@ void qmp_quit(Error **errp)
     qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_QMP_QUIT);
 }

+#include "qemu/error-report.h"
+
 void qmp_stop(Error **errp)
 {
+    error_report("injected failure");
+    abort();
     /* if there is a dump in background, we should wait until the dump
      * finished */
     if (qemu_system_dump_in_progress()) {
EOF

# This example loses the error message:

$ args='-display none -chardev socket,id=mon1,server=on,path=mon1.sock,wait=off 
-mon mon1,mode=control'
$ qemu-system-x86_64 $args < /dev/null &
[1] 18048
$ echo '{"execute":"qmp_capabilities"} 
{"execute":"human-monitor-command","arguments":{"command-line":"stop"}}' | ncat 
-U mon1.sock
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 8}, "package": 
"v8.1.0-2976-g4025fde-dirty"}, "capabilities": ["oob"]}}
{"return": {}}
Ncat: Connection reset by peer.
$
[1]+  Aborted                 qemu-system-x86_64 $args < /dev/null


# This example preserves the error message. I include it to show the ncat-based 
test is valid.

$ qemu-system-x86_64 $args < /dev/null &
[1] 18060
$ echo '{"execute":"qmp_capabilities"} {"execute":"stop"}' | ncat -U mon1.sock
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 8}, "package": 
"v8.1.0-2976-g4025fde-dirty"}, "capabilities": ["oob"]}}
{"return": {}}
injected failure                <============= qemu stderr
Ncat: Connection reset by peer.
$
[1]+  Aborted                 qemu-system-x86_64 $args < /dev/null

- Steve