diff options
45 files changed, 1080 insertions, 770 deletions
diff --git a/MAINTAINERS b/MAINTAINERS index b9a2171e9b..e3edd04982 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1404,6 +1404,7 @@ F: qmp.c F: monitor.c F: docs/*qmp-* F: scripts/qmp/ +F: tests/qmp-test.c T: git git://repo.or.cz/qemu/armbru.git qapi-next Register API diff --git a/block/nbd.c b/block/nbd.c index a7f9108fe5..f478f80b4a 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -278,7 +278,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp) goto done; } - iv = qobject_input_visitor_new(crumpled_addr, true); + iv = qobject_input_visitor_new(crumpled_addr); visit_type_SocketAddress(iv, NULL, &saddr, &local_err); if (local_err) { error_propagate(errp, local_err); diff --git a/block/nfs.c b/block/nfs.c index 890d5d4aff..3f43f6e26a 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -474,7 +474,7 @@ static NFSServer *nfs_config(QDict *options, Error **errp) goto out; } - iv = qobject_input_visitor_new(crumpled_addr, true); + iv = qobject_input_visitor_new(crumpled_addr); visit_type_NFSServer(iv, NULL, &server, &local_error); if (local_error) { error_propagate(errp, local_error); diff --git a/block/ssh.c b/block/ssh.c index 835932e6a4..278e66faa6 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -601,7 +601,7 @@ static InetSocketAddress *ssh_config(QDict *options, Error **errp) goto out; } - iv = qobject_input_visitor_new(crumpled_addr, true); + iv = qobject_input_visitor_new(crumpled_addr); visit_type_InetSocketAddress(iv, NULL, &inet, &local_error); if (local_error) { error_propagate(errp, local_error); diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 7eb7be12ab..6746c1052c 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -1138,7 +1138,7 @@ Example: Visitor *v; UserDefOneList *arg1 = NULL; - v = qobject_input_visitor_new(QOBJECT(args), true); + v = qobject_input_visitor_new(QOBJECT(args)); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 2de6377cca..150f6bf2c7 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -326,7 +326,12 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name, return; } } + visit_check_list(v, &err); visit_end_list(v, NULL); + if (err) { + error_propagate(errp, err); + return; + } break; } default: diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index e64b944d7c..d2b3aafdb4 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -16,6 +16,7 @@ extern Monitor *cur_mon; bool monitor_cur_is_qmp(void); +void monitor_init_qmp_commands(void); void monitor_init(Chardev *chr, int flags); void monitor_cleanup(void); diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h index 57651ea955..20578dcd48 100644 --- a/include/qapi/qmp/dispatch.h +++ b/include/qapi/qmp/dispatch.h @@ -34,18 +34,24 @@ typedef struct QmpCommand bool enabled; } QmpCommand; -void qmp_register_command(const char *name, QmpCommandFunc *fn, - QmpCommandOptions options); -void qmp_unregister_command(const char *name); -QmpCommand *qmp_find_command(const char *name); -QObject *qmp_dispatch(QObject *request); -void qmp_disable_command(const char *name); -void qmp_enable_command(const char *name); +typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList; + +void qmp_register_command(QmpCommandList *cmds, const char *name, + QmpCommandFunc *fn, QmpCommandOptions options); +void qmp_unregister_command(QmpCommandList *cmds, const char *name); +QmpCommand *qmp_find_command(QmpCommandList *cmds, const char *name); +QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request); +void qmp_disable_command(QmpCommandList *cmds, const char *name); +void qmp_enable_command(QmpCommandList *cmds, const char *name); + bool qmp_command_is_enabled(const QmpCommand *cmd); const char *qmp_command_name(const QmpCommand *cmd); bool qmp_has_success_response(const QmpCommand *cmd); QObject *qmp_build_error_object(Error *err); + typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque); -void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque); + +void qmp_for_each_command(QmpCommandList *cmds, qmp_cmd_callback_fn fn, + void *opaque); #endif diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 6586c9fa62..c82360f429 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -82,15 +82,6 @@ #define QERR_QGA_COMMAND_FAILED \ "Guest agent command failed, error was '%s'" -#define QERR_QMP_BAD_INPUT_OBJECT \ - "Expected '%s' in QMP input" - -#define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \ - "QMP input object member '%s' expects '%s'" - -#define QERR_QMP_EXTRA_MEMBER \ - "QMP input object member '%s' is unexpected" - #define QERR_SET_PASSWD_FAILED \ "Could not set password" diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h index cde328da9f..0b7633a38f 100644 --- a/include/qapi/qobject-input-visitor.h +++ b/include/qapi/qobject-input-visitor.h @@ -1,6 +1,7 @@ /* * Input Visitor * + * Copyright (C) 2017 Red Hat, Inc. * Copyright IBM, Corp. 2011 * * Authors: @@ -20,11 +21,42 @@ typedef struct QObjectInputVisitor QObjectInputVisitor; /* - * Return a new input visitor that converts a QObject to a QAPI object. + * Create a QObject input visitor for @obj * - * Set @strict to reject a parse that doesn't consume all keys of a - * dictionary; otherwise excess input is ignored. + * A QObject input visitor visit builds a QAPI object from a QObject. + * This simultaneously walks the QAPI object being built and the + * QObject. The latter walk starts at @obj. + * + * visit_type_FOO() creates an instance of QAPI type FOO. The visited + * QObject must match FOO. QDict matches struct/union types, QList + * matches list types, QString matches type 'str' and enumeration + * types, QInt matches integer types, QFloat matches type 'number', + * QBool matches type 'bool'. Type 'any' is matched by QObject. A + * QAPI alternate type is matched when one of its member types is. + * + * visit_start_struct() ... visit_end_struct() visits a QDict and + * creates a QAPI struct/union. Visits in between visit the + * dictionary members. visit_optional() is true when the QDict has + * this member. visit_check_struct() fails if unvisited members + * remain. + * + * visit_start_list() ... visit_end_list() visits a QList and creates + * a QAPI list. Visits in between visit list members, one after the + * other. visit_next_list() returns NULL when all QList members have + * been visited. visit_check_list() fails if unvisited members + * remain. + * + * visit_start_alternate() ... visit_end_alternate() visits a QObject + * and creates a QAPI alternate. The visit in between visits the same + * QObject and initializes the alternate member that is in use. + * + * Error messages refer to parts of @obj in JavaScript/Python syntax. + * For example, 'a.b[2]' refers to the second member of the QList + * member 'b' of the QDict member 'a' of QDict @obj. + * + * The caller is responsible for freeing the visitor with + * visit_free(). */ -Visitor *qobject_input_visitor_new(QObject *obj, bool strict); +Visitor *qobject_input_visitor_new(QObject *obj); #endif diff --git a/include/qapi/qobject-output-visitor.h b/include/qapi/qobject-output-visitor.h index 8241877bd7..9b990c318e 100644 --- a/include/qapi/qobject-output-visitor.h +++ b/include/qapi/qobject-output-visitor.h @@ -19,11 +19,38 @@ typedef struct QObjectOutputVisitor QObjectOutputVisitor; -/* - * Create a new QObject output visitor. +/** + * Create a QObject output visitor for @obj + * + * A QObject output visitor visit builds a QObject from QAPI Object. + * This simultaneously walks the QAPI object and the QObject being + * built. The latter walk starts at @obj. + * + * visit_type_FOO() creates a QObject for QAPI type FOO. It creates a + * QDict for struct/union types, a QList for list types, QString for + * type 'str' and enumeration types, QInt for integer types, QFloat + * for type 'number', QBool for type 'bool'. For type 'any', it + * increments the QObject's reference count. For QAPI alternate + * types, it creates the QObject for the member that is in use. + * + * visit_start_struct() ... visit_end_struct() visits a QAPI + * struct/union and creates a QDict. Visits in between visit the + * members. visit_optional() is true when the struct/union has this + * member. visit_check_struct() does nothing. + * + * visit_start_list() ... visit_end_list() visits a QAPI list and + * creates a QList. Visits in between visit list members, one after + * the other. visit_next_list() returns NULL when all QAPI list + * members have been visited. visit_check_list() does nothing. + * + * visit_start_alternate() ... visit_end_alternate() visits a QAPI + * alternate. The visit in between creates the QObject for the + * alternate member that is in use. + * + * Errors are not expected to happen. * - * If everything else succeeds, pass @result to visit_complete() to - * collect the result of the visit. + * The caller is responsible for freeing the visitor with + * visit_free(). */ Visitor *qobject_output_visitor_new(QObject **result); diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index 8bd47ee4bb..e87709db5c 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -61,6 +61,9 @@ struct Visitor /* Must be set */ GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size); + /* Optional; intended for input visitors */ + void (*check_list)(Visitor *v, Error **errp); + /* Must be set */ void (*end_list)(Visitor *v, void **list); @@ -102,8 +105,8 @@ struct Visitor /* Must be set to visit explicit null values. */ void (*type_null)(Visitor *v, const char *name, Error **errp); - /* Must be set for input visitors, optional otherwise. The core - * takes care of the return type in the public interface. */ + /* Must be set for input visitors to visit structs, optional otherwise. + The core takes care of the return type in the public interface. */ void (*optional)(Visitor *v, const char *name, bool *present); /* Must be set */ diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 9bb6cba237..1a1b62012b 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -66,12 +66,6 @@ * object, @name is the key associated with the value; and when * visiting a member of a list, @name is NULL. * - * FIXME: Clients must pass NULL for @name when visiting a member of a - * list, but this leads to poor error messages; it might be nicer to - * require a non-NULL name such as "key.0" for '{ "key": [ "value" ] - * }' if an error is encountered on "value" (or to have the visitor - * core auto-generate the nicer name). - * * The visit_type_FOO() functions expect a non-null @obj argument; * they allocate *@obj during input visits, leave it unchanged on * output visits, and recursively free any resources during a dealloc @@ -376,6 +370,19 @@ void visit_start_list(Visitor *v, const char *name, GenericList **list, GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size); /* + * Prepare for completing a list visit. + * + * @errp obeys typical error usage, and reports failures such as + * unvisited list tail remaining in the input stream. + * + * Should be called prior to visit_end_list() if all other + * intermediate visit steps were successful, to allow the visitor one + * last chance to report errors. May be skipped on a cleanup path, + * where there is no need to check for further errors. + */ +void visit_check_list(Visitor *v, Error **errp); + +/* * Complete a list visit started earlier. * * @list must match what was passed to the paired visit_start_list(). diff --git a/include/qemu/module.h b/include/qemu/module.h index 877cca7d7b..56dd218205 100644 --- a/include/qemu/module.h +++ b/include/qemu/module.h @@ -42,7 +42,6 @@ static void __attribute__((constructor)) do_qemu_init_ ## function(void) \ typedef enum { MODULE_INIT_BLOCK, MODULE_INIT_OPTS, - MODULE_INIT_QAPI, MODULE_INIT_QOM, MODULE_INIT_TRACE, MODULE_INIT_MAX @@ -50,7 +49,6 @@ typedef enum { #define block_init(function) module_init(function, MODULE_INIT_BLOCK) #define opts_init(function) module_init(function, MODULE_INIT_OPTS) -#define qapi_init(function) module_init(function, MODULE_INIT_QAPI) #define type_init(function) module_init(function, MODULE_INIT_QOM) #define trace_init(function) module_init(function, MODULE_INIT_TRACE) diff --git a/monitor.c b/monitor.c index b68944d93c..ec7623efdd 100644 --- a/monitor.c +++ b/monitor.c @@ -165,7 +165,7 @@ typedef struct { * When command qmp_capabilities succeeds, we go into command * mode. */ - bool in_command_mode; /* are we in command mode? */ + QmpCommandList *commands; } MonitorQMP; /* @@ -221,6 +221,8 @@ static int mon_refcount; static mon_cmd_t mon_cmds[]; static mon_cmd_t info_cmds[]; +QmpCommandList qmp_commands, qmp_cap_negotiation_commands; + Monitor *cur_mon; static QEMUClockType event_clock_type = QEMU_CLOCK_REALTIME; @@ -414,7 +416,8 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict) trace_monitor_protocol_event_emit(event, qdict); QLIST_FOREACH(mon, &mon_list, entry) { - if (monitor_is_qmp(mon) && mon->qmp.in_command_mode) { + if (monitor_is_qmp(mon) + && mon->qmp.commands != &qmp_cap_negotiation_commands) { monitor_json_emitter(mon, QOBJECT(qdict)); } } @@ -563,11 +566,6 @@ static void monitor_qapi_event_init(void) qmp_event_set_func_emit(monitor_qapi_event_queue); } -void qmp_qmp_capabilities(Error **errp) -{ - cur_mon->qmp.in_command_mode = true; -} - static void handle_hmp_command(Monitor *mon, const char *cmdline); static void monitor_data_init(Monitor *mon) @@ -919,7 +917,7 @@ CommandInfoList *qmp_query_commands(Error **errp) { CommandInfoList *list = NULL; - qmp_for_each_command(query_commands_cb, &list); + qmp_for_each_command(cur_mon->qmp.commands, query_commands_cb, &list); return list; } @@ -973,44 +971,67 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data, static void qmp_unregister_commands_hack(void) { #ifndef CONFIG_SPICE - qmp_unregister_command("query-spice"); + qmp_unregister_command(&qmp_commands, "query-spice"); #endif #ifndef TARGET_I386 - qmp_unregister_command("rtc-reset-reinjection"); + qmp_unregister_command(&qmp_commands, "rtc-reset-reinjection"); #endif #ifndef TARGET_S390X - qmp_unregister_command("dump-skeys"); + qmp_unregister_command(&qmp_commands, "dump-skeys"); #endif #ifndef TARGET_ARM - qmp_unregister_command("query-gic-capabilities"); + qmp_unregister_command(&qmp_commands, "query-gic-capabilities"); #endif #if !defined(TARGET_S390X) && !defined(TARGET_I386) - qmp_unregister_command("query-cpu-model-expansion"); + qmp_unregister_command(&qmp_commands, "query-cpu-model-expansion"); #endif #if !defined(TARGET_S390X) - qmp_unregister_command("query-cpu-model-baseline"); - qmp_unregister_command("query-cpu-model-comparison"); + qmp_unregister_command(&qmp_commands, "query-cpu-model-baseline"); + qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison"); #endif #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \ && !defined(TARGET_S390X) - qmp_unregister_command("query-cpu-definitions"); + qmp_unregister_command(&qmp_commands, "query-cpu-definitions"); #endif } -static void qmp_init_marshal(void) +void monitor_init_qmp_commands(void) { - qmp_register_command("query-qmp-schema", qmp_query_qmp_schema, + /* + * Two command lists: + * - qmp_commands contains all QMP commands + * - qmp_cap_negotiation_commands contains just + * "qmp_capabilities", to enforce capability negotiation + */ + + qmp_init_marshal(&qmp_commands); + + qmp_register_command(&qmp_commands, "query-qmp-schema", + qmp_query_qmp_schema, QCO_NO_OPTIONS); - qmp_register_command("device_add", qmp_device_add, + qmp_register_command(&qmp_commands, "device_add", qmp_device_add, QCO_NO_OPTIONS); - qmp_register_command("netdev_add", qmp_netdev_add, + qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add, QCO_NO_OPTIONS); - /* call it after the rest of qapi_init() */ - register_module_init(qmp_unregister_commands_hack, MODULE_INIT_QAPI); + qmp_unregister_commands_hack(); + + QTAILQ_INIT(&qmp_cap_negotiation_commands); + qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities", + qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS); } -qapi_init(qmp_init_marshal); +void qmp_qmp_capabilities(Error **errp) +{ + if (cur_mon->qmp.commands == &qmp_commands) { + error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, + "Capabilities negotiation is already complete, command " + "ignored"); + return; + } + + cur_mon->qmp.commands = &qmp_commands; +} /* set the current CPU defined by the user */ int monitor_set_cpu(int cpu_index) @@ -3679,87 +3700,10 @@ static int monitor_can_read(void *opaque) return (mon->suspend_cnt == 0) ? 1 : 0; } -static bool invalid_qmp_mode(const Monitor *mon, const char *cmd, - Error **errp) -{ - bool is_cap = g_str_equal(cmd, "qmp_capabilities"); - - if (is_cap && mon->qmp.in_command_mode) { - error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, - "Capabilities negotiation is already complete, command " - "'%s' ignored", cmd); - return true; - } - if (!is_cap && !mon->qmp.in_command_mode) { - error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, - "Expecting capabilities negotiation with " - "'qmp_capabilities' before command '%s'", cmd); - return true; - } - return false; -} - -/* - * Input object checking rules - * - * 1. Input object must be a dict - * 2. The "execute" key must exist - * 3. The "execute" key must be a string - * 4. If the "arguments" key exists, it must be a dict - * 5. If the "id" key exists, it can be anything (ie. json-value) - * 6. Any argument not listed above is considered invalid - */ -static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp) -{ - const QDictEntry *ent; - int has_exec_key = 0; - QDict *input_dict; - - input_dict = qobject_to_qdict(input_obj); - if (!input_dict) { - error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "object"); - return NULL; - } - - - for (ent = qdict_first(input_dict); ent; ent = qdict_next(input_dict, ent)){ - const char *arg_name = qdict_entry_key(ent); - const QObject *arg_obj = qdict_entry_value(ent); - - if (!strcmp(arg_name, "execute")) { - if (qobject_type(arg_obj) != QTYPE_QSTRING) { - error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER, - "execute", "string"); - return NULL; - } - has_exec_key = 1; - } else if (!strcmp(arg_name, "arguments")) { - if (qobject_type(arg_obj) != QTYPE_QDICT) { - error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER, - "arguments", "object"); - return NULL; - } - } else if (!strcmp(arg_name, "id")) { - /* Any string is acceptable as "id", so nothing to check */ - } else { - error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name); - return NULL; - } - } - - if (!has_exec_key) { - error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "execute"); - return NULL; - } - - return input_dict; -} - static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) { QObject *req, *rsp = NULL, *id = NULL; QDict *qdict = NULL; - const char *cmd_name; Monitor *mon = cur_mon; Error *err = NULL; @@ -3772,24 +3716,28 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) goto err_out; } - qdict = qmp_check_input_obj(req, &err); - if (!qdict) { - goto err_out; - } - - id = qdict_get(qdict, "id"); - qobject_incref(id); - qdict_del(qdict, "id"); - - cmd_name = qdict_get_str(qdict, "execute"); - trace_handle_qmp_command(mon, cmd_name); - - if (invalid_qmp_mode(mon, cmd_name, &err)) { - goto err_out; + qdict = qobject_to_qdict(req); + if (qdict) { + id = qdict_get(qdict, "id"); + qobject_incref(id); + qdict_del(qdict, "id"); + } /* else will fail qmp_dispatch() */ + + rsp = qmp_dispatch(cur_mon->qmp.commands, req); + + if (mon->qmp.commands == &qmp_cap_negotiation_commands) { + qdict = qdict_get_qdict(qobject_to_qdict(rsp), "error"); + if (qdict + && !g_strcmp0(qdict_get_try_str(qdict, "class"), + QapiErrorClass_lookup[ERROR_CLASS_COMMAND_NOT_FOUND])) { + /* Provide a more useful error message */ + qdict_del(qdict, "desc"); + qdict_put(qdict, "desc", + qstring_from_str("Expecting capabilities negotiation" + " with 'qmp_capabilities'")); + } } - rsp = qmp_dispatch(req); - err_out: if (err) { qdict = qdict_new(); @@ -3886,7 +3834,7 @@ static void monitor_qmp_event(void *opaque, int event) switch (event) { case CHR_EVENT_OPENED: - mon->qmp.in_command_mode = false; + mon->qmp.commands = &qmp_cap_negotiation_commands; data = get_qmp_greeting(); monitor_json_emitter(mon, data); qobject_decref(data); diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index a0a7c0e734..026d25b767 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -273,6 +273,16 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size) static void +opts_check_list(Visitor *v, Error **errp) +{ + /* + * FIXME should set error when unvisited elements remain. Mostly + * harmless, as the generated visits always visit all elements. + */ +} + + +static void opts_end_list(Visitor *v, void **obj) { OptsVisitor *ov = to_ov(v); @@ -528,6 +538,7 @@ opts_visitor_new(const QemuOpts *opts) { OptsVisitor *ov; + assert(opts); ov = g_malloc0(sizeof *ov); ov->visitor.type = VISITOR_INPUT; @@ -538,6 +549,7 @@ opts_visitor_new(const QemuOpts *opts) ov->visitor.start_list = &opts_start_list; ov->visitor.next_list = &opts_next_list; + ov->visitor.check_list = &opts_check_list; ov->visitor.end_list = &opts_end_list; ov->visitor.type_int64 = &opts_type_int64; diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index e6e93f02e6..43a09d147d 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -90,6 +90,14 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size) return v->next_list(v, tail, size); } +void visit_check_list(Visitor *v, Error **errp) +{ + trace_visit_check_list(v); + if (v->check_list) { + v->check_list(v, errp); + } +} + void visit_end_list(Visitor *v, void **obj) { trace_visit_end_list(v, obj); diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 48bec2072b..dc502129d8 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -30,8 +30,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) dict = qobject_to_qdict(request); if (!dict) { - error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, - "request is not a dictionary"); + error_setg(errp, "Expected '%s' in QMP input", "object"); return NULL; } @@ -42,26 +41,34 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp) if (!strcmp(arg_name, "execute")) { if (qobject_type(arg_obj) != QTYPE_QSTRING) { - error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute", - "string"); + error_setg(errp, "QMP input object member '%s' expects '%s'", + "execute", "string"); return NULL; } has_exec_key = true; - } else if (strcmp(arg_name, "arguments")) { - error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name); + } else if (!strcmp(arg_name, "arguments")) { + if (qobject_type(arg_obj) != QTYPE_QDICT) { + error_setg(errp, "QMP input object member '%s' expects '%s'", + "arguments", "object"); + return NULL; + } + } else { + error_setg(errp, "QMP input object member '%s' is unexpected", + arg_name); return NULL; } } if (!has_exec_key) { - error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "execute"); + error_setg(errp, "Expected '%s' in QMP input", "execute"); return NULL; } return dict; } -static QObject *do_qmp_dispatch(QObject *request, Error **errp) +static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request, + Error **errp) { Error *local_err = NULL; const char *command; @@ -75,7 +82,7 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp) } command = qdict_get_str(dict, "execute"); - cmd = qmp_find_command(command); + cmd = qmp_find_command(cmds, command); if (cmd == NULL) { error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has not been found", command); @@ -115,13 +122,13 @@ QObject *qmp_build_error_object(Error *err) error_get_pretty(err)); } -QObject *qmp_dispatch(QObject *request) +QObject *qmp_dispatch(QmpCommandList *cmds, QObject *request) { Error *err = NULL; QObject *ret; QDict *rsp; - ret = do_qmp_dispatch(request, &err); + ret = do_qmp_dispatch(cmds, request, &err); rsp = qdict_new(); if (err) { diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c index e8053686f3..5af484cd9a 100644 --- a/qapi/qmp-registry.c +++ b/qapi/qmp-registry.c @@ -15,11 +15,8 @@ #include "qemu/osdep.h" #include "qapi/qmp/dispatch.h" -static QTAILQ_HEAD(QmpCommandList, QmpCommand) qmp_commands = - QTAILQ_HEAD_INITIALIZER(qmp_commands); - -void qmp_register_command(const char *name, QmpCommandFunc *fn, - QmpCommandOptions options) +void qmp_register_command(QmpCommandList *cmds, const char *name, + QmpCommandFunc *fn, QmpCommandOptions options) { QmpCommand *cmd = g_malloc0(sizeof(*cmd)); @@ -27,22 +24,22 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn, cmd->fn = fn; cmd->enabled = true; cmd->options = options; - QTAILQ_INSERT_TAIL(&qmp_commands, cmd, node); + QTAILQ_INSERT_TAIL(cmds, cmd, node); } -void qmp_unregister_command(const char *name) +void qmp_unregister_command(QmpCommandList *cmds, const char *name) { - QmpCommand *cmd = qmp_find_command(name); + QmpCommand *cmd = qmp_find_command(cmds, name); - QTAILQ_REMOVE(&qmp_commands, cmd, node); + QTAILQ_REMOVE(cmds, cmd, node); g_free(cmd); } -QmpCommand *qmp_find_command(const char *name) +QmpCommand *qmp_find_command(QmpCommandList *cmds, const char *name) { QmpCommand *cmd; - QTAILQ_FOREACH(cmd, &qmp_commands, node) { + QTAILQ_FOREACH(cmd, cmds, node) { if (strcmp(cmd->name, name) == 0) { return cmd; } @@ -50,11 +47,12 @@ QmpCommand *qmp_find_command(const char *name) return NULL; } -static void qmp_toggle_command(const char *name, bool enabled) +static void qmp_toggle_command(QmpCommandList *cmds, const char *name, + bool enabled) { QmpCommand *cmd; - QTAILQ_FOREACH(cmd, &qmp_commands, node) { + QTAILQ_FOREACH(cmd, cmds, node) { if (strcmp(cmd->name, name) == 0) { cmd->enabled = enabled; return; @@ -62,14 +60,14 @@ static void qmp_toggle_command(const char *name, bool enabled) } } -void qmp_disable_command(const char *name) +void qmp_disable_command(QmpCommandList *cmds, const char *name) { - qmp_toggle_command(name, false); + qmp_toggle_command(cmds, name, false); } -void qmp_enable_command(const char *name) +void qmp_enable_command(QmpCommandList *cmds, const char *name) { - qmp_toggle_command(name, true); + qmp_toggle_command(cmds, name, true); } bool qmp_command_is_enabled(const QmpCommand *cmd) @@ -87,11 +85,12 @@ bool qmp_has_success_response(const QmpCommand *cmd) return !(cmd->options & QCO_NO_SUCCESS_RESP); } -void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque) +void qmp_for_each_command(QmpCommandList *cmds, qmp_cmd_callback_fn fn, + void *opaque) { QmpCommand *cmd; - QTAILQ_FOREACH(cmd, &qmp_commands, node) { + QTAILQ_FOREACH(cmd, cmds, node) { fn(cmd, opaque); } } diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 0063327b3b..d192727e0b 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -21,21 +21,19 @@ #include "qapi/qmp/types.h" #include "qapi/qmp/qerror.h" -#define QIV_STACK_SIZE 1024 - -typedef struct StackObject -{ - QObject *obj; /* Object being visited */ +typedef struct StackObject { + const char *name; /* Name of @obj in its parent, if any */ + QObject *obj; /* QDict or QList being visited */ void *qapi; /* sanity check that caller uses same pointer */ - GHashTable *h; /* If obj is dict: unvisited keys */ - const QListEntry *entry; /* If obj is list: unvisited tail */ + GHashTable *h; /* If @obj is QDict: unvisited keys */ + const QListEntry *entry; /* If @obj is QList: unvisited tail */ + unsigned index; /* If @obj is QList: list index of @entry */ - QSLIST_ENTRY(StackObject) node; + QSLIST_ENTRY(StackObject) node; /* parent */ } StackObject; -struct QObjectInputVisitor -{ +struct QObjectInputVisitor { Visitor visitor; /* Root of visit at visitor creation. */ @@ -45,8 +43,7 @@ struct QObjectInputVisitor * QDict or QList). */ QSLIST_HEAD(, StackObject) stack; - /* True to reject parse in visit_end_struct() if unvisited keys remain. */ - bool strict; + GString *errname; /* Accumulator for full_name() */ }; static QObjectInputVisitor *to_qiv(Visitor *v) @@ -54,9 +51,51 @@ static QObjectInputVisitor *to_qiv(Visitor *v) return container_of(v, QObjectInputVisitor, visitor); } -static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, - const char *name, - bool consume, Error **errp) +static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name, + int n) +{ + StackObject *so; + char buf[32]; + + if (qiv->errname) { + g_string_truncate(qiv->errname, 0); + } else { + qiv->errname = g_string_new(""); + } + + QSLIST_FOREACH(so , &qiv->stack, node) { + if (n) { + n--; + } else if (qobject_type(so->obj) == QTYPE_QDICT) { + g_string_prepend(qiv->errname, name ?: "<anonymous>"); + g_string_prepend_c(qiv->errname, '.'); + } else { + snprintf(buf, sizeof(buf), "[%u]", so->index); + g_string_prepend(qiv->errname, buf); + } + name = so->name; + } + assert(!n); + + if (name) { + g_string_prepend(qiv->errname, name); + } else if (qiv->errname->str[0] == '.') { + g_string_erase(qiv->errname, 0, 1); + } else if (!qiv->errname->str[0]) { + return "<anonymous>"; + } + + return qiv->errname->str; +} + +static const char *full_name(QObjectInputVisitor *qiv, const char *name) +{ + return full_name_nth(qiv, name, 0); +} + +static QObject *qobject_input_try_get_object(QObjectInputVisitor *qiv, + const char *name, + bool consume) { StackObject *tos; QObject *qobj; @@ -80,22 +119,37 @@ static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, bool removed = g_hash_table_remove(tos->h, name); assert(removed); } - if (!ret) { - error_setg(errp, QERR_MISSING_PARAMETER, name); - } } else { assert(qobject_type(qobj) == QTYPE_QLIST); assert(!name); - ret = qlist_entry_obj(tos->entry); - assert(ret); + if (tos->entry) { + ret = qlist_entry_obj(tos->entry); + if (consume) { + tos->entry = qlist_next(tos->entry); + } + } else { + ret = NULL; + } if (consume) { - tos->entry = qlist_next(tos->entry); + tos->index++; } } return ret; } +static QObject *qobject_input_get_object(QObjectInputVisitor *qiv, + const char *name, + bool consume, Error **errp) +{ + QObject *obj = qobject_input_try_get_object(qiv, name, consume); + + if (!obj) { + error_setg(errp, QERR_MISSING_PARAMETER, full_name(qiv, name)); + } + return obj; +} + static void qdict_add_key(const char *key, QObject *obj, void *opaque) { GHashTable *h = opaque; @@ -103,22 +157,25 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque) } static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv, - QObject *obj, void *qapi, - Error **errp) + const char *name, + QObject *obj, void *qapi) { GHashTable *h; StackObject *tos = g_new0(StackObject, 1); assert(obj); + tos->name = name; tos->obj = obj; tos->qapi = qapi; - if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) { + if (qobject_type(obj) == QTYPE_QDICT) { h = g_hash_table_new(g_str_hash, g_str_equal); qdict_iter(qobject_to_qdict(obj), qdict_add_key, h); tos->h = h; - } else if (qobject_type(obj) == QTYPE_QLIST) { + } else { + assert(qobject_type(obj) == QTYPE_QLIST); tos->entry = qlist_first(qobject_to_qlist(obj)); + tos->index = -1; } QSLIST_INSERT_HEAD(&qiv->stack, tos, node); @@ -130,19 +187,15 @@ static void qobject_input_check_struct(Visitor *v, Error **errp) { QObjectInputVisitor *qiv = to_qiv(v); StackObject *tos = QSLIST_FIRST(&qiv->stack); + GHashTableIter iter; + const char *key; assert(tos && !tos->entry); - if (qiv->strict) { - GHashTable *const top_ht = tos->h; - if (top_ht) { - GHashTableIter iter; - const char *key; - - g_hash_table_iter_init(&iter, top_ht); - if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) { - error_setg(errp, QERR_QMP_EXTRA_MEMBER, key); - } - } + + g_hash_table_iter_init(&iter, tos->h); + if (g_hash_table_iter_next(&iter, (void **)&key, NULL)) { + error_setg(errp, "Parameter '%s' is unexpected", + full_name(qiv, key)); } } @@ -170,7 +223,6 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj, { QObjectInputVisitor *qiv = to_qiv(v); QObject *qobj = qobject_input_get_object(qiv, name, true, errp); - Error *err = NULL; if (obj) { *obj = NULL; @@ -179,16 +231,12 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj, return; } if (qobject_type(qobj) != QTYPE_QDICT) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "QDict"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "object"); return; } - qobject_input_push(qiv, qobj, obj, &err); - if (err) { - error_propagate(errp, err); - return; - } + qobject_input_push(qiv, name, qobj, obj); if (obj) { *obj = g_malloc0(size); @@ -204,25 +252,21 @@ static void qobject_input_start_list(Visitor *v, const char *name, QObject *qobj = qobject_input_get_object(qiv, name, true, errp); const QListEntry *entry; + if (list) { + *list = NULL; + } if (!qobj) { return; } if (qobject_type(qobj) != QTYPE_QLIST) { - if (list) { - *list = NULL; - } - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "list"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "array"); return; } - entry = qobject_input_push(qiv, qobj, list, errp); - if (list) { - if (entry) { - *list = g_malloc0(size); - } else { - *list = NULL; - } + entry = qobject_input_push(qiv, name, qobj, list); + if (entry && list) { + *list = g_malloc0(size); } } @@ -230,15 +274,30 @@ static GenericList *qobject_input_next_list(Visitor *v, GenericList *tail, size_t size) { QObjectInputVisitor *qiv = to_qiv(v); - StackObject *so = QSLIST_FIRST(&qiv->stack); + StackObject *tos = QSLIST_FIRST(&qiv->stack); + + assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST); - if (!so->entry) { + if (!tos->entry) { return NULL; } tail->next = g_malloc0(size); return tail->next; } +static void qobject_input_check_list(Visitor *v, Error **errp) +{ + QObjectInputVisitor *qiv = to_qiv(v); + StackObject *tos = QSLIST_FIRST(&qiv->stack); + + assert(tos && tos->obj && qobject_type(tos->obj) == QTYPE_QLIST); + + if (tos->entry) { + error_setg(errp, "Only %u list elements expected in %s", + tos->index + 1, full_name_nth(qiv, NULL, 1)); + } +} + static void qobject_input_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, @@ -270,8 +329,8 @@ static void qobject_input_type_int64(Visitor *v, const char *name, int64_t *obj, } qint = qobject_to_qint(qobj); if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "integer"); return; } @@ -291,8 +350,8 @@ static void qobject_input_type_uint64(Visitor *v, const char *name, } qint = qobject_to_qint(qobj); if (!qint) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "integer"); return; } @@ -311,8 +370,8 @@ static void qobject_input_type_bool(Visitor *v, const char *name, bool *obj, } qbool = qobject_to_qbool(qobj); if (!qbool) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "boolean"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "boolean"); return; } @@ -332,8 +391,8 @@ static void qobject_input_type_str(Visitor *v, const char *name, char **obj, } qstr = qobject_to_qstring(qobj); if (!qstr) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "string"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "string"); return; } @@ -363,8 +422,8 @@ static void qobject_input_type_number(Visitor *v, const char *name, double *obj, return; } - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "number"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "number"); } static void qobject_input_type_any(Visitor *v, const char *name, QObject **obj, @@ -392,15 +451,15 @@ static void qobject_input_type_null(Visitor *v, const char *name, Error **errp) } if (qobject_type(qobj) != QTYPE_QNULL) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "null"); + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, + full_name(qiv, name), "null"); } } static void qobject_input_optional(Visitor *v, const char *name, bool *present) { QObjectInputVisitor *qiv = to_qiv(v); - QObject *qobj = qobject_input_get_object(qiv, name, false, NULL); + QObject *qobj = qobject_input_try_get_object(qiv, name, false); if (!qobj) { *present = false; @@ -413,6 +472,7 @@ static void qobject_input_optional(Visitor *v, const char *name, bool *present) static void qobject_input_free(Visitor *v) { QObjectInputVisitor *qiv = to_qiv(v); + while (!QSLIST_EMPTY(&qiv->stack)) { StackObject *tos = QSLIST_FIRST(&qiv->stack); @@ -421,10 +481,13 @@ static void qobject_input_free(Visitor *v) } qobject_decref(qiv->root); + if (qiv->errname) { + g_string_free(qiv->errname, TRUE); + } g_free(qiv); } -Visitor *qobject_input_visitor_new(QObject *obj, bool strict) +Visitor *qobject_input_visitor_new(QObject *obj) { QObjectInputVisitor *v; @@ -437,6 +500,7 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict) v->visitor.end_struct = qobject_input_pop; v->visitor.start_list = qobject_input_start_list; v->visitor.next_list = qobject_input_next_list; + v->visitor.check_list = qobject_input_check_list; v->visitor.end_list = qobject_input_pop; v->visitor.start_alternate = qobject_input_start_alternate; v->visitor.type_int64 = qobject_input_type_int64; @@ -448,7 +512,6 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool strict) v->visitor.type_null = qobject_input_type_null; v->visitor.optional = qobject_input_optional; v->visitor.free = qobject_input_free; - v->strict = strict; v->root = obj; qobject_incref(obj); diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index 8dfa561252..806b01ae3a 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -170,6 +170,35 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size) return tail->next; } +static void check_list(Visitor *v, Error **errp) +{ + const StringInputVisitor *siv = to_siv(v); + Range *r; + GList *cur_range; + + if (!siv->ranges || !siv->cur_range) { + return; + } + + r = siv->cur_range->data; + if (!r) { + return; + } + + if (!range_contains(r, siv->cur)) { + cur_range = g_list_next(siv->cur_range); + if (!cur_range) { + return; + } + r = cur_range->data; + if (!r) { + return; + } + } + + error_setg(errp, "Range contains too many values"); +} + static void end_list(Visitor *v, void **obj) { StringInputVisitor *siv = to_siv(v); @@ -182,12 +211,6 @@ static void parse_type_int64(Visitor *v, const char *name, int64_t *obj, { StringInputVisitor *siv = to_siv(v); - if (!siv->string) { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "integer"); - return; - } - if (parse_str(siv, name, errp) < 0) { return; } @@ -242,13 +265,7 @@ static void parse_type_size(Visitor *v, const char *name, uint64_t *obj, Error *err = NULL; uint64_t val; - if (siv->string) { - parse_option_size(name, siv->string, &val, &err); - } else { - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "size"); - return; - } + parse_option_size(name, siv->string, &val, &err); if (err) { error_propagate(errp, err); return; @@ -262,19 +279,17 @@ static void parse_type_bool(Visitor *v, const char *name, bool *obj, { StringInputVisitor *siv = to_siv(v); - if (siv->string) { - if (!strcasecmp(siv->string, "on") || - !strcasecmp(siv->string, "yes") || - !strcasecmp(siv->string, "true")) { - *obj = true; - return; - } - if (!strcasecmp(siv->string, "off") || - !strcasecmp(siv->string, "no") || - !strcasecmp(siv->string, "false")) { - *obj = false; - return; - } + if (!strcasecmp(siv->string, "on") || + !strcasecmp(siv->string, "yes") || + !strcasecmp(siv->string, "true")) { + *obj = true; + return; + } + if (!strcasecmp(siv->string, "off") || + !strcasecmp(siv->string, "no") || + !strcasecmp(siv->string, "false")) { + *obj = false; + return; } error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -285,13 +300,8 @@ static void parse_type_str(Visitor *v, const char *name, char **obj, Error **errp) { StringInputVisitor *siv = to_siv(v); - if (siv->string) { - *obj = g_strdup(siv->string); - } else { - *obj = NULL; - error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", - "string"); - } + + *obj = g_strdup(siv->string); } static void parse_type_number(Visitor *v, const char *name, double *obj, @@ -302,10 +312,8 @@ static void parse_type_number(Visitor *v, const char *name, double *obj, double val; errno = 0; - if (siv->string) { - val = strtod(siv->string, &endp); - } - if (!siv->string || errno || endp == siv->string || *endp) { + val = strtod(siv->string, &endp); + if (errno || endp == siv->string || *endp) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "number"); return; @@ -314,18 +322,6 @@ static void parse_type_number(Visitor *v, const char *name, double *obj, *obj = val; } -static void parse_optional(Visitor *v, const char *name, bool *present) -{ - StringInputVisitor *siv = to_siv(v); - - if (!siv->string) { - *present = false; - return; - } - - *present = true; -} - static void string_input_free(Visitor *v) { StringInputVisitor *siv = to_siv(v); @@ -339,6 +335,7 @@ Visitor *string_input_visitor_new(const char *str) { StringInputVisitor *v; + assert(str); v = g_malloc0(sizeof(*v)); v->visitor.type = VISITOR_INPUT; @@ -350,8 +347,8 @@ Visitor *string_input_visitor_new(const char *str) v->visitor.type_number = parse_type_number; v->visitor.start_list = start_list; v->visitor.next_list = next_list; + v->visitor.check_list = check_list; v->visitor.end_list = end_list; - v->visitor.optional = parse_optional; v->visitor.free = string_input_free; v->string = str; diff --git a/qapi/trace-events b/qapi/trace-events index 9cbb61b2bd..339cacf0ad 100644 --- a/qapi/trace-events +++ b/qapi/trace-events @@ -8,6 +8,7 @@ visit_end_struct(void *v, void *obj) "v=%p obj=%p" visit_start_list(void *v, const char *name, void *obj, size_t size) "v=%p name=%s obj=%p size=%zu" visit_next_list(void *v, void *tail, size_t size) "v=%p tail=%p size=%zu" +visit_check_list(void *v) "v=%p" visit_end_list(void *v, void *obj) "v=%p obj=%p" visit_start_alternate(void *v, const char *name, void *obj, size_t size, bool promote_int) "v=%p name=%s obj=%p size=%zu promote_int=%d" diff --git a/qga/commands.c b/qga/commands.c index edd3e830e6..4d92946820 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -75,7 +75,7 @@ struct GuestAgentInfo *qmp_guest_info(Error **errp) GuestAgentInfo *info = g_new0(GuestAgentInfo, 1); info->version = g_strdup(QEMU_VERSION); - qmp_for_each_command(qmp_command_info, info); + qmp_for_each_command(&ga_commands, qmp_command_info, info); return info; } diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h index 63e9d398ae..3e8a4acff2 100644 --- a/qga/guest-agent-core.h +++ b/qga/guest-agent-core.h @@ -18,7 +18,9 @@ typedef struct GAState GAState; typedef struct GACommandState GACommandState; + extern GAState *ga_state; +extern QmpCommandList ga_commands; GList *ga_command_blacklist_init(GList *blacklist); void ga_command_state_init(GAState *s, GACommandState *cs); diff --git a/qga/main.c b/qga/main.c index 538e4ee299..9435bc74b8 100644 --- a/qga/main.c +++ b/qga/main.c @@ -92,6 +92,7 @@ struct GAState { }; struct GAState *ga_state; +QmpCommandList ga_commands; /* commands that are safe to issue while filesystems are frozen */ static const char *ga_freeze_whitelist[] = { @@ -370,7 +371,7 @@ static void ga_disable_non_whitelisted(QmpCommand *cmd, void *opaque) } if (!whitelisted) { g_debug("disabling command: %s", name); - qmp_disable_command(name); + qmp_disable_command(&ga_commands, name); } } @@ -383,7 +384,7 @@ static void ga_enable_non_blacklisted(QmpCommand *cmd, void *opaque) if (g_list_find_custom(blacklist, name, ga_strcmp) == NULL && !qmp_command_is_enabled(cmd)) { g_debug("enabling command: %s", name); - qmp_enable_command(name); + qmp_enable_command(&ga_commands, name); } } @@ -420,7 +421,7 @@ void ga_set_frozen(GAState *s) return; } /* disable all non-whitelisted (for frozen state) commands */ - qmp_for_each_command(ga_disable_non_whitelisted, NULL); + qmp_for_each_command(&ga_commands, ga_disable_non_whitelisted, NULL); g_warning("disabling logging due to filesystem freeze"); ga_disable_logging(s); s->frozen = true; @@ -456,7 +457,7 @@ void ga_unset_frozen(GAState *s) } /* enable all disabled, non-blacklisted commands */ - qmp_for_each_command(ga_enable_non_blacklisted, s->blacklist); + qmp_for_each_command(&ga_commands, ga_enable_non_blacklisted, s->blacklist); s->frozen = false; if (!ga_delete_file(s->state_filepath_isfrozen)) { g_warning("unable to delete %s, fsfreeze may not function properly", @@ -555,7 +556,7 @@ static void process_command(GAState *s, QDict *req) g_assert(req); g_debug("processing command"); - rsp = qmp_dispatch(QOBJECT(req)); + rsp = qmp_dispatch(&ga_commands, QOBJECT(req)); if (rsp) { ret = send_response(s, rsp); if (ret < 0) { @@ -1119,7 +1120,7 @@ static void config_parse(GAConfig *config, int argc, char **argv) break; case 'b': { if (is_help_option(optarg)) { - qmp_for_each_command(ga_print_cmd, NULL); + qmp_for_each_command(&ga_commands, ga_print_cmd, NULL); exit(EXIT_SUCCESS); } config->blacklist = g_list_concat(config->blacklist, @@ -1247,7 +1248,7 @@ static int run_agent(GAState *s, GAConfig *config) s->deferred_options.log_filepath = config->log_filepath; } ga_disable_logging(s); - qmp_for_each_command(ga_disable_non_whitelisted, NULL); + qmp_for_each_command(&ga_commands, ga_disable_non_whitelisted, NULL); } else { if (config->daemonize) { become_daemon(config->pid_filepath); @@ -1277,7 +1278,7 @@ static int run_agent(GAState *s, GAConfig *config) s->blacklist = config->blacklist; do { g_debug("disabling command: %s", (char *)l->data); - qmp_disable_command(l->data); + qmp_disable_command(&ga_commands, l->data); l = g_list_next(l); } while (l); } @@ -1321,7 +1322,7 @@ int main(int argc, char **argv) config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; - module_call_init(MODULE_INIT_QAPI); + qga_qmp_init_marshal(&ga_commands); init_dfl_pathnames(); config_load(config); diff --git a/qmp.c b/qmp.c index dfaabac1a6..fa82b598c6 100644 --- a/qmp.c +++ b/qmp.c @@ -675,7 +675,7 @@ void qmp_object_add(const char *type, const char *id, pdict = qdict_new(); } - v = qobject_input_visitor_new(QOBJECT(pdict), true); + v = qobject_input_visitor_new(QOBJECT(pdict)); obj = user_creatable_add_type(type, id, pdict, v, errp); visit_free(v); if (obj) { diff --git a/qom/qom-qobject.c b/qom/qom-qobject.c index 447e4a0560..4aec20d73c 100644 --- a/qom/qom-qobject.c +++ b/qom/qom-qobject.c @@ -22,8 +22,8 @@ void object_property_set_qobject(Object *obj, QObject *value, const char *name, Error **errp) { Visitor *v; - /* TODO: Should we reject, rather than ignore, excess input? */ - v = qobject_input_visitor_new(value, false); + + v = qobject_input_visitor_new(value); object_property_set(obj, v, name, errp); visit_free(v); } diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 09e8467d90..0c05449cb6 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -130,7 +130,7 @@ def gen_marshal(name, arg_type, boxed, ret_type): push_indent() ret += mcgen(''' - v = qobject_input_visitor_new(QOBJECT(args), true); + v = qobject_input_visitor_new(QOBJECT(args)); visit_start_struct(v, NULL, NULL, 0, &err); if (err) { goto out; @@ -198,7 +198,8 @@ def gen_register_command(name, success_response): options = 'QCO_NO_SUCCESS_RESP' ret = mcgen(''' - qmp_register_command("%(name)s", qmp_marshal_%(c_name)s, %(opts)s); + qmp_register_command(cmds, "%(name)s", + qmp_marshal_%(c_name)s, %(opts)s); ''', name=name, c_name=c_name(name), opts=options) @@ -208,14 +209,15 @@ def gen_register_command(name, success_response): def gen_registry(registry): ret = mcgen(''' -static void qmp_init_marshal(void) +void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds) { -''') + QTAILQ_INIT(cmds); + +''', + c_prefix=c_name(prefix, protect=False)) ret += registry ret += mcgen(''' } - -qapi_init(qmp_init_marshal); ''') return ret @@ -291,7 +293,6 @@ fdef.write(mcgen(''' #include "qemu-common.h" #include "qemu/module.h" #include "qapi/qmp/types.h" -#include "qapi/qmp/dispatch.h" #include "qapi/visitor.h" #include "qapi/qobject-output-visitor.h" #include "qapi/qobject-input-visitor.h" @@ -306,10 +307,12 @@ fdef.write(mcgen(''' fdecl.write(mcgen(''' #include "%(prefix)sqapi-types.h" #include "qapi/qmp/qdict.h" +#include "qapi/qmp/dispatch.h" #include "qapi/error.h" +void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds); ''', - prefix=prefix)) + prefix=prefix, c_prefix=c_name(prefix, protect=False))) schema = QAPISchema(input_file) gen = QAPISchemaGenCommandVisitor() diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py index 541644e350..fb72c61d02 100644 --- a/scripts/qapi-introspect.py +++ b/scripts/qapi-introspect.py @@ -64,7 +64,7 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor): # generate C # TODO can generate awfully long lines jsons.extend(self._jsons) - name = prefix + 'qmp_schema_json' + name = c_name(prefix, protect=False) + 'qmp_schema_json' self.decl = mcgen(''' extern const char %(c_name)s[]; ''', diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 96f2491c16..330b9f321b 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -133,6 +133,9 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error } } + if (!err) { + visit_check_list(v, &err); + } visit_end_list(v, (void **)obj); if (err && visit_is_input(v)) { qapi_free_%(c_name)s(*obj); diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index 2a894eec65..4ea3a2de80 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -346,7 +346,7 @@ static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info, } if (qdict) { - visitor = qobject_input_visitor_new(info->props, true); + visitor = qobject_input_visitor_new(info->props); visit_start_struct(visitor, NULL, NULL, 0, errp); if (*errp) { object_unref(obj); diff --git a/tests/Makefile.include b/tests/Makefile.include index 364ef1bd23..ace4e80464 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -28,7 +28,6 @@ check-unit-y += tests/test-clone-visitor$(EXESUF) gcov-files-test-clone-visitor-y = qapi/qapi-clone-visitor.c check-unit-y += tests/test-qobject-input-visitor$(EXESUF) gcov-files-test-qobject-input-visitor-y = qapi/qobject-input-visitor.c -check-unit-y += tests/test-qobject-input-strict$(EXESUF) check-unit-y += tests/test-qmp-commands$(EXESUF) gcov-files-test-qmp-commands-y = qapi/qmp-dispatch.c check-unit-y += tests/test-string-input-visitor$(EXESUF) @@ -133,7 +132,9 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh # All QTests for now are POSIX-only, but the dependencies are # really in libqtest, not in the testcases themselves. -check-qtest-generic-y = tests/device-introspect-test$(EXESUF) +check-qtest-generic-y = tests/qmp-test$(EXESUF) +gcov-files-generic-y = monitor.c qapi/qmp-dispatch.c +check-qtest-generic-y += tests/device-introspect-test$(EXESUF) gcov-files-generic-y = qdev-monitor.c qmp.c gcov-files-ipack-y += hw/ipack/ipack.c @@ -487,7 +488,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \ tests/test-coroutine.o tests/test-string-output-visitor.o \ tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \ tests/test-clone-visitor.o \ - tests/test-qobject-input-visitor.o tests/test-qobject-input-strict.o \ + tests/test-qobject-input-visitor.o \ tests/test-qmp-commands.o tests/test-visitor-serialization.o \ tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \ tests/test-opts-visitor.o tests/test-qmp-event.o \ @@ -596,7 +597,6 @@ tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) tests/test-qobject-output-visitor$(EXESUF): tests/test-qobject-output-visitor.o $(test-qapi-obj-y) tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o $(test-qapi-obj-y) tests/test-qobject-input-visitor$(EXESUF): tests/test-qobject-input-visitor.o $(test-qapi-obj-y) -tests/test-qobject-input-strict$(EXESUF): tests/test-qobject-input-strict.o $(test-qapi-obj-y) tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marshal.o $(test-qapi-obj-y) tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y) tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o $(test-qapi-obj-y) @@ -653,6 +653,7 @@ libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o libqos-virtio-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o +tests/qmp-test$(EXESUF): tests/qmp-test.o tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o tests/rtc-test$(EXESUF): tests/rtc-test.o tests/m48t59-test$(EXESUF): tests/m48t59-test.o diff --git a/tests/check-qnull.c b/tests/check-qnull.c index b50bb8a81a..8dd1c9686f 100644 --- a/tests/check-qnull.c +++ b/tests/check-qnull.c @@ -47,7 +47,7 @@ static void qnull_visit_test(void) g_assert(qnull_.refcnt == 1); obj = qnull(); - v = qobject_input_visitor_new(obj, true); + v = qobject_input_visitor_new(obj); qobject_decref(obj); visit_type_null(v, NULL, &error_abort); visit_free(v); diff --git a/tests/libqtest.c b/tests/libqtest.c index 3a0e0d63a7..ca6b641963 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -149,7 +149,7 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data) g_hook_prepend(&abrt_hooks, hook); } -QTestState *qtest_init(const char *extra_args) +QTestState *qtest_init_without_qmp_handshake(const char *extra_args) { QTestState *s; int sock, qmpsock, i; @@ -204,10 +204,6 @@ QTestState *qtest_init(const char *extra_args) s->irq_level[i] = false; } - /* Read the QMP greeting and then do the handshake */ - qtest_qmp_discard_response(s, ""); - qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); - if (getenv("QTEST_STOP")) { kill(s->qemu_pid, SIGSTOP); } @@ -219,6 +215,17 @@ QTestState *qtest_init(const char *extra_args) return s; } +QTestState *qtest_init(const char *extra_args) +{ + QTestState *s = qtest_init_without_qmp_handshake(extra_args); + + /* Read the QMP greeting and then do the handshake */ + qtest_qmp_discard_response(s, ""); + qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); + + return s; +} + void qtest_quit(QTestState *s) { qtest_instances = g_list_remove(qtest_instances, s); @@ -442,14 +449,20 @@ void qmp_fd_sendv(int fd, const char *fmt, va_list ap) if (qobj) { int log = getenv("QTEST_LOG") != NULL; QString *qstr = qobject_to_json(qobj); - const char *str = qstring_get_str(qstr); - size_t size = qstring_get_length(qstr); + const char *str; + + /* + * BUG: QMP doesn't react to input until it sees a newline, an + * object, or an array. Work-around: give it a newline. + */ + qstring_append_chr(qstr, '\n'); + str = qstring_get_str(qstr); if (log) { fprintf(stderr, "%s", str); } /* Send QMP request */ - socket_send(fd, str, size); + socket_send(fd, str, qstring_get_length(qstr)); QDECREF(qstr); qobject_decref(qobj); diff --git a/tests/libqtest.h b/tests/libqtest.h index 90f182e1d8..2c9962d94f 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -32,6 +32,14 @@ extern QTestState *global_qtest; QTestState *qtest_init(const char *extra_args); /** + * qtest_init_without_qmp_handshake: + * @extra_args: other arguments to pass to QEMU. + * + * Returns: #QTestState instance. + */ +QTestState *qtest_init_without_qmp_handshake(const char *extra_args); + +/** * qtest_quit: * @s: #QTestState instance to operate on. * diff --git a/tests/qmp-test.c b/tests/qmp-test.c new file mode 100644 index 0000000000..5d0260b2be --- /dev/null +++ b/tests/qmp-test.c @@ -0,0 +1,139 @@ +/* + * QMP protocol test cases + * + * Copyright (c) 2017 Red Hat Inc. + * + * Authors: + * Markus Armbruster <armbru@redhat.com>, + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "libqtest.h" +#include "qapi-visit.h" +#include "qapi/error.h" +#include "qapi/qobject-input-visitor.h" +#include "qapi/visitor.h" + +const char common_args[] = "-nodefaults -machine none"; + +static const char *get_error_class(QDict *resp) +{ + QDict *error = qdict_get_qdict(resp, "error"); + const char *desc = qdict_get_try_str(error, "desc"); + + g_assert(desc); + return error ? qdict_get_try_str(error, "class") : NULL; +} + +static void test_version(QObject *version) +{ + Visitor *v; + VersionInfo *vinfo; + + g_assert(version); + v = qobject_input_visitor_new(version); + visit_type_VersionInfo(v, "version", &vinfo, &error_abort); + qapi_free_VersionInfo(vinfo); + visit_free(v); +} + +static void test_malformed(void) +{ + QDict *resp; + + /* Not even a dictionary */ + resp = qmp("null"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* No "execute" key */ + resp = qmp("{}"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* "execute" isn't a string */ + resp = qmp("{ 'execute': true }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* "arguments" isn't a dictionary */ + resp = qmp("{ 'execute': 'no-such-cmd', 'arguments': [] }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); + + /* extra key */ + resp = qmp("{ 'execute': 'no-such-cmd', 'extra': true }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + QDECREF(resp); +} + +static void test_qmp_protocol(void) +{ + QDict *resp, *q, *ret; + QList *capabilities; + + global_qtest = qtest_init_without_qmp_handshake(common_args); + + /* Test greeting */ + resp = qmp_receive(); + q = qdict_get_qdict(resp, "QMP"); + g_assert(q); + test_version(qdict_get(q, "version")); + capabilities = qdict_get_qlist(q, "capabilities"); + g_assert(capabilities && qlist_empty(capabilities)); + QDECREF(resp); + + /* Test valid command before handshake */ + resp = qmp("{ 'execute': 'query-version' }"); + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); + QDECREF(resp); + + /* Test malformed commands before handshake */ + test_malformed(); + + /* Test handshake */ + resp = qmp("{ 'execute': 'qmp_capabilities' }"); + ret = qdict_get_qdict(resp, "return"); + g_assert(ret && !qdict_size(ret)); + QDECREF(resp); + + /* Test repeated handshake */ + resp = qmp("{ 'execute': 'qmp_capabilities' }"); + g_assert_cmpstr(get_error_class(resp), ==, "CommandNotFound"); + QDECREF(resp); + + /* Test valid command */ + resp = qmp("{ 'execute': 'query-version' }"); + test_version(qdict_get(resp, "return")); + QDECREF(resp); + + /* Test malformed commands */ + test_malformed(); + + /* Test 'id' */ + resp = qmp("{ 'execute': 'query-name', 'id': 'cookie#1' }"); + ret = qdict_get_qdict(resp, "return"); + g_assert(ret); + g_assert_cmpstr(qdict_get_try_str(resp, "id"), ==, "cookie#1"); + QDECREF(resp); + + /* Test command failure with 'id' */ + resp = qmp("{ 'execute': 'human-monitor-command', 'id': 2 }"); + g_assert_cmpstr(get_error_class(resp), ==, "GenericError"); + g_assert_cmpint(qdict_get_int(resp, "id"), ==, 2); + QDECREF(resp); + + qtest_end(); +} + +int main(int argc, char *argv[]) +{ + g_test_init(&argc, &argv, NULL); + + qtest_add_func("qmp/protocol", test_qmp_protocol); + + return g_test_run(); +} diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c index 0a9e75f1bb..2238f8efe5 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -172,6 +172,81 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data) /* test cases */ +static void +test_opts_range_unvisited(void) +{ + intList *list = NULL; + intList *tail; + QemuOpts *opts; + Visitor *v; + + opts = qemu_opts_parse(qemu_find_opts("userdef"), "ilist=0-2", false, + &error_abort); + + v = opts_visitor_new(opts); + + visit_start_struct(v, NULL, NULL, 0, &error_abort); + + /* Would be simpler if the visitor genuinely supported virtual walks */ + visit_start_list(v, "ilist", (GenericList **)&list, sizeof(*list), + &error_abort); + tail = list; + visit_type_int(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 0); + tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list)); + g_assert(tail); + visit_type_int(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 1); + tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*list)); + g_assert(tail); + visit_check_list(v, &error_abort); /* BUG: unvisited tail not reported */ + visit_end_list(v, (void **)&list); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + qapi_free_intList(list); + visit_free(v); + qemu_opts_del(opts); +} + +static void +test_opts_range_beyond(void) +{ + Error *err = NULL; + intList *list = NULL; + intList *tail; + QemuOpts *opts; + Visitor *v; + int64_t val; + + opts = qemu_opts_parse(qemu_find_opts("userdef"), "ilist=0", false, + &error_abort); + + v = opts_visitor_new(opts); + + visit_start_struct(v, NULL, NULL, 0, &error_abort); + + /* Would be simpler if the visitor genuinely supported virtual walks */ + visit_start_list(v, "ilist", (GenericList **)&list, sizeof(*list), + &error_abort); + tail = list; + visit_type_int(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 0); + tail = (intList *)visit_next_list(v, (GenericList *)tail, sizeof(*tail)); + g_assert(!tail); + visit_type_int(v, NULL, &val, &err); + error_free_or_abort(&err); + visit_end_list(v, (void **)&list); + + visit_check_struct(v, &error_abort); + visit_end_struct(v, NULL); + + qapi_free_intList(list); + visit_free(v); + qemu_opts_del(opts); +} + int main(int argc, char **argv) { @@ -263,6 +338,11 @@ main(int argc, char **argv) add_test("/visitor/opts/i64/range/2big/full", &expect_fail, "i64=-0x8000000000000000-0x7fffffffffffffff"); + g_test_add_func("/visitor/opts/range/unvisited", + test_opts_range_unvisited); + g_test_add_func("/visitor/opts/range/beyond", + test_opts_range_beyond); + g_test_run(); return 0; } diff --git a/tests/test-qga.c b/tests/test-qga.c index 868b02a40f..ae97b575c1 100644 --- a/tests/test-qga.c +++ b/tests/test-qga.c @@ -213,7 +213,7 @@ static void test_qga_invalid_args(gconstpointer fix) desc = qdict_get_try_str(error, "desc"); g_assert_cmpstr(class, ==, "GenericError"); - g_assert_cmpstr(desc, ==, "QMP input object member 'foo' is unexpected"); + g_assert_cmpstr(desc, ==, "Parameter 'foo' is unexpected"); QDECREF(ret); } diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c index ff944811e8..0f81a98be2 100644 --- a/tests/test-qmp-commands.c +++ b/tests/test-qmp-commands.c @@ -8,6 +8,8 @@ #include "tests/test-qapi-types.h" #include "tests/test-qapi-visit.h" +static QmpCommandList qmp_commands; + void qmp_user_def_cmd(Error **errp) { } @@ -94,7 +96,7 @@ static void test_dispatch_cmd(void) qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd"))); - resp = qmp_dispatch(QOBJECT(req)); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req)); assert(resp != NULL); assert(!qdict_haskey(qobject_to_qdict(resp), "error")); @@ -111,7 +113,7 @@ static void test_dispatch_cmd_failure(void) qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2"))); - resp = qmp_dispatch(QOBJECT(req)); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req)); assert(resp != NULL); assert(qdict_haskey(qobject_to_qdict(resp), "error")); @@ -125,7 +127,7 @@ static void test_dispatch_cmd_failure(void) qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd"))); - resp = qmp_dispatch(QOBJECT(req)); + resp = qmp_dispatch(&qmp_commands, QOBJECT(req)); assert(resp != NULL); assert(qdict_haskey(qobject_to_qdict(resp), "error")); @@ -139,7 +141,7 @@ static QObject *test_qmp_dispatch(QDict *req) QDict *resp; QObject *ret; - resp_obj = qmp_dispatch(QOBJECT(req)); + resp_obj = qmp_dispatch(&qmp_commands, QOBJECT(req)); assert(resp_obj); resp = qobject_to_qdict(resp_obj); assert(resp && !qdict_haskey(resp, "error")); @@ -244,7 +246,7 @@ static void test_dealloc_partial(void) ud2_dict = qdict_new(); qdict_put_obj(ud2_dict, "string0", QOBJECT(qstring_from_str(text))); - v = qobject_input_visitor_new(QOBJECT(ud2_dict), true); + v = qobject_input_visitor_new(QOBJECT(ud2_dict)); visit_type_UserDefTwo(v, NULL, &ud2, &err); visit_free(v); QDECREF(ud2_dict); @@ -273,7 +275,7 @@ int main(int argc, char **argv) g_test_add_func("/0.15/dealloc_types", test_dealloc_types); g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial); - module_call_init(MODULE_INIT_QAPI); + test_qmp_init_marshal(&qmp_commands); g_test_run(); return 0; diff --git a/tests/test-qobject-input-strict.c b/tests/test-qobject-input-strict.c deleted file mode 100644 index 4087ea3dd3..0000000000 --- a/tests/test-qobject-input-strict.c +++ /dev/null @@ -1,381 +0,0 @@ -/* - * QObject Input Visitor unit-tests (strict mode). - * - * Copyright (C) 2011-2012, 2015 Red Hat Inc. - * - * Authors: - * Luiz Capitulino <lcapitulino@redhat.com> - * Paolo Bonzini <pbonzini@redhat.com> - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - */ - -#include "qemu/osdep.h" - -#include "qemu-common.h" -#include "qapi/error.h" -#include "qapi/qobject-input-visitor.h" -#include "test-qapi-types.h" -#include "test-qapi-visit.h" -#include "qapi/qmp/types.h" -#include "qapi/qmp/qjson.h" -#include "test-qmp-introspect.h" -#include "qmp-introspect.h" -#include "qapi-visit.h" - -typedef struct TestInputVisitorData { - QObject *obj; - Visitor *qiv; -} TestInputVisitorData; - -static void validate_teardown(TestInputVisitorData *data, - const void *unused) -{ - qobject_decref(data->obj); - data->obj = NULL; - - if (data->qiv) { - visit_free(data->qiv); - data->qiv = NULL; - } -} - -/* The various test_init functions are provided instead of a test setup - function so that the JSON string used by the tests are kept in the test - functions (and not in main()). */ -static Visitor *validate_test_init_internal(TestInputVisitorData *data, - const char *json_string, - va_list *ap) -{ - validate_teardown(data, NULL); - - data->obj = qobject_from_jsonv(json_string, ap); - g_assert(data->obj); - - data->qiv = qobject_input_visitor_new(data->obj, true); - g_assert(data->qiv); - return data->qiv; -} - -static GCC_FMT_ATTR(2, 3) -Visitor *validate_test_init(TestInputVisitorData *data, - const char *json_string, ...) -{ - Visitor *v; - va_list ap; - - va_start(ap, json_string); - v = validate_test_init_internal(data, json_string, &ap); - va_end(ap); - return v; -} - -/* similar to validate_test_init(), but does not expect a string - * literal/format json_string argument and so can be used for - * programatically generated strings (and we can't pass in programatically - * generated strings via %s format parameters since qobject_from_jsonv() - * will wrap those in double-quotes and treat the entire object as a - * string) - */ -static Visitor *validate_test_init_raw(TestInputVisitorData *data, - const char *json_string) -{ - return validate_test_init_internal(data, json_string, NULL); -} - - -static void test_validate_struct(TestInputVisitorData *data, - const void *unused) -{ - TestStruct *p = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }"); - - visit_type_TestStruct(v, NULL, &p, &error_abort); - g_free(p->string); - g_free(p); -} - -static void test_validate_struct_nested(TestInputVisitorData *data, - const void *unused) -{ - UserDefTwo *udp = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'string0': 'string0', " - "'dict1': { 'string1': 'string1', " - "'dict2': { 'userdef': { 'integer': 42, " - "'string': 'string' }, 'string': 'string2'}}}"); - - visit_type_UserDefTwo(v, NULL, &udp, &error_abort); - qapi_free_UserDefTwo(udp); -} - -static void test_validate_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefOneList *head = NULL; - Visitor *v; - - v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]"); - - visit_type_UserDefOneList(v, NULL, &head, &error_abort); - qapi_free_UserDefOneList(head); -} - -static void test_validate_union_native_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefNativeListUnion *tmp = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'type': 'integer', 'data' : [ 1, 2 ] }"); - - visit_type_UserDefNativeListUnion(v, NULL, &tmp, &error_abort); - qapi_free_UserDefNativeListUnion(tmp); -} - -static void test_validate_union_flat(TestInputVisitorData *data, - const void *unused) -{ - UserDefFlatUnion *tmp = NULL; - Visitor *v; - - v = validate_test_init(data, - "{ 'enum1': 'value1', " - "'integer': 41, " - "'string': 'str', " - "'boolean': true }"); - - visit_type_UserDefFlatUnion(v, NULL, &tmp, &error_abort); - qapi_free_UserDefFlatUnion(tmp); -} - -static void test_validate_alternate(TestInputVisitorData *data, - const void *unused) -{ - UserDefAlternate *tmp = NULL; - Visitor *v; - - v = validate_test_init(data, "42"); - - visit_type_UserDefAlternate(v, NULL, &tmp, &error_abort); - qapi_free_UserDefAlternate(tmp); -} - -static void test_validate_fail_struct(TestInputVisitorData *data, - const void *unused) -{ - TestStruct *p = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }"); - - visit_type_TestStruct(v, NULL, &p, &err); - error_free_or_abort(&err); - g_assert(!p); -} - -static void test_validate_fail_struct_nested(TestInputVisitorData *data, - const void *unused) -{ - UserDefTwo *udp = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}"); - - visit_type_UserDefTwo(v, NULL, &udp, &err); - error_free_or_abort(&err); - g_assert(!udp); -} - -static void test_validate_fail_struct_missing(TestInputVisitorData *data, - const void *unused) -{ - Error *err = NULL; - Visitor *v; - QObject *any; - GenericAlternate *alt; - bool present; - int en; - int64_t i64; - uint32_t u32; - int8_t i8; - char *str; - double dbl; - - v = validate_test_init(data, "{}"); - visit_start_struct(v, NULL, NULL, 0, &error_abort); - visit_start_struct(v, "struct", NULL, 0, &err); - error_free_or_abort(&err); - visit_start_list(v, "list", NULL, 0, &err); - error_free_or_abort(&err); - visit_start_alternate(v, "alternate", &alt, sizeof(*alt), false, &err); - error_free_or_abort(&err); - visit_optional(v, "optional", &present); - g_assert(!present); - visit_type_enum(v, "enum", &en, EnumOne_lookup, &err); - error_free_or_abort(&err); - visit_type_int(v, "i64", &i64, &err); - error_free_or_abort(&err); - visit_type_uint32(v, "u32", &u32, &err); - error_free_or_abort(&err); - visit_type_int8(v, "i8", &i8, &err); - error_free_or_abort(&err); - visit_type_str(v, "i8", &str, &err); - error_free_or_abort(&err); - visit_type_number(v, "dbl", &dbl, &err); - error_free_or_abort(&err); - visit_type_any(v, "any", &any, &err); - error_free_or_abort(&err); - visit_type_null(v, "null", &err); - error_free_or_abort(&err); - visit_end_struct(v, NULL); -} - -static void test_validate_fail_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefOneList *head = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]"); - - visit_type_UserDefOneList(v, NULL, &head, &err); - error_free_or_abort(&err); - g_assert(!head); -} - -static void test_validate_fail_union_native_list(TestInputVisitorData *data, - const void *unused) -{ - UserDefNativeListUnion *tmp = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, - "{ 'type': 'integer', 'data' : [ 'string' ] }"); - - visit_type_UserDefNativeListUnion(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void test_validate_fail_union_flat(TestInputVisitorData *data, - const void *unused) -{ - UserDefFlatUnion *tmp = NULL; - Error *err = NULL; - Visitor *v; - - v = validate_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }"); - - visit_type_UserDefFlatUnion(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data, - const void *unused) -{ - UserDefFlatUnion2 *tmp = NULL; - Error *err = NULL; - Visitor *v; - - /* test situation where discriminator field ('enum1' here) is missing */ - v = validate_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }"); - - visit_type_UserDefFlatUnion2(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void test_validate_fail_alternate(TestInputVisitorData *data, - const void *unused) -{ - UserDefAlternate *tmp; - Visitor *v; - Error *err = NULL; - - v = validate_test_init(data, "3.14"); - - visit_type_UserDefAlternate(v, NULL, &tmp, &err); - error_free_or_abort(&err); - g_assert(!tmp); -} - -static void do_test_validate_qmp_introspect(TestInputVisitorData *data, - const char *schema_json) -{ - SchemaInfoList *schema = NULL; - Visitor *v; - - v = validate_test_init_raw(data, schema_json); - - visit_type_SchemaInfoList(v, NULL, &schema, &error_abort); - g_assert(schema); - - qapi_free_SchemaInfoList(schema); -} - -static void test_validate_qmp_introspect(TestInputVisitorData *data, - const void *unused) -{ - do_test_validate_qmp_introspect(data, test_qmp_schema_json); - do_test_validate_qmp_introspect(data, qmp_schema_json); -} - -static void validate_test_add(const char *testpath, - TestInputVisitorData *data, - void (*test_func)(TestInputVisitorData *data, const void *user_data)) -{ - g_test_add(testpath, TestInputVisitorData, data, NULL, test_func, - validate_teardown); -} - -int main(int argc, char **argv) -{ - TestInputVisitorData testdata; - - g_test_init(&argc, &argv, NULL); - - validate_test_add("/visitor/input-strict/pass/struct", - &testdata, test_validate_struct); - validate_test_add("/visitor/input-strict/pass/struct-nested", - &testdata, test_validate_struct_nested); - validate_test_add("/visitor/input-strict/pass/list", - &testdata, test_validate_list); - validate_test_add("/visitor/input-strict/pass/union-flat", - &testdata, test_validate_union_flat); - validate_test_add("/visitor/input-strict/pass/alternate", - &testdata, test_validate_alternate); - validate_test_add("/visitor/input-strict/pass/union-native-list", - &testdata, test_validate_union_native_list); - validate_test_add("/visitor/input-strict/fail/struct", - &testdata, test_validate_fail_struct); - validate_test_add("/visitor/input-strict/fail/struct-nested", - &testdata, test_validate_fail_struct_nested); - validate_test_add("/visitor/input-strict/fail/struct-missing", - &testdata, test_validate_fail_struct_missing); - validate_test_add("/visitor/input-strict/fail/list", - &testdata, test_validate_fail_list); - validate_test_add("/visitor/input-strict/fail/union-flat", - &testdata, test_validate_fail_union_flat); - validate_test_add("/visitor/input-strict/fail/union-flat-no-discriminator", - &testdata, test_validate_fail_union_flat_no_discrim); - validate_test_add("/visitor/input-strict/fail/alternate", - &testdata, test_validate_fail_alternate); - validate_test_add("/visitor/input-strict/fail/union-native-list", - &testdata, test_validate_fail_union_native_list); - validate_test_add("/visitor/input-strict/pass/qmp-introspect", - &testdata, test_validate_qmp_introspect); - - g_test_run(); - - return 0; -} diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c index 945404a9e0..94305f58ca 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -5,6 +5,7 @@ * * Authors: * Luiz Capitulino <lcapitulino@redhat.com> + * Paolo Bonzini <pbonzini@redhat.com> * * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. @@ -19,6 +20,9 @@ #include "test-qapi-visit.h" #include "qapi/qmp/types.h" #include "qapi/qmp/qjson.h" +#include "test-qmp-introspect.h" +#include "qmp-introspect.h" +#include "qapi-visit.h" typedef struct TestInputVisitorData { QObject *obj; @@ -49,7 +53,7 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data, data->obj = qobject_from_jsonv(json_string, ap); g_assert(data->obj); - data->qiv = qobject_input_visitor_new(data->obj, false); + data->qiv = qobject_input_visitor_new(data->obj); g_assert(data->qiv); return data->qiv; } @@ -290,14 +294,14 @@ static void test_visitor_in_null(TestInputVisitorData *data, * when input is not null. */ - v = visitor_input_test_init(data, "{ 'a': null, 'b': '' }"); + v = visitor_input_test_init(data, "{ 'a': null, 'b': '', 'c': null }"); visit_start_struct(v, NULL, NULL, 0, &error_abort); visit_type_null(v, "a", &error_abort); - visit_type_str(v, "a", &tmp, &err); - g_assert(!tmp); - error_free_or_abort(&err); visit_type_null(v, "b", &err); error_free_or_abort(&err); + visit_type_str(v, "c", &tmp, &err); + g_assert(!tmp); + error_free_or_abort(&err); visit_check_struct(v, &error_abort); visit_end_struct(v, NULL); } @@ -833,6 +837,230 @@ static void test_visitor_in_wrong_type(TestInputVisitorData *data, error_free_or_abort(&err); } +static void test_visitor_in_fail_struct(TestInputVisitorData *data, + const void *unused) +{ + TestStruct *p = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }"); + + visit_type_TestStruct(v, NULL, &p, &err); + error_free_or_abort(&err); + g_assert(!p); +} + +static void test_visitor_in_fail_struct_nested(TestInputVisitorData *data, + const void *unused) +{ + UserDefTwo *udp = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}"); + + visit_type_UserDefTwo(v, NULL, &udp, &err); + error_free_or_abort(&err); + g_assert(!udp); +} + +static void test_visitor_in_fail_struct_in_list(TestInputVisitorData *data, + const void *unused) +{ + UserDefOneList *head = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]"); + + visit_type_UserDefOneList(v, NULL, &head, &err); + error_free_or_abort(&err); + g_assert(!head); +} + +static void test_visitor_in_fail_struct_missing(TestInputVisitorData *data, + const void *unused) +{ + Error *err = NULL; + Visitor *v; + QObject *any; + GenericAlternate *alt; + bool present; + int en; + int64_t i64; + uint32_t u32; + int8_t i8; + char *str; + double dbl; + + v = visitor_input_test_init(data, "{ 'sub': [ {} ] }"); + visit_start_struct(v, NULL, NULL, 0, &error_abort); + visit_start_struct(v, "struct", NULL, 0, &err); + error_free_or_abort(&err); + visit_start_list(v, "list", NULL, 0, &err); + error_free_or_abort(&err); + visit_start_alternate(v, "alternate", &alt, sizeof(*alt), false, &err); + error_free_or_abort(&err); + visit_optional(v, "optional", &present); + g_assert(!present); + visit_type_enum(v, "enum", &en, EnumOne_lookup, &err); + error_free_or_abort(&err); + visit_type_int(v, "i64", &i64, &err); + error_free_or_abort(&err); + visit_type_uint32(v, "u32", &u32, &err); + error_free_or_abort(&err); + visit_type_int8(v, "i8", &i8, &err); + error_free_or_abort(&err); + visit_type_str(v, "i8", &str, &err); + error_free_or_abort(&err); + visit_type_number(v, "dbl", &dbl, &err); + error_free_or_abort(&err); + visit_type_any(v, "any", &any, &err); + error_free_or_abort(&err); + visit_type_null(v, "null", &err); + error_free_or_abort(&err); + visit_start_list(v, "sub", NULL, 0, &error_abort); + visit_start_struct(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, "i64", &i64, &err); + error_free_or_abort(&err); + visit_end_struct(v, NULL); + visit_end_list(v, NULL); + visit_end_struct(v, NULL); +} + +static void test_visitor_in_fail_list(TestInputVisitorData *data, + const void *unused) +{ + int64_t i64 = -1; + Error *err = NULL; + Visitor *v; + + /* Unvisited list tail */ + + v = visitor_input_test_init(data, "[ 1, 2, 3 ]"); + + visit_start_list(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 1); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 2); + visit_check_list(v, &err); + error_free_or_abort(&err); + visit_end_list(v, NULL); + + /* Visit beyond end of list */ + v = visitor_input_test_init(data, "[]"); + + visit_start_list(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, NULL, &i64, &err); + error_free_or_abort(&err); + visit_end_list(v, NULL); +} + +static void test_visitor_in_fail_list_nested(TestInputVisitorData *data, + const void *unused) +{ + int64_t i64 = -1; + Error *err = NULL; + Visitor *v; + + /* Unvisited nested list tail */ + + v = visitor_input_test_init(data, "[ 0, [ 1, 2, 3 ] ]"); + + visit_start_list(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 0); + visit_start_list(v, NULL, NULL, 0, &error_abort); + visit_type_int(v, NULL, &i64, &error_abort); + g_assert_cmpint(i64, ==, 1); + visit_check_list(v, &err); + error_free_or_abort(&err); + visit_end_list(v, NULL); + visit_check_list(v, &error_abort); + visit_end_list(v, NULL); +} + +static void test_visitor_in_fail_union_native_list(TestInputVisitorData *data, + const void *unused) +{ + UserDefNativeListUnion *tmp = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, + "{ 'type': 'integer', 'data' : [ 'string' ] }"); + + visit_type_UserDefNativeListUnion(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void test_visitor_in_fail_union_flat(TestInputVisitorData *data, + const void *unused) +{ + UserDefFlatUnion *tmp = NULL; + Error *err = NULL; + Visitor *v; + + v = visitor_input_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }"); + + visit_type_UserDefFlatUnion(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void test_visitor_in_fail_union_flat_no_discrim(TestInputVisitorData *data, + const void *unused) +{ + UserDefFlatUnion2 *tmp = NULL; + Error *err = NULL; + Visitor *v; + + /* test situation where discriminator field ('enum1' here) is missing */ + v = visitor_input_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }"); + + visit_type_UserDefFlatUnion2(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void test_visitor_in_fail_alternate(TestInputVisitorData *data, + const void *unused) +{ + UserDefAlternate *tmp; + Visitor *v; + Error *err = NULL; + + v = visitor_input_test_init(data, "3.14"); + + visit_type_UserDefAlternate(v, NULL, &tmp, &err); + error_free_or_abort(&err); + g_assert(!tmp); +} + +static void do_test_visitor_in_qmp_introspect(TestInputVisitorData *data, + const char *schema_json) +{ + SchemaInfoList *schema = NULL; + Visitor *v; + + v = visitor_input_test_init_raw(data, schema_json); + + visit_type_SchemaInfoList(v, NULL, &schema, &error_abort); + g_assert(schema); + + qapi_free_SchemaInfoList(schema); +} + +static void test_visitor_in_qmp_introspect(TestInputVisitorData *data, + const void *unused) +{ + do_test_visitor_in_qmp_introspect(data, test_qmp_schema_json); + do_test_visitor_in_qmp_introspect(data, qmp_schema_json); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -893,6 +1121,28 @@ int main(int argc, char **argv) NULL, test_visitor_in_native_list_string); input_visitor_test_add("/visitor/input/native_list/number", NULL, test_visitor_in_native_list_number); + input_visitor_test_add("/visitor/input/fail/struct", + NULL, test_visitor_in_fail_struct); + input_visitor_test_add("/visitor/input/fail/struct-nested", + NULL, test_visitor_in_fail_struct_nested); + input_visitor_test_add("/visitor/input/fail/struct-in-list", + NULL, test_visitor_in_fail_struct_in_list); + input_visitor_test_add("/visitor/input/fail/struct-missing", + NULL, test_visitor_in_fail_struct_missing); + input_visitor_test_add("/visitor/input/fail/list", + NULL, test_visitor_in_fail_list); + input_visitor_test_add("/visitor/input/fail/list-nested", + NULL, test_visitor_in_fail_list_nested); + input_visitor_test_add("/visitor/input/fail/union-flat", + NULL, test_visitor_in_fail_union_flat); + input_visitor_test_add("/visitor/input/fail/union-flat-no-discriminator", + NULL, test_visitor_in_fail_union_flat_no_discrim); + input_visitor_test_add("/visitor/input/fail/alternate", + NULL, test_visitor_in_fail_alternate); + input_visitor_test_add("/visitor/input/fail/union-native-list", + NULL, test_visitor_in_fail_union_native_list); + input_visitor_test_add("/visitor/input/qmp-introspect", + NULL, test_visitor_in_qmp_introspect); g_test_run(); diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c index 7f10e2582f..6db850bc89 100644 --- a/tests/test-string-input-visitor.c +++ b/tests/test-string-input-visitor.c @@ -39,6 +39,8 @@ static Visitor *visitor_input_test_init(TestInputVisitorData *data, const char *string) { + visitor_input_teardown(data, NULL); + data->v = string_input_visitor_new(string); g_assert(data->v); return data->v; @@ -57,43 +59,138 @@ static void test_visitor_in_int(TestInputVisitorData *data, g_assert(!err); g_assert_cmpint(res, ==, value); - visitor_input_teardown(data, unused); - v = visitor_input_test_init(data, "not an int"); visit_type_int(v, NULL, &res, &err); error_free_or_abort(&err); } +static void check_ilist(Visitor *v, int64_t *expected, size_t n) +{ + int64List *res = NULL; + int64List *tail; + int i; + + visit_type_int64List(v, NULL, &res, &error_abort); + tail = res; + for (i = 0; i < n; i++) { + g_assert(tail); + g_assert_cmpint(tail->value, ==, expected[i]); + tail = tail->next; + } + g_assert(!tail); + + qapi_free_int64List(res); +} + +static void check_ulist(Visitor *v, uint64_t *expected, size_t n) +{ + uint64List *res = NULL; + uint64List *tail; + int i; + + /* BUG: unsigned numbers above INT64_MAX don't work */ + for (i = 0; i < n; i++) { + if (expected[i] > INT64_MAX) { + Error *err = NULL; + visit_type_uint64List(v, NULL, &res, &err); + error_free_or_abort(&err); + return; + } + } + + visit_type_uint64List(v, NULL, &res, &error_abort); + tail = res; + for (i = 0; i < n; i++) { + g_assert(tail); + g_assert_cmpuint(tail->value, ==, expected[i]); + tail = tail->next; + } + g_assert(!tail); + + qapi_free_uint64List(res); +} + static void test_visitor_in_intList(TestInputVisitorData *data, const void *unused) { - int64_t value[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20}; - int16List *res = NULL, *tmp; + /* Note: the visitor *sorts* ranges *unsigned* */ + int64_t expect1[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20 }; + int64_t expect2[] = { 32767, -32768, -32767 }; + int64_t expect3[] = { INT64_MAX, INT64_MIN }; + uint64_t expect4[] = { UINT64_MAX }; Error *err = NULL; + int64List *res = NULL; + int64List *tail; Visitor *v; - int i = 0; + int64_t val; + + /* Valid lists */ v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8"); + check_ilist(v, expect1, ARRAY_SIZE(expect1)); - visit_type_int16List(v, NULL, &res, &error_abort); - tmp = res; - while (i < sizeof(value) / sizeof(value[0])) { - g_assert(tmp); - g_assert_cmpint(tmp->value, ==, value[i++]); - tmp = tmp->next; - } - g_assert(!tmp); + v = visitor_input_test_init(data, "32767,-32768--32767"); + check_ilist(v, expect2, ARRAY_SIZE(expect2)); + + v = visitor_input_test_init(data, + "-9223372036854775808,9223372036854775807"); + check_ilist(v, expect3, ARRAY_SIZE(expect3)); + + v = visitor_input_test_init(data, "18446744073709551615"); + check_ulist(v, expect4, ARRAY_SIZE(expect4)); + + /* Empty list is invalid (weird) */ - qapi_free_int16List(res); + v = visitor_input_test_init(data, ""); + visit_type_int64List(v, NULL, &res, &err); + error_free_or_abort(&err); - visitor_input_teardown(data, unused); + /* Not a list */ v = visitor_input_test_init(data, "not an int list"); - visit_type_int16List(v, NULL, &res, &err); + visit_type_int64List(v, NULL, &res, &err); error_free_or_abort(&err); g_assert(!res); + + /* Unvisited list tail */ + + v = visitor_input_test_init(data, "0,2-3"); + + /* Would be simpler if the visitor genuinely supported virtual walks */ + visit_start_list(v, NULL, (GenericList **)&res, sizeof(*res), + &error_abort); + tail = res; + visit_type_int64(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 0); + tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res)); + g_assert(tail); + visit_type_int64(v, NULL, &tail->value, &error_abort); + g_assert_cmpint(tail->value, ==, 2); + tail = (int64List *)visit_next_list(v, (GenericList *)tail, sizeof(*res)); + g_assert(tail); + + visit_check_list(v, &err); + error_free_or_abort(&err); + visit_end_list(v, (void **)&res); + + qapi_free_int64List(res); + + /* Visit beyond end of list */ + v = visitor_input_test_init(data, "0"); + + visit_start_list(v, NULL, (GenericList **)&res, sizeof(*res), + &error_abort); + tail = res; + visit_type_int64(v, NULL, &tail->value, &err); + g_assert_cmpint(tail->value, ==, 0); + visit_type_int64(v, NULL, &val, &err); + g_assert_cmpint(val, ==, 1); /* BUG */ + visit_check_list(v, &error_abort); + visit_end_list(v, (void **)&res); + + qapi_free_int64List(res); } static void test_visitor_in_bool(TestInputVisitorData *data, @@ -108,35 +205,30 @@ static void test_visitor_in_bool(TestInputVisitorData *data, visit_type_bool(v, NULL, &res, &err); g_assert(!err); g_assert_cmpint(res, ==, true); - visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "yes"); visit_type_bool(v, NULL, &res, &err); g_assert(!err); g_assert_cmpint(res, ==, true); - visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "on"); visit_type_bool(v, NULL, &res, &err); g_assert(!err); g_assert_cmpint(res, ==, true); - visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "false"); visit_type_bool(v, NULL, &res, &err); g_assert(!err); g_assert_cmpint(res, ==, false); - visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "no"); visit_type_bool(v, NULL, &res, &err); g_assert(!err); g_assert_cmpint(res, ==, false); - visitor_input_teardown(data, unused); v = visitor_input_test_init(data, "off"); @@ -190,8 +282,6 @@ static void test_visitor_in_enum(TestInputVisitorData *data, visit_type_EnumOne(v, NULL, &res, &err); g_assert(!err); g_assert_cmpint(i, ==, res); - - visitor_input_teardown(data, NULL); } } @@ -224,30 +314,24 @@ static void test_visitor_in_fuzz(TestInputVisitorData *data, v = visitor_input_test_init(data, buf); visit_type_int(v, NULL, &ires, NULL); - visitor_input_teardown(data, NULL); v = visitor_input_test_init(data, buf); visit_type_intList(v, NULL, &ilres, NULL); qapi_free_intList(ilres); - visitor_input_teardown(data, NULL); v = visitor_input_test_init(data, buf); visit_type_bool(v, NULL, &bres, NULL); - visitor_input_teardown(data, NULL); v = visitor_input_test_init(data, buf); visit_type_number(v, NULL, &nres, NULL); - visitor_input_teardown(data, NULL); v = visitor_input_test_init(data, buf); sres = NULL; visit_type_str(v, NULL, &sres, NULL); g_free(sres); - visitor_input_teardown(data, NULL); v = visitor_input_test_init(data, buf); visit_type_EnumOne(v, NULL, &eres, NULL); - visitor_input_teardown(data, NULL); } } diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c index 66b2b1c564..c7e64f022c 100644 --- a/tests/test-visitor-serialization.c +++ b/tests/test-visitor-serialization.c @@ -1040,7 +1040,7 @@ static void qmp_deserialize(void **native_out, void *datap, obj = qobject_from_json(qstring_get_str(output_json)); QDECREF(output_json); - d->qiv = qobject_input_visitor_new(obj, true); + d->qiv = qobject_input_visitor_new(obj); qobject_decref(obj_orig); qobject_decref(obj); visit(d->qiv, native_out, errp); diff --git a/trace-events b/trace-events index 728855743c..b07a09ba95 100644 --- a/trace-events +++ b/trace-events @@ -65,7 +65,6 @@ xen_remap_bucket(uint64_t index) "index %#"PRIx64 xen_map_cache_return(void* ptr) "%p" # monitor.c -handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\"" monitor_protocol_event_handler(uint32_t event, void *qdict) "event=%d data=%p" monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p" monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) "event=%d data=%p rate=%" PRId64 diff --git a/vl.c b/vl.c index 16a3b5ed8b..71b75ef8a0 100644 --- a/vl.c +++ b/vl.c @@ -2988,7 +2988,7 @@ int main(int argc, char **argv, char **envp) qemu_init_exec_dir(argv[0]); module_call_init(MODULE_INIT_QOM); - module_call_init(MODULE_INIT_QAPI); + monitor_init_qmp_commands(); qemu_add_opts(&qemu_drive_opts); qemu_add_drive_opts(&qemu_legacy_drive_opts); |