On Tue, 12 Jul 2011 08:46:13 -0500 Michael Roth <mdr...@linux.vnet.ibm.com> wrote:
> On 07/12/2011 08:16 AM, Luiz Capitulino wrote: > > On Mon, 11 Jul 2011 19:05:58 -0500 > > Michael Roth<mdr...@linux.vnet.ibm.com> wrote: > > > >> On 07/07/2011 09:32 AM, Luiz Capitulino wrote: > >>> On Tue, 5 Jul 2011 08:02:32 -0500 > >>> Michael Roth<mdr...@linux.vnet.ibm.com> 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<mdr...@linux.vnet.ibm.com> > >>>> --- > >>>> 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<aligu...@us.ibm.com> > >>>> + * > >>>> + * 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 :) > > >> > >>>> + 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<aligu...@us.ibm.com> > >>>> + * > >>>> + * 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 } }" > >>>> > >>> > >> > > >