QMP: Save default control monitor for emitting async events
PATCH 1/1 QMP: Save default control monitor for emitting async events
When using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by monitor_protocol_event() always go to cur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor).
One solution is to save the default QMP session in another monitor pointer (ala cur_mon) and always direct asynchronous events to that monitor...
monitor.c +6 -2
@@ -128,6 +128,7 @@ static const mon_cmd_t mon_cmds[];static const mon_cmd_t info_cmds[];Monitor *cur_mon =NULL ; +Monitor *cur_mon_control =NULL ;static void monitor_command_cb (Monitor *mon,const char *cmdline,void *opaque);
@@ -334,11 +335,11 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) {QDict *qmp;const char *event_name; -Monitor *mon = cur_mon; +Monitor *mon = cur_mon_control;assert (event <QEVENT_MAX ); -if (!monitor_ctrl_mode (mon)) +if (!mon)return ;switch (event) {
@@ -4283,6 +4284,9 @@ void monitor_init(CharDriverState *chr, int flags)QLIST_INSERT_HEAD (&mon_list, mon, entry);if (!cur_mon || (flags &MONITOR_IS_DEFAULT )) cur_mon = mon; +if (!cur_mon_control || (flags &MONITOR_IS_DEFAULT )) +if (flags &MONITOR_USE_CONTROL ) + cur_mon_control = mon; }static void bdrv_password_cb (Monitor *mon,const char *password,void *opaque)
monitor.h +1 -0
@@ -7,6 +7,7 @@#include "block.h" extern Monitor *cur_mon; +extern Monitor *cur_mon_control;/* flags for monitor_init */ #define MONITOR_IS_DEFAULT 0x01
qemu-tool.c +1 -0
@@ -34,6 +34,7 @@ void qemu_service_io(void) }Monitor *cur_mon; +Monitor *cur_mon_control;void monitor_printf (Monitor *mon,const char *fmt, ...) {
Signature · 2 lines
Thanks, Adam
Quoted 5 lines · On Thu, 14 Jan 2010 15:20:10 -0600 Adam Litke <[email protected]> wrote:
When using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by
monitor_protocol_event()always go tocur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor).
I think we have two problems here:
1. Async messages are only delivered for the default Monitor, so if the QMP Monitor is not the default one it won't get them (not a bug if well documented, but it's annoying)
2. On a multiple QMP Monitor setup, only one of them will get async messages
This patch fixes 1. but the best fix would be to solve both problems,
as QMP Monitors have to be equally capable IMO.
There's an array with all Monitors IIRC, maybe we could loop through it
and delivery the message to each QMP ones of them?
Quoted 67 lines
One solution is to save the default QMP session in another monitor pointer (ala
cur_mon) and always direct asynchronous events to that monitor...Signed-off-by Adam Litke <[email protected]>monitor.c +6
-2@@ -128,6 +128,7 @@ static const mon_cmd_t mon_cmds[];static const mon_cmd_t info_cmds[];Monitor *cur_mon =NULL ; +Monitor *cur_mon_control =NULL ;static void monitor_command_cb (Monitor *mon,const char *cmdline,void *opaque);@@ -334,11 +335,11 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) {QDict *qmp;const char *event_name; -Monitor *mon = cur_mon; +Monitor *mon = cur_mon_control;assert (event <QEVENT_MAX ); -if (!monitor_ctrl_mode (mon)) +if (!mon)return ;switch (event) {@@ -4283,6 +4284,9 @@ void monitor_init(CharDriverState *chr, int flags)QLIST_INSERT_HEAD (&mon_list, mon, entry);if (!cur_mon || (flags &MONITOR_IS_DEFAULT )) cur_mon = mon; +if (!cur_mon_control || (flags &MONITOR_IS_DEFAULT )) +if (flags &MONITOR_USE_CONTROL ) + cur_mon_control = mon; }static void bdrv_password_cb (Monitor *mon,const char *password,void *opaque)monitor.h +1
-0@@ -7,6 +7,7 @@#include "block.h" extern Monitor *cur_mon; +extern Monitor *cur_mon_control;/* flags for monitor_init */ #define MONITOR_IS_DEFAULT 0x01qemu-tool.c +1
-0@@ -34,6 +34,7 @@ void qemu_service_io(void) }Monitor *cur_mon; +Monitor *cur_mon_control;void monitor_printf (Monitor *mon,const char *fmt, ...) {
Quoted 15 lines · Luiz Capitulino
Quoted 5 lines · On Thu, 14 Jan 2010 15:20:10 -0600 Adam Litke <[email protected]> wrote:
When using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by
monitor_protocol_event()always go tocur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor).I think we have two problems here:
1. Async messages are only delivered for the default Monitor, so if the QMP Monitor is not the default one it won't get them (not a bug if well documented, but it's annoying)
Quoted 7 lines
2. On a multiple QMP Monitor setup, only one of them will get async messages
This patch fixes 1. but the best fix would be to solve both problems,
as QMP Monitors have to be equally capable IMO.There's an array with all Monitors IIRC, maybe we could loop through it
and delivery the message to each QMP ones of them?
Sure. This was the other way I was considering. I'll spin up a patch and we can see how it looks.
Signature · 2 lines
Thanks, Adam
PATCH 1/1 QMP: Emit asynchronous events on all QMP monitors
In reply to Luiz CapitulinoWhen using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by monitor_protocol_event() always go to cur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor). Additionally, only one QMP session is currently able to receive async messages.
To avoid this confusion, scan through the list of monitors and emit the message on each QMP monitor.
monitor.c +7 -5
@@ -334,13 +334,10 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) {QDict *qmp;const char *event_name; -Monitor *mon = cur_mon; +Monitor *mon;assert (event <QEVENT_MAX ); -if (!monitor_ctrl_mode (mon)) -return ; -switch (event) {case QEVENT_DEBUG : event_name ="DEBUG" ;
@@ -373,7 +370,12 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)qdict_put_obj (qmp,"data" , data); } -monitor_json_emitter (mon,QOBJECT (qmp)); + QLIST_FOREACH(mon, &mon_list, entry) { +if (!monitor_ctrl_mode (mon)) +return ; + +monitor_json_emitter (mon,QOBJECT (qmp)); + }QDECREF (qmp); }
Signature · 2 lines
Thanks, Adam
[Qemu-devel] Re: [PATCH] QMP: Emit asynchronous events on all QMP monitors
Quoted 32 lines · On Fri, 15 Jan 2010 08:34:02 -0600 Adam Litke <[email protected]> wrote:
When using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by
monitor_protocol_event()always go tocur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor). Additionally, only one QMP session is currently able to receive async messages.To avoid this confusion, scan through the list of monitors and emit the message on each QMP monitor.
Signed-off-by Adam Litke <[email protected]>monitor.c +1
-4@@ -334,13 +334,10 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) {QDict *qmp;const char *event_name; -Monitor *mon = cur_mon; +Monitor *mon;assert (event <QEVENT_MAX ); -if (!monitor_ctrl_mode (mon)) -return ; -switch (event) {case QEVENT_DEBUG : event_name ="DEBUG" ;
monitor.c void monitor_protocol_event(MonitorEvent event, QObject *data) · line 378
Quoted 10 lines
monitor.c +6
-1@@ -373,7 +370,12 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)qdict_put_obj (qmp,"data" , data); } -monitor_json_emitter (mon,QOBJECT (qmp)); + QLIST_FOREACH(mon, &mon_list, entry) { +if (!monitor_ctrl_mode (mon)) +return ; + +monitor_json_emitter (mon,QOBJECT (qmp)); + }
The function will return on the first !QMP Monitor, the
right QLIST_FOREACH() body is:
if (monitor_ctrl_mode(mon)) {
monitor_json_emitter(mon, QOBJECT(qmp));
}
I'll ACK the respin.
Quoted 5 lines
monitor.c +0
-0QDECREF (qmp); }
Adam Litke
Acked-by Luiz Capitulino[Qemu-devel] Re: [PATCH][RESPIN] QMP: Emit asynchronous events on all QMP monitors
Quoted 8 lines · Luiz Capitulino
The function will return on the first !QMP Monitor, the
rightQLIST_FOREACH()body is:if (
monitor_ctrl_mode(mon)) {
monitor_json_emitter(mon, QOBJECT(qmp));
}I'll ACK the respin.
Ah right, of course. Thanks and here it is.
When using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by monitor_protocol_event() always go to cur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor). Additionally, only one QMP session is currently able to receive async messages.
To avoid this confusion, scan through the list of monitors and emit the message on each QMP monitor.
monitor.c +6 -5
@@ -334,13 +334,10 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) {QDict *qmp;const char *event_name; -Monitor *mon = cur_mon; +Monitor *mon;assert (event <QEVENT_MAX ); -if (!monitor_ctrl_mode (mon)) -return ; -switch (event) {case QEVENT_DEBUG : event_name ="DEBUG" ;
@@ -373,7 +370,11 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)qdict_put_obj (qmp,"data" , data); } -monitor_json_emitter (mon,QOBJECT (qmp)); + QLIST_FOREACH(mon, &mon_list, entry) { +if (monitor_ctrl_mode (mon)) { +monitor_json_emitter (mon,QOBJECT (qmp)); + } + }QDECREF (qmp); }
Signature · 2 lines
Thanks, Adam
[Qemu-devel] Re: [PATCH][RESPIN] QMP: Emit asynchronous events on all QMP monitors
Quoted 19 lines · On Fri, 15 Jan 2010 09:16:03 -0600 Adam Litke <[email protected]> wrote:
Quoted 8 lines · Luiz Capitulino
The function will return on the first !QMP Monitor, the
rightQLIST_FOREACH()body is:if (
monitor_ctrl_mode(mon)) {
monitor_json_emitter(mon, QOBJECT(qmp));
}I'll ACK the respin.
Ah right, of course. Thanks and here it is.
When using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by
monitor_protocol_event()always go tocur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor). Additionally, only one QMP session is currently able to receive async messages.
Quoted 5 lines
To avoid this confusion, scan through the list of monitors and emit the message on each QMP monitor.
Signed-off-by Adam Litke <[email protected]>Acked-by Luiz Capitulino <[email protected]>
Now it's good, hope Anthony's script can get a patch from a thread,
this is probably good for stable too.
Btw, we have a lot of QMP tasks in case you're interested :-)
Quoted 35 lines
monitor.c +6
-5@@ -334,13 +334,10 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) {QDict *qmp;const char *event_name; -Monitor *mon = cur_mon; +Monitor *mon;assert (event <QEVENT_MAX ); -if (!monitor_ctrl_mode (mon)) -return ; -switch (event) {case QEVENT_DEBUG : event_name ="DEBUG" ;@@ -373,7 +370,11 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)qdict_put_obj (qmp,"data" , data); } -monitor_json_emitter (mon,QOBJECT (qmp)); + QLIST_FOREACH(mon, &mon_list, entry) { +if (monitor_ctrl_mode (mon)) { +monitor_json_emitter (mon,QOBJECT (qmp)); + } + }QDECREF (qmp); }
Anthony Liguori
AppliedRe: [Qemu-devel] [PATCH] QMP: Emit asynchronous events on all QMP monitors
In reply to Adam LitkeQuoted 7 lines · Adam Litke
When using a control/QMP monitor in tandem with a regular monitor, asynchronous messages can get lost depending on the order of the QEMU program arguments. QEMU events issued by
monitor_protocol_event()always go tocur_mon. If the user monitor was specified on the command line first (or it has ,default), the message will be directed to the user monitor (not the QMP monitor). Additionally, only one QMP session is currently able to receive async messages.
Quoted 5 lines
To avoid this confusion, scan through the list of monitors and emit the message on each QMP monitor.
Signed-off-by Adam Litke<[email protected]>
Applied. Thanks.
Regards,
Anthony Liguori
Quoted 36 lines
monitor.c +7
-5@@ -334,13 +334,10 @@ void monitor_protocol_event(MonitorEvent event, QObject *data) {QDict *qmp;const char *event_name; -Monitor *mon = cur_mon; +Monitor *mon;assert (event<QEVENT_MAX ); -if (!monitor_ctrl_mode (mon)) -return ; -switch (event) {case QEVENT_DEBUG : event_name ="DEBUG" ;@@ -373,7 +370,12 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)qdict_put_obj (qmp,"data" , data); } -monitor_json_emitter (mon,QOBJECT (qmp)); + QLIST_FOREACH(mon,&mon_list, entry) { +if (!monitor_ctrl_mode (mon)) +return ; + +monitor_json_emitter (mon,QOBJECT (qmp)); + }QDECREF (qmp); }