| Message ID | 20170505201128.12099-2-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/05/2017 03:11 PM, Eduardo Habkost wrote: > This will allow visitors to make decisions based on the supported qtypes > of a given alternate type. The new parameter can replace the old > 'promote_int' argument, as qobject-input-visitor can simply check if > QTYPE_QINT is set in supported_qtypes. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Changes v1 -> v2: > * Change supported_qtypes to uint32_t (Eric Blake) > * Replace assert() on all generated visitor functions with a > single QEMU_BUILD_BUG_ON() on visit_start_alternate() > * Extra spaces around "|" on generated visitor code > (Eric Blake) > * Don't use bitops.h and just use (1U << QTYPE_FOO) > (Markus Armbruster) Reviewed-by: Eric Blake <eblake@redhat.com>
Eduardo Habkost <ehabkost@redhat.com> writes: > This will allow visitors to make decisions based on the supported qtypes > of a given alternate type. The new parameter can replace the old > 'promote_int' argument, as qobject-input-visitor can simply check if > QTYPE_QINT is set in supported_qtypes. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Might conflict with Marc-André's work, which I haven't reviewed, yet. Should be easy enough to resolve, though. > --- > Changes v1 -> v2: > * Change supported_qtypes to uint32_t (Eric Blake) > * Replace assert() on all generated visitor functions with a > single QEMU_BUILD_BUG_ON() on visit_start_alternate() > * Extra spaces around "|" on generated visitor code > (Eric Blake) > * Don't use bitops.h and just use (1U << QTYPE_FOO) > (Markus Armbruster) > --- > include/qapi/visitor.h | 5 +++-- > include/qapi/visitor-impl.h | 2 +- > scripts/qapi-visit.py | 12 ++++++------ > qapi/qapi-visit-core.c | 9 ++++++--- > qapi/qapi-clone-visitor.c | 3 ++- > qapi/qapi-dealloc-visitor.c | 3 ++- > qapi/qobject-input-visitor.c | 6 ++++-- > qapi/trace-events | 2 +- > 8 files changed, 25 insertions(+), 17 deletions(-) > > diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h > index 1a1b62012b..8f5a223714 100644 > --- a/include/qapi/visitor.h > +++ b/include/qapi/visitor.h > @@ -408,7 +408,8 @@ void visit_end_list(Visitor *v, void **list); > * the qtype of the next thing to be visited, stored in (*@obj)->type. > * Other visitors will leave @obj unchanged. > * > - * If @promote_int, treat integers as QTYPE_FLOAT. > + * @supported_qtypes is a bit mask indicating which QTypes are supported > + * by the alternate. > * > * If successful, this must be paired with visit_end_alternate() with > * the same @obj to clean up, even if visiting the contents of the > @@ -416,7 +417,7 @@ void visit_end_list(Visitor *v, void **list); > */ > void visit_start_alternate(Visitor *v, const char *name, > GenericAlternate **obj, size_t size, > - bool promote_int, Error **errp); > + uint32_t supported_qtypes, Error **errp); > > /* > * Finish visiting an alternate type. > diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h > index e87709db5c..8afcde0f5d 100644 > --- a/include/qapi/visitor-impl.h > +++ b/include/qapi/visitor-impl.h > @@ -71,7 +71,7 @@ struct Visitor > * optional for output visitors. */ > void (*start_alternate)(Visitor *v, const char *name, > GenericAlternate **obj, size_t size, > - bool promote_int, Error **errp); > + uint32_t supported_qtypes, Error **errp); > > /* Optional, needed for dealloc visitor */ > void (*end_alternate)(Visitor *v, void **obj); > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 5737aefa05..41c54982e2 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -161,20 +161,20 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s *obj, Error > > > def gen_visit_alternate(name, variants): > - promote_int = 'true' > + qtypes = ['(1U << %s)' % (var.type.alternate_qtype()) > + for var in variants.variants] > + supported_qtypes = ' | '.join(qtypes) > ret = '' > - for var in variants.variants: > - if var.type.alternate_qtype() == 'QTYPE_QINT': > - promote_int = 'false' Note for later: * promote_int is true if and only if we have no integer variant. * supported_qtypes is the set of the variants' qtypes > > ret += mcgen(''' > > void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp) > { > Error *err = NULL; > + uint32_t supported_qtypes = %(supported_qtypes)s; The initializer is of type unsigned. I'd make the variable unsigned, too. Not worth a respin, though. > > visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj), > - %(promote_int)s, &err); > + supported_qtypes, &err); > if (err) { > goto out; > } > @@ -183,7 +183,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error > } > switch ((*obj)->type) { > ''', > - c_name=c_name(name), promote_int=promote_int) > + c_name=c_name(name), supported_qtypes=supported_qtypes) > > for var in variants.variants: > ret += mcgen(''' > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c > index 43a09d147d..8d62914393 100644 > --- a/qapi/qapi-visit-core.c > +++ b/qapi/qapi-visit-core.c > @@ -106,15 +106,18 @@ void visit_end_list(Visitor *v, void **obj) > > void visit_start_alternate(Visitor *v, const char *name, > GenericAlternate **obj, size_t size, > - bool promote_int, Error **errp) > + uint32_t supported_qtypes, > + Error **errp) > { > Error *err = NULL; > > + QEMU_BUILD_BUG_ON(QTYPE__MAX > 32); Aha, I guess this is why you picked uint32_t. Nevermind then. > + > assert(obj && size >= sizeof(GenericAlternate)); > assert(!(v->type & VISITOR_OUTPUT) || *obj); > - trace_visit_start_alternate(v, name, obj, size, promote_int); > + trace_visit_start_alternate(v, name, obj, size, supported_qtypes); > if (v->start_alternate) { > - v->start_alternate(v, name, obj, size, promote_int, &err); > + v->start_alternate(v, name, obj, size, supported_qtypes, &err); > } > if (v->type & VISITOR_INPUT) { > assert(v->start_alternate && !err != !*obj); > diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c > index 34086cbfc0..2e40da5981 100644 > --- a/qapi/qapi-clone-visitor.c > +++ b/qapi/qapi-clone-visitor.c > @@ -70,7 +70,8 @@ static GenericList *qapi_clone_next_list(Visitor *v, GenericList *tail, > > static void qapi_clone_start_alternate(Visitor *v, const char *name, > GenericAlternate **obj, size_t size, > - bool promote_int, Error **errp) > + uint32_t supported_qtypes, > + Error **errp) > { > qapi_clone_start_struct(v, name, (void **)obj, size, errp); > } > diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c > index e39457bc79..23b64c21a4 100644 > --- a/qapi/qapi-dealloc-visitor.c > +++ b/qapi/qapi-dealloc-visitor.c > @@ -38,7 +38,8 @@ static void qapi_dealloc_end_struct(Visitor *v, void **obj) > > static void qapi_dealloc_start_alternate(Visitor *v, const char *name, > GenericAlternate **obj, size_t size, > - bool promote_int, Error **errp) > + uint32_t supported_qtypes, > + Error **errp) > { > } > > diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c > index 865e948ac0..25997ee816 100644 > --- a/qapi/qobject-input-visitor.c > +++ b/qapi/qobject-input-visitor.c > @@ -338,7 +338,8 @@ static void qobject_input_check_list(Visitor *v, Error **errp) > > static void qobject_input_start_alternate(Visitor *v, const char *name, > GenericAlternate **obj, size_t size, > - bool promote_int, Error **errp) > + uint32_t supported_qtypes, > + Error **errp) > { > QObjectInputVisitor *qiv = to_qiv(v); > QObject *qobj = qobject_input_get_object(qiv, name, false, errp); > @@ -349,7 +350,8 @@ static void qobject_input_start_alternate(Visitor *v, const char *name, > } > *obj = g_malloc0(size); > (*obj)->type = qobject_type(qobj); > - if (promote_int && (*obj)->type == QTYPE_QINT) { Old condition: the alternate has no integer variant, and we got an integer value. > + if (!(supported_qtypes & (1U << QTYPE_QINT)) && > + (*obj)->type == QTYPE_QINT) { New condition: same. Good. > (*obj)->type = QTYPE_QFLOAT; > } > } > diff --git a/qapi/trace-events b/qapi/trace-events > index 339cacf0ad..b15a55b797 100644 > --- a/qapi/trace-events > +++ b/qapi/trace-events > @@ -11,7 +11,7 @@ visit_next_list(void *v, void *tail, size_t size) "v=%p tail=%p size=%zu" > visit_check_list(void *v) "v=%p" > visit_end_list(void *v, void *obj) "v=%p obj=%p" > > -visit_start_alternate(void *v, const char *name, void *obj, size_t size, bool promote_int) "v=%p name=%s obj=%p size=%zu promote_int=%d" > +visit_start_alternate(void *v, const char *name, void *obj, size_t size, uint32_t supported_qtypes) "v=%p name=%s obj=%p size=%zu supported_qtypes=0x%x" > visit_end_alternate(void *v, void *obj) "v=%p obj=%p" > > visit_optional(void *v, const char *name, bool *present) "v=%p name=%s present=%p" Reviewed-by: Markus Armbruster <armbru@redhat.com>
On Wed, May 10, 2017 at 03:16:56PM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > This will allow visitors to make decisions based on the supported qtypes > > of a given alternate type. The new parameter can replace the old > > 'promote_int' argument, as qobject-input-visitor can simply check if > > QTYPE_QINT is set in supported_qtypes. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > Might conflict with Marc-André's work, which I haven't reviewed, yet. > Should be easy enough to resolve, though. This series is now low-priority for me, as I'm not sure about the need for "feature=force" in x86. I won't mind having to rebase this later.
Eduardo Habkost <ehabkost@redhat.com> writes: > On Wed, May 10, 2017 at 03:16:56PM +0200, Markus Armbruster wrote: >> Eduardo Habkost <ehabkost@redhat.com> writes: >> >> > This will allow visitors to make decisions based on the supported qtypes >> > of a given alternate type. The new parameter can replace the old >> > 'promote_int' argument, as qobject-input-visitor can simply check if >> > QTYPE_QINT is set in supported_qtypes. >> > >> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> >> >> Might conflict with Marc-André's work, which I haven't reviewed, yet. >> Should be easy enough to resolve, though. > > This series is now low-priority for me, as I'm not sure about the > need for "feature=force" in x86. I won't mind having to rebase > this later. Okay. I rather like PATCH 1, and may pick it into my own (unpublished) rework of the alternate visiting code.
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 1a1b62012b..8f5a223714 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -408,7 +408,8 @@ void visit_end_list(Visitor *v, void **list); * the qtype of the next thing to be visited, stored in (*@obj)->type. * Other visitors will leave @obj unchanged. * - * If @promote_int, treat integers as QTYPE_FLOAT. + * @supported_qtypes is a bit mask indicating which QTypes are supported + * by the alternate. * * If successful, this must be paired with visit_end_alternate() with * the same @obj to clean up, even if visiting the contents of the @@ -416,7 +417,7 @@ void visit_end_list(Visitor *v, void **list); */ void visit_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, - bool promote_int, Error **errp); + uint32_t supported_qtypes, Error **errp); /* * Finish visiting an alternate type. diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h index e87709db5c..8afcde0f5d 100644 --- a/include/qapi/visitor-impl.h +++ b/include/qapi/visitor-impl.h @@ -71,7 +71,7 @@ struct Visitor * optional for output visitors. */ void (*start_alternate)(Visitor *v, const char *name, GenericAlternate **obj, size_t size, - bool promote_int, Error **errp); + uint32_t supported_qtypes, Error **errp); /* Optional, needed for dealloc visitor */ void (*end_alternate)(Visitor *v, void **obj); diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 5737aefa05..41c54982e2 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -161,20 +161,20 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s *obj, Error def gen_visit_alternate(name, variants): - promote_int = 'true' + qtypes = ['(1U << %s)' % (var.type.alternate_qtype()) + for var in variants.variants] + supported_qtypes = ' | '.join(qtypes) ret = '' - for var in variants.variants: - if var.type.alternate_qtype() == 'QTYPE_QINT': - promote_int = 'false' ret += mcgen(''' void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp) { Error *err = NULL; + uint32_t supported_qtypes = %(supported_qtypes)s; visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj), - %(promote_int)s, &err); + supported_qtypes, &err); if (err) { goto out; } @@ -183,7 +183,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error } switch ((*obj)->type) { ''', - c_name=c_name(name), promote_int=promote_int) + c_name=c_name(name), supported_qtypes=supported_qtypes) for var in variants.variants: ret += mcgen(''' diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 43a09d147d..8d62914393 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -106,15 +106,18 @@ void visit_end_list(Visitor *v, void **obj) void visit_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, - bool promote_int, Error **errp) + uint32_t supported_qtypes, + Error **errp) { Error *err = NULL; + QEMU_BUILD_BUG_ON(QTYPE__MAX > 32); + assert(obj && size >= sizeof(GenericAlternate)); assert(!(v->type & VISITOR_OUTPUT) || *obj); - trace_visit_start_alternate(v, name, obj, size, promote_int); + trace_visit_start_alternate(v, name, obj, size, supported_qtypes); if (v->start_alternate) { - v->start_alternate(v, name, obj, size, promote_int, &err); + v->start_alternate(v, name, obj, size, supported_qtypes, &err); } if (v->type & VISITOR_INPUT) { assert(v->start_alternate && !err != !*obj); diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c index 34086cbfc0..2e40da5981 100644 --- a/qapi/qapi-clone-visitor.c +++ b/qapi/qapi-clone-visitor.c @@ -70,7 +70,8 @@ static GenericList *qapi_clone_next_list(Visitor *v, GenericList *tail, static void qapi_clone_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, - bool promote_int, Error **errp) + uint32_t supported_qtypes, + Error **errp) { qapi_clone_start_struct(v, name, (void **)obj, size, errp); } diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c index e39457bc79..23b64c21a4 100644 --- a/qapi/qapi-dealloc-visitor.c +++ b/qapi/qapi-dealloc-visitor.c @@ -38,7 +38,8 @@ static void qapi_dealloc_end_struct(Visitor *v, void **obj) static void qapi_dealloc_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, - bool promote_int, Error **errp) + uint32_t supported_qtypes, + Error **errp) { } diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 865e948ac0..25997ee816 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -338,7 +338,8 @@ static void qobject_input_check_list(Visitor *v, Error **errp) static void qobject_input_start_alternate(Visitor *v, const char *name, GenericAlternate **obj, size_t size, - bool promote_int, Error **errp) + uint32_t supported_qtypes, + Error **errp) { QObjectInputVisitor *qiv = to_qiv(v); QObject *qobj = qobject_input_get_object(qiv, name, false, errp); @@ -349,7 +350,8 @@ static void qobject_input_start_alternate(Visitor *v, const char *name, } *obj = g_malloc0(size); (*obj)->type = qobject_type(qobj); - if (promote_int && (*obj)->type == QTYPE_QINT) { + if (!(supported_qtypes & (1U << QTYPE_QINT)) && + (*obj)->type == QTYPE_QINT) { (*obj)->type = QTYPE_QFLOAT; } } diff --git a/qapi/trace-events b/qapi/trace-events index 339cacf0ad..b15a55b797 100644 --- a/qapi/trace-events +++ b/qapi/trace-events @@ -11,7 +11,7 @@ visit_next_list(void *v, void *tail, size_t size) "v=%p tail=%p size=%zu" visit_check_list(void *v) "v=%p" visit_end_list(void *v, void *obj) "v=%p obj=%p" -visit_start_alternate(void *v, const char *name, void *obj, size_t size, bool promote_int) "v=%p name=%s obj=%p size=%zu promote_int=%d" +visit_start_alternate(void *v, const char *name, void *obj, size_t size, uint32_t supported_qtypes) "v=%p name=%s obj=%p size=%zu supported_qtypes=0x%x" visit_end_alternate(void *v, void *obj) "v=%p obj=%p" visit_optional(void *v, const char *name, bool *present) "v=%p name=%s present=%p"
This will allow visitors to make decisions based on the supported qtypes of a given alternate type. The new parameter can replace the old 'promote_int' argument, as qobject-input-visitor can simply check if QTYPE_QINT is set in supported_qtypes. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v1 -> v2: * Change supported_qtypes to uint32_t (Eric Blake) * Replace assert() on all generated visitor functions with a single QEMU_BUILD_BUG_ON() on visit_start_alternate() * Extra spaces around "|" on generated visitor code (Eric Blake) * Don't use bitops.h and just use (1U << QTYPE_FOO) (Markus Armbruster) --- include/qapi/visitor.h | 5 +++-- include/qapi/visitor-impl.h | 2 +- scripts/qapi-visit.py | 12 ++++++------ qapi/qapi-visit-core.c | 9 ++++++--- qapi/qapi-clone-visitor.c | 3 ++- qapi/qapi-dealloc-visitor.c | 3 ++- qapi/qobject-input-visitor.c | 6 ++++-- qapi/trace-events | 2 +- 8 files changed, 25 insertions(+), 17 deletions(-)