qmp: Wean off qerror_report()

The traditional QMP command handler interface

    int qmp_FOO(Monitor *mon, const QDict *params, QObject **ret_data);

doesn't provide for returning an Error object.  Instead, the handler
is expected to stash it in the monitor with qerror_report().

When we rebased QMP on top of QAPI, we didn't change this interface.
Instead, commit 776574d introduced "middle mode" as a temporary aid
for converting existing QMP commands to QAPI one by one.  More than
three years later, we're still using it.

Middle mode has two effects:

* Instead of the native input marshallers

      static void qmp_marshal_input_FOO(QDict *, QObject **, Error **)

  it generates input marshallers conforming to the traditional QMP
  command handler interface.

* It suppresses generation of code to register them with
  qmp_register_command()

  This permits giving them internal linkage.

As long as we need qmp-commands.hx, we can't use the registry behind
qmp_register_command(), so the latter has to stay for now.

The former has to go to get rid of qerror_report().  Changing all QMP
commands to fit the QAPI mold in one go was impractical back when we
started, but by now there are just a few stragglers left:
do_qmp_capabilities(), qmp_qom_set(), qmp_qom_get(), qmp_object_add(),
qmp_netdev_add(), do_device_add().

Switch middle mode to generate native input marshallers, and adapt the
stragglers.  Simplifies both the monitor code and the stragglers.

Rename do_qmp_capabilities() to qmp_capabilities(), and
do_device_add() to qmp_device_add, because that's how QMP command
handlers are named today.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@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-13 17:25:50 +01:00
parent 8b53a19675
commit 485febc6d1
11 changed files with 50 additions and 126 deletions

5
hmp.c
View File

@ -1507,7 +1507,10 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
void hmp_device_add(Monitor *mon, const QDict *qdict)
{
do_device_add(mon, qdict, NULL);
Error *err = NULL;
qmp_device_add((QDict *)qdict, NULL, &err);
hmp_handle_error(mon, &err);
}
void hmp_device_del(Monitor *mon, const QDict *qdict)

View File

@ -44,10 +44,9 @@ void monitor_read_command(Monitor *mon, int show_prompt);
int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
void *opaque);
int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret);
void qmp_qom_set(QDict *qdict, QObject **ret, Error **errp);
void qmp_qom_get(QDict *qdict, QObject **ret, Error **errp);
void qmp_object_add(QDict *qdict, QObject **ret, Error **errp);
void object_add(const char *type, const char *id, const QDict *qdict,
Visitor *v, Error **errp);

View File

@ -9,7 +9,8 @@
void hmp_info_qtree(Monitor *mon, const QDict *qdict);
void hmp_info_qdm(Monitor *mon, const QDict *qdict);
void hmp_info_qom_tree(Monitor *mon, const QDict *dict);
int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp);
int qdev_device_help(QemuOpts *opts);
DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);

View File

@ -197,7 +197,7 @@ void net_cleanup(void);
void hmp_host_net_add(Monitor *mon, const QDict *qdict);
void hmp_host_net_remove(Monitor *mon, const QDict *qdict);
void netdev_add(QemuOpts *opts, Error **errp);
int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret);
void qmp_netdev_add(QDict *qdict, QObject **ret, Error **errp);
int net_hub_id_for_client(NetClientState *nc, int *id);
NetClientState *net_hub_port_find(int hub_id);

View File

@ -122,7 +122,7 @@ typedef struct mon_cmd_t {
const char *help;
union {
void (*cmd)(Monitor *mon, const QDict *qdict);
int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data);
void (*cmd_new)(QDict *params, QObject **ret_data, Error **errp);
} mhandler;
/* @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
@ -563,11 +563,9 @@ static void monitor_qapi_event_init(void)
qmp_event_set_func_emit(monitor_qapi_event_queue);
}
static int do_qmp_capabilities(Monitor *mon, const QDict *params,
QObject **ret_data)
static void qmp_capabilities(QDict *params, QObject **ret_data, Error **errp)
{
mon->qmp.in_command_mode = true;
return 0;
cur_mon->qmp.in_command_mode = true;
}
static void handle_hmp_command(Monitor *mon, const char *cmdline);
@ -4725,7 +4723,7 @@ static int monitor_can_read(void *opaque)
static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd,
Error **errp)
{
bool is_cap = cmd->mhandler.cmd_new == do_qmp_capabilities;
bool is_cap = cmd->mhandler.cmd_new == qmp_capabilities;
if (is_cap && mon->qmp.in_command_mode) {
error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
@ -5044,17 +5042,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
goto err_out;
}
if (cmd->mhandler.cmd_new(mon, args, &data)) {
/* Command failed... */
if (!mon->error) {
/* ... without setting an error, so make one up */
error_setg(&local_err, QERR_UNDEFINED_ERROR);
}
}
if (mon->error) {
error_set(&local_err, mon->error->err_class, "%s",
mon->error->err_msg);
}
cmd->mhandler.cmd_new(args, &data, &local_err);
err_out:
monitor_protocol_emitter(mon, data, local_err);
@ -5126,7 +5114,7 @@ static QObject *get_qmp_greeting(void)
{
QObject *ver = NULL;
qmp_marshal_input_query_version(NULL, NULL, &ver);
qmp_marshal_input_query_version(NULL, &ver, NULL);
return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities': []}}",ver);
}

View File

@ -1090,7 +1090,7 @@ void netdev_add(QemuOpts *opts, Error **errp)
net_client_init(opts, 1, errp);
}
int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
void qmp_netdev_add(QDict *qdict, QObject **ret, Error **errp)
{
Error *local_err = NULL;
QemuOptsList *opts_list;
@ -1098,26 +1098,22 @@ int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
opts_list = qemu_find_opts_err("netdev", &local_err);
if (local_err) {
goto exit_err;
goto out;
}
opts = qemu_opts_from_qdict(opts_list, qdict, &local_err);
if (local_err) {
goto exit_err;
goto out;
}
netdev_add(opts, &local_err);
if (local_err) {
qemu_opts_del(opts);
goto exit_err;
goto out;
}
return 0;
exit_err:
qerror_report_err(local_err);
error_free(local_err);
return -1;
out:
error_propagate(errp, local_err);
}
void qmp_netdev_del(const char *id, Error **errp)

View File

@ -756,7 +756,7 @@ void hmp_info_qom_tree(Monitor *mon, const QDict *dict)
print_qom_composition(mon, obj, 0);
}
int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
{
Error *local_err = NULL;
QemuOpts *opts;
@ -764,23 +764,20 @@ int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &local_err);
if (local_err) {
qerror_report_err(local_err);
error_free(local_err);
return -1;
error_propagate(errp, local_err);
return;
}
if (!monitor_cur_is_qmp() && qdev_device_help(opts)) {
qemu_opts_del(opts);
return 0;
return;
}
dev = qdev_device_add(opts, &local_err);
if (!dev) {
qerror_report_err(local_err);
error_free(local_err);
error_propagate(errp, local_err);
qemu_opts_del(opts);
return -1;
return;
}
object_unref(OBJECT(dev));
return 0;
}
void qmp_device_del(const char *id, Error **errp)

View File

@ -276,7 +276,7 @@ EQMP
.args_type = "device:O",
.params = "driver[,prop=value][,...]",
.help = "add device, like -device on the command line",
.mhandler.cmd_new = do_device_add,
.mhandler.cmd_new = qmp_device_add,
},
SQMP
@ -1983,7 +1983,7 @@ EQMP
.args_type = "",
.params = "",
.help = "enable QMP capabilities",
.mhandler.cmd_new = do_qmp_capabilities,
.mhandler.cmd_new = qmp_capabilities,
},
SQMP

55
qmp.c
View File

@ -227,57 +227,37 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
}
/* FIXME: teach qapi about how to pass through Visitors */
int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret)
void qmp_qom_set(QDict *qdict, QObject **ret, Error **errp)
{
const char *path = qdict_get_str(qdict, "path");
const char *property = qdict_get_str(qdict, "property");
QObject *value = qdict_get(qdict, "value");
Error *local_err = NULL;
Object *obj;
obj = object_resolve_path(path, NULL);
if (!obj) {
error_set(&local_err, ERROR_CLASS_DEVICE_NOT_FOUND,
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
"Device '%s' not found", path);
goto out;
return;
}
object_property_set_qobject(obj, value, property, &local_err);
out:
if (local_err) {
qerror_report_err(local_err);
error_free(local_err);
return -1;
}
return 0;
object_property_set_qobject(obj, value, property, errp);
}
int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret)
void qmp_qom_get(QDict *qdict, QObject **ret, Error **errp)
{
const char *path = qdict_get_str(qdict, "path");
const char *property = qdict_get_str(qdict, "property");
Error *local_err = NULL;
Object *obj;
obj = object_resolve_path(path, NULL);
if (!obj) {
error_set(&local_err, ERROR_CLASS_DEVICE_NOT_FOUND,
error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
"Device '%s' not found", path);
goto out;
return;
}
*ret = object_property_get_qobject(obj, property, &local_err);
out:
if (local_err) {
qerror_report_err(local_err);
error_free(local_err);
return -1;
}
return 0;
*ret = object_property_get_qobject(obj, property, errp);
}
void qmp_set_password(const char *protocol, const char *password,
@ -673,36 +653,25 @@ out:
object_unref(obj);
}
int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret)
void qmp_object_add(QDict *qdict, QObject **ret, Error **errp)
{
const char *type = qdict_get_str(qdict, "qom-type");
const char *id = qdict_get_str(qdict, "id");
QObject *props = qdict_get(qdict, "props");
const QDict *pdict = NULL;
Error *local_err = NULL;
QmpInputVisitor *qiv;
if (props) {
pdict = qobject_to_qdict(props);
if (!pdict) {
error_setg(&local_err, QERR_INVALID_PARAMETER_TYPE,
"props", "dict");
goto out;
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict");
return;
}
}
qiv = qmp_input_visitor_new(props);
object_add(type, id, pdict, qmp_input_get_visitor(qiv), &local_err);
object_add(type, id, pdict, qmp_input_get_visitor(qiv), errp);
qmp_input_visitor_cleanup(qiv);
out:
if (local_err) {
qerror_report_err(local_err);
error_free(local_err);
return -1;
}
return 0;
}
void qmp_object_del(const char *id, Error **errp)

View File

@ -195,12 +195,10 @@ out:
return ret
def gen_marshal_input_decl(name, args, ret_type, middle_mode):
if middle_mode:
return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict, QObject **ret)' % c_name(name)
else:
return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
ret = 'void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
if not middle_mode:
ret = "static " + ret
return ret
def gen_marshal_input(name, args, ret_type, middle_mode):
hdr = gen_marshal_input_decl(name, args, ret_type, middle_mode)
@ -212,11 +210,6 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
''',
header=hdr)
if middle_mode:
ret += mcgen('''
QDict *args = (QDict *)qdict;
''')
if ret_type:
if is_c_ptr(ret_type):
retval = " %s retval = NULL;" % c_type(ret_type)
@ -252,36 +245,14 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
ret += mcgen('''
out:
''')
if not middle_mode:
ret += mcgen('''
error_propagate(errp, local_err);
''')
ret += mcgen('''
error_propagate(errp, local_err);
%(visitor_input_block_cleanup)s
}
''',
visitor_input_block_cleanup=gen_visitor_input_block(args,
dealloc=True))
if middle_mode:
ret += mcgen('''
if (local_err) {
qerror_report_err(local_err);
error_free(local_err);
return -1;
}
return 0;
''')
else:
ret += mcgen('''
return;
''')
ret += mcgen('''
}
''')
return ret
def gen_registry(commands):

View File

@ -202,7 +202,7 @@ bool enable_timestamp_msg;
* Format arguments like vsprintf(). The result should not contain
* newlines.
* Prepend the current location and append a newline.
* It's wrong to call this in a QMP monitor. Use qerror_report() there.
* It's wrong to call this in a QMP monitor. Use error_setg() there.
*/
void error_vreport(const char *fmt, va_list ap)
{
@ -226,7 +226,7 @@ void error_vreport(const char *fmt, va_list ap)
* Format arguments like sprintf(). The result should not contain
* newlines.
* Prepend the current location and append a newline.
* It's wrong to call this in a QMP monitor. Use qerror_report() there.
* It's wrong to call this in a QMP monitor. Use error_setg() there.
*/
void error_report(const char *fmt, ...)
{