monitor/hmp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
monitor_parse_arguments() also need to known the current monitoar:
(gdb) bt
#0 0x0000555555ac6a6d in mon_get_cpu_sync (mon=0x0, synchronize=synchronize@entry=true) at ../monitor/misc.c:270
#1 0x0000555555ac6b4a in mon_get_cpu () at ../monitor/misc.c:294
#2 0x0000555555ac80fd in get_monitor_def (pval=pval@entry=0x7fffffffcc78, name=name@entry=0x7fffffffcc80 "pc") at ../monitor/misc.c:1669
#3 0x000055555583fa8a in expr_unary (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:387
#4 0x000055555583fb32 in expr_prod (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:421
#5 0x000055555583fbcc in expr_logic (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:455
#6 0x000055555583f82c in expr_sum (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:484
#7 0x000055555583fc97 in get_expr (mon=mon@entry=0x5555568a75a0, pval=pval@entry=0x7fffffffce18, pp=pp@entry=0x7fffffffce08) at ../monitor/hmp.c:511
#8 0x00005555558409b1 in monitor_parse_arguments (mon=mon@entry=0x5555568a75a0, cmd=0x555556561e40 <hmp_cmds+7040>, cmd=0x555556561e40 <hmp_cmds+7040>, endp=0x7fffffffd288) at ../monitor/hmp.c:876
#9 0x0000555555841796 in handle_hmp_command (mon=mon@entry=0x5555568a75a0, cmdline=0x5555568b12b3 "$pc", cmdline@entry=0x5555568b12b0 "xp $pc") at ../monitor/hmp.c:1073
Therefore update current monitor as soon as possible to avoid
hmp/xp command failure.
Fixes: ff04108a0e36 ("hmp: Update current monitor only in handle_hmp_command()")
Signed-off-by: lichun <lichun@ruijie.com.cn>
---
monitor/hmp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/monitor/hmp.c b/monitor/hmp.c
index c5cd9d3..ee5413e 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1072,52 +1072,52 @@ static void handle_hmp_command_co(void *opaque)
}
void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
{
QDict *qdict;
const HMPCommand *cmd;
const char *cmd_start = cmdline;
trace_handle_hmp_command(mon, cmdline);
+ /* old_mon is non-NULL when called from qmp_human_monitor_command() */
+ Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
+
cmd = monitor_parse_command(mon, cmdline, &cmdline, hmp_cmds);
if (!cmd) {
return;
}
qdict = monitor_parse_arguments(&mon->common, &cmdline, cmd);
if (!qdict) {
while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
cmdline--;
}
monitor_printf(&mon->common, "Try \"help %.*s\" for more information\n",
(int)(cmdline - cmd_start), cmd_start);
return;
}
if (!cmd->coroutine) {
- /* old_mon is non-NULL when called from qmp_human_monitor_command() */
- Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
cmd->cmd(&mon->common, qdict);
- monitor_set_cur(qemu_coroutine_self(), old_mon);
} else {
HandleHmpCommandCo data = {
.mon = &mon->common,
.cmd = cmd,
.qdict = qdict,
.done = false,
};
Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data);
- monitor_set_cur(co, &mon->common);
aio_co_enter(qemu_get_aio_context(), co);
AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done);
}
+ monitor_set_cur(qemu_coroutine_self(), old_mon);
qobject_unref(qdict);
}
static void cmd_completion(MonitorHMP *mon, const char *name, const char *list)
{
const char *p, *pstart;
char cmd[128];
int len;
--
1.8.3.1
Am 13.11.2020 um 12:13 hat lichun geschrieben: > monitor_parse_arguments() also need to known the current monitoar: > (gdb) bt > #0 0x0000555555ac6a6d in mon_get_cpu_sync (mon=0x0, synchronize=synchronize@entry=true) at ../monitor/misc.c:270 > #1 0x0000555555ac6b4a in mon_get_cpu () at ../monitor/misc.c:294 > #2 0x0000555555ac80fd in get_monitor_def (pval=pval@entry=0x7fffffffcc78, name=name@entry=0x7fffffffcc80 "pc") at ../monitor/misc.c:1669 > #3 0x000055555583fa8a in expr_unary (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:387 > #4 0x000055555583fb32 in expr_prod (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:421 > #5 0x000055555583fbcc in expr_logic (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:455 > #6 0x000055555583f82c in expr_sum (mon=mon@entry=0x5555568a75a0) at ../monitor/hmp.c:484 > #7 0x000055555583fc97 in get_expr (mon=mon@entry=0x5555568a75a0, pval=pval@entry=0x7fffffffce18, pp=pp@entry=0x7fffffffce08) at ../monitor/hmp.c:511 > #8 0x00005555558409b1 in monitor_parse_arguments (mon=mon@entry=0x5555568a75a0, cmd=0x555556561e40 <hmp_cmds+7040>, cmd=0x555556561e40 <hmp_cmds+7040>, endp=0x7fffffffd288) at ../monitor/hmp.c:876 > #9 0x0000555555841796 in handle_hmp_command (mon=mon@entry=0x5555568a75a0, cmdline=0x5555568b12b3 "$pc", cmdline@entry=0x5555568b12b0 "xp $pc") at ../monitor/hmp.c:1073 > Therefore update current monitor as soon as possible to avoid > hmp/xp command failure. > > Fixes: ff04108a0e36 ("hmp: Update current monitor only in handle_hmp_command()") > Signed-off-by: lichun <lichun@ruijie.com.cn> > --- > monitor/hmp.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/monitor/hmp.c b/monitor/hmp.c > index c5cd9d3..ee5413e 100644 > --- a/monitor/hmp.c > +++ b/monitor/hmp.c > @@ -1072,52 +1072,52 @@ static void handle_hmp_command_co(void *opaque) > } > > void handle_hmp_command(MonitorHMP *mon, const char *cmdline) > { > QDict *qdict; > const HMPCommand *cmd; > const char *cmd_start = cmdline; > > trace_handle_hmp_command(mon, cmdline); > > + /* old_mon is non-NULL when called from qmp_human_monitor_command() */ > + Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); > + > cmd = monitor_parse_command(mon, cmdline, &cmdline, hmp_cmds); > if (!cmd) { > return; > } Now the monitor isn't changed back in all early return cases. > > qdict = monitor_parse_arguments(&mon->common, &cmdline, cmd); > if (!qdict) { > while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) { > cmdline--; > } > monitor_printf(&mon->common, "Try \"help %.*s\" for more information\n", > (int)(cmdline - cmd_start), cmd_start); > return; > } > > if (!cmd->coroutine) { > - /* old_mon is non-NULL when called from qmp_human_monitor_command() */ > - Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common); > cmd->cmd(&mon->common, qdict); > - monitor_set_cur(qemu_coroutine_self(), old_mon); > } else { > HandleHmpCommandCo data = { > .mon = &mon->common, > .cmd = cmd, > .qdict = qdict, > .done = false, > }; > Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data); > - monitor_set_cur(co, &mon->common); Removing this line is wrong, we still need to set the current monitor for co, which is not qemu_coroutine_self() self. > aio_co_enter(qemu_get_aio_context(), co); > AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done); > } > + monitor_set_cur(qemu_coroutine_self(), old_mon); > > qobject_unref(qdict); > } With the above bugs fixed, this approach is one option to fix the bug. Personally, if it's possible with reasonable effort, I would prefer the other way, which is making sure that monitor_cur() isn't used, but the Monitor pointer is just passed down. This would be a bigger change, but it wouldn't only fix the bug, but also clean up the code and make it more maintainable. I can try to write a patch series to do it this way and see how it goes. Kevin
© 2016 - 2024 Red Hat, Inc.