From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47459) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QgQUQ-0002DB-Vx for qemu-devel@nongnu.org; Mon, 11 Jul 2011 20:06:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QgQUM-0002M5-PV for qemu-devel@nongnu.org; Mon, 11 Jul 2011 20:06:14 -0400 Received: from e2.ny.us.ibm.com ([32.97.182.142]:55667) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QgQUM-0002L9-8T for qemu-devel@nongnu.org; Mon, 11 Jul 2011 20:06:10 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e2.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p6BNj1NT021541 for ; Mon, 11 Jul 2011 19:45:01 -0400 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p6C060jd122870 for ; Mon, 11 Jul 2011 20:06:00 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p6BI5VDa022455 for ; Mon, 11 Jul 2011 12:05:32 -0600 Message-ID: <4E1B8FE6.8060202@linux.vnet.ibm.com> Date: Mon, 11 Jul 2011 19:05:58 -0500 From: Michael Roth MIME-Version: 1.0 References: <1309870965-27066-1-git-send-email-mdroth@linux.vnet.ibm.com> <1309870965-27066-6-git-send-email-mdroth@linux.vnet.ibm.com> <20110707113234.328bc500@doriath> In-Reply-To: <20110707113234.328bc500@doriath> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 05/18] qapi: add QMP input visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: aliguori@linux.vnet.ibm.com, agl@linux.vnet.ibm.com, qemu-devel@nongnu.org, Jes.Sorensen@redhat.com On 07/07/2011 09:32 AM, Luiz Capitulino wrote: > On Tue, 5 Jul 2011 08:02:32 -0500 > Michael Roth wrote: > >> A type of Visiter class that is used to walk a qobject's >> structure and assign each entry to the corresponding native C type. >> Command marshaling function will use this to pull out QMP command >> parameters recieved over the wire and pass them as native arguments >> to the corresponding C functions. >> >> Signed-off-by: Michael Roth >> --- >> Makefile.objs | 2 +- >> qapi/qmp-input-visitor.c | 264 ++++++++++++++++++++++++++++++++++++++++++++++ >> qapi/qmp-input-visitor.h | 27 +++++ >> qerror.h | 3 + >> 4 files changed, 295 insertions(+), 1 deletions(-) >> create mode 100644 qapi/qmp-input-visitor.c >> create mode 100644 qapi/qmp-input-visitor.h >> >> diff --git a/Makefile.objs b/Makefile.objs >> index 0077014..997ecef 100644 >> --- a/Makefile.objs >> +++ b/Makefile.objs >> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o >> ###################################################################### >> # qapi >> >> -qapi-nested-y = qapi-visit-core.o >> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o >> qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y)) >> >> vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) >> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c >> new file mode 100644 >> index 0000000..80912bb >> --- /dev/null >> +++ b/qapi/qmp-input-visitor.c >> @@ -0,0 +1,264 @@ >> +/* >> + * Input Visitor >> + * >> + * Copyright IBM, Corp. 2011 >> + * >> + * Authors: >> + * Anthony Liguori >> + * >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. >> + * See the COPYING.LIB file in the top-level directory. >> + * >> + */ >> + >> +#include "qmp-input-visitor.h" >> +#include "qemu-queue.h" >> +#include "qemu-common.h" >> +#include "qemu-objects.h" >> +#include "qerror.h" >> + >> +#define QIV_STACK_SIZE 1024 >> + >> +typedef struct StackObject >> +{ >> + const QObject *obj; >> + const QListEntry *entry; >> +} StackObject; >> + >> +struct QmpInputVisitor >> +{ >> + Visitor visitor; >> + const QObject *obj; >> + StackObject stack[QIV_STACK_SIZE]; >> + int nb_stack; >> +}; >> + >> +static QmpInputVisitor *to_qiv(Visitor *v) >> +{ >> + return container_of(v, QmpInputVisitor, visitor); >> +} >> + >> +static const QObject *qmp_input_get_object(QmpInputVisitor *qiv, const char *name) >> +{ >> + const QObject *qobj; >> + >> + if (qiv->nb_stack == 0) { >> + qobj = qiv->obj; >> + } else { >> + qobj = qiv->stack[qiv->nb_stack - 1].obj; >> + } >> + >> + if (name&& qobject_type(qobj) == QTYPE_QDICT) { >> + return qdict_get(qobject_to_qdict(qobj), name); >> + } else if (qiv->nb_stack> 0&& qobject_type(qobj) == QTYPE_QLIST) { >> + return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry); >> + } >> + >> + return qobj; >> +} >> + >> +static void qmp_input_push(QmpInputVisitor *qiv, const QObject *obj, Error **errp) >> +{ >> + qiv->stack[qiv->nb_stack].obj = obj; >> + if (qobject_type(obj) == QTYPE_QLIST) { >> + qiv->stack[qiv->nb_stack].entry = qlist_first(qobject_to_qlist(obj)); >> + } >> + qiv->nb_stack++; >> + >> + if (qiv->nb_stack>= QIV_STACK_SIZE) { >> + error_set(errp, QERR_BUFFER_OVERRUN); >> + return; >> + } >> +} >> + >> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp) >> +{ >> + qiv->nb_stack--; >> + if (qiv->nb_stack< 0) { >> + error_set(errp, QERR_BUFFER_OVERRUN); >> + return; >> + } >> +} >> + >> +static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind, const char *name, size_t size, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + const QObject *qobj = qmp_input_get_object(qiv, name); >> + >> + if (!qobj || qobject_type(qobj) != QTYPE_QDICT) { >> + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict"); >> + return; >> + } >> + >> + qmp_input_push(qiv, qobj, errp); >> + if (error_is_set(errp)) { >> + return; >> + } >> + >> + if (obj) { >> + *obj = qemu_mallocz(size); >> + } >> +} >> + >> +static void qmp_input_end_struct(Visitor *v, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + >> + qmp_input_pop(qiv, errp); >> +} >> + >> +static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + const QObject *qobj = qmp_input_get_object(qiv, name); >> + >> + if (!qobj || qobject_type(qobj) != QTYPE_QLIST) { >> + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list"); >> + return; >> + } >> + >> + qmp_input_push(qiv, qobj, errp); >> +} >> + >> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + GenericList *entry; >> + StackObject *so =&qiv->stack[qiv->nb_stack - 1]; >> + >> + if (so->entry == NULL) { >> + return NULL; >> + } >> + >> + entry = qemu_mallocz(sizeof(*entry)); >> + if (*list) { >> + so->entry = qlist_next(so->entry); >> + if (so->entry == NULL) { >> + qemu_free(entry); >> + return NULL; >> + } >> + (*list)->next = entry; >> + } >> + *list = entry; >> + >> + >> + return entry; >> +} >> + >> +static void qmp_input_end_list(Visitor *v, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + >> + qmp_input_pop(qiv, errp); >> +} >> + >> +static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + const QObject *qobj = qmp_input_get_object(qiv, name); >> + >> + if (!qobj || qobject_type(qobj) != QTYPE_QINT) { >> + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer"); >> + return; >> + } >> + >> + *obj = qint_get_int(qobject_to_qint(qobj)); >> +} >> + >> +static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + const QObject *qobj = qmp_input_get_object(qiv, name); >> + >> + if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) { >> + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean"); >> + return; >> + } >> + >> + *obj = qbool_get_int(qobject_to_qbool(qobj)); >> +} >> + >> +static void qmp_input_type_str(Visitor *v, char **obj, const char *name, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + const QObject *qobj = qmp_input_get_object(qiv, name); >> + >> + if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) { >> + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string"); >> + return; >> + } >> + >> + *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj))); >> +} >> + >> +static void qmp_input_type_number(Visitor *v, double *obj, const char *name, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + const QObject *qobj = qmp_input_get_object(qiv, name); >> + >> + if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) { >> + error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double"); >> + return; >> + } >> + >> + *obj = qfloat_get_double(qobject_to_qfloat(qobj)); >> +} >> + >> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp) >> +{ >> + int64_t value; >> + qmp_input_type_int(v,&value, name, errp); >> + *obj = value; >> +} >> + >> +static void qmp_input_start_optional(Visitor *v, bool *present, >> + const char *name, Error **errp) >> +{ >> + QmpInputVisitor *qiv = to_qiv(v); >> + const QObject *qobj = qmp_input_get_object(qiv, name); >> + >> + if (!qobj) { >> + *present = false; >> + return; >> + } >> + >> + *present = true; >> +} >> + >> +static void qmp_input_end_optional(Visitor *v, Error **errp) >> +{ >> +} >> + >> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v) >> +{ >> + return&v->visitor; >> +} >> + >> +void qmp_input_visitor_cleanup(QmpInputVisitor *v) >> +{ > > We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and > decrefing here. > I think it was either that, or just making the QObject arg to qmp_input_visitor_new() const, to make it clear it was caller-freed. >> + qemu_free(v); >> +} >> + >> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj) >> +{ >> + QmpInputVisitor *v; >> + >> + v = qemu_mallocz(sizeof(*v)); >> + >> + v->visitor.start_struct = qmp_input_start_struct; >> + v->visitor.end_struct = qmp_input_end_struct; >> + v->visitor.start_list = qmp_input_start_list; >> + v->visitor.next_list = qmp_input_next_list; >> + v->visitor.end_list = qmp_input_end_list; >> + v->visitor.type_enum = qmp_input_type_enum; >> + v->visitor.type_int = qmp_input_type_int; >> + v->visitor.type_bool = qmp_input_type_bool; >> + v->visitor.type_str = qmp_input_type_str; >> + v->visitor.type_number = qmp_input_type_number; >> + v->visitor.start_optional = qmp_input_start_optional; >> + v->visitor.end_optional = qmp_input_end_optional; >> + >> + v->obj = obj; >> + >> + return v; >> +} >> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h >> new file mode 100644 >> index 0000000..0f77916 >> --- /dev/null >> +++ b/qapi/qmp-input-visitor.h >> @@ -0,0 +1,27 @@ >> +/* >> + * Input Visitor >> + * >> + * Copyright IBM, Corp. 2011 >> + * >> + * Authors: >> + * Anthony Liguori >> + * >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. >> + * See the COPYING.LIB file in the top-level directory. >> + * >> + */ >> + >> +#ifndef QMP_INPUT_VISITOR_H >> +#define QMP_INPUT_VISITOR_H >> + >> +#include "qapi-visit-core.h" >> +#include "qobject.h" >> + >> +typedef struct QmpInputVisitor QmpInputVisitor; >> + >> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj); >> +void qmp_input_visitor_cleanup(QmpInputVisitor *v); >> + >> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v); >> + >> +#endif >> diff --git a/qerror.h b/qerror.h >> index 16c830d..9a9fa5b 100644 >> --- a/qerror.h >> +++ b/qerror.h >> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj); >> #define QERR_JSON_PARSE_ERROR \ >> "{ 'class': 'JSONParseError', 'data': { 'message': %s } }" >> >> +#define QERR_BUFFER_OVERRUN \ >> + "{ 'class': 'BufferOverrun', 'data': {} }" >> + > > You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory, > but BufferOverrrun is probably fine. > >> #define QERR_KVM_MISSING_CAP \ >> "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }" >> >