Commit Graph

269 Commits

Author SHA1 Message Date
Paolo Bonzini
164dafd174 remove preconfig state
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>
2020-12-15 12:51:48 -05:00
Markus Armbruster
9e1b9c6c2d ui: Improve a client_migrate_info error message
client_migrate_info reports spice_server_migrate_connect() failure as
"An undefined error has occurred".  Improve to "Could not set up
display for migration".

QERR_UNDEFINED_ERROR is now unused.  Drop.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201113082626.2725812-6-armbru@redhat.com>
2020-12-10 17:16:44 +01:00
Markus Armbruster
9272186d3d ui: Improve some set_passwd, expire_password error messages
set_passwd and expire_password reject invalid "protocol" with "Invalid
parameter 'protocol'".  Misleading; the parameter is valid, its value
isn't.  Improve to "Parameter 'protocol' expects 'vnc' or 'spice'".

expire_password fails with "Could not set password".  Misleading;
improve to "Could not set password expire time".

QERR_SET_PASSWD_FAILED is now unused.  Drop.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201113082626.2725812-5-armbru@redhat.com>
2020-12-10 17:16:44 +01:00
Markus Armbruster
b0d7be2a62 block: Improve some block-commit, block-stream error messages
block-commit defaults @base-node to the deepest backing image.  When
there is none, it fails with "Base 'NULL' not found".  Improve to
"There is no backing image".

block-commit and block-stream reject a @base argument that doesn't
resolve with "Base 'BASE' not found".  Commit 6b33f3ae8b "qemu-img:
Improve commit invalid base message" improved this message in
qemu-img.  Improve it here, too: "Can't find '%s' in the backing
chain".

QERR_BASE_NOT_FOUND is now unused.  Drop.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201113082626.2725812-4-armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
2020-12-10 17:16:44 +01:00
Markus Armbruster
f820af8743 qerror: Eliminate QERR_ macros used in just one place
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201113082626.2725812-3-armbru@redhat.com>
2020-12-10 17:16:44 +01:00
Markus Armbruster
ac114d5cd5 qerror: Drop unused QERR_ macros
QERR_INVALID_BLOCK_FORMAT is dead since commit e6641719fe "block:
Always pass NULL as drv for bdrv_open()", 2015-09-14.

QERR_INVALID_PASSWORD is dead since commit c01c214b69 "block: remove
all encryption handling APIs", 2017-07-11.

Bury them.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201113082626.2725812-2-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-12-10 17:16:44 +01:00
Paolo Bonzini
372bcb2585 qapi, qemu-options: make all parsing visitors parse boolean options the same
OptsVisitor, StringInputVisitor and the keyval visitor have
three different ideas of how a human could write the value of
a boolean option.  Pay homage to the backwards-compatibility
gods and make the new common helper accept all four sets (on/off,
true/false, y/n and yes/no), but remove case-insensitivity.

Since OptsVisitor is supposed to match qemu-options, adjust
it as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201103161339.447118-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-11-04 12:00:40 -05:00
Eric Blake
9812e7125b qapi: Add QAPI_LIST_PREPEND() macro
block.c has a useful macro QAPI_LIST_ADD() for inserting at the front
of any QAPI-generated list; move it from block.c to qapi/util.h so
more places can use it, including one earlier place in block.c, and
rename it to something more obvious (since we also have a lot of
places that append, rather than prepend, to a list).

There are many more places in the codebase that can benefit from using
the macro, but converting them will be left to later patches.

In theory, all QAPI list types are child classes of GenericList; but
in practice, that relationship is not explicitly spelled out in the C
type declarations (rather, it is something that happens implicitly due
to C compatible layouts), and the macro does not actually depend on
the GenericList type.  We considered moving GenericList from visitor.h
into util.h to group related code; however, such a move would be
awkward if we do not also move GenericAlternate.  Unfortunately,
moving GenericAlternate would introduce its own problems of
declaration circularity (qapi-builtin-types.h needs a complete
definition of QEnumLookup from util.h, but GenericAlternate needs a
complete definition of QType from qapi-builtin-types.h).

Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201027050556.269064-3-eblake@redhat.com>
[eblake: s/ADD/PREPEND/ per suggestion by Markus]
2020-10-30 15:10:14 -05:00
Kevin Wolf
9ce44e2ce2 qmp: Move dispatcher to a coroutine
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>
2020-10-09 07:08:20 +02:00
Kevin Wolf
04f22362f1 qapi: Add a 'coroutine' flag for commands
This patch adds a new 'coroutine' flag to QMP command definitions that
tells the QMP dispatcher that the command handler is safe to be run in a
coroutine.

The documentation of the new flag pretends that this flag is already
used as intended, which it isn't yet after this patch. We'll implement
this in another patch in this series.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201005155855.256490-9-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>
2020-10-09 07:08:19 +02:00
Kevin Wolf
41725fa7ed qmp: Call monitor_set_cur() only in qmp_dispatch()
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>
2020-10-09 07:08:19 +02:00
Philippe Mathieu-Daudé
192cf54ac5 qapi/error: Check format string argument in error_*prepend()
error_propagate_prepend() "behaves like error_prepend()", and
error_prepend() uses "formatting @fmt, ... like printf()".
error_prepend() checks its format string argument, but
error_propagate_prepend() does not. Fix by addint the format
attribute to error_propagate_prepend() and error_vprepend().

This would have caught the bug fixed in the previous commit.

Missed in commit 4b5766488f "error: Fix use of error_prepend() with
&error_fatal, &error_abort".

Inspired-by: Stefan Weil <sw@weilnetz.de>
Suggested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200723171205.14949-1-philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-07-24 15:03:09 +02:00
Vladimir Sementsov-Ogievskiy
8220f3ac74 scripts: Coccinelle script to use ERRP_GUARD()
Script adds ERRP_GUARD() macro invocations where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)

Usage example:
spatch --sp-file scripts/coccinelle/errp-guard.cocci \
 --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
 --max-width 80 FILES...

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200707165037.1026246-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci]
2020-07-10 15:18:09 +02:00
Vladimir Sementsov-Ogievskiy
ae7c80a7bd error: New macro ERRP_GUARD()
Introduce a new ERRP_GUARD() macro, to be used at start of functions
with an errp OUT parameter.

It has three goals:

1. Fix issue with error_fatal and error_prepend/error_append_hint: the
user can't see this additional information, because exit() happens in
error_setg earlier than information is added. [Reported by Greg Kurz]

2. Fix issue with error_abort and error_propagate: when we wrap
error_abort by local_err+error_propagate, the resulting coredump will
refer to error_propagate and not to the place where error happened.
(the macro itself doesn't fix the issue, but it allows us to [3.] drop
the local_err+error_propagate pattern, which will definitely fix the
issue) [Reported by Kevin Wolf]

3. Drop local_err+error_propagate pattern, which is used to workaround
void functions with errp parameter, when caller wants to know resulting
status. (Note: actually these functions could be merely updated to
return int error code).

To achieve these goals, later patches will add invocations
of this macro at the start of functions with either use
error_prepend/error_append_hint (solving 1) or which use
local_err+error_propagate to check errors, switching those
functions to use *errp instead (solving 2 and 3).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Merge comments properly with recent commit "error: Document Error API
usage rules", and edit for clarity.  Put ERRP_AUTO_PROPAGATE() before
its helpers, and touch up style.  Tweak commit message.]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200707165037.1026246-2-armbru@redhat.com>
[Rename ERRP_AUTO_PROPAGATE() to ERRP_GUARD(), tweak commit message
again]
2020-07-10 15:18:09 +02:00
Markus Armbruster
012d4c96e2 qapi: Make visitor functions taking Error ** return bool, not void
See recent commit "error: Document Error API usage rules" for
rationale.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200707160613.848843-18-armbru@redhat.com>
2020-07-10 15:18:08 +02:00
Markus Armbruster
e3fe3988d7 error: Document Error API usage rules
This merely codifies existing practice, with one exception: the rule
advising against returning void, where existing practice is mixed.

When the Error API was created, we adopted the (unwritten) rule to
return void when the function returns no useful value on success,
unlike GError, which recommends to return true on success and false on
error then.

When a function returns a distinct error value, say false, a checked
call that passes the error up looks like

    if (!frobnicate(..., errp)) {
        handle the error...
    }

When it returns void, we need

    Error *err = NULL;

    frobnicate(..., &err);
    if (err) {
        handle the error...
        error_propagate(errp, err);
    }

Not only is this more verbose, it also creates an Error object even
when @errp is null, &error_abort or &error_fatal.

People got tired of the additional boilerplate, and started to ignore
the unwritten rule.  The result is confusion among developers about
the preferred usage.

Make the rule advising against returning void official by putting it
in writing.  This will hopefully reduce confusion.

Update the examples accordingly.

The remainder of this series will update a substantial amount of code
to honor the rule.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-4-armbru@redhat.com>
[Tweak prose as per advice from Eric]
2020-07-10 15:01:06 +02:00
Markus Armbruster
9aac7d486c error: Improve error.h's big comment
Add headlines to the big comment.

Explain examples for NULL, &error_abort and &error_fatal argument
better.

Tweak rationale for error_propagate_prepend().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200707160613.848843-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
2020-07-10 15:01:06 +02:00
Markus Armbruster
47ff5ac81e error: Fix examples in error.h's big comment
Mark a bad example more clearly.  Fix the error_propagate_prepend()
example.  Add a missing declaration and a second error pileup example.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200707160613.848843-2-armbru@redhat.com>
2020-07-10 15:01:06 +02:00
Markus Armbruster
1f5842487a qapi: Only input visitors can actually fail
The previous few commits have made this more obvious, and removed the
one exception.  Time to clarify the documentation, and drop dead error
checking.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-13-armbru@redhat.com>
2020-04-30 07:26:40 +02:00
Markus Armbruster
8b7ce95b46 qapi: Fix Visitor contract for start_alternate()
The contract demands v->start_alternate() for input and dealloc
visitors, but visit_start_alternate() actually requires it for input
and clone visitors.  Fix the contract, and delete superfluous
qapi_dealloc_start_alternate().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-8-armbru@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
8e08bf4ea2 qapi: Assert incomplete object occurs only in dealloc visitor
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-7-armbru@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
554d6586ae qapi: Polish prose in visitor.h
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-6-armbru@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
c5460d5e19 qapi: Document @errp usage more thoroughly in visitor.h
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-5-armbru@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
782586c771 qapi: Fix typo in visit_start_list()'s contract
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-4-armbru@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
294c90662a qapi: Fix the virtual walk example in visitor.h's big comment
Call visit_check_list().  Missed in commit a4a1c70dc7 "qapi: Make
input visitors detect unvisited list tails".

Drop an irrelevant error_propagate() while there.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-3-armbru@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
3777d36e67 qapi: Belatedly update visitor.h's big comment for QAPI modules
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200424084338.26803-2-armbru@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
7b1cd1c65a qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()
qdict_iter() has just three uses and no test coverage.  Replace by
qdict_first(), qdict_next() for more concise code and less type
punning.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200415083048.14339-5-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
2f2ec11179 qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead
qlist_iter() has just three uses outside tests/.  Replace by
QLIST_FOREACH_ENTRY() for more concise code and less type punning.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200415083048.14339-4-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30 06:51:15 +02:00
Markus Armbruster
64594e2bcc qobject: Clean up QLIST_FOREACH_ENTRY()
QLIST_FOREACH_ENTRY() traverses a tail queue manually.  Use
QTAILQ_FIRST() and QTAILQ_NEXT() instead.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200415083048.14339-2-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2020-04-30 06:51:15 +02:00
Marc-André Lureau
f0ccc00be1 qmp: constify QmpCommand and list
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>
2020-03-17 21:43:12 +01:00
Marc-André Lureau
164c374b75 qstring: add qstring_free()
Similar to g_string_free(), optionally return the underlying char*.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20200110153039.1379601-10-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-01-24 20:59:13 +01:00
Vladimir Sementsov-Ogievskiy
49fbc7236d error: make Error **errp const where it is appropriate
Mostly, Error ** is for returning error from the function, so the
callee sets it. However these three functions get already filled errp
parameter. They don't change the pointer itself, only change the
internal state of referenced Error object. So we can make it
Error *const * errp, to stress the behavior. It will also help
coccinelle script (in future) to distinguish such cases from common
errp usage.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-4-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message typo fixed]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2019-12-18 08:36:16 +01:00
Markus Armbruster
81b49004e0 qapi: Make visit_next_list()'s comment less confusing
visit_next_list() returns non-null on success, null on failure.  The
comment's phrasing "until NULL return or error occurs" is needlessly
confusing.  Scratch the "or error occurs" part.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190802122325.16520-1-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2019-09-24 11:03:44 +02:00
Markus Armbruster
abb3d37d0c qapi: Split error.json off common.json
In my "build everything" tree, changing a type in qapi/common.json
triggers a recompile of some 3600 out of 6600 objects (not counting
tests and objects that don't depend on qemu/osdep.h).

One common dependency is QapiErrorClass: it's used only in in
qapi/error.h, which uses nothing else, and is widely included.

Move QapiErrorClass from common.json to new error.json.  Touching
common.json now recompiles only some 2900 objects.

Cc: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190812052359.30071-4-armbru@redhat.com>
2019-08-16 13:31:51 +02:00
Marc-André Lureau
0b69f6f72c qapi: remove qmp_unregister_command()
This command is no longer needed, the schema has compile-time
configuration conditions.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190214152251.2073-16-armbru@redhat.com>
2019-02-18 14:44:05 +01:00
Markus Armbruster
a95291007b qapi: Eliminate indirection through qmp_event_get_func_emit()
The qapi_event_send_FOO() functions emit events like this:

    QMPEventFuncEmit emit;

    emit = qmp_event_get_func_emit();
    if (!emit) {
        return;
    }

    qmp = qmp_event_build_dict("FOO");
    [put event arguments into @qmp...]

    emit(QAPI_EVENT_FOO, qmp);

The value of qmp_event_get_func_emit() depends only on the program:

* In qemu-system-FOO, it's always monitor_qapi_event_queue.

* In tests/test-qmp-event, it's always event_test_emit.

* In all other programs, it's always null.

This is exactly the kind of dependence the linker is supposed to
resolve; we don't actually need an indirection.

Note that things would fall apart if we linked more than one QAPI
schema into a single program: each set of qapi_event_send_FOO() uses
its own event enumeration, yet they share a single emit function.
Which takes the event enumeration as an argument.  Which one if
there's more than one?

More seriously: how does this work even now?  qemu-system-FOO wants
QAPIEvent, and passes a function taking that to
qmp_event_set_func_emit().  test-qmp-event wants test_QAPIEvent, and
passes a function taking that to qmp_event_set_func_emit().

It works by type trickery, of course:

    typedef void (*QMPEventFuncEmit)(unsigned event, QDict *dict);

    void qmp_event_set_func_emit(QMPEventFuncEmit emit);

    QMPEventFuncEmit qmp_event_get_func_emit(void);

We use unsigned instead of the enumeration type.  Relies on both
enumerations boiling down to unsigned, which happens to be true for
the compilers we use.

Clean this up as follows:

* Generate qapi_event_send_FOO() that call PREFIX_qapi_event_emit()
  instead of the value of qmp_event_set_func_emit().

* Generate a prototype for PREFIX_qapi_event_emit() into
  qapi-events.h.

* PREFIX_ is empty for qapi/qapi-schema.json, and test_ for
  tests/qapi-schema/qapi-schema-test.json.  It's qga_ for
  qga/qapi-schema.json, and doc-good- for
  tests/qapi-schema/doc-good.json, but those don't define any events.

* Rename monitor_qapi_event_queue() to qapi_event_emit() instead of
  passing it to qmp_event_set_func_emit().  This takes care of
  qemu-system-FOO.

* Rename event_test_emit() to test_qapi_event_emit() instead of
  passing it to qmp_event_set_func_emit().  This takes care of
  tests/test-qmp-event.

* Add a qapi_event_emit() that does nothing to stubs/monitor.c.  This
  takes care of all other programs that link code emitting QMP events.

* Drop qmp_event_set_func_emit(), qmp_event_get_func_emit().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20181218182234.28876-3-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[Commit message typos fixed]
2019-01-24 10:01:05 +01:00
David Hildenbrand
c9fba9de89 qapi: Rewrite string-input-visitor's integer and list parsing
The input visitor has some problems right now, especially
- unsigned type "Range" is used to process signed ranges, resulting in
  inconsistent behavior and ugly/magical code
- uint64_t are parsed like int64_t, so big uint64_t values are not
  supported and error messages are misleading
- lists/ranges of int64_t are accepted although no list is parsed and
  we should rather report an error
- lists/ranges are preparsed using int64_t, making it hard to
  implement uint64_t values or uint64_t lists
- types that don't support lists don't bail out
- visiting beyond the end of a list is not handled properly
- we don't actually parse lists, we parse *sets*: members are sorted,
  and duplicates eliminated

So let's rewrite it by getting rid of usage of the type "Range" and
properly supporting lists of int64_t and uint64_t (including ranges of
both types), fixing the above mentioned issues.

Lists of other types are not supported and will properly report an
error. Virtual walks are now supported.

Tests have to be fixed up:
- Two BUGs were hardcoded that are fixed now
- The string-input-visitor now actually returns a parsed list and not
  an ordered set.

Please note that no users/callers have to be fixed up. Candidates using
visit_type_uint16List() and friends are:
- backends/hostmem.c:host_memory_backend_set_host_nodes()
-- Code can deal with duplicates/unsorted lists
- numa.c::query_memdev()
-- via object_property_get_uint16List(), the list will still be sorted
   and without duplicates (via host_memory_backend_get_host_nodes())
- qapi-visit.c::visit_type_Memdev_members()
- qapi-visit.c::visit_type_NumaNodeOptions_members()
- qapi-visit.c::visit_type_RockerOfDpaGroup_members
- qapi-visit.c::visit_type_RxFilterInfo_members()
-- Not used with string-input-visitor.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20181121164421.20780-7-david@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2018-12-13 19:10:06 +01:00
Markus Armbruster
4b5766488f error: Fix use of error_prepend() with &error_fatal, &error_abort
From include/qapi/error.h:

  * Pass an existing error to the caller with the message modified:
  *     error_propagate(errp, err);
  *     error_prepend(errp, "Could not frobnicate '%s': ", name);

Fei Li pointed out that doing error_propagate() first doesn't work
well when @errp is &error_fatal or &error_abort: the error_prepend()
is never reached.

Since I doubt fixing the documentation will stop people from getting
it wrong, introduce error_propagate_prepend(), in the hope that it
lures people away from using its constituents in the wrong order.
Update the instructions in error.h accordingly.

Convert existing error_prepend() next to error_propagate to
error_propagate_prepend().  If any of these get reached with
&error_fatal or &error_abort, the error messages improve.  I didn't
check whether that's the case anywhere.

Cc: Fei Li <fli@suse.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20181017082702.5581-2-armbru@redhat.com>
2018-10-19 14:51:34 +02:00
Alberto Garcia
4c9ab1e693 scripts: Remove check-qerror.sh
qerror.h contains leftovers from the now-defunct QError API.

There's only a handful of string macros left, and no one is supposed
to add anything else. The check-qerror.sh script was used to make sure
that all definitions on the qerror.c and qerror.h files were sorted
alphabetically. The former was removed three years ago, and the latter
is now in a different location, so the script doesn't even work (as
a matter of fact the alphabetical order was broken last time someone
added a macro -also in 2015- and no one seemed to notice).

There's no point in fixing this script so let's just remove it.
The rogue macro is also moved to its correct location.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20181017151738.20299-1-berto@igalia.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2018-10-19 14:40:15 +02:00
Peter Maydell
efd1d5229f Monitor patches for 2018-09-01
-----BEGIN PGP SIGNATURE-----
 
 iQIcBAABAgAGBQJbinLMAAoJEDhwtADrkYZTtBgQAIAa1Z6KUOjWWxzewpowl9E5
 5gaRTAv02LPlOg0Xg0QtYDch+JpeLhdtMQEtLCIiHWRdj84b//NkpbTf72rwn72G
 1BY/3DjLtf3eYuYrdwF3Qb5WTPXAwzVfYt0lQYrFZl/71qpvXPdTh5K0jRAXLfMm
 +NkbA22jhg4mz83fan+AygdoPjidpjYZIpv0Kac9h67TLEP+eKcRBVFadozqskvW
 aFEX/5PGO/tDV7g+0lVx1AYzbPcmCE+ItP+egOKhVxZKZhX1bw3nFLc3I9u4ieI0
 fXDJVY811tQoF2t+01sFVwPX/tDtmOqXBZpivX7OorA5JXdTcqyS8ZgPrmU+OVWI
 58vUKJ4F+EzXYg9/lyMwWRTuqKQpHUuZEUQYr5Yr1lRz+umWyVKHhRgMlvyNRnUL
 DHEmcCBlO0WkhbbfqNPB7H9rPbvsaPKqTMGfAUxOWiaFHxRrCSXJDd0z168yYtw2
 raLk+hqaek3yvbbeo9puTSI93YzmdvywqsVoVQDLlyyICtwK/WRJp50JFQv6tp6E
 TkIevT/E4ba+YGowvm0jCCSxv6WKXMZUQgxzrFCpyhDzf3lEgXJS7dYYvkubsfBZ
 kK3zOWCdMCIHZiqtd10LAUQ9Rj4k42WjYgVs2aWP4caWisoCEIqlNzeMK0OXScbK
 hEgmZfawMd2a0sR7kIdI
 =n9J0
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/armbru/tags/pull-monitor-2018-09-01' into staging

Monitor patches for 2018-09-01

# gpg: Signature made Sat 01 Sep 2018 12:06:52 BST
# gpg:                using RSA key 3870B400EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* remotes/armbru/tags/pull-monitor-2018-09-01:
  monitor: no need to save need_resume
  Revert "qmp: isolate responses into io thread"
  qmp: constify qmp_is_oob()
  monitor: consitify qmp_send_response() QDict argument
  monitor: accept input on resume
  monitor: simplify monitor_qmp_setup_handlers_bh

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2018-09-24 15:43:33 +01:00
Marc-André Lureau
2aa788f5cb qmp: constify qmp_is_oob()
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180829134043.31706-3-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2018-08-30 16:08:47 +02:00
Peter Xu
3ab72385b2 qapi: Drop qapi_event_send_FOO()'s Error ** argument
The generated qapi_event_send_FOO() take an Error ** argument.  They
can't actually fail, because all they do with the argument is passing it
to functions that can't fail: the QObject output visitor, and the
@qmp_emit callback, which is either monitor_qapi_event_queue() or
event_test_emit().

Drop the argument, and pass &error_abort to the QObject output visitor
and @qmp_emit instead.

Suggested-by: Eric Blake <eblake@redhat.com>
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180815133747.25032-4-peterx@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message rewritten, update to qapi-code-gen.txt corrected]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2018-08-28 18:21:38 +02:00
Markus Armbruster
37aded92c2 json: Update references to RFC 7159 to RFC 8259
RFC 8259 (December 2017) obsoletes RFC 7159 (March 2014).

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180823164025.12553-59-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
2018-08-24 20:27:14 +02:00
Markus Armbruster
86cdf9ec8d json: Clean up headers
The JSON parser has three public headers, json-lexer.h, json-parser.h,
json-streamer.h.  They all contain stuff that is of no interest
outside qobject/json-*.c.

Collect the public interface in include/qapi/qmp/json-parser.h, and
everything else in qobject/json-parser-int.h.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-54-armbru@redhat.com>
2018-08-24 20:26:37 +02:00
Markus Armbruster
812ce33ead qobject: Drop superfluous includes of qemu-common.h
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-53-armbru@redhat.com>
2018-08-24 20:26:37 +02:00
Markus Armbruster
abe7c2067c json: Make JSONToken opaque outside json-parser.c
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-52-armbru@redhat.com>
2018-08-24 20:26:37 +02:00
Markus Armbruster
a2731e08ee json: Unbox tokens queue in JSONMessageParser
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-51-armbru@redhat.com>
2018-08-24 20:26:37 +02:00
Markus Armbruster
f9277915ee json: Fix streamer not to ignore trailing unterminated structures
json_message_process_token() 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 json_parser_parse().  If a non-empty sequence of tokens remains at
the end of the parse, it's silently ignored.  check-qjson.c cases
unterminated_array(), unterminated_array_comma(), unterminated_dict(),
unterminated_dict_comma() demonstrate this bug.

Fix as follows.  Introduce a JSON_END_OF_INPUT token.  When the
streamer receives it, it feeds the accumulated tokens to
json_parser_parse().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-46-armbru@redhat.com>
2018-08-24 20:26:37 +02:00
Markus Armbruster
84a56f38b2 json: Pass lexical errors and limit violations to callback
The callback to consume JSON values takes QObject *json, Error *err.
If both are null, the callback is supposed to make up an error by
itself.  This sucks.

qjson.c's consume_json() neglects to do so, which makes
qobject_from_json() null instead of failing.  I consider that a bug.

The culprit is json_message_process_token(): it passes two null
pointers when it runs into a lexical error or a limit violation.  Fix
it to pass a proper Error object then.  Update the callbacks:

* monitor.c's handle_qmp_command(): the code to make up an error is
  now dead, drop it.

* qga/main.c's process_event(): lumps the "both null" case together
  with the "not a JSON object" case.  The former is now gone.  The
  error message "Invalid JSON syntax" is misleading for the latter.
  Improve it to "Input must be a JSON object".

* qobject/qjson.c's consume_json(): no update; check-qjson
  demonstrates qobject_from_json() now sets an error on lexical
  errors, but still doesn't on some other errors.

* tests/libqtest.c's qmp_response(): the Error object is now reliable,
  so use it to improve the error message.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-40-armbru@redhat.com>
2018-08-24 20:26:37 +02:00
Markus Armbruster
2cbd15aa6f json: Treat unwanted interpolation as lexical error
The JSON parser optionally supports interpolation.  The lexer
recognizes interpolation tokens unconditionally.  The parser rejects
them when interpolation is disabled, in parse_interpolation().
However, it neglects to set an error then, which can make
json_parser_parse() fail without setting an error.

Move the check for unwanted interpolation from the parser's
parse_interpolation() into the lexer's finite state machine.  When
interpolation is disabled, '%' is now handled like any other
unexpected character.

The next commit will improve how such lexical errors are handled.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-39-armbru@redhat.com>
2018-08-24 20:26:37 +02:00