Re: [Qemu-devel] [RFC][PATCH v2 09/17] qmp proxy: core code for proxying qmp requests to guest

[Stefan Hajnoczi]

On Mon, Apr 18, 2011 at 4:02 PM, Michael Roth <address@hidden> wrote:
> +static int qmp_proxy_cancel_request(QmpProxy *p, QmpProxyRequest *r)
> +{
> +    if (r && r->cb) {
> +        r->cb(r->opaque, NULL, NULL);
> +    }
> +
> +    return 0;
> +}
> +
> +static int qmp_proxy_cancel_all(QmpProxy *p)
> +{
> +    QmpProxyRequest *r, *tmp;
> +    QTAILQ_FOREACH_SAFE(r, &p->requests, entry, tmp) {
> +        qmp_proxy_cancel_request(p, r);
> +        QTAILQ_REMOVE(&p->requests, r, entry);
> +    }
> +
> +    return 0;
> +}

qmp_proxy_cancel_all() will remove requests from the list.
qmp_proxy_cancel_request() will not remove it from the list.  This
could cause confusion in the future if someone adds a call to
qmp_proxy_cancel_request() without realizing that it will not remove
the request from the list.  The two function's names are similar, it
would be nice if they acted the same way.

> +static void qmp_proxy_process_event(JSONMessageParser *parser, QList *tokens)
> +{
> +    QmpProxy *p = container_of(parser, QmpProxy, parser);
> +    QmpProxyRequest *r;
> +    QObject *obj;
> +    QDict *qdict;
> +    Error *err = NULL;
> +
> +    fprintf(stderr, "qmp proxy: called\n");
> +    obj = json_parser_parse_err(tokens, NULL, &err);
> +    if (!obj) {
> +        fprintf(stderr, "qmp proxy: failed to parse\n");
> +        return;
> +    } else {
> +        fprintf(stderr, "qmp proxy: parse successful\n");
> +        qdict = qobject_to_qdict(obj);
> +    }
> +
> +    if (qdict_haskey(qdict, "_control_event")) {
> +        /* handle transport-level control event */
> +        qmp_proxy_process_control_event(p, qdict);
> +    } else if (qdict_haskey(qdict, "return")) {
> +        /* handle proxied qmp command response */
> +        fprintf(stderr, "received return\n");
> +        r = QTAILQ_FIRST(&p->requests);
> +        if (!r) {
> +            fprintf(stderr, "received return, but no request queued\n");

QDECREF(qdict)?

> +            return;
> +        }
> +        /* XXX: can't assume type here */
> +        fprintf(stderr, "recieved response for cmd: %s\nreturn: %s\n",
> +                r->name, qstring_get_str(qobject_to_json(QOBJECT(qdict))));
> +        r->cb(r->opaque, qdict_get(qdict, "return"), NULL);
> +        QTAILQ_REMOVE(&p->requests, r, entry);
> +        qemu_free(r);
> +        fprintf(stderr, "done handling response\n");
> +    } else {
> +        fprintf(stderr, "received invalid payload format\n");
> +    }
> +
> +    QDECREF(qdict);
> +}
> +void qmp_proxy_send_request(QmpProxy *p, const char *name,
> +                            const QDict *args, Error **errp,
> +                            QmpGuestCompletionFunc *cb, void *opaque)
> +{
> +    QmpProxyRequest *r = qemu_mallocz(sizeof(QmpProxyRequest));
> +    QDict *payload = qdict_new();
> +    QString *json;
> +
> +    /* TODO: don't really need to hold on to name/args after encoding */
> +    r->name = name;
> +    r->args = args;
> +    r->cb = cb;
> +    r->opaque = opaque;
> +
> +    qdict_put_obj(payload, "execute", QOBJECT(qstring_from_str(r->name)));
> +    /* TODO: casting a const so we can add it to our dictionary. bad. */
> +    qdict_put_obj(payload, "arguments", QOBJECT((QDict *)args));
> +
> +    json = qobject_to_json(QOBJECT((QDict *)payload));
> +    if (!json) {
> +        goto out_bad;
> +    }
> +
> +    QTAILQ_INSERT_TAIL(&p->requests, r, entry);
> +    g_string_append(p->tx, qstring_get_str(json));
> +    QDECREF(json);
> +    qmp_proxy_write(p);
> +    return;
> +
> +out_bad:
> +    cb(opaque, NULL, NULL);
> +    qemu_free(r);

Need to free payload?

> +}
> +
> +QmpProxy *qmp_proxy_new(CharDriverState *chr)
> +{
> +    QmpProxy *p = qemu_mallocz(sizeof(QmpProxy));
> +
> +    signal_init(&guest_agent_up_event);
> +    signal_init(&guest_agent_reset_event);
> +
> +    /* there's a reason for this madness */

Helpful comment :)

> +    p->tx_timer = qemu_new_timer(rt_clock, qmp_proxy_write_handler, p);
> +    p->tx_timer_interval = 10;
> +    p->tx = g_string_new("");
> +    p->chr = chr;
> +    json_message_parser_init(&p->parser, qmp_proxy_process_event);
> +    QTAILQ_INIT(&p->requests);
> +
> +    return p;
> +}
> +
> +void qmp_proxy_close(QmpProxy *p)
> +{
> +    qmp_proxy_cancel_all(p);
> +    g_string_free(p->tx, TRUE);

Free tx_timer?

> +    qemu_free(p);
> +}