OOB commands were introduced in commit cf869d53172.  Unfortunately, we
ran into a regression, and had to disable them by default 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").  Time to re-enable OOB.

This patch partly reverts be933ffc23 (monitor: new parameter "x-oob"),
and turns it on again for non-MUX QMPs.  Note that we can't enable
Out-Of-Band for monitors with MUX-typed chardev backends, because not
all the chardev frontends can run without main thread, or can run in
multiple threads.

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

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/monitor/monitor.h |  1 -
 monitor.c                 | 22 ++++++----------------
 tests/libqtest.c          |  2 +-
 tests/qmp-test.c          |  2 +-
 vl.c                      |  5 -----
 5 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 76873c0d40..281480a1e5 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 c2c9853f75..a465b69c62 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4524,19 +4524,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 (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);
-        }
-    }
+    /*
+     * Note: we can't enable Out-Of-Band for monitors with MUX-typed
+     * chardev backends, because not all the chardev frontends can run
+     * without main thread, or can run in multiple threads.
+     */
+    bool use_oob = (flags & MONITOR_USE_CONTROL) && !CHARDEV_IS_MUX(chr);
 
     monitor_data_init(mon, false, use_oob);
 
@@ -4626,9 +4619,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 d635c5bea0..ebd92f22f6 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -231,7 +231,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 ?: "");
         execlp("/bin/sh", "sh", "-c", command, NULL);
         exit(1);
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 4ae2245484..5302bd07b9 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -143,7 +143,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 10dd690e30..0206f0c512 100644
--- a/vl.c
+++ b/vl.c
@@ -2323,11 +2323,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");
     chr = qemu_chr_find(chardev);
     if (chr == NULL) {
-- 
2.17.1

Hi

On Wed, Sep 5, 2018 at 10:29 AM Peter Xu <peterx@redhat.com> wrote:
>
> OOB commands were introduced in commit cf869d53172.  Unfortunately, we
> ran into a regression, and had to disable them by default 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").  Time to re-enable OOB.
>
> This patch partly reverts be933ffc23 (monitor: new parameter "x-oob"),
> and turns it on again for non-MUX QMPs.  Note that we can't enable
> Out-Of-Band for monitors with MUX-typed chardev backends, because not
> all the chardev frontends can run without main thread, or can run in
> multiple threads.
>
> Some trivial touch-up in the test code is required to make sure qmp-test
> won't break.
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  include/monitor/monitor.h |  1 -
>  monitor.c                 | 22 ++++++----------------
>  tests/libqtest.c          |  2 +-
>  tests/qmp-test.c          |  2 +-
>  vl.c                      |  5 -----
>  5 files changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 76873c0d40..281480a1e5 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 c2c9853f75..a465b69c62 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4524,19 +4524,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 (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);
> -        }
> -    }

much nicer without exit() :)

> +    /*
> +     * Note: we can't enable Out-Of-Band for monitors with MUX-typed
> +     * chardev backends, because not all the chardev frontends can run
> +     * without main thread, or can run in multiple threads.
> +     */
> +    bool use_oob = (flags & MONITOR_USE_CONTROL) && !CHARDEV_IS_MUX(chr);
>
>      monitor_data_init(mon, false, use_oob);
>
> @@ -4626,9 +4619,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 d635c5bea0..ebd92f22f6 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -231,7 +231,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 ?: "");
>          execlp("/bin/sh", "sh", "-c", command, NULL);
>          exit(1);
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index 4ae2245484..5302bd07b9 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -143,7 +143,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 10dd690e30..0206f0c512 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2323,11 +2323,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");
>      chr = qemu_chr_find(chardev);
>      if (chr == NULL) {
> --
> 2.17.1
>
>


-- 
Marc-André Lureau