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>
---
include/qapi/visitor.h | 5 +++--
include/qapi/visitor-impl.h | 2 +-
scripts/qapi-visit.py | 14 ++++++++------
qapi/qapi-visit-core.c | 7 ++++---
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..8c2bff4a05 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);
+ unsigned long 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..50c2b2feef 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);
+ unsigned long 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..ebf7e67109 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -161,20 +161,21 @@ 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 = ['BIT(%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;
+ unsigned long supported_qtypes = %(supported_qtypes)s;
+ assert(QTYPE__MAX < BITS_PER_LONG);
visit_start_alternate(v, name, (GenericAlternate **)obj, sizeof(**obj),
- %(promote_int)s, &err);
+ supported_qtypes, &err);
if (err) {
goto out;
}
@@ -183,7 +184,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('''
@@ -375,6 +376,7 @@ h_comment = '''
fdef.write(mcgen('''
#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
#include "qemu-common.h"
#include "qapi/error.h"
#include "%(prefix)sqapi-visit.h"
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 43a09d147d..784ba1b756 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -106,15 +106,16 @@ 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)
+ unsigned long supported_qtypes,
+ Error **errp)
{
Error *err = NULL;
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..5550871488 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)
+ unsigned long 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..ef83d1420b 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)
+ unsigned long supported_qtypes,
+ Error **errp)
{
}
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 865e948ac0..393220b3a6 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -22,6 +22,7 @@
#include "qapi/qmp/types.h"
#include "qapi/qmp/qerror.h"
#include "qemu/cutils.h"
+#include "qemu/bitops.h"
#include "qemu/option.h"
typedef struct StackObject {
@@ -338,7 +339,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)
+ unsigned long supported_qtypes,
+ Error **errp)
{
QObjectInputVisitor *qiv = to_qiv(v);
QObject *qobj = qobject_input_get_object(qiv, name, false, errp);
@@ -349,7 +351,7 @@ 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 & BIT(QTYPE_QINT)) && (*obj)->type == QTYPE_QINT) {
(*obj)->type = QTYPE_QFLOAT;
}
}
diff --git a/qapi/trace-events b/qapi/trace-events
index 339cacf0ad..db42dd8353 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, unsigned long supported_qtypes) "v=%p name=%s obj=%p size=%zu supported_qtypes=0x%lx"
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"
--
2.11.0.259.g40922b1
On 05/02/2017 03:31 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>
> ---
> @@ -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);
> + unsigned long supported_qtypes, Error **errp);
Why unsigned long (which is platform-dependent in size)? At the moment,
even unsigned char happens to be long enough, although I probably would
have used uint32_t.
Oh, I see, it's because you use the BIT() macros from bitops.h, which
are hardcoded to unsigned long.
> +++ b/scripts/qapi-visit.py
> @@ -161,20 +161,21 @@ 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 = ['BIT(%s)' % (var.type.alternate_qtype())
> + for var in variants.variants]
> + supported_qtypes = '|'.join(qtypes)
Do you want ' | '.join(qtypes), so that at least the generated code
still follows recommended operator spacing? (The line is long no matter
what, though, and that's not worth worrying about.)
> 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;
> + unsigned long supported_qtypes = %(supported_qtypes)s;
>
> + assert(QTYPE__MAX < BITS_PER_LONG);
Do we really have to generate a separate copy of this assert in every
generated function? Especially when we know it is true by construction,
that seems like a lot. Having the assertion once in a .c file rather
than generated in multiple functions might be acceptable, though.
> +++ b/qapi/qobject-input-visitor.c
> @@ -349,7 +351,7 @@ 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 & BIT(QTYPE_QINT)) && (*obj)->type == QTYPE_QINT) {
Experimenting, does this read any better:
if (!extract32(supported_qtypes, QTYPE_QINT, 1) && ...
which would be another argument for uint32_t instead of unsigned long in
the signature.
The idea makes sense, but I'm still not necessarily sold on using a long.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On Tue, May 02, 2017 at 04:29:32PM -0500, Eric Blake wrote:
> On 05/02/2017 03:31 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>
> > ---
>
> > @@ -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);
> > + unsigned long supported_qtypes, Error **errp);
>
> Why unsigned long (which is platform-dependent in size)? At the moment,
> even unsigned char happens to be long enough, although I probably would
> have used uint32_t.
>
> Oh, I see, it's because you use the BIT() macros from bitops.h, which
> are hardcoded to unsigned long.
Yep. But I don't see a problem with using uint32_t or a simple
int.
>
> > +++ b/scripts/qapi-visit.py
> > @@ -161,20 +161,21 @@ 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 = ['BIT(%s)' % (var.type.alternate_qtype())
> > + for var in variants.variants]
> > + supported_qtypes = '|'.join(qtypes)
>
> Do you want ' | '.join(qtypes), so that at least the generated code
> still follows recommended operator spacing? (The line is long no matter
> what, though, and that's not worth worrying about.)
I can do that in v2.
>
> > 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;
> > + unsigned long supported_qtypes = %(supported_qtypes)s;
> >
> > + assert(QTYPE__MAX < BITS_PER_LONG);
>
> Do we really have to generate a separate copy of this assert in every
> generated function? Especially when we know it is true by construction,
> that seems like a lot. Having the assertion once in a .c file rather
> than generated in multiple functions might be acceptable, though.
I will probably do this as a single QEMU_BUILD_BUG_ON near
visit_start_alternate().
>
> > +++ b/qapi/qobject-input-visitor.c
>
> > @@ -349,7 +351,7 @@ 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 & BIT(QTYPE_QINT)) && (*obj)->type == QTYPE_QINT) {
>
> Experimenting, does this read any better:
>
> if (!extract32(supported_qtypes, QTYPE_QINT, 1) && ...
>
> which would be another argument for uint32_t instead of unsigned long in
> the signature.
I am more used to see this written as "if (s & (1UL << n))" than
as "if (extract32(s, n, 1))", so I'm not sure.
I see some extract32(..., ..., 1) cases in the tree, so it's not
as unusual as I thought. I will probably give it a try.
>
> The idea makes sense, but I'm still not necessarily sold on using a long.
Thanks!
--
Eduardo
© 2016 - 2026 Red Hat, Inc.