qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).