All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: "Daniel P. Berrange" <berrange@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] chardev's and fd's in monitors
Date: Thu, 20 Oct 2016 08:53:23 +0000	[thread overview]
Message-ID: <CAJ+F1CLMQVWZ+=a7zv_YMPOdKR93nGJdypnYn_tRDwq10zCv+w@mail.gmail.com> (raw)
In-Reply-To: <20161020083730.GC12145@redhat.com>

Hi

On Thu, Oct 20, 2016 at 11:38 AM Daniel P. Berrange <berrange@redhat.com>
wrote:

> On Wed, Oct 19, 2016 at 07:06:16PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrange (berrange@redhat.com) wrote:
> > > On Wed, Oct 19, 2016 at 02:16:05PM +0200, Markus Armbruster wrote:
> > > > "Daniel P. Berrange" <berrange@redhat.com> 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

  reply	other threads:[~2016-10-20  8:53 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-12 19:15 [Qemu-devel] chardev's and fd's in monitors Dr. David Alan Gilbert
2016-10-12 20:02 ` Marc-André Lureau
2016-10-13 15:47   ` Dr. David Alan Gilbert
2016-10-18 10:04 ` Daniel P. Berrange
2016-10-18 11:32   ` Dr. David Alan Gilbert
2016-10-18 11:41     ` Marc-André Lureau
2016-10-18 11:44       ` Marc-André Lureau
2016-10-18 12:01     ` Daniel P. Berrange
2016-10-18 13:25       ` Dr. David Alan Gilbert
2016-10-18 13:35         ` Daniel P. Berrange
2016-10-18 13:52           ` Dr. David Alan Gilbert
2016-10-18 14:01             ` Daniel P. Berrange
2016-10-18 18:53               ` Dr. David Alan Gilbert
2016-10-19  7:45                 ` Daniel P. Berrange
2016-10-19  8:00               ` Markus Armbruster
2016-10-19  8:12                 ` Dr. David Alan Gilbert
2016-10-19  8:42                   ` Daniel P. Berrange
2016-10-19  9:48                     ` Markus Armbruster
2016-10-19 10:05                       ` Dr. David Alan Gilbert
2016-10-19 10:16                         ` Daniel P. Berrange
2016-10-19 12:16                           ` Markus Armbruster
2016-10-19 12:21                             ` Daniel P. Berrange
2016-10-19 18:06                               ` Dr. David Alan Gilbert
2016-10-20  8:37                                 ` Daniel P. Berrange
2016-10-20  8:53                                   ` Marc-André Lureau [this message]
2016-10-20 10:45                                     ` Markus Armbruster
2016-10-20 16:56                                   ` Paolo Bonzini
2016-10-21  9:12                                     ` Markus Armbruster
2016-10-21 21:06                                       ` Paolo Bonzini
2016-10-24  7:07                                         ` Markus Armbruster
2016-10-21  9:35                                     ` Daniel P. Berrange
2016-10-20 16:59                                   ` Dr. David Alan Gilbert
2016-10-20  8:55                                 ` Markus Armbruster
2016-10-20  9:03                                   ` Daniel P. Berrange
2016-10-20  9:58                                     ` Dr. David Alan Gilbert
2016-10-20 10:42                                       ` Markus Armbruster
2016-10-20 11:01                                         ` Dr. David Alan Gilbert
2016-10-20 11:10                                           ` Daniel P. Berrange
2016-10-20 11:45                                             ` Markus Armbruster
2016-10-20 11:08                                         ` Daniel P. Berrange
2016-10-20 11:57                                           ` Markus Armbruster
2016-10-20 17:56                                             ` Dr. David Alan Gilbert
2016-10-21  9:06                                               ` Markus Armbruster
2016-10-21  9:37                                                 ` Daniel P. Berrange
2016-10-21 11:56                                                   ` Dr. David Alan Gilbert
2016-10-21  9:45                                                 ` Dr. David Alan Gilbert
2016-10-19 12:26               ` Paolo Bonzini
2016-10-19 17:01                 ` Dr. David Alan Gilbert
2016-10-19 20:51                   ` Paolo Bonzini
2016-10-20  8:34                     ` Daniel P. Berrange
2016-10-18 12:08   ` Markus Armbruster
2016-10-18 12:13     ` Daniel P. Berrange
2016-10-18 12:43       ` Dr. David Alan Gilbert
2016-10-18 10:06 ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJ+F1CLMQVWZ+=a7zv_YMPOdKR93nGJdypnYn_tRDwq10zCv+w@mail.gmail.com' \
    --to=marcandre.lureau@gmail.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.