[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 07/15] monitor: unify global init
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [RFC 07/15] monitor: unify global init |
Date: |
Tue, 19 Sep 2017 16:35:28 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 09/14/2017 02:50 AM, Peter Xu wrote:
> There are many places for monitor init its globals, at least:
>
> - monitor_init_qmp_commands() at the very beginning
> - single function to init monitor_lock
> - in the first entry of monitor_init() using "is_first_init"
>
> Unify them a bit.
>
> Signed-off-by: Peter Xu <address@hidden>
> ---
> include/monitor/monitor.h | 2 +-
> monitor.c | 25 ++++++++++---------------
> vl.c | 3 ++-
> 3 files changed, 13 insertions(+), 17 deletions(-)
>
>
> +void monitor_init_globals(void)
> +{
> + monitor_init_qmp_commands();
> + monitor_qapi_event_init();
> + sortcmdlist();
> + qemu_mutex_init(&monitor_lock);
> +}
Are we sure that this new function is called sooner than any access to
monitor_lock,
> -static void __attribute__((constructor)) monitor_lock_init(void)
> -{
> - qemu_mutex_init(&monitor_lock);
> -}
especially since the old code initialized the lock REALLY early?
> diff --git a/vl.c b/vl.c
> index fb1f05b..850cf55 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3049,7 +3049,6 @@ int main(int argc, char **argv, char **envp)
> qemu_init_exec_dir(argv[0]);
>
> module_call_init(MODULE_INIT_QOM);
> - monitor_init_qmp_commands();
>
> qemu_add_opts(&qemu_drive_opts);
> qemu_add_drive_opts(&qemu_legacy_drive_opts);
> @@ -4587,6 +4586,8 @@ int main(int argc, char **argv, char **envp)
>
> parse_numa_opts(current_machine);
>
> + monitor_init_globals();
> +
Pre-patch, a breakpoint on main() and on monitor_lock_init() fires on
monitor_lock_init() first, prior to main.
Breakpoint 2, monitor_lock_init () at /home/eblake/qemu/monitor.c:4089
4089 qemu_mutex_init(&monitor_lock);
(gdb) c
Continuing.
[New Thread 0x7fffce225700 (LWP 26380)]
Thread 1 "qemu-system-x86" hit Breakpoint 1, main (argc=5,
argv=0x7fffffffdc88, envp=0x7fffffffdcb8) at vl.c:3077
3077 {
Post-patch, the mutex is not initialized until well after main(). So
the real question is what (if anything) is using the lock in between
those two points?
Hmm - it may be that we needed it back before commit 05875687, when we
really did depend on MODULE_INIT_QAPI, but it is something we forgot to
cleanup in the meantime?
If nothing else, the commit message should call out that dropping
__attribute__((constructor)) nonsense is intentional (if it was indeed
nonsense).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [RFC 02/15] qobject: allow NULL for qstring_get_str(), (continued)
- [Qemu-devel] [RFC 03/15] qobject: introduce qobject_to_str(), Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 04/15] monitor: move skip_flush into monitor_data_init, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 05/15] qjson: add "opaque" field to JSONMessageParser, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 06/15] monitor: move the cur_mon hack deeper for QMP, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 07/15] monitor: unify global init, Peter Xu, 2017/09/14
- Re: [Qemu-devel] [RFC 07/15] monitor: unify global init,
Eric Blake <=
- [Qemu-devel] [RFC 08/15] monitor: create IO thread, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 09/15] monitor: allow to use IO thread for parsing, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 10/15] monitor: introduce monitor_qmp_respond(), Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 11/15] monitor: separate QMP parser and dispatcher, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 12/15] monitor: enable IO thread for (qmp & !mux) typed, Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 13/15] qapi: introduce new cmd option "allow-oob", Peter Xu, 2017/09/14
- [Qemu-devel] [RFC 14/15] qmp: support out-of-band (oob) execution, Peter Xu, 2017/09/14