[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier
From: |
Wolfgang Bumiller |
Subject: |
Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier |
Date: |
Thu, 20 Sep 2018 10:02:22 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Thu, Sep 20, 2018 at 10:59:56AM +0800, Peter Xu wrote:
> On Wed, Sep 19, 2018 at 04:58:06PM +0200, Wolfgang Bumiller wrote:
> > On Wed, Sep 19, 2018 at 03:36:02PM +0200, Markus Armbruster wrote:
> > > Peter Xu <address@hidden> writes:
> > > > Indeed. So we have these ordering constraints:
> > > >
> > > > init mon locks [1]
> > > > cli parsing
> > > > daemonize
> > > > create mon iothread [2]
> > > >
> > > > I can't figure out a way unless we split the global init routine for
> > > > the monitors, possibly (and simply) by postpone creation of the
> > > > iothread. Thoughts?
> > >
> > > Instead of creating @mon_iothread eagerly in monitor_init_globals(), we
> > > could perhaps create it lazily when we create the first monitor with
> > > mon->use_io_thread.
> >
> > Sounds reasonable. I'm about to head out, but glancing through the code
> > just now I noticed a minor discrepancy we should either document or
> > fixup:
> > Looking at monitor_suspend() and monitor_resume(), the suspend()
> > function calls aio_notify() on the mon_iothread's context if the monitor
> > flags contains MONITOR_USE_CONTROL, but mon->use_io_thread is equivalent
> > to having MONITOR_USE_OOB in the flags. The resume() function
> > additionally guards that same call with a mon->use_io_thread check.
> > I wonder if the difference is there on purpose, but I doubt it.
>
> Yes I don't think there's a purpose of it, though I would suspect that
> Markus would even like that use_io_thread check to go away at least in
> the past (to avoid different code path for oob and non-oob).
>
> But if we're going to create the iothread when referencing it the
> first time then we possibly want the check in both suspend and resume
> paths so that we won't need to create it when not needed.
>
> > Every
> > other access seems to be guarded by mon->use_io_thread.
> > (Finally, monitor_cleanup() would need to not call iothread_stop/destroy
> > if mon_iothread is NULL)
> >
>
> Wolfgang, do you wanna post a patch for it?
Can do.
After taking another look, btw., I'm wondering whether it would make
sense to move the monitor_init_globals() call to after os_daemonize().
Between its current new place and os_daemonize() isn't much, the big
part is mostly the argument handling which seems to mostly parse argv[]
into QemuOptsList, which is consumed only after daemonization.
There's also bdrv_init_with_whitelist() which calls all the block_init()
functions which should only call bdrv_register.
Either way, spawning the iothread on demand can still make sense, as
does updating the check in resume()/suspend().
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Wolfgang Bumiller, 2018/09/05
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Peter Xu, 2018/09/05
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Markus Armbruster, 2018/09/19
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Wolfgang Bumiller, 2018/09/19
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Peter Xu, 2018/09/19
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier,
Wolfgang Bumiller <=
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Peter Xu, 2018/09/20
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Wolfgang Bumiller, 2018/09/20
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Markus Armbruster, 2018/09/20
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Wolfgang Bumiller, 2018/09/20
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Markus Armbruster, 2018/09/20
- Re: [Qemu-devel] [PULL 6/7] monitor: move init global earlier, Wolfgang Bumiller, 2018/09/21