Re: [Qemu-devel] [PATCH for-2.12 3/8] monitor: new parameter "x-oob"

[Peter Xu <peterx@redhat.com>]

On Mon, Mar 26, 2018 at 11:10:30AM +0200, Marc-André Lureau wrote:

[...]

> > @@ -4568,12 +4569,26 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
> >  void monitor_init(Chardev *chr, int flags)
> >  {
> >      Monitor *mon = g_malloc(sizeof(*mon));
> > +    bool use_readline = flags & MONITOR_USE_READLINE;
> > +    bool use_oob = flags & MONITOR_USE_OOB;
> > +
> > +    if (use_oob) {
> > +        if (CHARDEV_IS_MUX(chr)) {
> > +            error_report("Monitor Out-Of-Band is not supported with "
> > +                         "MUX typed chardev backend");
> > +            exit(1);
> > +        }
> > +        if (use_readline) {
> > +            error_report("Monitor Out-Of-band is only supported by QMP");
> > +            exit(1);
> > +        }
> > +    }
> 
> I would rather see the error reporting / exit in vl.c:mon_init_func()
> function, to have a single place for exit()

But there can be other callers for monitor_init() so I would assume
checking it here would be safer (though for now indeed mon_init_func()
is the only one that can pass OOB flag in).

[...]

> Other than that, it looks fine.
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks for your quick reviews!

-- 
Peter Xu