From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:48360) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qgdjn-0000y4-5B for qemu-devel@nongnu.org; Tue, 12 Jul 2011 10:15:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qgdjk-0002yJ-6p for qemu-devel@nongnu.org; Tue, 12 Jul 2011 10:14:58 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:48271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qgdjj-0002xx-Is for qemu-devel@nongnu.org; Tue, 12 Jul 2011 10:14:55 -0400 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by e6.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p6CDojTK023786 for ; Tue, 12 Jul 2011 09:50:45 -0400 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p6CEEoLE125386 for ; Tue, 12 Jul 2011 10:14:50 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p6CEESns025990 for ; Tue, 12 Jul 2011 10:14:28 -0400 Message-ID: <4E1C56BF.5070008@linux.vnet.ibm.com> Date: Tue, 12 Jul 2011 09:14:23 -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> <4E1B8FE6.8060202@linux.vnet.ibm.com> <20110712101631.22d636a8@doriath> <4E1C5025.4060007@linux.vnet.ibm.com> <20110712105356.4963c9d9@doriath> In-Reply-To: <20110712105356.4963c9d9@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/12/2011 08:53 AM, Luiz Capitulino wrote: > On Tue, 12 Jul 2011 08:46:13 -0500 > Michael Roth wrote: > >> On 07/12/2011 08:16 AM, Luiz Capitulino wrote: >>> On Mon, 11 Jul 2011 19:05:58 -0500 >>> Michael Roth wrote: >>> >>>> 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. >>> >>> No, because (at least theoretically) a client could free it before walking >>> the struct with the visitor functions. The rule is: if you store it, you >>> have to increment its reference or get its ownership. >>> >>> >> >> Makes sense. Sucks to have to remove the const for that though...would a >> temporary cast to non-const be acceptable in a case like this? > > Well, if we have to change it, it's not const anymore :) > Heh, yah, silly question. Though from a documentation/api perspective, const also helps in terms of making it clear that the dict entries are read-only... but not to the point of violating the semantics of const i suppose. Only other thing we could do is store a copy instead, but that's probably unnecessarily costly, particularly with guest-file-write >> >>>> >>>>>> + 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 } }" >>>>>> >>>>> >>>> >>> >> >