On 03/26/2018 04:10 AM, Marc-André Lureau wrote:
Hi
On Mon, Mar 26, 2018 at 8:38 AM, Peter Xu <pet...@redhat.com> wrote:
Add new parameter to optionally enable Out-Of-Band for a QMP server.
An example command line:
./qemu-system-x86_64 -chardev stdio,id=char0 \
-mon chardev=char0,mode=control,x-oob=on
By default, Out-Of-Band is off.
It is not allowed if either MUX or non-QMP is detected, since
Out-Of-Band is currently only for QMP, and non-MUX chardev backends.
Worth documenting in the commit message at least that even when OOB is
enabled, the client must STILL opt-in to using it by replying correctly
to qmp_capabilities, as well as mention that in the future, we may
remove x-oob and rely on JUST qmp_capabilities for enabling OOB.
@@ -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);
Should these two checks be swapped? Otherwise, if you use a MUX-typed
chardev for HMP, the message implies that switching chardev backend
might make it work, even though if you actually do that you'd then get
the failure for not being QMP.
+ }
+ }
I would rather see the error reporting / exit in vl.c:mon_init_func()
function, to have a single place for exit()
To do that, monitor_init() should change signatures to take Error
**errp. Probably worth doing if you spin a v2 of this series (adding
the parameter can be done as a separate patch, although there are only 5
callers in the tree so adjusting the callers at the same time is
probably not that hard to review).
Other than that, it looks fine.
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
Okay, I'll see how my review goes on the rest of the series before
deciding whether to request a v2.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
