[v3,05/21] qapi: add QMP input visitor

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 <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs            |    2 +-
 qapi/qmp-input-visitor.c |  251 ++++++++++++++++++++++++++++++++++++++++++++++
 qapi/qmp-input-visitor.h |   27 +++++
 qerror.h                 |    3 +
 4 files changed, 282 insertions(+), 1 deletions(-)
 create mode 100644 qapi/qmp-input-visitor.c
 create mode 100644 qapi/qmp-input-visitor.h

On Mon, 13 Jun 2011 21:31:10 -0500
Michael Roth <mdroth@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 <mdroth@linux.vnet.ibm.com>
> ---
>  Makefile.objs            |    2 +-
>  qapi/qmp-input-visitor.c |  251 ++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/qmp-input-visitor.h |   27 +++++
>  qerror.h                 |    3 +
>  4 files changed, 282 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 68d7b5a..2eb90b8 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -367,7 +367,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..9344d37
> --- /dev/null
> +++ b/qapi/qmp-input-visitor.c

No license info. It seems to be missing in more files.

> @@ -0,0 +1,251 @@
> +#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
> +{
> +    QObject *obj;
> +    const  QListEntry *entry;
> +} StackObject;
> +
> +struct QmpInputVisitor
> +{
> +    Visitor visitor;
> +    QObject *obj;
> +    StackObject stack[QIV_STACK_SIZE];
> +    int nb_stack;
> +};
> +
> +static QmpInputVisitor *to_qiv(Visitor *v)
> +{
> +    return container_of(v, QmpInputVisitor, visitor);
> +}
> +
> +static QObject *qmp_input_get_object(QmpInputVisitor *qiv, const char *name)
> +{
> +    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, 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_QAPI_VISITOR_STACK_OVERRUN);

QAPI is something internal to qemu, the error name and description
should be more generic like "not enough memory" or "not enough space".

> +        return;
> +    }
> +}
> +
> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
> +{
> +    qiv->nb_stack--;
> +    if (qiv->nb_stack < 0) {
> +        error_set(errp, QERR_QAPI_VISITOR_STACK_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);
> +    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);
> +    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);
> +    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);
> +    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);
> +    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);
> +    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);
> +    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)
> +{

You're not decrementing the reference to v->obj. Actually, we should also
increment the reference to it before storing or document that the reference
ownership is transferred to this function.

> +    qemu_free(v);
> +}
> +
> +QmpInputVisitor *qmp_input_visitor_new(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..3f798f0
> --- /dev/null
> +++ b/qapi/qmp-input-visitor.h
> @@ -0,0 +1,27 @@
> +/*
> + * Input Visitor
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori   <aliguori@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(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..7a89a50 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_QAPI_VISITOR_STACK_OVERRUN \
> +    "{ 'class': 'QAPIVisitorStackOverrun', 'data': {} }"
> +
>  #define QERR_KVM_MISSING_CAP \
>      "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
>

On 06/15/2011 11:11 AM, Luiz Capitulino wrote:
> On Mon, 13 Jun 2011 21:31:10 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
<snip>
>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
>> +{
>> +    return&v->visitor;
>> +}
>> +
>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
>> +{
>
> You're not decrementing the reference to v->obj. Actually, we should also
> increment the reference to it before storing or document that the reference
> ownership is transferred to this function.
>

Ownership never gets transferred to the visitor in the dispatch path; 
the caller is supposed to free it. I thought there was some manipulation 
elsewhere that prevented us from using a const argument to 
qmp_input_visitor_new() instead, but it looks like we can. Otherwise, 
I'd prefer to do an inc+dec to make this more explicit.

>> +    qemu_free(v);
>> +}
>> +
>> +QmpInputVisitor *qmp_input_visitor_new(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..3f798f0
>> --- /dev/null
>> +++ b/qapi/qmp-input-visitor.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Input Visitor
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + *  Anthony Liguori<aliguori@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(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..7a89a50 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_QAPI_VISITOR_STACK_OVERRUN \
>> +    "{ 'class': 'QAPIVisitorStackOverrun', 'data': {} }"
>> +
>>   #define QERR_KVM_MISSING_CAP \
>>       "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
>>
>

On Wed, 15 Jun 2011 11:56:03 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 06/15/2011 11:11 AM, Luiz Capitulino wrote:
> > On Mon, 13 Jun 2011 21:31:10 -0500
> > Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
> >
> <snip>
> >> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
> >> +{
> >> +    return&v->visitor;
> >> +}
> >> +
> >> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
> >> +{
> >
> > You're not decrementing the reference to v->obj. Actually, we should also
> > increment the reference to it before storing or document that the reference
> > ownership is transferred to this function.
> >
> 
> Ownership never gets transferred to the visitor in the dispatch path; 
> the caller is supposed to free it. I thought there was some manipulation 
> elsewhere that prevented us from using a const argument to 
> qmp_input_visitor_new() instead, but it looks like we can. Otherwise, 
> I'd prefer to do an inc+dec to make this more explicit.

Yeah, I prefer that too.

> 
> >> +    qemu_free(v);
> >> +}
> >> +
> >> +QmpInputVisitor *qmp_input_visitor_new(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..3f798f0
> >> --- /dev/null
> >> +++ b/qapi/qmp-input-visitor.h
> >> @@ -0,0 +1,27 @@
> >> +/*
> >> + * Input Visitor
> >> + *
> >> + * Copyright IBM, Corp. 2011
> >> + *
> >> + * Authors:
> >> + *  Anthony Liguori<aliguori@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(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..7a89a50 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_QAPI_VISITOR_STACK_OVERRUN \
> >> +    "{ 'class': 'QAPIVisitorStackOverrun', 'data': {} }"
> >> +
> >>   #define QERR_KVM_MISSING_CAP \
> >>       "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
> >>
> >
>