Re: [Qemu-devel] [RFC v6 15/27] monitor: let suspend/resume work even with QMPs

[Stefan Hajnoczi]

On Mon, Dec 25, 2017 at 11:26:13AM +0800, Peter Xu wrote:
> On Thu, Dec 21, 2017 at 07:27:38PM +0800, Fam Zheng wrote:
> > On Tue, 12/19 16:45, Peter Xu wrote:
> > > One thing to mention is that for QMPs that are using IOThreads, we need
> > > an explicit kick for the IOThread in case it is sleeping.
> > > 
> > > Since at it, add traces for the operations.
> > > 
> > > Signed-off-by: Peter Xu <address@hidden>
> > > ---
> > >  monitor.c    | 26 +++++++++++++++++++++-----
> > >  trace-events |  1 +
> > >  2 files changed, 22 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/monitor.c b/monitor.c
> > > index 844508d134..5f05f2e9da 100644
> > > --- a/monitor.c
> > > +++ b/monitor.c
> > > @@ -3992,19 +3992,35 @@ static void monitor_command_cb(void *opaque, 
> > > const char *cmdline,
> > >  
> > >  int monitor_suspend(Monitor *mon)
> > >  {
> > > -    if (!mon->rs)
> > > -        return -ENOTTY;
> > 
> > Please add to the commit message why the mon->rs check is dropped, for this 
> > and
> > the other one.
> 
> I thought it would be clear enough since mon->rs is only used by HMP
> and the subject tells us that this patch is adding support for QMP.
> But... sure I can add one more sentence for that!

This change breaks hmp.c:hmp_migrate():

  if (monitor_suspend(mon) < 0) {
      monitor_printf(mon, "terminal does not allow synchronous "
                     "migration, continuing detached\n");
      return;
  }

mon->rs is used by HMP code to determine whether this is an interactive
terminal.  Both live migration and password prompting rely on this
because they must be forbidden when doing so would be impossible (e.g.
GDB stub tunneling HMP commands).

Suspending the QMP monitor is useful, but please don't break the HMP
code.

Stefan

signature.asc
Description: PGP signature