On Mon, May 28, 2018 at 05:19:08PM +0200, Markus Armbruster wrote: [...]
> >> > >> > + * Meanwhile it can also be used even at the end of main. Let's keep > >> > + * it initialized for the whole lifecycle of QEMU. > >> > + */ > >> > >> Awkward question, since our main() is such a tangled mess, but here goes > >> anyway... The existing place to initialize monitor.c's globals is > >> monitor_init_globals(). But that one runs too late, I guess: > >> parse_add_fd() runs earlier, and calls monitor_fdset_add_fd(). Unclean > >> even without this lock; no module should be used before its > >> initialization function runs. Sure we can't run monitor_init_globals() > >> sufficiently early? > > > > Please see the comment for monitor_init_globals(): > > > > /* > > * Note: qtest_enabled() (which is used in monitor_qapi_event_init()) > > * depends on configure_accelerator() above. > > */ > > monitor_init_globals(); > > > > So I guess it won't work to directly move it earlier. The init > > dependency of QEMU is really complicated. I'll be fine now that we > > mark totally independent init functions (like this one, which is a > > mutex init only) as constructor, then we can save some time on > > ordering issue. > > Let me rephrase. There's a preexisting issue: main() calls monitor.c > functions before calling its initialization function > monitor_init_globals(). This needs to be cleaned up. Would you be > willing to do it? > > Possible solutions: > > * Perhaps we can move monitor_init_globals() up and/or the code calling > into monitor.c early down sufficiently. > > * Calculate event_clock_type on each use instead of ahead of time. It's > qtest_enabled ? QEMU_CLOCK_VIRTUAL : QEMU_CLOCK_REALTIME, and neither > of its users needs to be fast. Then move monitor_init_globals before > the code calling into monitor.c. Indeed. Obviously you thought a step further. :) > > I'm not opposed to use of constructors for self-contained initialization > (no calls to other modules). But I don't like initialization spread > over multiple functions. Since this work will actually decide where I should init this new fdset lock, so I'll try to do that altogether within the series. Thanks for your suggestions! It makes sense. -- Peter Xu
