From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58391 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q3EY7-0007nY-HK for qemu-devel@nongnu.org; Fri, 25 Mar 2011 17:28:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q3EY5-0001L0-TL for qemu-devel@nongnu.org; Fri, 25 Mar 2011 17:28:03 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:43657) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q3EY5-0001Kr-Ew for qemu-devel@nongnu.org; Fri, 25 Mar 2011 17:28:01 -0400 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by e32.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p2PLHExZ016257 for ; Fri, 25 Mar 2011 15:17:14 -0600 Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p2PLRxcL105394 for ; Fri, 25 Mar 2011 15:27:59 -0600 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2PLRx23020171 for ; Fri, 25 Mar 2011 15:27:59 -0600 Message-ID: <4D8D08DE.8050805@us.ibm.com> Date: Fri, 25 Mar 2011 16:27:58 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1301082479-4058-1-git-send-email-mdroth@linux.vnet.ibm.com> <1301082479-4058-8-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1301082479-4058-8-git-send-email-mdroth@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RFC][PATCH v1 07/12] qmp proxy: core code for proxying qmp requests to guest List-Id: qemu-devel.nongnu.org 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 03/25/2011 02:47 PM, Michael Roth wrote: > This provides a QmpProxy class, 1 instance of which is shared by all QMP > servers/sessions to send/receive QMP requests/responses between QEMU and > the QEMU guest agent. > > A single qmp_proxy_send_request() is the only interface currently needed > by a QMP session, QAPI/QMP's existing async support handles all the work > of doing callbacks and routing responses to the proper session. > > Currently the class requires a path to a listening socket that either > corresponds to the chardev that the guest agent is communicating > through, or a local socket so we can communicate with a host-side > "guest" agent for testing purposes. > > A subsequent patch will introduce a new chardev that sets up the > socket chardev and initializes the QmpProxy instance to abstract this > away from the user. Unifying this with local "guest" agent support may > not be feasible, so another command-line option may be needed support > host-side-only testing. > > Signed-off-by: Michael Roth > --- > qmp-core.c | 8 ++ > qmp-core.h | 7 +- > qmp-proxy-core.h | 20 ++++ > qmp-proxy.c | 335 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > vl.c | 1 + > 5 files changed, 365 insertions(+), 6 deletions(-) > create mode 100644 qmp-proxy-core.h > create mode 100644 qmp-proxy.c > > diff --git a/qmp-core.c b/qmp-core.c > index 9f3d182..dab50a1 100644 > --- a/qmp-core.c > +++ b/qmp-core.c > @@ -937,7 +937,15 @@ void qmp_async_complete_command(QmpCommandState *cmd, QObject *retval, Error *er > qemu_free(cmd); > } > > +extern QmpProxy *qmp_proxy_default; > + > void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp, > QmpGuestCompletionFunc *cb, void *opaque) > { > + if (!qmp_proxy_default) { > + /* TODO: should set errp here */ > + fprintf(stderr, "qmp proxy: no guest proxy found\n"); > + return; > + } > + qmp_proxy_send_request(qmp_proxy_default, name, args, errp, cb, opaque); > } > diff --git a/qmp-core.h b/qmp-core.h > index b676020..114d290 100644 > --- a/qmp-core.h > +++ b/qmp-core.h > @@ -4,6 +4,7 @@ > #include "monitor.h" > #include "qmp-marshal-types.h" > #include "error_int.h" > +#include "qmp-proxy-core.h" > > struct QmpCommandState > { > @@ -85,11 +86,5 @@ int qmp_state_get_fd(QmpState *sess); > } \ > } while(0) > > -typedef void (QmpGuestCompletionFunc)(void *opaque, QObject *ret_data, Error *err); > - > -void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp, > - QmpGuestCompletionFunc *cb, void *opaque); > - > - > #endif > > diff --git a/qmp-proxy-core.h b/qmp-proxy-core.h > new file mode 100644 > index 0000000..47ac85d > --- /dev/null > +++ b/qmp-proxy-core.h > @@ -0,0 +1,20 @@ > +#ifndef QMP_PROXY_CORE_H > +#define QMP_PROXY_CORE_H > + > +#define QMP_PROXY_PATH_DEFAULT "/tmp/qmp-proxy.sock" > + > +typedef void (QmpGuestCompletionFunc)(void *opaque, QObject *ret_data, > + Error *err); > + > +void qmp_guest_dispatch(const char *name, const QDict *args, Error **errp, > + QmpGuestCompletionFunc *cb, void *opaque); > + > +typedef struct QmpProxy QmpProxy; > + > +void qmp_proxy_send_request(QmpProxy *p, const char *name, > + const QDict *args, Error **errp, > + QmpGuestCompletionFunc *cb, void *opaque); > +QmpProxy *qmp_proxy_new(const char *channel_path); > +void qmp_proxy_close(QmpProxy *p); > + > +#endif > diff --git a/qmp-proxy.c b/qmp-proxy.c > new file mode 100644 > index 0000000..eaa6e6e > --- /dev/null > +++ b/qmp-proxy.c > @@ -0,0 +1,335 @@ > +/* > + * QMP definitions for communicating with guest agent > + * > + * Copyright IBM Corp. 2011 > + * > + * Authors: > + * Michael Roth > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qmp.h" > +#include "qmp-core.h" > +#include "qemu-queue.h" > +#include "json-parser.h" > +#include "json-streamer.h" > +#include "qemu_socket.h" > + > +#define QMP_SENTINEL 0xFF > + > +typedef struct QmpProxyRequest { > + const char *name; > + const QDict *args; > + QmpGuestCompletionFunc *cb; > + void *opaque; > + QString *json; > + QTAILQ_ENTRY(QmpProxyRequest) entry; > +} QmpProxyRequest; > + > +typedef struct QmpProxyWriteState { > + QmpProxyRequest *current_request; > + const char *buf; > + size_t size; > + size_t pos; > + bool use_sentinel; > +} QmpProxyWriteState; > + > +typedef struct QmpProxyReadState { > + char *buf; > + size_t size; > + size_t pos; > +} QmpProxyReadState; I suspect you should use GStrings for both the rx and tx buffers. See the QmpUnixSession for an example. > +struct QmpProxy { > + int fd; > + const char *path; > + QmpProxyWriteState write_state; > + QmpProxyReadState read_state; > + JSONMessageParser parser; > + QTAILQ_HEAD(, QmpProxyRequest) sent_requests; > + QTAILQ_HEAD(, QmpProxyRequest) queued_requests; > + QString *xport_event; > + QString *xport_event_sending; > +}; > + > +static void qmp_proxy_read_handler(void *opaque); > +static void qmp_proxy_write_handler(void *opaque); > + > +static int qmp_proxy_cancel_request(QmpProxy *p, QmpProxyRequest *r) > +{ > + if (r->name) { > + if (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->queued_requests, entry, tmp) { > + qmp_proxy_cancel_request(p, r); > + QTAILQ_REMOVE(&p->queued_requests, r, entry); > + } > + QTAILQ_FOREACH_SAFE(r,&p->sent_requests, entry, tmp) { > + qmp_proxy_cancel_request(p, r); > + QTAILQ_REMOVE(&p->queued_requests, r, entry); > + } > + > + return 0; > +} > + > +static void qmp_proxy_send_host_ack(QmpProxy *p, int session_id) > +{ > + QDict *evt = qdict_new(); > + > + /* only the last ack matters, nuke any outstanding ones. need to rethink > + * this approach if a host->guest reset event is added > + */ > + if (p->xport_event) { > + QDECREF(p->xport_event); > + } > + > + qdict_put_obj(evt, "_xport_event", QOBJECT(qstring_from_str("host_ack"))); > + qdict_put_obj(evt, "_xport_arg_sid", QOBJECT(qint_from_int(session_id))); I don't quite follow what this is doing. > + p->xport_event = qobject_to_json(QOBJECT(evt)); > + > + qemu_set_fd_handler(p->fd, qmp_proxy_read_handler, > + qmp_proxy_write_handler, p); > +} > + > +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; > + const char *cmd; > + int session_id; > + > + 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); > + } > + > + /* check for transport-only commands/events */ > + if (qdict_haskey(qdict, "_xport_event")) { > + cmd = qdict_get_try_str(qdict, "_xport_event"); > + if (cmd&& strcmp(cmd, "guest_init") == 0) { > + /* reset outstanding requests, then send an ack with the > + * session id they passed us > + */ > + session_id = qdict_get_try_int(qdict, "_xport_arg_sid", 0); > + if (!session_id) { > + fprintf(stderr, "received invalid guest_init event\n"); > + } > + qmp_proxy_cancel_all(p); > + qmp_proxy_send_host_ack(p, session_id); > + > + return; > + } > + } else if (qdict_haskey(qdict, "return")) { > + fprintf(stderr, "received return\n"); > + r = QTAILQ_FIRST(&p->sent_requests); > + if (!r) { > + fprintf(stderr, "received return, but no request queued\n"); > + 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)))); > + /* TODO: we don't know what kind of qtype the return value is, what > + * really need is for the qmp async callbacks to handle demarshalling > + * for us, for now we just pass the whole response up the stack, which > + * means everything except commands with no return value, like > + * guest-ping, will result in errors reported to the client > + */ > + r->cb(r->opaque, QOBJECT(qdict), NULL); > + QTAILQ_REMOVE(&p->sent_requests, r, entry); > + fprintf(stderr, "done handling response\n"); > + } else { > + fprintf(stderr, "received invalid payload format\n"); > + } > +} > + > +static void qmp_proxy_read_handler(void *opaque) > +{ > + QmpProxy *p = opaque; > + char buf[4096]; > + int ret; > + > + do { > + ret = read(p->fd, buf, 4096); > + if (ret == -1) { > + if (errno != EAGAIN&& errno != EINTR) { > + fprintf(stderr, "qmp proxy: error reading request: %s", > + strerror(errno)); > + } > + return; > + } else if (ret == 0) { > + /* TODO: is this recoverable? should only happen for hot-unplug > + * in the chardev case, but for testing via a local guest agent > + * we may want to do some special handling... > + */ > + fprintf(stderr, "qmp proxy: connection closed unexpectedly"); > + qmp_proxy_cancel_all(p); > + qemu_set_fd_handler(p->fd, NULL, NULL, p); > + return; > + } > + buf[ret] = 0; You have a buffer overflow here. > + json_message_parser_feed(&p->parser, (char *)buf, ret); cast is unnecessary. > + } while (ret> 0); > +} > + > +static void qmp_proxy_write_handler(void *opaque) > +{ > + QmpProxy *p = opaque; > + QmpProxyWriteState s = p->write_state; > + QmpProxyRequest *r; > + char sentinel = QMP_SENTINEL; > + int ret; > + > +send_another: Better to avoid being clever here and have a separate send_once function and then a loop() that calls send_once. Regards, Anthony Liguori