Policy "crash" calls abort() when deprecated input is received.
Bugs in integration tests may mask the error from policy "reject".
Provide a larger hammer: crash outright. Masking that seems unlikely.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210318155519.1224118-12-armbru@redhat.com>
This policy rejects deprecated input, and thus permits "testing the
future". Implement it for QMP command arguments: reject commands with
deprecated ones. Example: when QEMU is run with -compat
deprecated-input=reject, then
{"execute": "eject", "arguments": {"device": "cd"}}
fails like this
{"error": {"class": "GenericError", "desc": "Deprecated parameter 'device' disabled by policy"}}
When the deprecated parameter is removed, the error will change to
{"error": {"class": "GenericError", "desc": "Parameter 'device' is unexpected"}}
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210318155519.1224118-11-armbru@redhat.com>
This policy rejects deprecated input, and thus permits "testing the
future". Implement it for QMP commands: make deprecated ones fail.
Example: when QEMU is run with -compat deprecated-input=reject, then
{"execute": "query-cpus"}
fails like this
{"error": {"class": "CommandNotFound", "desc": "Deprecated command query-cpus disabled by policy"}}
When the deprecated command is removed, the error will change to
{"error": {"class": "CommandNotFound", "desc": "The command query-cpus has not been found"}}
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210318155519.1224118-10-armbru@redhat.com>
This policy suppresses deprecated bits in output, and thus permits
"testing the future". Implement it for QMP command results. Example:
when QEMU is run with -compat deprecated-output=hide, then
{"execute": "query-cpus-fast"}
yields
{"return": [{"thread-id": 9805, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
instead of
{"return": [{"arch": "x86", "thread-id": 22436, "props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "qom-path": "/machine/unattached/device[0]", "cpu-index": 0, "target": "x86_64"}]}
Note the suppression of deprecated member "arch".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210318155519.1224118-4-armbru@redhat.com>
New option -compat lets you configure what to do when deprecated
interfaces get used. This is intended for testing users of the
management interfaces. It is experimental.
-compat deprecated-input=<input-policy> configures what to do when
deprecated input is received. Input policy can be "accept" (accept
silently), or "reject" (reject the request with an error).
-compat deprecated-output=<out-policy> configures what to do when
deprecated output is sent. Output policy can be "accept" (pass on
unchanged), or "hide" (filter out the deprecated parts).
Default is "accept". Policies other than "accept" are implemented
later in this series.
For now, -compat covers only syntactic aspects of QMP, i.e. stuff
tagged with feature 'deprecated'. We may want to extend it to cover
semantic aspects, CLI, and experimental features.
Note that there is no good way for management application to detect
presence of -compat: it's not visible output of query-qmp-schema or
query-command-line-options. Tolerable, because it's meant for
testing. If running with -compat fails, skip the test.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210318155519.1224118-3-armbru@redhat.com>
qmp_disable_command() now takes an optional error string to return a
more explicit error message.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1928806
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
*fix up 80+ char line
Signed-off-by: Michael Roth <michael.roth@amd.com>
The preconfig state is only used if -incoming is not specified, which
makes the RunState state machine more tricky than it need be. However
there is already an equivalent condition which works even with -incoming,
namely qdev_hotplug. Use it instead of a separate runstate.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This moves the QMP dispatcher to a coroutine and runs all QMP command
handlers that declare 'coroutine': true in coroutine context so they
can avoid blocking the main loop while doing I/O or waiting for other
events.
For commands that are not declared safe to run in a coroutine, the
dispatcher drops out of coroutine context by calling the QMP command
handler from a bottom half.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201005155855.256490-10-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This way, a monitor command handler will still be able to access the
current monitor, but when it yields, all other code code will correctly
get NULL from monitor_cur().
This uses a hash table to map the coroutine pointer to the current
monitor of that coroutine. Outside of coroutine context, we associate
the current monitor with the leader coroutine of the current thread.
Approaches to implement some form of coroutine local storage directly in
the coroutine core code have been considered and discarded because they
didn't end up being much more generic than the hash table and their
performance impact on coroutines not using coroutine local storage was
unclear. As the block layer uses a coroutine per I/O request, this is a
fast path and we have to be careful. It's safest to just stay out of
this path with code only used by the monitor.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20201005155855.256490-8-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The correct way to set the current monitor for a coroutine handler will
be different than for a blocking handler, so monitor_set_cur() needs to
be called in qmp_dispatch().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201005155855.256490-7-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Direct leak of 4120 byte(s) in 1 object(s) allocated from:
#0 0x7fa114931887 in __interceptor_calloc (/lib64/libasan.so.6+0xb0887)
#1 0x7fa1144ad8f0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x588f0)
#2 0x561e3c9c8897 in qmp_object_add /home/elmarco/src/qemu/qom/qom-qmp-cmds.c:291
#3 0x561e3cf48736 in qmp_dispatch /home/elmarco/src/qemu/qapi/qmp-dispatch.c:155
#4 0x561e3c8efb36 in monitor_qmp_dispatch /home/elmarco/src/qemu/monitor/qmp.c:145
#5 0x561e3c8f09ed in monitor_qmp_bh_dispatcher /home/elmarco/src/qemu/monitor/qmp.c:234
#6 0x561e3d08c993 in aio_bh_call /home/elmarco/src/qemu/util/async.c:136
#7 0x561e3d08d0a5 in aio_bh_poll /home/elmarco/src/qemu/util/async.c:164
#8 0x561e3d0a535a in aio_dispatch /home/elmarco/src/qemu/util/aio-posix.c:380
#9 0x561e3d08e3ca in aio_ctx_dispatch /home/elmarco/src/qemu/util/async.c:298
#10 0x7fa1144a776e in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x5276e)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200325184723.2029630-3-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Since 0b69f6f72c "qapi: remove
qmp_unregister_command()", the command list can be declared const.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Message-Id: <20200316171824.2319695-1-marcandre.lureau@redhat.com>
[Rebased]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200317115459.31821-25-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
We convert the request object to a QDict twice: first in
qmp_dispatch() to get the request ID, and then again in
qmp_dispatch_check_obj(), which converts to QDict, then checks and
returns it. We can't get the request ID from the latter, because it's
null when the qdict flunks the checks.
Move the checked conversion to QDict from qmp_dispatch_check_obj() to
qmp_dispatch(), and drop the duplicate there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200317115459.31821-24-armbru@redhat.com>
Both functions check @request is a QDict, and both have code for
QCO_NO_SUCCESS_RESP. This wasn't the case back when they were
created. It's a sign of muddled responsibilities. Inline. The next
commits will clean up some more.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200317115459.31821-22-armbru@redhat.com>
If a command is disabled an error is reported. But due to usage of
error_setg() the class of the error is GenericError which does not
help callers in distinguishing this case from a case where a qmp
command fails regularly due to other reasons.
We used to use class CommandDisabled until the great error
simplification (commit de253f1491 for QMP and commit 93b91c59db for
qemu-ga, both v1.2.0).
Use CommandNotFound error class, which is close enough.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Message-Id: <faeb030e6a1044f0fd88208edfdb1c5fafe5def9.1567171655.git.mprivozn@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Test update squashed in, commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
sysemu/sysemu.h is a rather unfocused dumping ground for stuff related
to the system-emulator. Evidence:
* It's included widely: in my "build everything" tree, changing
sysemu/sysemu.h still triggers a recompile of some 1100 out of 6600
objects (not counting tests and objects that don't depend on
qemu/osdep.h, down from 5400 due to the previous two commits).
* It pulls in more than a dozen additional headers.
Split stuff related to run state management into its own header
sysemu/runstate.h.
Touching sysemu/sysemu.h now recompiles some 850 objects. qemu/uuid.h
also drops from 1100 to 850, and qapi/qapi-types-run-state.h from 4400
to 4200. Touching new sysemu/runstate.h recompiles some 500 objects.
Since I'm touching MAINTAINERS to add sysemu/runstate.h anyway, also
add qemu/main-loop.h.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190812052359.30071-30-armbru@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
[Unbreak OS-X build]
There are no harm but just looks weird to return bool in
pointer-returning function. Introduced in 69240fe62d with the whole
failure-checking "if" chunk.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20190325154748.66381-1-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Let qmp_dispatch() copy the 'id' field. That way any qmp client will
conform to the specification, including QGA. Furthermore, it
simplifies the work for qemu monitor.
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
The classical way to structure parser and lexer is to have the client
call the parser to get an abstract syntax tree, the parser call the
lexer to get the next token, and the lexer call some function to get
input characters.
Another way to structure them would be to have the client feed
characters to the lexer, the lexer feed tokens to the parser, and the
parser feed abstract syntax trees to some callback provided by the
client. This way is more easily integrated into an event loop that
dispatches input characters as they arrive.
Our JSON parser is kind of between the two. The lexer feeds tokens to
a "streamer" instead of a real parser. The streamer accumulates
tokens until it got the sequence of tokens that comprise a single JSON
value (it counts curly braces and square brackets to decide). It
feeds those token sequences to a callback provided by the client. The
callback passes each token sequence to the parser, and gets back an
abstract syntax tree.
I figure it was done that way to make a straightforward recursive
descent parser possible. "Get next token" becomes "pop the first
token off the token sequence". Drawback: we need to store a complete
token sequence. Each token eats 13 + input characters + malloc
overhead bytes.
Observations:
1. This is not the only way to use recursive descent. If we replaced
"get next token" by a coroutine yield, we could do without a
streamer.
2. The lexer reports errors by passing a JSON_ERROR token to the
streamer. This communicates the offending input characters and
their location, but no more.
3. The streamer reports errors by passing a null token sequence to the
callback. The (already poor) lexical error information is thrown
away.
4. Having the callback receive a token sequence duplicates the code to
convert token sequence to abstract syntax tree in every callback.
5. Known bug: the streamer silently drops incomplete token sequences.
This commit rectifies 4. by lifting the call of the parser from the
callbacks into the streamer. Later commits will address 3. and 5.
The lifting removes a bug from qjson.c's parse_json(): it passed a
pointer to a non-null Error * in certain cases, as demonstrated by
check-qjson.c.
json_parser_parse() is now unused. It's a stupid wrapper around
json_parser_parse_err(). Drop it, and rename json_parser_parse_err()
to json_parser_parse().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-35-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-28-armbru@redhat.com>
By using the more specific type, we get fewer downcasts. The
downcasts are safe, but not obviously so, at least not locally.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-24-armbru@redhat.com>
All callers of qmp_build_error_object() duplicate the code to wrap it
in a response object. Replace it by qmp_error_response() that
captures the duplicated code, including error_free().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-23-armbru@redhat.com>
handle_qmp_command() reports certain errors right away. This is wrong
when OOB is enabled, because the errors can "jump the queue" then, as
the previous commit demonstrates.
To fix, we need to delay errors until dispatch. Do that for semantic
errors, mostly by reverting ill-advised parts of commit cf869d5317
"qmp: support out-of-band (oob) execution". Bonus: doesn't run
qmp_dispatch_check_obj() twice, once in handle_qmp_command(), and
again in do_qmp_dispatch(). That's also due to commit cf869d5317.
The next commit will fix queue jumping for syntax errors.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-18-armbru@redhat.com>
Commit cf869d5317 "qmp: support out-of-band (oob) execution" added a
general mechanism for command-independent arguments just for an
out-of-band flag:
The "control" key is introduced to store this extra flag. "control"
field is used to store arguments that are shared by all the commands,
rather than command specific arguments. Let "run-oob" be the first.
However, it failed to reject unknown members of "control". For
instance, in QMP command
{"execute": "query-name", "id": 42, "control": {"crap": true}}
"crap" gets silently ignored.
Instead of fixing this, revert the general "control" mechanism
(because YAGNI), and do it the way I initially proposed, with key
"exec-oob". Simpler code, simpler interface.
An out-of-band command
{"execute": "migrate-pause", "id": 42, "control": {"run-oob": true}}
becomes
{"exec-oob": "migrate-pause", "id": 42}
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-13-armbru@redhat.com>
[Commit message typo fixed]
Commit cf869d5317 "qmp: support out-of-band (oob) execution"
accidentally made qemu-ga accept and ignore "control". Fix that.
Out-of-band execution in a monitor that doesn't support it now fails
with
{"error": {"class": "GenericError", "desc": "QMP input member 'control' is unexpected"}}
instead of
{"error": {"class": "GenericError", "desc": "Please enable out-of-band first for the session during capabilities negotiation"}}
The old description is suboptimal when out-of-band cannot not be
enabled, or the command doesn't support out-of-band execution.
The new description is a bit unspecific, but it'll do.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-12-armbru@redhat.com>
Commit cf869d5317 "qmp: support out-of-band (oob) execution" changed
how we check "id":
Note that in the patch I exported qmp_dispatch_check_obj() to be
used to check the request earlier, and at the same time allowed
"id" field to be there since actually we always allow that.
The part after "and" is ill-advised: it makes qemu-ga accept and
ignore "id". Revert.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-10-armbru@redhat.com>
This option allows pausing QEMU in the new RUN_STATE_PRECONFIG state,
allowing the configuration of QEMU from QMP before the machine jumps
into board initialization code of machine_run_board_init()
The intent is to allow management to query machine state and additionally
configure it using previous query results within one QEMU instance
(i.e. eliminate the need to start QEMU twice, 1st to query board specific
parameters and 2nd for actual VM start using query results for
additional parameters).
The new option complements -S option and could be used with or without
it. The difference is that -S pauses QEMU when the machine is completely
initialized with all devices wired up and ready to execute guest code
(QEMU needs only to unpause VCPUs to let guest execute its code),
while the "preconfig" option pauses QEMU early before board specific init
callback (machine_run_board_init) is executed and allows the configuration
of machine parameters which will be used by board init code.
When early introspection/configuration is done, command 'exit-preconfig'
should be used to exit RUN_STATE_PRECONFIG and transition to the next
requested state (i.e. if -S is used then QEMU will pause the second
time when board/device initialization is completed or start guest
execution if -S isn't provided on CLI)
PS:
Initially 'preconfig' is planned to be used for configuring numa
topology depending on board specified possible cpus layout.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1526059483-42847-1-git-send-email-imammedo@redhat.com>
[ehabkost: Changed "since 2.13" to "since 3.0"]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Now that we can safely call QOBJECT() on QObject * as well as its
subtypes, we can have macros qobject_ref() / qobject_unref() that work
everywhere instead of having to use QINCREF() / QDECREF() for QObject
and qobject_incref() / qobject_decref() for its subtypes.
The replacement is mechanical, except I broke a long line, and added a
cast in monitor_qmp_cleanup_req_queue_locked(). Unlike
qobject_decref(), qobject_unref() doesn't accept void *.
Note that the new macros evaluate their argument exactly once, thus no
need to shout them.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180419150145.24795-4-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Rebased, semantic conflict resolved, commit message improved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Having "allow-oob":true for a command does not mean that this command
will always be run in out-of-band mode. The out-of-band quick path will
only be executed if we specify the extra "run-oob" flag when sending the
QMP request:
{ "execute": "command-that-allows-oob",
"arguments": { ... },
"control": { "run-oob": true } }
The "control" key is introduced to store this extra flag. "control"
field is used to store arguments that are shared by all the commands,
rather than command specific arguments. Let "run-oob" be the first.
Note that in the patch I exported qmp_dispatch_check_obj() to be used to
check the request earlier, and at the same time allowed "id" field to be
there since actually we always allow that.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-19-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase to qobject_to(), spelling fix]
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch was generated using the following Coccinelle script:
@@
expression Obj;
@@
(
- qobject_to_qnum(Obj)
+ qobject_to(QNum, Obj)
|
- qobject_to_qstring(Obj)
+ qobject_to(QString, Obj)
|
- qobject_to_qdict(Obj)
+ qobject_to(QDict, Obj)
|
- qobject_to_qlist(Obj)
+ qobject_to(QList, Obj)
|
- qobject_to_qbool(Obj)
+ qobject_to(QBool, Obj)
)
and a bit of manual fix-up for overly long lines and three places in
tests/check-qjson.c that Coccinelle did not find.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20180224154033.29559-4-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: swap order from qobject_to(o, X), rebase to master, also a fix
to latent false-positive compiler complaint about hw/i386/acpi-build.c]
Signed-off-by: Eric Blake <eblake@redhat.com>
This cleanup makes the number of objects depending on qapi/qmp/qdict.h
drop from 4550 (out of 4743) to 368 in my "build everything" tree.
For qapi/qmp/qobject.h, the number drops from 4552 to 390.
While there, separate #include from file comment with a blank line.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180201111846.21846-13-armbru@redhat.com>
qapi/qmp/types.h is a convenience header to include a number of
qapi/qmp/ headers. Since we rarely need all of the headers
qapi/qmp/types.h includes, we bypass it most of the time. Most of the
places that use it don't need all the headers, either.
Include the necessary headers directly, and drop qapi/qmp/types.h.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180201111846.21846-9-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180201111846.21846-6-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180201111846.21846-4-armbru@redhat.com>
The QERR_ macros are leftovers from the days of "rich" error objects.
QERR_QMP_BAD_INPUT_OBJECT, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
QERR_QMP_EXTRA_MEMBER are used in just one place now, except for one
use that has crept into qobject-input-visitor.c.
Drop these macros, to make the (bad) error messages more visible.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-10-git-send-email-armbru@redhat.com>
qmp_check_input_obj() duplicates qmp_dispatch_check_obj(), except the
latter screws up an error message. handle_qmp_command() runs first
the former, then the latter via qmp_dispatch(), masking the screwup.
qemu-ga also masks the screwup, because it also duplicates checks,
just differently.
qmp_check_input_obj() exists because handle_qmp_command() needs to
examine the command before dispatching it. The previous commit got
rid of this need, except for a tracepoint, and a bit of "id" code that
relies on qdict not being null.
Fix up the error message in qmp_dispatch_check_obj(), drop
qmp_check_input_obj() and the tracepoint. Protect the "id" code with
a conditional.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-9-git-send-email-armbru@redhat.com>
The command registry encapsulates a single command list. Give the
functions using it a parameter instead. Define suitable command lists
in monitor, guest agent and test-qmp-commands.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1488544368-30622-6-git-send-email-armbru@redhat.com>
[Debugging turds buried]
Reviewed-by: Eric Blake <eblake@redhat.com>
The value of key 'arguments' must be a JSON object. qemu-ga neglects
to check, and crashes. To reproduce, send
{ 'execute': 'guest-sync', 'arguments': [] }
to qemu-ga.
do_qmp_dispatch() uses qdict_get_qdict() to get the arguments. When
not a JSON object, this gets a null pointer, which flows through the
generated marshalling function to qobject_input_visitor_new(), where
it fails the assertion. qmp_dispatch_check_obj() needs to catch this
error.
QEMU isn't affected, because it runs qmp_check_input_obj() first,
which basically duplicates qmp_dispatch_check_obj()'s checks, plus the
missing one.
Fix by copying the missing one from qmp_check_input_obj() to
qmp_dispatch_check_obj().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1488544368-30622-2-git-send-email-armbru@redhat.com>
qobject_to_qdict(obj) returns NULL when obj isn't a QDict. Check
that instead of qobject_type(obj) == QTYPE_QDICT.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1487363905-9480-8-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
'qjson.h' is not a QObject subtype; include this file directly in
.c files that are using it, rather than abusing qmp/types.h for
that purpose.
Meanwhile, for files that include a list of individual QObject
subtypes, it's easier to just use qmp/types.h for that purpose.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-2-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Ever since QMP was first added back in commit 43c20a43, we have
never had any QmpCommandType other than QCT_NORMAL. It's
pointless to carry around the cruft.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1461879932-9020-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef. Since then, we've moved to include qemu/osdep.h
everywhere. Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h. That's in excess of
100KiB of crap most .c files don't actually need.
Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h. Include qapi/error.h in .c files that need it and don't
get it now. Include qapi-types.h in qom/object.h for uint16List.
Update scripts/clean-includes accordingly. Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h
comment quoted above similarly.
This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third. Unfortunately, the number depending on
qapi-types.h shrinks only a little. More work is needed for that one.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1454089805-5470-8-git-send-email-peter.maydell@linaro.org
The qapi enum ErrorClass is unusual that it uses 'CamelCase' names,
contrary to our documented convention of preferring 'lower-case'.
However, this enum is entrenched in the API; we cannot change
what strings QMP outputs. Meanwhile, we want to simplify how
c_enum_const() is used to generate enum constants, by moving away
from the heuristics of camel_to_upper() to a more straightforward
c_name(N).upper() - but doing so will rename all of the ErrorClass
constants and cause churn to all client files, where the new names
are aesthetically less pleasing (ERROR_CLASS_DEVICENOTFOUND looks
like we can't make up our minds on whether to break between words).
So as always in computer science, solve the problem by some more
indirection: rename the qapi type to QapiErrorClass, and add a
new enum ErrorClass in error.h whose members are aliases of the
qapi type, but with the spelling expected elsewhere in the tree.
Then, when c_enum_const() changes the munging, we only have to
adjust the one alias spot.
Suggested by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1447836791-369-26-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
These macros expand into error class enumeration constant, comma,
string. Unclean. Has been that way since commit 13f59ae.
The error class is always ERROR_CLASS_GENERIC_ERROR since the previous
commit.
Clean up as follows:
* Prepend every use of a QERR_ macro by ERROR_CLASS_GENERIC_ERROR, and
delete it from the QERR_ macro. No change after preprocessing.
* Rewrite error_set(ERROR_CLASS_GENERIC_ERROR, ...) into
error_setg(...). Again, no change after preprocessing.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
