Re: [Qemu-devel] [RFC v2 2/8] monitor: allow monitor to create thread to poll

[Dr. David Alan Gilbert]

* Peter Xu (address@hidden) wrote:
> On Fri, Aug 25, 2017 at 10:30:42AM +0100, Dr. David Alan Gilbert wrote:
> 
> [...]
> 
> > > >   c) As mentioned on irc there's fun to be had with cur_mon and error
> > > >      handling - in my local world I have cur_mon declared as __thread
> > > >      but never got around to thinking aobut what should set it up.
> > > >      There's also 'wavcapture: Convert to error_report' that I posted
> > > >      in March that got rid of some uses of cur_mon in wavcapture.c
> > > >      for error_report.
> > > 
> > > Yeh.  I at least also see a positive ACK from Markus in the other
> > > thread for per-thread cur_mon, sounds like this is the right way to
> > > go.
> > > 
> > > To setup cur_mon, what I can think of is create wrapper for
> > > pthread_create() in qemu_thread_create().  I see that we have done
> > > similar thing in util/qemu-thread-win32.c for Windows.  With that we
> > > can setup the cur_mon before going into real thread function but in
> > > the right context, though we may need one more parameter for current
> > > qemu_thread_create():
> > > 
> > > void qemu_thread_create(QemuThread *thread, const char *name,
> > >                        void *(*start_routine)(void*),
> > >                        void *arg, int mode, Monitor *mon);
> > > 
> > > Then we can specify monitor for any new thread (default to cur_mon).
> > > For per-monitor threads, I think we need to pass in that specific mon.
> > > 
> > > Is this doable?
> > 
> > That would mean changing all the qemu_thread_create calls, but yes
> > I guess is doable.  I'd thought the other way, perhaps you inherit
> > Monitor except in the case of when the monitor creates threads.
> 
> Do you mean setup cur_mon in monitor threads?
> 
> I'm afraid that may not be enough, since after we mark cur_mon as
> __thread variable, it should be NULL for each newly created threads,
> then we need to init them for every thread.  Or anything I missed?

Right, I thought you'd modify qemu_thread_create to setup cur_mon in
new threads to the same as the parent.

Dave

> [...]
> 
> > > > 
> > > >   d) I wonder if it's better to have thread as a flag, so that you have
> > > >      to explicitly ask for a monitor to have it's own thread.
> > > 
> > > This should be doable.  Would a new parameter for "-qmp" and "-hmp"
> > > suffice?
> > 
> > Yes.
> 
> (I meant "-monitor" when saying "-hmp")
> 
> Hmm, it seems not easy to simply add a new parameter for it, since we
> used "," already to parse the chardev params in monitor codes, like
> the usage of:
> 
>   -qmp telnet::8888,server,nowait
> 
> So I cannot simply do:
> 
>   -qmp telnet::8888,server,nowait,threaded=on
> 
> Or it will be treated for a parameter for chardev type "telnet".
> 
> I can at least add something similar to QEMU_OPTION_qmp_pretty, like:
> QEMU_OPTION_qmp_threaded, and maybe I also need
> QEMU_OPTION_monitor_pretty.  But I am thinking whether there can be
> anything better.  Any suggestion from anyone?
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK