Commit Graph

5 Commits

Author SHA1 Message Date
Markus Armbruster
5d75648b56 qapi: Generate QAPIEvent stuff into separate files
Having to include qapi-events.h just for QAPIEvent is suboptimal, but
quite tolerable now.  It'll become problematic when we have events
conditional on the target, because then qapi-events.h won't be usable
from target-independent code anymore.  Avoid that by generating it
into separate files.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20190214152251.2073-6-armbru@redhat.com>
2019-02-18 14:44:04 +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
Peter Xu
62aa1d887f monitor: Fix unsafe sharing of @cur_mon among threads
@cur_mon is null unless the main thread is running monitor code, either
HMP code within monitor_read(), or QMP code within
monitor_qmp_dispatch().

Use of @cur_mon outside the main thread is therefore unsafe.

Most of its uses are in monitor command handlers.  These run in the main
thread.

However, there are also uses hiding elsewhere, such as in
error_vprintf(), and thus error_report(), making these functions unsafe
outside the main thread.  No such unsafe uses are known at this time.
Regardless, this is an unnecessary trap.  It's an ancient trap, though.

More recently, commit cf869d5317 "qmp: support out-of-band (oob)
execution" spiced things up: the monitor I/O thread assigns to @cur_mon
when executing commands out-of-band.  Having two threads save, set and
restore @cur_mon without synchronization is definitely unsafe.  We can
end up with @cur_mon null while the main thread runs monitor code, or
non-null while it runs non-monitor code.

We could fix this by making the I/O thread not mess with @cur_mon, but
that would leave the trap armed and ready.

Instead, make @cur_mon thread-local.  It's now reliably null unless the
thread is running monitor code.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[peterx: update subject and commit message written by Markus]
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180720033451.32710-1-peterx@redhat.com>
2018-07-23 14:00:03 +02:00
Marc-André Lureau
0ec7b3e7f2 char: rename CharDriverState Chardev
Pick a uniform chardev type name.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-01-27 18:07:59 +01:00
Paolo Bonzini
869e9aecbe stubs: merge all monitor stubs in one file, remove monitor_cur_is_qmp stub
monitor_cur_is_qmp was previously used by other stubs, but it's not
since 397d30e ("qemu-error: remove dependency of stubs on monitor",
2016-11-01).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-01-16 17:52:35 +01:00