[Qemu-devel] [PULL 09/11] monitor: Remove "x-oob", offer capability "oob" unconditionally

[Markus Armbruster]

From: Peter Xu <address@hidden>

Out-of-band command execution was introduced in commit cf869d53172.
Unfortunately, we ran into a regression, and had to turn it into an
experimental option for 2.12 (commit be933ffc23).

  http://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06231.html

The regression has since been fixed (commit 951702f39c7 "monitor: bind
dispatch bh to iohandler context").  A thorough re-review of OOB
commands led to a few more issues, which have also been addressed.

This patch partly reverts be933ffc23 (monitor: new parameter "x-oob"),
and makes QMP monitors again offer capability "oob" whenever they can
provide it, i.e. when the monitor's character device is capable of
running in an I/O thread.

Some trivial touch-up in the test code is required to make sure qmp-test
won't break.

Reviewed-by: Markus Armbruster <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Signed-off-by: Peter Xu <address@hidden>
Message-Id: <address@hidden>
[Conflict with "monitor: check if chardev can switch gcontext for OOB"
resolved, commit message updated]
Signed-off-by: Markus Armbruster <address@hidden>
---
 include/monitor/monitor.h |  1 -
 monitor.c                 | 23 +++++------------------
 tests/libqtest.c          |  2 +-
 tests/qmp-test.c          |  2 +-
 vl.c                      |  5 -----
 5 files changed, 7 insertions(+), 26 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 0c0a37d8cb..c1b40a9cac 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -13,7 +13,6 @@ extern __thread Monitor *cur_mon;
 #define MONITOR_USE_READLINE  0x02
 #define MONITOR_USE_CONTROL   0x04
 #define MONITOR_USE_PRETTY    0x08
-#define MONITOR_USE_OOB       0x10
 
 #define QMP_REQ_QUEUE_LEN_MAX 8
 
diff --git a/monitor.c b/monitor.c
index d2529260ed..6e81b09294 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4567,22 +4567,12 @@ 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 (!qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)) {
-            error_report("Monitor out-of-band is not supported with "
-                         "%s typed chardev backend",
-                         object_get_typename(OBJECT(chr)));
-            exit(1);
-        }
-        if (use_readline) {
-            error_report("Monitor out-of-band is only supported by QMP");
-            exit(1);
-        }
-    }
-
-    monitor_data_init(mon, false, use_oob);
+    /* Note: we run QMP monitor in I/O thread when @chr supports that */
+    monitor_data_init(mon, false,
+                      (flags & MONITOR_USE_CONTROL)
+                      && qemu_chr_has_feature(chr,
+                                              QEMU_CHAR_FEATURE_GCONTEXT));
 
     qemu_chr_fe_init(&mon->chr, chr, &error_abort);
     mon->flags = flags;
@@ -4677,9 +4667,6 @@ QemuOptsList qemu_mon_opts = {
         },{
             .name = "pretty",
             .type = QEMU_OPT_BOOL,
-        },{
-            .name = "x-oob",
-            .type = QEMU_OPT_BOOL,
         },
         { /* end of list */ }
     },
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 75e07e16e7..225a123649 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -224,7 +224,7 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob,
                               "-display none "
                               "%s", qemu_binary, socket_path,
                               getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
-                              qmp_socket_path, use_oob ? ",x-oob=on" : "",
+                              qmp_socket_path, "",
                               extra_args ?: "");
 
     g_test_message("starting QEMU: %s", command);
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 7517be4654..5ba22af6a0 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -116,7 +116,7 @@ static void test_qmp_protocol(void)
     g_assert(q);
     test_version(qdict_get(q, "version"));
     capabilities = qdict_get_qlist(q, "capabilities");
-    g_assert(capabilities && qlist_empty(capabilities));
+    g_assert(capabilities);
     qobject_unref(resp);
 
     /* Test valid command before handshake */
diff --git a/vl.c b/vl.c
index a5ae5f23d2..2a8b2ee16d 100644
--- a/vl.c
+++ b/vl.c
@@ -2322,11 +2322,6 @@ static int mon_init_func(void *opaque, QemuOpts *opts, 
Error **errp)
     if (qemu_opt_get_bool(opts, "pretty", 0))
         flags |= MONITOR_USE_PRETTY;
 
-    /* OOB is off by default */
-    if (qemu_opt_get_bool(opts, "x-oob", 0)) {
-        flags |= MONITOR_USE_OOB;
-    }
-
     chardev = qemu_opt_get(opts, "chardev");
     if (!chardev) {
         error_report("chardev is required");
-- 
2.17.2