Re: [Qemu-devel] chardev's and fd's in monitors

[Marc-André Lureau]

Hi

On Thu, Oct 20, 2016 at 11:38 AM Daniel P. Berrange <address@hidden>
wrote:

> On Wed, Oct 19, 2016 at 07:06:16PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (address@hidden) wrote:
> > > On Wed, Oct 19, 2016 at 02:16:05PM +0200, Markus Armbruster wrote:
> > > > "Daniel P. Berrange" <address@hidden> writes:
> > > >
> > > > > On Wed, Oct 19, 2016 at 11:05:53AM +0100, Dr. David Alan Gilbert
> wrote:
> > > > >>
> > > > >> We need a way to be able to report an error without plumbing
> error_setg
> > > > >> up the stack; if you're saying error_report isn't suitable then we
> > > > >> should just recommend we switch everything in migration back to
> > > > >> fprintf(stderr,
> > > >
> > > > In the cases where error_report() isn't suitable, fprintf() is just
> as
> > > > unsuitable for the exact same reasons.
> > > >
> > > > > Well both error_report() + fprintf  are broken from POV of anything
> > > > > using QMP. error_report() is slightly less broken for HMP,
> > > >
> > > > error_report() is not broken at all for HMP code.  The trouble is
> code
> > > > that can't know whether it's running in a context where
> error_report()
> > > > is suitable.
> > > >
> > > > >                                                            but
> doesn't
> > > > > help QMP.
> > > >
> > > > Correct.
> > > >
> > > > > In the short term we should just make error_report be  threadsafe
> in
> > > > > its usage of the monitor.
> > > >
> > > > Any problems left once cur_mon is thread-local (which it should be
> > > > anyway)?
> > >
> > > If we make cur_mon a thread-local, then error_report() is equivalent
> > > to fprintf(stderr) for the migration code, since the migration
> > > code runs in a different thread thread, and so would always see
> > > cur_mon == NULL.
> >
> > Yes, that would become safe; it does sound the best fix for the current
> > worry.
> >
> > If we had that, then why not wire up error_report to pass errors back to
> QMP
> > as well?
>
> You have a problem of context - if you have multiple monitors, how do
> you know which to send the error back to if you're not in the event
> loop thread, and thus cur_mon is NULL. With Marc-Andre's series which
> allows proper async command processing it gets even harder, because
> there's potentially many outstanding commands associated with a monitor
> and you need to decide which the error should be given to.
>

I would think the contrary, it gets potentially easier as it may actually
keep the associated monitor (the one that triggered the command) with the
'async' context.

-- 
Marc-André Lureau