From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46037) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEiCe-0000DL-OF for qemu-devel@nongnu.org; Tue, 26 Apr 2011 09:21:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QEiCd-0004Ae-LX for qemu-devel@nongnu.org; Tue, 26 Apr 2011 09:21:20 -0400 Received: from mail-yi0-f45.google.com ([209.85.218.45]:64436) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QEiCd-0004AZ-IE for qemu-devel@nongnu.org; Tue, 26 Apr 2011 09:21:19 -0400 Received: by yib19 with SMTP id 19so270731yib.4 for ; Tue, 26 Apr 2011 06:21:19 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1303138953-1334-10-git-send-email-mdroth@linux.vnet.ibm.com> References: <1303138953-1334-1-git-send-email-mdroth@linux.vnet.ibm.com> <1303138953-1334-10-git-send-email-mdroth@linux.vnet.ibm.com> Date: Tue, 26 Apr 2011 14:21:19 +0100 Message-ID: From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC][PATCH v2 09/17] qmp proxy: core code for proxying qmp requests to guest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: aliguori@linux.vnet.ibm.com, agl@linux.vnet.ibm.com, qemu-devel@nongnu.org, Jes.Sorensen@redhat.com On Mon, Apr 18, 2011 at 4:02 PM, Michael Roth w= rote: > +static int qmp_proxy_cancel_request(QmpProxy *p, QmpProxyRequest *r) > +{ > + =A0 =A0if (r && r->cb) { > + =A0 =A0 =A0 =A0r->cb(r->opaque, NULL, NULL); > + =A0 =A0} > + > + =A0 =A0return 0; > +} > + > +static int qmp_proxy_cancel_all(QmpProxy *p) > +{ > + =A0 =A0QmpProxyRequest *r, *tmp; > + =A0 =A0QTAILQ_FOREACH_SAFE(r, &p->requests, entry, tmp) { > + =A0 =A0 =A0 =A0qmp_proxy_cancel_request(p, r); > + =A0 =A0 =A0 =A0QTAILQ_REMOVE(&p->requests, r, entry); > + =A0 =A0} > + > + =A0 =A0return 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 *to= kens) > +{ > + =A0 =A0QmpProxy *p =3D container_of(parser, QmpProxy, parser); > + =A0 =A0QmpProxyRequest *r; > + =A0 =A0QObject *obj; > + =A0 =A0QDict *qdict; > + =A0 =A0Error *err =3D NULL; > + > + =A0 =A0fprintf(stderr, "qmp proxy: called\n"); > + =A0 =A0obj =3D json_parser_parse_err(tokens, NULL, &err); > + =A0 =A0if (!obj) { > + =A0 =A0 =A0 =A0fprintf(stderr, "qmp proxy: failed to parse\n"); > + =A0 =A0 =A0 =A0return; > + =A0 =A0} else { > + =A0 =A0 =A0 =A0fprintf(stderr, "qmp proxy: parse successful\n"); > + =A0 =A0 =A0 =A0qdict =3D qobject_to_qdict(obj); > + =A0 =A0} > + > + =A0 =A0if (qdict_haskey(qdict, "_control_event")) { > + =A0 =A0 =A0 =A0/* handle transport-level control event */ > + =A0 =A0 =A0 =A0qmp_proxy_process_control_event(p, qdict); > + =A0 =A0} else if (qdict_haskey(qdict, "return")) { > + =A0 =A0 =A0 =A0/* handle proxied qmp command response */ > + =A0 =A0 =A0 =A0fprintf(stderr, "received return\n"); > + =A0 =A0 =A0 =A0r =3D QTAILQ_FIRST(&p->requests); > + =A0 =A0 =A0 =A0if (!r) { > + =A0 =A0 =A0 =A0 =A0 =A0fprintf(stderr, "received return, but no request= queued\n"); QDECREF(qdict)? > + =A0 =A0 =A0 =A0 =A0 =A0return; > + =A0 =A0 =A0 =A0} > + =A0 =A0 =A0 =A0/* XXX: can't assume type here */ > + =A0 =A0 =A0 =A0fprintf(stderr, "recieved response for cmd: %s\nreturn: = %s\n", > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0r->name, qstring_get_str(qobject_to_json= (QOBJECT(qdict)))); > + =A0 =A0 =A0 =A0r->cb(r->opaque, qdict_get(qdict, "return"), NULL); > + =A0 =A0 =A0 =A0QTAILQ_REMOVE(&p->requests, r, entry); > + =A0 =A0 =A0 =A0qemu_free(r); > + =A0 =A0 =A0 =A0fprintf(stderr, "done handling response\n"); > + =A0 =A0} else { > + =A0 =A0 =A0 =A0fprintf(stderr, "received invalid payload format\n"); > + =A0 =A0} > + > + =A0 =A0QDECREF(qdict); > +} > +void qmp_proxy_send_request(QmpProxy *p, const char *name, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const QDict *arg= s, Error **errp, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0QmpGuestCompleti= onFunc *cb, void *opaque) > +{ > + =A0 =A0QmpProxyRequest *r =3D qemu_mallocz(sizeof(QmpProxyRequest)); > + =A0 =A0QDict *payload =3D qdict_new(); > + =A0 =A0QString *json; > + > + =A0 =A0/* TODO: don't really need to hold on to name/args after encodin= g */ > + =A0 =A0r->name =3D name; > + =A0 =A0r->args =3D args; > + =A0 =A0r->cb =3D cb; > + =A0 =A0r->opaque =3D opaque; > + > + =A0 =A0qdict_put_obj(payload, "execute", QOBJECT(qstring_from_str(r->na= me))); > + =A0 =A0/* TODO: casting a const so we can add it to our dictionary. bad= . */ > + =A0 =A0qdict_put_obj(payload, "arguments", QOBJECT((QDict *)args)); > + > + =A0 =A0json =3D qobject_to_json(QOBJECT((QDict *)payload)); > + =A0 =A0if (!json) { > + =A0 =A0 =A0 =A0goto out_bad; > + =A0 =A0} > + > + =A0 =A0QTAILQ_INSERT_TAIL(&p->requests, r, entry); > + =A0 =A0g_string_append(p->tx, qstring_get_str(json)); > + =A0 =A0QDECREF(json); > + =A0 =A0qmp_proxy_write(p); > + =A0 =A0return; > + > +out_bad: > + =A0 =A0cb(opaque, NULL, NULL); > + =A0 =A0qemu_free(r); Need to free payload? > +} > + > +QmpProxy *qmp_proxy_new(CharDriverState *chr) > +{ > + =A0 =A0QmpProxy *p =3D qemu_mallocz(sizeof(QmpProxy)); > + > + =A0 =A0signal_init(&guest_agent_up_event); > + =A0 =A0signal_init(&guest_agent_reset_event); > + > + =A0 =A0/* there's a reason for this madness */ Helpful comment :) > + =A0 =A0p->tx_timer =3D qemu_new_timer(rt_clock, qmp_proxy_write_handler= , p); > + =A0 =A0p->tx_timer_interval =3D 10; > + =A0 =A0p->tx =3D g_string_new(""); > + =A0 =A0p->chr =3D chr; > + =A0 =A0json_message_parser_init(&p->parser, qmp_proxy_process_event); > + =A0 =A0QTAILQ_INIT(&p->requests); > + > + =A0 =A0return p; > +} > + > +void qmp_proxy_close(QmpProxy *p) > +{ > + =A0 =A0qmp_proxy_cancel_all(p); > + =A0 =A0g_string_free(p->tx, TRUE); Free tx_timer? > + =A0 =A0qemu_free(p); > +}