OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

monitor: Drop broken, unused asynchronous command interface

Browse Source 
The asynchronous monitor command interface goes back to commit 940cc30
(Jan 2010).  Added a third case to command execution.  The hope back
then according to the commit message was that all commands get
converted to the asynchronous interface, killing off the other two
cases.  Didn't happen.

The initial asynchronous commands balloon and info balloon were
converted back to synchronous long ago (commit 96637bc and d72f32),
with commit messages calling the asynchronous interface "not fully
working" and "deprecated".  The only other user went away in commit
3b5704b.

New code generally uses synchronous commands and asynchronous events.

What exactly is still "not fully working" with asynchronous commands?
Well, here's a bug that defeats actual asynchronous use pretty
reliably: the reply's ID is wrong (and has always been wrong) unless
you use the command synchronously!  To reproduce, we need an
asynchronous command, so we have to go back before commit 3b5704b.
Run QEMU with spice:

    $ qemu-system-x86_64 -nodefaults -S -spice port=5900,disable-ticketing -qmp stdio
    {"QMP": {"version": {"qemu": {"micro": 94, "minor": 2, "major": 2}, "package": ""}, "capabilities": []}}

Connect a spice client in another terminal:

    $ remote-viewer spice://localhost:5900

Set up a migration destination dummy in a third terminal:

    $ socat TCP-LISTEN:12345 STDIO

Now paste the following into the QMP monitor:

    { "execute": "qmp_capabilities", "id": "i0" }
    { "execute": "client_migrate_info", "id": "i1", "arguments": { "protocol": "spice", "hostname": "localhost", "port": 12345 } }
    { "execute": "query-kvm", "id": "i2" }

Produces two replies immediately, one to qmp_capabilities, and one to
query-kvm:

    {"return": {}, "id": "i0"}
    {"return": {"enabled": false, "present": true}, "id": "i2"}

Both are correct.  Two lines of debug output from libspice-server not
shown.

Now EOF socat's standard input to make it close the connection.  This
makes the asynchronous client_migrate_info complete.  It replies:

    {"return": {}}

Bug: "id": "i1" is missing.  Two lines of debug output from
libspice-server not shown.  Cherry on top: storage for the missing ID
is leaked.

Get rid of this stuff before somebody hurts himself with it.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
This commit is contained in:
Markus Armbruster 2015-03-05 14:35:26 +01:00
parent b821cbe274
commit 65207c59d9
2 changed files with 2 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
include/monitor/monitor.h
View File

@ -16,9 +16,6 @@ extern Monitor *default_mon;
#define MONITOR_USE_CONTROL 0x04
#define MONITOR_USE_PRETTY 0x08
/* flags for monitor commands */
#define MONITOR_CMD_ASYNC 0x0001
int monitor_cur_is_qmp(void);
void monitor_init(CharDriverState *chr, int flags);
@ -43,8 +40,6 @@ void monitor_flush(Monitor *mon);
int monitor_set_cpu(int cpu_index);
int monitor_get_cpu_index(void);
typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
void monitor_set_error(Monitor *mon, QError *qerror);
void monitor_read_command(Monitor *mon, int show_prompt);
int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,

70
monitor.c
View File

@ -118,12 +118,6 @@
*
*/
typedef struct MonitorCompletionData MonitorCompletionData;
struct MonitorCompletionData {
Monitor *mon;
void (*user_print)(Monitor *mon, const QObject *data);
};
typedef struct mon_cmd_t {
const char *name;
const char *args_type;
@ -133,10 +127,7 @@ typedef struct mon_cmd_t {
union {
void (*cmd)(Monitor *mon, const QDict *qdict);
int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data);
int (*cmd_async)(Monitor *mon, const QDict *params,
MonitorCompletion *cb, void *opaque);
} mhandler;
int flags;
/* @sub_table is a list of 2nd level of commands. If it do not exist,
* mhandler should be used. If it exist, sub_table[?].mhandler should be
* used, and mhandler of 1st level plays the role of help function.
@ -394,11 +385,6 @@ static inline int handler_is_qobject(const mon_cmd_t *cmd)
return cmd->user_print != NULL;
}
static inline bool handler_is_async(const mon_cmd_t *cmd)
{
return cmd->flags & MONITOR_CMD_ASYNC;
}
static inline int monitor_has_error(const Monitor *mon)
{
return mon->error != NULL;
@ -917,45 +903,6 @@ static void hmp_trace_file(Monitor *mon, const QDict *qdict)
}
#endif
static void user_monitor_complete(void *opaque, QObject *ret_data)
{
MonitorCompletionData *data = (MonitorCompletionData *)opaque;
if (ret_data) {
data->user_print(data->mon, ret_data);
}
monitor_resume(data->mon);
g_free(data);
}
static void qmp_monitor_complete(void *opaque, QObject *ret_data)
{
monitor_protocol_emitter(opaque, ret_data);
}
static int qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
const QDict *params)
{
return cmd->mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
}
static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
const QDict *params)
{
int ret;
MonitorCompletionData *cb_data = g_malloc(sizeof(*cb_data));
cb_data->mon = mon;
cb_data->user_print = cmd->user_print;
monitor_suspend(mon);
ret = cmd->mhandler.cmd_async(mon, params,
user_monitor_complete, cb_data);
if (ret < 0) {
monitor_resume(mon);
g_free(cb_data);
}
}
static void hmp_info_help(Monitor *mon, const QDict *qdict)
{
help_cmd(mon, "info");
@ -4121,9 +4068,7 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
if (!cmd)
goto out;
if (handler_is_async(cmd)) {
user_async_cmd_handler(mon, cmd, qdict);
} else if (handler_is_qobject(cmd)) {
if (handler_is_qobject(cmd)) {
QObject *data = NULL;
/* XXX: ignores the error code */
@ -5054,8 +4999,6 @@ static QDict *qmp_check_input_obj(QObject *input_obj)
"object");
return NULL;
}
} else if (!strcmp(arg_name, "id")) {
/* FIXME: check duplicated IDs for async commands */
} else {
qerror_report(QERR_QMP_EXTRA_MEMBER, arg_name);
return NULL;
@ -5134,16 +5077,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
goto err_out;
}
if (handler_is_async(cmd)) {
err = qmp_async_cmd_handler(mon, cmd, args);
if (err) {
/* emit the error response */
goto err_out;
}
} else {
qmp_call_cmd(mon, cmd, args);
}
qmp_call_cmd(mon, cmd, args);
goto out;
err_out: