Commit 4eaca8de authored by Marc-André Lureau's avatar Marc-André Lureau Committed by Michael Roth
Browse files

qmp: common 'id' handling & make QGA conform to QMP spec 



Let qmp_dispatch() copy the 'id' field. That way any qmp client will
conform to the specification, including QGA. Furthermore, it
simplifies the work for qemu monitor.

CC: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: default avatarMarc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMichael Roth <mdroth@linux.vnet.ibm.com>
parent 781f2b3d

monitor.c

+12 −21
Original line number Diff line number Diff line
@@ -250,8 +250,6 @@ QEMUBH *qmp_dispatcher_bh; 
struct QMPRequest {

    /* Owner of the request */

    Monitor *mon;

    /* "id" field of the request */

    QObject *id;

    /*

     * Request object to be handled or Error to be reported

     * (exactly one of them is non-null)
@@ -353,7 +351,6 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func, 


static void qmp_request_free(QMPRequest *req)

{

    qobject_unref(req->id);

    qobject_unref(req->req);

    error_free(req->err);

    g_free(req);
@@ -4108,18 +4105,14 @@ static int monitor_can_read(void *opaque) 
 * Null @rsp can only happen for commands with QCO_NO_SUCCESS_RESP.

 * Nothing is emitted then.

 */

static void monitor_qmp_respond(Monitor *mon, QDict *rsp, QObject *id)

static void monitor_qmp_respond(Monitor *mon, QDict *rsp)

{

    if (rsp) {

        if (id) {

            qdict_put_obj(rsp, "id", qobject_ref(id));

        }



        qmp_send_response(mon, rsp);

    }

}



static void monitor_qmp_dispatch(Monitor *mon, QObject *req, QObject *id)

static void monitor_qmp_dispatch(Monitor *mon, QObject *req)

{

    Monitor *old_mon;

    QDict *rsp;
@@ -4144,7 +4137,7 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject *req, QObject *id) 
        }

    }



    monitor_qmp_respond(mon, rsp, id);

    monitor_qmp_respond(mon, rsp);

    qobject_unref(rsp);

}
@@ -4208,13 +4201,15 @@ static void monitor_qmp_bh_dispatcher(void *data) 
        mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1;

    qemu_mutex_unlock(&mon->qmp.qmp_queue_lock);

    if (req_obj->req) {

        trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: "");

        monitor_qmp_dispatch(mon, req_obj->req, req_obj->id);

        QDict *qdict = qobject_to(QDict, req_obj->req);

        QObject *id = qdict ? qdict_get(qdict, "id") : NULL;

        trace_monitor_qmp_cmd_in_band(qobject_get_try_str(id) ?: "");

        monitor_qmp_dispatch(mon, req_obj->req);

    } else {

        assert(req_obj->err);

        rsp = qmp_error_response(req_obj->err);

        req_obj->err = NULL;

        monitor_qmp_respond(mon, rsp, NULL);

        monitor_qmp_respond(mon, rsp);

        qobject_unref(rsp);

    }
@@ -4239,8 +4234,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) 


    qdict = qobject_to(QDict, req);

    if (qdict) {

        id = qobject_ref(qdict_get(qdict, "id"));

        qdict_del(qdict, "id");

        id = qdict_get(qdict, "id");

    } /* else will fail qmp_dispatch() */



    if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
@@ -4251,17 +4245,14 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) 


    if (qdict && qmp_is_oob(qdict)) {

        /* OOB commands are executed immediately */

        trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id)

                                          ?: "");

        monitor_qmp_dispatch(mon, req, id);

        trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id) ?: "");

        monitor_qmp_dispatch(mon, req);

        qobject_unref(req);

        qobject_unref(id);

        return;

    }



    req_obj = g_new0(QMPRequest, 1);

    req_obj->mon = mon;

    req_obj->id = id;

    req_obj->req = req;

    req_obj->err = err;
@@ -4281,7 +4272,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) 


    /*

     * Put the request to the end of queue so that requests will be

     * handled in time order.  Ownership for req_obj, req, id,

     * handled in time order.  Ownership for req_obj, req,

     * etc. will be delivered to the handler side.

     */

    assert(mon->qmp.qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX);

qapi/qmp-dispatch.c

+8 −2
Original line number Diff line number Diff line
@@ -58,6 +58,8 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, bool allow_oob, 
                           "QMP input member 'arguments' must be an object");

                return NULL;

            }

        } else if (!strcmp(arg_name, "id")) {

            continue;

        } else {

            error_setg(errp, "QMP input member '%s' is unexpected",

                       arg_name);
@@ -165,11 +167,11 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, 
                    bool allow_oob)

{

    Error *err = NULL;

    QObject *ret;

    QDict *dict = qobject_to(QDict, request);

    QObject *ret, *id = dict ? qdict_get(dict, "id") : NULL;

    QDict *rsp;



    ret = do_qmp_dispatch(cmds, request, allow_oob, &err);



    if (err) {

        rsp = qmp_error_response(err);

    } else if (ret) {
@@ -180,5 +182,9 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request, 
        rsp = NULL;

    }



    if (rsp && id) {

        qdict_put_obj(rsp, "id", qobject_ref(id));

    }



    return rsp;

}

tests/test-qga.c

+5 −8
Original line number Diff line number Diff line
@@ -225,18 +225,15 @@ static void test_qga_ping(gconstpointer fix) 
    qobject_unref(ret);

}



static void test_qga_invalid_id(gconstpointer fix)

static void test_qga_id(gconstpointer fix)

{

    const TestFixture *fixture = fix;

    QDict *ret, *error;

    const char *class;

    QDict *ret;



    ret = qmp_fd(fixture->fd, "{'execute': 'guest-ping', 'id': 1}");

    g_assert_nonnull(ret);



    error = qdict_get_qdict(ret, "error");

    class = qdict_get_try_str(error, "class");

    g_assert_cmpstr(class, ==, "GenericError");

    qmp_assert_no_error(ret);

    g_assert_cmpint(qdict_get_int(ret, "id"), ==, 1);



    qobject_unref(ret);

}
@@ -992,7 +989,7 @@ int main(int argc, char **argv) 
    g_test_add_data_func("/qga/file-ops", &fix, test_qga_file_ops);

    g_test_add_data_func("/qga/file-write-read", &fix, test_qga_file_write_read);

    g_test_add_data_func("/qga/get-time", &fix, test_qga_get_time);

    g_test_add_data_func("/qga/invalid-id", &fix, test_qga_invalid_id);

    g_test_add_data_func("/qga/id", &fix, test_qga_id);

    g_test_add_data_func("/qga/invalid-oob", &fix, test_qga_invalid_oob);

    g_test_add_data_func("/qga/invalid-cmd", &fix, test_qga_invalid_cmd);

    g_test_add_data_func("/qga/invalid-args", &fix, test_qga_invalid_args);