All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Daniel P . Berrange" <berrange@redhat.com>,
	peterx@redhat.com, "Eric Blake" <eblake@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: [Qemu-devel] [PATCH 8/9] monitor: remove "x-oob", turn oob on by default
Date: Wed,  4 Jul 2018 16:45:06 +0800	[thread overview]
Message-ID: <20180704084507.14560-9-peterx@redhat.com> (raw)
In-Reply-To: <20180704084507.14560-1-peterx@redhat.com>

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 d6ab70cae2..0cb0538a31 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -13,7 +13,6 @@ extern Monitor *cur_mon;
 #define MONITOR_USE_READLINE  0x02
 #define MONITOR_USE_CONTROL   0x04
 #define MONITOR_USE_PRETTY    0x08
-#define MONITOR_USE_OOB       0x10
 
 bool monitor_cur_is_qmp(void);
 
diff --git a/monitor.c b/monitor.c
index 9b78cf1c63..9eb9f06599 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4626,19 +4626,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);
 
@@ -4735,9 +4728,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 098af6aec4..c5cb3f925c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -213,7 +213,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 57c1296573..033d610b0e 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -89,7 +89,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 29b1830259..e087cc1b72 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");
     chr = qemu_chr_find(chardev);
     if (chr == NULL) {
-- 
2.17.1

  parent reply	other threads:[~2018-07-04  8:45 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-04  8:44 [Qemu-devel] [PATCH 0/9] monitor: enable OOB by default Peter Xu
2018-07-04  8:44 ` [Qemu-devel] [PATCH 1/9] monitor: simplify monitor_qmp_setup_handlers_bh Peter Xu
2018-07-05  5:44   ` Markus Armbruster
2018-07-04  8:45 ` [Qemu-devel] [PATCH 2/9] qapi: allow build_params to return "void" Peter Xu
2018-07-05  6:02   ` Markus Armbruster
2018-07-05  6:18     ` Peter Xu
2018-07-04  8:45 ` [Qemu-devel] [PATCH 3/9] qapi: remove error checks for event emission Peter Xu
2018-07-05  8:43   ` Markus Armbruster
2018-07-05  9:17     ` Peter Xu
2018-07-04  8:45 ` [Qemu-devel] [PATCH 4/9] monitor: move need_resume flag into monitor struct Peter Xu
2018-07-05  8:51   ` Markus Armbruster
2018-07-05  9:49     ` Peter Xu
2018-07-05 11:09       ` Markus Armbruster
2018-07-05 11:32         ` Marc-André Lureau
2018-07-05 12:01           ` Peter Xu
2018-07-04  8:45 ` [Qemu-devel] [PATCH 5/9] monitor: suspend monitor instead of send CMD_DROP Peter Xu
2018-07-05 16:47   ` Eric Blake
2018-07-06  3:49     ` Peter Xu
2018-07-06  8:00       ` Markus Armbruster
2018-07-06  8:09   ` Markus Armbruster
2018-07-06  9:39     ` Peter Xu
2018-07-06 13:19       ` Markus Armbruster
2018-07-10  4:27         ` Peter Xu
2018-07-12  6:10           ` Markus Armbruster
2018-07-12 13:23             ` Markus Armbruster
2018-07-04  8:45 ` [Qemu-devel] [PATCH 6/9] qapi: remove COMMAND_DROPPED event Peter Xu
2018-07-04  8:45 ` [Qemu-devel] [PATCH 7/9] monitor: restrict response queue length too Peter Xu
2018-07-04  8:45 ` Peter Xu [this message]
2018-07-04  8:45 ` [Qemu-devel] [PATCH 9/9] Revert "tests: Add parameter to qtest_init_without_qmp_handshake" Peter Xu
2018-07-16 17:18 ` [Qemu-devel] [PATCH 0/9] monitor: enable OOB by default Markus Armbruster
2018-07-17 12:08   ` Peter Xu
2018-07-18  8:47     ` Peter Xu
2018-07-18 13:09       ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180704084507.14560-9-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=eblake@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.