Commit 8258292e authored by Peter Xu's avatar Peter Xu Committed by Markus Armbruster
Browse files

monitor: Remove "x-oob", offer capability "oob" unconditionally 

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

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



The regression has since been fixed (commit 951702f3 "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 be933ffc (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: default avatarMarkus Armbruster <armbru@redhat.com>
Reviewed-by: default avatarMarc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: default avatarPeter Xu <peterx@redhat.com>
Message-Id: <20181009062718.1914-4-peterx@redhat.com>
[Conflict with "monitor: check if chardev can switch gcontext for OOB"
resolved, commit message updated]
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
parent 9ab84470

include/monitor/monitor.h

+0 −1
Original line number Diff line number Diff line
@@ -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

monitor.c

+5 −18
Original line number Diff line number Diff line
@@ -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 */ }

    },

tests/libqtest.c

+1 −1
Original line number Diff line number Diff line
@@ -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);

tests/qmp-test.c

+1 −1
Original line number Diff line number Diff line
@@ -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 */

vl.c

+0 −5
Original line number Diff line number Diff line
@@ -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");