[Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force"

[Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force"

[Eduardo Habkost]

This series implements "-cpu feature=force", to allow a feature
to be forcefully enabled even if the underlying accelerator
report it as unsupported. This feature should be used only for
testing and debugging.

We use a new QAPI alternate type to keep command-line
compatibility, and to keep compatibility with code that reads the
existing feature QOM properties and expects boolean values.

Eduardo Habkost (4):
  visitor: Add 'supported_qtypes' parameter to visit_start_alternate()
  string-input-visitor: Support alternate types
  tests: Add [+-]feature and feature=on|off test cases
  x86: Support feature=force on the command-line

 qapi-schema.json                        |  32 +++++++++
 include/qapi/visitor.h                  |   5 +-
 include/qapi/visitor-impl.h             |   2 +-
 scripts/qapi-visit.py                   |  14 ++--
 target/i386/cpu.h                       |   2 +
 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/string-input-visitor.c             |  71 ++++++++++++++++---
 target/i386/cpu.c                       |  55 +++++++++++----
 tests/test-string-input-visitor.c       |  89 ++++++++++++++++++++++++
 tests/test-x86-cpuid-compat.c           | 119 ++++++++++++++++++++++++++++++++
 qapi/trace-events                       |   2 +-
 tests/qapi-schema/qapi-schema-test.json |   8 +++
 15 files changed, 379 insertions(+), 39 deletions(-)

-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 1/4] visitor: Add 'supported_qtypes' parameter to visit_start_alternate()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 1/4] visitor: Add 'supported_qtypes' parameter to visit_start_alternate()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3046 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/4] visitor: Add 'supported_qtypes' parameter to visit_start_alternate()

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH 1/4] visitor: Add 'supported_qtypes' parameter to visit_start_alternate()

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> 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.

Yes, please.

>> 
>> >      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().

Yes, please.

>> 
>> > +++ 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.

I think (s & (1ul << n)) reads okay.  extract32() I have to look up.  If
we used it all the time, extract32() might be the better choice.

>> The idea makes sense, but I'm still not necessarily sold on using a long.
>
> Thanks!

The generalization from bool to set of QType makes sense to me.
However, bitmap.h/bitops.h seems overkill.  It's for bit vectors of
arbitrary size, represented as unsigned long[].  What we got here is a
need for just eight bits, vanishingly unlikely to grow much.

If we decide we like BIT(n) so much better than the obvious (1ul << n),
let's at least include just bitops.h like you did in one place, not
bitmaps.h like you did in the other place.

As long as we're using no more than BIT(), there's no real need for
unsigned long[1] (which you shortened to just unsigned long).  Plain
unsigned would do.  I could also accept Eric's uint32_t.

[Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eduardo Habkost]

When parsing alternates from a string, there are some limitations in
what we can do, but it is a valid use case in some situations. We can
support booleans, integer types, and enums.

This will be used to support 'feature=force' in -cpu options, while
keeping 'feature=on|off|true|false' represented as boolean values.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qapi/string-input-visitor.c             | 71 ++++++++++++++++++++++----
 tests/test-string-input-visitor.c       | 89 +++++++++++++++++++++++++++++++++
 tests/qapi-schema/qapi-schema-test.json |  8 +++
 3 files changed, 158 insertions(+), 10 deletions(-)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index c089491c24..bf8f58748b 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -19,6 +19,7 @@
 #include "qemu/option.h"
 #include "qemu/queue.h"
 #include "qemu/range.h"
+#include "qemu/bitops.h"
 
 
 struct StringInputVisitor
@@ -278,21 +279,34 @@ static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,
     *obj = val;
 }
 
+static int try_parse_bool(const char *s, bool *result)
+{
+    if (!strcasecmp(s, "on") ||
+        !strcasecmp(s, "yes") ||
+        !strcasecmp(s, "true")) {
+        if (result) {
+            *result = true;
+        }
+        return 0;
+    }
+    if (!strcasecmp(s, "off") ||
+        !strcasecmp(s, "no") ||
+        !strcasecmp(s, "false")) {
+        if (result) {
+            *result = false;
+        }
+        return 0;
+    }
+
+    return -1;
+}
+
 static void parse_type_bool(Visitor *v, const char *name, bool *obj,
                             Error **errp)
 {
     StringInputVisitor *siv = to_siv(v);
 
-    if (!strcasecmp(siv->string, "on") ||
-        !strcasecmp(siv->string, "yes") ||
-        !strcasecmp(siv->string, "true")) {
-        *obj = true;
-        return;
-    }
-    if (!strcasecmp(siv->string, "off") ||
-        !strcasecmp(siv->string, "no") ||
-        !strcasecmp(siv->string, "false")) {
-        *obj = false;
+    if (try_parse_bool(siv->string, obj) == 0) {
         return;
     }
 
@@ -326,6 +340,42 @@ static void parse_type_number(Visitor *v, const char *name, double *obj,
     *obj = val;
 }
 
+/* Support for alternates on string-input-visitor is limited, because
+ * the input string doesn't have any type information.
+ *
+ * Supported alternate member types:
+ * 1) enums
+ * 2) integer types
+ * 3) booleans (but only if the there's no enum variant
+ *    containing "on", "off", "true", or "false" as members)
+ *
+ * UNSUPPORTED alternate member types:
+ * 1) strings
+ * 2) complex types
+ */
+static void start_alternate(Visitor *v, const char *name,
+                            GenericAlternate **obj, size_t size,
+                            unsigned long supported_qtypes, Error **errp)
+{
+    StringInputVisitor *siv = to_siv(v);
+    QType t = QTYPE_QSTRING;
+
+    if (supported_qtypes & BIT(QTYPE_QBOOL)) {
+        if (try_parse_bool(siv->string, NULL) == 0) {
+            t = QTYPE_QBOOL;
+        }
+    }
+
+    if (supported_qtypes & BIT(QTYPE_QINT)) {
+        if (parse_str(siv, name, NULL) == 0) {
+            t = QTYPE_QINT;
+        }
+    }
+
+    *obj = g_malloc0(size);
+    (*obj)->type = t;
+}
+
 static void string_input_free(Visitor *v)
 {
     StringInputVisitor *siv = to_siv(v);
@@ -353,6 +403,7 @@ Visitor *string_input_visitor_new(const char *str)
     v->visitor.next_list = next_list;
     v->visitor.check_list = check_list;
     v->visitor.end_list = end_list;
+    v->visitor.start_alternate = start_alternate;
     v->visitor.free = string_input_free;
 
     v->string = str;
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 79313a7f7a..1e867d62c9 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -290,6 +290,91 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
     }
 }
 
+static void test_visitor_in_alt_bool_enum(TestInputVisitorData *data,
+                                 const void *unused)
+{
+    Error *err = NULL;
+    Visitor *v;
+    AltBoolEnum *be = NULL;
+
+    v = visitor_input_test_init(data, "true");
+    visit_type_AltBoolEnum(v, NULL, &be, &err);
+    g_assert(!err);
+    g_assert_cmpint(be->type, ==, QTYPE_QBOOL);
+    g_assert(be->u.b);
+    qapi_free_AltBoolEnum(be);
+
+    v = visitor_input_test_init(data, "off");
+    visit_type_AltBoolEnum(v, NULL, &be, &err);
+    g_assert(!err);
+    g_assert_cmpint(be->type, ==, QTYPE_QBOOL);
+    g_assert(!be->u.b);
+    qapi_free_AltBoolEnum(be);
+
+    v = visitor_input_test_init(data, "value2");
+    visit_type_AltBoolEnum(v, NULL, &be, &err);
+    g_assert(!err);
+    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
+    g_assert_cmpint(be->u.e, ==, ENUM_ONE_VALUE2);
+    qapi_free_AltBoolEnum(be);
+
+    v = visitor_input_test_init(data, "value100");
+    visit_type_AltBoolEnum(v, NULL, &be, &err);
+    error_free_or_abort(&err);
+    qapi_free_AltBoolEnum(be);
+
+    v = visitor_input_test_init(data, "10");
+    visit_type_AltBoolEnum(v, NULL, &be, &err);
+    error_free_or_abort(&err);
+    qapi_free_AltBoolEnum(be);
+}
+
+static void test_visitor_in_alt_enum_int(TestInputVisitorData *data,
+                                 const void *unused)
+{
+    Error *err = NULL;
+    Visitor *v;
+    AltOnOffInt *be = NULL;
+
+    v = visitor_input_test_init(data, "on");
+    visit_type_AltOnOffInt(v, NULL, &be, &err);
+    g_assert(!err);
+    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
+    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_ON);
+    qapi_free_AltOnOffInt(be);
+
+    v = visitor_input_test_init(data, "off");
+    visit_type_AltOnOffInt(v, NULL, &be, &err);
+    g_assert(!err);
+    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
+    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_OFF);
+    qapi_free_AltOnOffInt(be);
+
+    v = visitor_input_test_init(data, "auto");
+    visit_type_AltOnOffInt(v, NULL, &be, &err);
+    g_assert(!err);
+    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
+    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_AUTO);
+    qapi_free_AltOnOffInt(be);
+
+    v = visitor_input_test_init(data, "-12345");
+    visit_type_AltOnOffInt(v, NULL, &be, &err);
+    g_assert(!err);
+    g_assert_cmpint(be->type, ==, QTYPE_QINT);
+    g_assert_cmpint(be->u.i, ==, -12345);
+    qapi_free_AltOnOffInt(be);
+
+    v = visitor_input_test_init(data, "true");
+    visit_type_AltOnOffInt(v, NULL, &be, &err);
+    error_free_or_abort(&err);
+    qapi_free_AltOnOffInt(be);
+
+    v = visitor_input_test_init(data, "value2");
+    visit_type_AltOnOffInt(v, NULL, &be, &err);
+    error_free_or_abort(&err);
+    qapi_free_AltOnOffInt(be);
+}
+
 /* Try to crash the visitors */
 static void test_visitor_in_fuzz(TestInputVisitorData *data,
                                  const void *unused)
@@ -366,6 +451,10 @@ int main(int argc, char **argv)
                             &in_visitor_data, test_visitor_in_string);
     input_visitor_test_add("/string-visitor/input/enum",
                             &in_visitor_data, test_visitor_in_enum);
+    input_visitor_test_add("/string-visitor/input/alternate/bool_enum",
+                            &in_visitor_data, test_visitor_in_alt_bool_enum);
+    input_visitor_test_add("/string-visitor/input/alternate/enum_int",
+                            &in_visitor_data, test_visitor_in_alt_enum_int);
     input_visitor_test_add("/string-visitor/input/fuzz",
                             &in_visitor_data, test_visitor_in_fuzz);
 
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 842ea3c5e3..231c8952e8 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -106,6 +106,14 @@
 { 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } }
 { 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } }
 
+
+# for testing string-input-visitor handling of alternates:
+{ 'enum': 'TestOnOffAuto',
+  'data': [ 'on', 'off', 'auto' ] }
+
+{ 'alternate': 'AltBoolEnum', 'data': { 'b': 'bool', 'e': 'EnumOne' } }
+{ 'alternate': 'AltOnOffInt', 'data': { 'o': 'TestOnOffAuto', 'i': 'int' } }
+
 # for testing native lists
 { 'union': 'UserDefNativeListUnion',
   'data': { 'integer': ['int'],
-- 
2.11.0.259.g40922b1

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

On 05/02/2017 03:31 PM, Eduardo Habkost wrote:
> When parsing alternates from a string, there are some limitations in
> what we can do, but it is a valid use case in some situations. We can
> support booleans, integer types, and enums.
> 
> This will be used to support 'feature=force' in -cpu options, while
> keeping 'feature=on|off|true|false' represented as boolean values.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---

>  
> +/* Support for alternates on string-input-visitor is limited, because
> + * the input string doesn't have any type information.
> + *
> + * Supported alternate member types:
> + * 1) enums
> + * 2) integer types
> + * 3) booleans (but only if the there's no enum variant
> + *    containing "on", "off", "true", or "false" as members)
> + *
> + * UNSUPPORTED alternate member types:
> + * 1) strings
> + * 2) complex types
> + */
> +static void start_alternate(Visitor *v, const char *name,
> +                            GenericAlternate **obj, size_t size,
> +                            unsigned long supported_qtypes, Error **errp)
> +{
> +    StringInputVisitor *siv = to_siv(v);
> +    QType t = QTYPE_QSTRING;

Why do you document string as unsupported, and yet default to
QTYPE_QSTRING?  I don't see how a string is fundamentally different from
an enum (no alternate can have both at the same time, an alternate with
either type will have QTYPE_QSTRING set in supported_qtypes).

> +
> +    if (supported_qtypes & BIT(QTYPE_QBOOL)) {
> +        if (try_parse_bool(siv->string, NULL) == 0) {
> +            t = QTYPE_QBOOL;
> +        }
> +    }
> +
> +    if (supported_qtypes & BIT(QTYPE_QINT)) {
> +        if (parse_str(siv, name, NULL) == 0) {
> +            t = QTYPE_QINT;
> +        }
> +    }
> +
> +    *obj = g_malloc0(size);
> +    (*obj)->type = t;

Should you raise an error if you couldn't match the input with
supported_qtypes, rather than just blindly returning QTYPE_QSTRING?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eduardo Habkost]

On Tue, May 02, 2017 at 04:37:03PM -0500, Eric Blake wrote:
> On 05/02/2017 03:31 PM, Eduardo Habkost wrote:
> > When parsing alternates from a string, there are some limitations in
> > what we can do, but it is a valid use case in some situations. We can
> > support booleans, integer types, and enums.
> > 
> > This will be used to support 'feature=force' in -cpu options, while
> > keeping 'feature=on|off|true|false' represented as boolean values.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> 
> >  
> > +/* Support for alternates on string-input-visitor is limited, because
> > + * the input string doesn't have any type information.
> > + *
> > + * Supported alternate member types:
> > + * 1) enums
> > + * 2) integer types
> > + * 3) booleans (but only if the there's no enum variant
> > + *    containing "on", "off", "true", or "false" as members)
> > + *
> > + * UNSUPPORTED alternate member types:
> > + * 1) strings
> > + * 2) complex types
> > + */
> > +static void start_alternate(Visitor *v, const char *name,
> > +                            GenericAlternate **obj, size_t size,
> > +                            unsigned long supported_qtypes, Error **errp)
> > +{
> > +    StringInputVisitor *siv = to_siv(v);
> > +    QType t = QTYPE_QSTRING;
> 
> Why do you document string as unsupported, and yet default to
> QTYPE_QSTRING?  I don't see how a string is fundamentally different from
> an enum (no alternate can have both at the same time, an alternate with
> either type will have QTYPE_QSTRING set in supported_qtypes).

Strings are indistinguishable from enums inside
start_alternate(), that's true. But I wanted to explicitly
document 'str' as unsupported by string-input-visitor because
some string values would necessarily overlap with the string
representation of other variants.

> 
> > +
> > +    if (supported_qtypes & BIT(QTYPE_QBOOL)) {
> > +        if (try_parse_bool(siv->string, NULL) == 0) {
> > +            t = QTYPE_QBOOL;
> > +        }
> > +    }
> > +
> > +    if (supported_qtypes & BIT(QTYPE_QINT)) {
> > +        if (parse_str(siv, name, NULL) == 0) {
> > +            t = QTYPE_QINT;
> > +        }
> > +    }
> > +
> > +    *obj = g_malloc0(size);
> > +    (*obj)->type = t;
> 
> Should you raise an error if you couldn't match the input with
> supported_qtypes, rather than just blindly returning QTYPE_QSTRING?

The generated visitors calling visit_start_alternate() already
generate a (more specific and more useful) error message when
they see unsupported types.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> When parsing alternates from a string, there are some limitations in
> what we can do, but it is a valid use case in some situations. We can
> support booleans, integer types, and enums.
>
> This will be used to support 'feature=force' in -cpu options, while
> keeping 'feature=on|off|true|false' represented as boolean values.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qapi/string-input-visitor.c             | 71 ++++++++++++++++++++++----
>  tests/test-string-input-visitor.c       | 89 +++++++++++++++++++++++++++++++++
>  tests/qapi-schema/qapi-schema-test.json |  8 +++
>  3 files changed, 158 insertions(+), 10 deletions(-)
>
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index c089491c24..bf8f58748b 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -19,6 +19,7 @@
>  #include "qemu/option.h"
>  #include "qemu/queue.h"
>  #include "qemu/range.h"
> +#include "qemu/bitops.h"
>  
>  
>  struct StringInputVisitor
> @@ -278,21 +279,34 @@ static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,
>      *obj = val;
>  }
>  
> +static int try_parse_bool(const char *s, bool *result)
> +{
> +    if (!strcasecmp(s, "on") ||
> +        !strcasecmp(s, "yes") ||
> +        !strcasecmp(s, "true")) {
> +        if (result) {
> +            *result = true;
> +        }
> +        return 0;
> +    }
> +    if (!strcasecmp(s, "off") ||
> +        !strcasecmp(s, "no") ||
> +        !strcasecmp(s, "false")) {
> +        if (result) {
> +            *result = false;
> +        }
> +        return 0;
> +    }
> +
> +    return -1;
> +}
> +
>  static void parse_type_bool(Visitor *v, const char *name, bool *obj,
>                              Error **errp)
>  {
>      StringInputVisitor *siv = to_siv(v);
>  
> -    if (!strcasecmp(siv->string, "on") ||
> -        !strcasecmp(siv->string, "yes") ||
> -        !strcasecmp(siv->string, "true")) {
> -        *obj = true;
> -        return;
> -    }
> -    if (!strcasecmp(siv->string, "off") ||
> -        !strcasecmp(siv->string, "no") ||
> -        !strcasecmp(siv->string, "false")) {
> -        *obj = false;
> +    if (try_parse_bool(siv->string, obj) == 0) {
>          return;
>      }
>  
> @@ -326,6 +340,42 @@ static void parse_type_number(Visitor *v, const char *name, double *obj,
>      *obj = val;
>  }
>  
> +/* Support for alternates on string-input-visitor is limited, because

Please wing both ends of your comments, like this:

   /*
    * Comment text
    */

> + * the input string doesn't have any type information.
> + *
> + * Supported alternate member types:
> + * 1) enums
> + * 2) integer types

Better add "(but only if there 's no enum member consisting only of
digits)".

> + * 3) booleans (but only if the there's no enum variant

Make that "(but only if there's no enum member".

> + *    containing "on", "off", "true", or "false" as members)

Begs the question what happens when you violate these restrictions.

> + *
> + * UNSUPPORTED alternate member types:
> + * 1) strings
> + * 2) complex types
> + */

This comment explains the visitor's restrictions for alternate types.
It does not explain how the function works.  The proper place for
explaining the visitor's restriction is the comment in
string-input-visitor.h, which you need to update anyway, because right
now it claims visiting alternates isn't implemented.

> +static void start_alternate(Visitor *v, const char *name,
> +                            GenericAlternate **obj, size_t size,
> +                            unsigned long supported_qtypes, Error **errp)
> +{
> +    StringInputVisitor *siv = to_siv(v);
> +    QType t = QTYPE_QSTRING;

We usually call such variables @type or @qtype.

QTYPE_QSTRING means enum here.  Worth a comment, I think.

> +
> +    if (supported_qtypes & BIT(QTYPE_QBOOL)) {
> +        if (try_parse_bool(siv->string, NULL) == 0) {
> +            t = QTYPE_QBOOL;
> +        }
> +    }
> +
> +    if (supported_qtypes & BIT(QTYPE_QINT)) {
> +        if (parse_str(siv, name, NULL) == 0) {

parse_str() alters *siv.  Why is that harmless?

Aside: parse_str() is basically horrible, but that's not your fault.

> +            t = QTYPE_QINT;
> +        }
> +    }
> +
> +    *obj = g_malloc0(size);
> +    (*obj)->type = t;
> +}
> +
>  static void string_input_free(Visitor *v)
>  {
>      StringInputVisitor *siv = to_siv(v);
> @@ -353,6 +403,7 @@ Visitor *string_input_visitor_new(const char *str)
>      v->visitor.next_list = next_list;
>      v->visitor.check_list = check_list;
>      v->visitor.end_list = end_list;
> +    v->visitor.start_alternate = start_alternate;
>      v->visitor.free = string_input_free;
>  
>      v->string = str;
> diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
> index 79313a7f7a..1e867d62c9 100644
> --- a/tests/test-string-input-visitor.c
> +++ b/tests/test-string-input-visitor.c
> @@ -290,6 +290,91 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
>      }
>  }
>  
> +static void test_visitor_in_alt_bool_enum(TestInputVisitorData *data,
> +                                 const void *unused)

Indentation's off.

> +{
> +    Error *err = NULL;
> +    Visitor *v;
> +    AltBoolEnum *be = NULL;
> +
> +    v = visitor_input_test_init(data, "true");
> +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> +    g_assert(!err);

What's wrong with &error_abort?  More of the same below.

> +    g_assert_cmpint(be->type, ==, QTYPE_QBOOL);
> +    g_assert(be->u.b);
> +    qapi_free_AltBoolEnum(be);
> +
> +    v = visitor_input_test_init(data, "off");
> +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> +    g_assert(!err);
> +    g_assert_cmpint(be->type, ==, QTYPE_QBOOL);
> +    g_assert(!be->u.b);
> +    qapi_free_AltBoolEnum(be);
> +
> +    v = visitor_input_test_init(data, "value2");
> +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> +    g_assert(!err);
> +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> +    g_assert_cmpint(be->u.e, ==, ENUM_ONE_VALUE2);
> +    qapi_free_AltBoolEnum(be);
> +
> +    v = visitor_input_test_init(data, "value100");
> +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> +    error_free_or_abort(&err);
> +    qapi_free_AltBoolEnum(be);
> +
> +    v = visitor_input_test_init(data, "10");
> +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> +    error_free_or_abort(&err);
> +    qapi_free_AltBoolEnum(be);
> +}
> +
> +static void test_visitor_in_alt_enum_int(TestInputVisitorData *data,
> +                                 const void *unused)

Indentation's off.

> +{
> +    Error *err = NULL;
> +    Visitor *v;
> +    AltOnOffInt *be = NULL;
> +
> +    v = visitor_input_test_init(data, "on");
> +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> +    g_assert(!err);
> +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> +    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_ON);
> +    qapi_free_AltOnOffInt(be);
> +
> +    v = visitor_input_test_init(data, "off");
> +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> +    g_assert(!err);
> +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> +    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_OFF);
> +    qapi_free_AltOnOffInt(be);
> +
> +    v = visitor_input_test_init(data, "auto");
> +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> +    g_assert(!err);
> +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> +    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_AUTO);
> +    qapi_free_AltOnOffInt(be);
> +
> +    v = visitor_input_test_init(data, "-12345");
> +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> +    g_assert(!err);
> +    g_assert_cmpint(be->type, ==, QTYPE_QINT);
> +    g_assert_cmpint(be->u.i, ==, -12345);
> +    qapi_free_AltOnOffInt(be);
> +
> +    v = visitor_input_test_init(data, "true");
> +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> +    error_free_or_abort(&err);
> +    qapi_free_AltOnOffInt(be);
> +
> +    v = visitor_input_test_init(data, "value2");
> +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> +    error_free_or_abort(&err);
> +    qapi_free_AltOnOffInt(be);
> +}
> +

Could we use tests covering alternates that don't satisfy the string
visitor's restrictions?  Like OnOffAuto + bool.

>  /* Try to crash the visitors */
>  static void test_visitor_in_fuzz(TestInputVisitorData *data,
>                                   const void *unused)
> @@ -366,6 +451,10 @@ int main(int argc, char **argv)
>                              &in_visitor_data, test_visitor_in_string);
>      input_visitor_test_add("/string-visitor/input/enum",
>                              &in_visitor_data, test_visitor_in_enum);
> +    input_visitor_test_add("/string-visitor/input/alternate/bool_enum",
> +                            &in_visitor_data, test_visitor_in_alt_bool_enum);
> +    input_visitor_test_add("/string-visitor/input/alternate/enum_int",
> +                            &in_visitor_data, test_visitor_in_alt_enum_int);
>      input_visitor_test_add("/string-visitor/input/fuzz",
>                              &in_visitor_data, test_visitor_in_fuzz);
>  
> diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> index 842ea3c5e3..231c8952e8 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -106,6 +106,14 @@
>  { 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } }
>  { 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } }
>  
> +
> +# for testing string-input-visitor handling of alternates:
> +{ 'enum': 'TestOnOffAuto',
> +  'data': [ 'on', 'off', 'auto' ] }
> +
> +{ 'alternate': 'AltBoolEnum', 'data': { 'b': 'bool', 'e': 'EnumOne' } }
> +{ 'alternate': 'AltOnOffInt', 'data': { 'o': 'TestOnOffAuto', 'i': 'int' } }
> +
>  # for testing native lists
>  { 'union': 'UserDefNativeListUnion',
>    'data': { 'integer': ['int'],

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eduardo Habkost]

On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > When parsing alternates from a string, there are some limitations in
> > what we can do, but it is a valid use case in some situations. We can
> > support booleans, integer types, and enums.
> >
> > This will be used to support 'feature=force' in -cpu options, while
> > keeping 'feature=on|off|true|false' represented as boolean values.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  qapi/string-input-visitor.c             | 71 ++++++++++++++++++++++----
> >  tests/test-string-input-visitor.c       | 89 +++++++++++++++++++++++++++++++++
> >  tests/qapi-schema/qapi-schema-test.json |  8 +++
> >  3 files changed, 158 insertions(+), 10 deletions(-)
> >
> > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > index c089491c24..bf8f58748b 100644
> > --- a/qapi/string-input-visitor.c
> > +++ b/qapi/string-input-visitor.c
> > @@ -19,6 +19,7 @@
> >  #include "qemu/option.h"
> >  #include "qemu/queue.h"
> >  #include "qemu/range.h"
> > +#include "qemu/bitops.h"
> >  
> >  
> >  struct StringInputVisitor
> > @@ -278,21 +279,34 @@ static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,
> >      *obj = val;
> >  }
> >  
> > +static int try_parse_bool(const char *s, bool *result)
> > +{
> > +    if (!strcasecmp(s, "on") ||
> > +        !strcasecmp(s, "yes") ||
> > +        !strcasecmp(s, "true")) {
> > +        if (result) {
> > +            *result = true;
> > +        }
> > +        return 0;
> > +    }
> > +    if (!strcasecmp(s, "off") ||
> > +        !strcasecmp(s, "no") ||
> > +        !strcasecmp(s, "false")) {
> > +        if (result) {
> > +            *result = false;
> > +        }
> > +        return 0;
> > +    }
> > +
> > +    return -1;
> > +}
> > +
> >  static void parse_type_bool(Visitor *v, const char *name, bool *obj,
> >                              Error **errp)
> >  {
> >      StringInputVisitor *siv = to_siv(v);
> >  
> > -    if (!strcasecmp(siv->string, "on") ||
> > -        !strcasecmp(siv->string, "yes") ||
> > -        !strcasecmp(siv->string, "true")) {
> > -        *obj = true;
> > -        return;
> > -    }
> > -    if (!strcasecmp(siv->string, "off") ||
> > -        !strcasecmp(siv->string, "no") ||
> > -        !strcasecmp(siv->string, "false")) {
> > -        *obj = false;
> > +    if (try_parse_bool(siv->string, obj) == 0) {
> >          return;
> >      }
> >  
> > @@ -326,6 +340,42 @@ static void parse_type_number(Visitor *v, const char *name, double *obj,
> >      *obj = val;
> >  }
> >  
> > +/* Support for alternates on string-input-visitor is limited, because
> 
> Please wing both ends of your comments, like this:
> 
>    /*
>     * Comment text
>     */

Will do.

> 
> > + * the input string doesn't have any type information.
> > + *
> > + * Supported alternate member types:
> > + * 1) enums
> > + * 2) integer types
> 
> Better add "(but only if there 's no enum member consisting only of
> digits)".

Oh, I didn't know this was possible.

> 
> > + * 3) booleans (but only if the there's no enum variant
> 
> Make that "(but only if there's no enum member".

Will do.


> 
> > + *    containing "on", "off", "true", or "false" as members)
> 
> Begs the question what happens when you violate these restrictions.

Right now, we don't detect those cases and behavior is undefined.
I think it will be a good idea to give start_alternate() enough
information to detect those cases (by adding a 'const char *const
enum_table[]' parameter).

> 
> > + *
> > + * UNSUPPORTED alternate member types:
> > + * 1) strings
> > + * 2) complex types
> > + */
> 
> This comment explains the visitor's restrictions for alternate types.
> It does not explain how the function works.  The proper place for
> explaining the visitor's restriction is the comment in
> string-input-visitor.h, which you need to update anyway, because right
> now it claims visiting alternates isn't implemented.

Right. I will take a look.

> 
> > +static void start_alternate(Visitor *v, const char *name,
> > +                            GenericAlternate **obj, size_t size,
> > +                            unsigned long supported_qtypes, Error **errp)
> > +{
> > +    StringInputVisitor *siv = to_siv(v);
> > +    QType t = QTYPE_QSTRING;
> 
> We usually call such variables @type or @qtype.

I will change it.

> 
> QTYPE_QSTRING means enum here.  Worth a comment, I think.

I will update it.

> 
> > +
> > +    if (supported_qtypes & BIT(QTYPE_QBOOL)) {
> > +        if (try_parse_bool(siv->string, NULL) == 0) {
> > +            t = QTYPE_QBOOL;
> > +        }
> > +    }
> > +
> > +    if (supported_qtypes & BIT(QTYPE_QINT)) {
> > +        if (parse_str(siv, name, NULL) == 0) {
> 
> parse_str() alters *siv.  Why is that harmless?

I remember concluding it was harmless, but I don't remember the
details.

I will probably drop support for integers in the next version,
and let whoever needs integer member support on alternates fix
parse_str().

> 
> Aside: parse_str() is basically horrible, but that's not your fault.

That's an understatement. :)

> 
> > +            t = QTYPE_QINT;
> > +        }
> > +    }
> > +
> > +    *obj = g_malloc0(size);
> > +    (*obj)->type = t;
> > +}
> > +
> >  static void string_input_free(Visitor *v)
> >  {
> >      StringInputVisitor *siv = to_siv(v);
> > @@ -353,6 +403,7 @@ Visitor *string_input_visitor_new(const char *str)
> >      v->visitor.next_list = next_list;
> >      v->visitor.check_list = check_list;
> >      v->visitor.end_list = end_list;
> > +    v->visitor.start_alternate = start_alternate;
> >      v->visitor.free = string_input_free;
> >  
> >      v->string = str;
> > diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
> > index 79313a7f7a..1e867d62c9 100644
> > --- a/tests/test-string-input-visitor.c
> > +++ b/tests/test-string-input-visitor.c
> > @@ -290,6 +290,91 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
> >      }
> >  }
> >  
> > +static void test_visitor_in_alt_bool_enum(TestInputVisitorData *data,
> > +                                 const void *unused)
> 
> Indentation's off.

Will fix it.

> 
> > +{
> > +    Error *err = NULL;
> > +    Visitor *v;
> > +    AltBoolEnum *be = NULL;
> > +
> > +    v = visitor_input_test_init(data, "true");
> > +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> > +    g_assert(!err);
> 
> What's wrong with &error_abort?  More of the same below.

Most test cases in test-string-input-visitor.c use
g_asserto(!err) instead of error_abort.

I prefer the former, because I read &error_abort as "if this
causes an error, something is wrong with the caller [the test
code itself]", and g_assert(!err) as "if this assert is
triggered, the test code detected a bug".

Also, if that function call returns an error, I want to know
which line on which test case failed. &error_abort would hide
that information.

> 
> > +    g_assert_cmpint(be->type, ==, QTYPE_QBOOL);
> > +    g_assert(be->u.b);
> > +    qapi_free_AltBoolEnum(be);
> > +
> > +    v = visitor_input_test_init(data, "off");
> > +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> > +    g_assert(!err);
> > +    g_assert_cmpint(be->type, ==, QTYPE_QBOOL);
> > +    g_assert(!be->u.b);
> > +    qapi_free_AltBoolEnum(be);
> > +
> > +    v = visitor_input_test_init(data, "value2");
> > +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> > +    g_assert(!err);
> > +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> > +    g_assert_cmpint(be->u.e, ==, ENUM_ONE_VALUE2);
> > +    qapi_free_AltBoolEnum(be);
> > +
> > +    v = visitor_input_test_init(data, "value100");
> > +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> > +    error_free_or_abort(&err);
> > +    qapi_free_AltBoolEnum(be);
> > +
> > +    v = visitor_input_test_init(data, "10");
> > +    visit_type_AltBoolEnum(v, NULL, &be, &err);
> > +    error_free_or_abort(&err);
> > +    qapi_free_AltBoolEnum(be);
> > +}
> > +
> > +static void test_visitor_in_alt_enum_int(TestInputVisitorData *data,
> > +                                 const void *unused)
> 
> Indentation's off.

Will fix it.

> 
> > +{
> > +    Error *err = NULL;
> > +    Visitor *v;
> > +    AltOnOffInt *be = NULL;
> > +
> > +    v = visitor_input_test_init(data, "on");
> > +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> > +    g_assert(!err);
> > +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> > +    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_ON);
> > +    qapi_free_AltOnOffInt(be);
> > +
> > +    v = visitor_input_test_init(data, "off");
> > +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> > +    g_assert(!err);
> > +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> > +    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_OFF);
> > +    qapi_free_AltOnOffInt(be);
> > +
> > +    v = visitor_input_test_init(data, "auto");
> > +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> > +    g_assert(!err);
> > +    g_assert_cmpint(be->type, ==, QTYPE_QSTRING);
> > +    g_assert_cmpint(be->u.o, ==, TEST_ON_OFF_AUTO_AUTO);
> > +    qapi_free_AltOnOffInt(be);
> > +
> > +    v = visitor_input_test_init(data, "-12345");
> > +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> > +    g_assert(!err);
> > +    g_assert_cmpint(be->type, ==, QTYPE_QINT);
> > +    g_assert_cmpint(be->u.i, ==, -12345);
> > +    qapi_free_AltOnOffInt(be);
> > +
> > +    v = visitor_input_test_init(data, "true");
> > +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> > +    error_free_or_abort(&err);
> > +    qapi_free_AltOnOffInt(be);
> > +
> > +    v = visitor_input_test_init(data, "value2");
> > +    visit_type_AltOnOffInt(v, NULL, &be, &err);
> > +    error_free_or_abort(&err);
> > +    qapi_free_AltOnOffInt(be);
> > +}
> > +
> 
> Could we use tests covering alternates that don't satisfy the string
> visitor's restrictions?  Like OnOffAuto + bool.

We could, but right now the visitor is unable to detect those
cases, so behavior is undefined. I will try to address that in
the next version.

> 
> >  /* Try to crash the visitors */
> >  static void test_visitor_in_fuzz(TestInputVisitorData *data,
> >                                   const void *unused)
> > @@ -366,6 +451,10 @@ int main(int argc, char **argv)
> >                              &in_visitor_data, test_visitor_in_string);
> >      input_visitor_test_add("/string-visitor/input/enum",
> >                              &in_visitor_data, test_visitor_in_enum);
> > +    input_visitor_test_add("/string-visitor/input/alternate/bool_enum",
> > +                            &in_visitor_data, test_visitor_in_alt_bool_enum);
> > +    input_visitor_test_add("/string-visitor/input/alternate/enum_int",
> > +                            &in_visitor_data, test_visitor_in_alt_enum_int);
> >      input_visitor_test_add("/string-visitor/input/fuzz",
> >                              &in_visitor_data, test_visitor_in_fuzz);
> >  
> > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
> > index 842ea3c5e3..231c8952e8 100644
> > --- a/tests/qapi-schema/qapi-schema-test.json
> > +++ b/tests/qapi-schema/qapi-schema-test.json
> > @@ -106,6 +106,14 @@
> >  { 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } }
> >  { 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } }
> >  
> > +
> > +# for testing string-input-visitor handling of alternates:
> > +{ 'enum': 'TestOnOffAuto',
> > +  'data': [ 'on', 'off', 'auto' ] }
> > +
> > +{ 'alternate': 'AltBoolEnum', 'data': { 'b': 'bool', 'e': 'EnumOne' } }
> > +{ 'alternate': 'AltOnOffInt', 'data': { 'o': 'TestOnOffAuto', 'i': 'int' } }
> > +
> >  # for testing native lists
> >  { 'union': 'UserDefNativeListUnion',
> >    'data': { 'integer': ['int'],

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > When parsing alternates from a string, there are some limitations in
>> > what we can do, but it is a valid use case in some situations. We can
>> > support booleans, integer types, and enums.

By the way, the same restrictions apply to the "keyval" variant of the
QObject input visitor.  It's a known problem stated here:

    Message-ID: <8737exuz6u.fsf@dusky.pond.sub.org>
    https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00046.html

However, I failed to document it properly in the source.

>> >
>> > This will be used to support 'feature=force' in -cpu options, while
>> > keeping 'feature=on|off|true|false' represented as boolean values.
>> >
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> >  qapi/string-input-visitor.c             | 71 ++++++++++++++++++++++----
>> >  tests/test-string-input-visitor.c       | 89 +++++++++++++++++++++++++++++++++
>> >  tests/qapi-schema/qapi-schema-test.json |  8 +++
>> >  3 files changed, 158 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
>> > index c089491c24..bf8f58748b 100644
>> > --- a/qapi/string-input-visitor.c
>> > +++ b/qapi/string-input-visitor.c
>> > @@ -19,6 +19,7 @@
>> >  #include "qemu/option.h"
>> >  #include "qemu/queue.h"
>> >  #include "qemu/range.h"
>> > +#include "qemu/bitops.h"
>> >  
>> >  
>> >  struct StringInputVisitor
>> > @@ -278,21 +279,34 @@ static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,
>> >      *obj = val;
>> >  }
>> >  
>> > +static int try_parse_bool(const char *s, bool *result)
>> > +{
>> > +    if (!strcasecmp(s, "on") ||
>> > +        !strcasecmp(s, "yes") ||
>> > +        !strcasecmp(s, "true")) {
>> > +        if (result) {
>> > +            *result = true;
>> > +        }
>> > +        return 0;
>> > +    }
>> > +    if (!strcasecmp(s, "off") ||
>> > +        !strcasecmp(s, "no") ||
>> > +        !strcasecmp(s, "false")) {
>> > +        if (result) {
>> > +            *result = false;
>> > +        }
>> > +        return 0;
>> > +    }
>> > +
>> > +    return -1;
>> > +}
>> > +
>> >  static void parse_type_bool(Visitor *v, const char *name, bool *obj,
>> >                              Error **errp)
>> >  {
>> >      StringInputVisitor *siv = to_siv(v);
>> >  
>> > -    if (!strcasecmp(siv->string, "on") ||
>> > -        !strcasecmp(siv->string, "yes") ||
>> > -        !strcasecmp(siv->string, "true")) {
>> > -        *obj = true;
>> > -        return;
>> > -    }
>> > -    if (!strcasecmp(siv->string, "off") ||
>> > -        !strcasecmp(siv->string, "no") ||
>> > -        !strcasecmp(siv->string, "false")) {
>> > -        *obj = false;
>> > +    if (try_parse_bool(siv->string, obj) == 0) {
>> >          return;
>> >      }
>> >  
>> > @@ -326,6 +340,42 @@ static void parse_type_number(Visitor *v, const char *name, double *obj,
>> >      *obj = val;
>> >  }
>> >  
>> > +/* Support for alternates on string-input-visitor is limited, because
>> 
>> Please wing both ends of your comments, like this:
>> 
>>    /*
>>     * Comment text
>>     */
>
> Will do.
>
>> 
>> > + * the input string doesn't have any type information.
>> > + *
>> > + * Supported alternate member types:
>> > + * 1) enums
>> > + * 2) integer types
>> 
>> Better add "(but only if there 's no enum member consisting only of
>> digits)".
>
> Oh, I didn't know this was possible.

Special exception in the naming rules prompted by enum QKeyCode.  Quote
qapi-code-gen.txt:

    All names must begin with a letter, and contain only ASCII letters,
    digits, hyphen, and underscore.  There are two exceptions: enum values
    may start with a digit, and names that are downstream extensions (see
    section Downstream extensions) start with underscore.

>> > + * 3) booleans (but only if the there's no enum variant
>> 
>> Make that "(but only if there's no enum member".
>
> Will do.
>
>
>> 
>> > + *    containing "on", "off", "true", or "false" as members)
>> 
>> Begs the question what happens when you violate these restrictions.
>
> Right now, we don't detect those cases and behavior is undefined.
> I think it will be a good idea to give start_alternate() enough
> information to detect those cases (by adding a 'const char *const
> enum_table[]' parameter).

Alternate types that won't work with the string input visitor can be
detected at compile time (by qapi.py), but not their actual use.  Pity.

Do we actually use alternates that violate the restrictions?  If not, we
could simply restrict alternates so they work with *all* visitors.  If
we ever run into an actual need for alternates that don't, we'll be no
worse off than now.

Let's review existing alternates outside tests:

* Qcow2OverlapChecks: struct + enum
* BlockdevRef: struct + str
* GuestFileWhence: int + enum (all enum members start with a letter)

Restricting alternates looks practical to me.  Eric, what do you think?

>> > + *
>> > + * UNSUPPORTED alternate member types:
>> > + * 1) strings
>> > + * 2) complex types
>> > + */
>> 
>> This comment explains the visitor's restrictions for alternate types.
>> It does not explain how the function works.  The proper place for
>> explaining the visitor's restriction is the comment in
>> string-input-visitor.h, which you need to update anyway, because right
>> now it claims visiting alternates isn't implemented.
>
> Right. I will take a look.
>
>> 
>> > +static void start_alternate(Visitor *v, const char *name,
>> > +                            GenericAlternate **obj, size_t size,
>> > +                            unsigned long supported_qtypes, Error **errp)
>> > +{
>> > +    StringInputVisitor *siv = to_siv(v);
>> > +    QType t = QTYPE_QSTRING;
>> 
>> We usually call such variables @type or @qtype.
>
> I will change it.
>
>> 
>> QTYPE_QSTRING means enum here.  Worth a comment, I think.
>
> I will update it.
>
>> 
>> > +
>> > +    if (supported_qtypes & BIT(QTYPE_QBOOL)) {
>> > +        if (try_parse_bool(siv->string, NULL) == 0) {
>> > +            t = QTYPE_QBOOL;
>> > +        }
>> > +    }
>> > +
>> > +    if (supported_qtypes & BIT(QTYPE_QINT)) {
>> > +        if (parse_str(siv, name, NULL) == 0) {
>> 
>> parse_str() alters *siv.  Why is that harmless?
>
> I remember concluding it was harmless, but I don't remember the
> details.
>
> I will probably drop support for integers in the next version,
> and let whoever needs integer member support on alternates fix
> parse_str().

Any idea that lets me avoid looking at parse_str() is tempting.

If we choose to have alternates that can work only with some visitors,
then restricting this visitor in its own special ways seems quite
tolerable.

But if we restrict alternates to the ones that can work with all
visitors, it would be nice if all visitors that can do alternates at all
can do all alternates.  Mind, "nice", not "must have to get patches
accepted".

>> Aside: parse_str() is basically horrible, but that's not your fault.
>
> That's an understatement. :)
>
>> 
>> > +            t = QTYPE_QINT;
>> > +        }
>> > +    }
>> > +
>> > +    *obj = g_malloc0(size);
>> > +    (*obj)->type = t;
>> > +}
>> > +
>> >  static void string_input_free(Visitor *v)
>> >  {
>> >      StringInputVisitor *siv = to_siv(v);
>> > @@ -353,6 +403,7 @@ Visitor *string_input_visitor_new(const char *str)
>> >      v->visitor.next_list = next_list;
>> >      v->visitor.check_list = check_list;
>> >      v->visitor.end_list = end_list;
>> > +    v->visitor.start_alternate = start_alternate;
>> >      v->visitor.free = string_input_free;
>> >  
>> >      v->string = str;
>> > diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
>> > index 79313a7f7a..1e867d62c9 100644
>> > --- a/tests/test-string-input-visitor.c
>> > +++ b/tests/test-string-input-visitor.c
>> > @@ -290,6 +290,91 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
>> >      }
>> >  }
>> >  
>> > +static void test_visitor_in_alt_bool_enum(TestInputVisitorData *data,
>> > +                                 const void *unused)
>> 
>> Indentation's off.
>
> Will fix it.
>
>> 
>> > +{
>> > +    Error *err = NULL;
>> > +    Visitor *v;
>> > +    AltBoolEnum *be = NULL;
>> > +
>> > +    v = visitor_input_test_init(data, "true");
>> > +    visit_type_AltBoolEnum(v, NULL, &be, &err);
>> > +    g_assert(!err);
>> 
>> What's wrong with &error_abort?  More of the same below.
>
> Most test cases in test-string-input-visitor.c use
> g_asserto(!err) instead of error_abort.
>
> I prefer the former, because I read &error_abort as "if this
> causes an error, something is wrong with the caller [the test
> code itself]", and g_assert(!err) as "if this assert is
> triggered, the test code detected a bug".

Better not read them that way, because you'll misread a couple of
hundred instances in tests/ that don't make this distinction.

> Also, if that function call returns an error, I want to know
> which line on which test case failed. &error_abort would hide
> that information.

Point taken.  On the flip side, with &error_abort you immediately see
this is a negative test.

This file already uses both, so I'm not going to force you pick the one
I prefer.

[...]

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]

On 05/04/2017 03:06 AM, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
>> On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
>>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>>
>>>> When parsing alternates from a string, there are some limitations in
>>>> what we can do, but it is a valid use case in some situations. We can
>>>> support booleans, integer types, and enums.
> 
> By the way, the same restrictions apply to the "keyval" variant of the
> QObject input visitor.  It's a known problem stated here:
> 
>     Message-ID: <8737exuz6u.fsf@dusky.pond.sub.org>
>     https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00046.html
> 
> However, I failed to document it properly in the source.
> 

>>> Begs the question what happens when you violate these restrictions.
>>
>> Right now, we don't detect those cases and behavior is undefined.
>> I think it will be a good idea to give start_alternate() enough
>> information to detect those cases (by adding a 'const char *const
>> enum_table[]' parameter).
> 
> Alternate types that won't work with the string input visitor can be
> detected at compile time (by qapi.py), but not their actual use.  Pity.
> 
> Do we actually use alternates that violate the restrictions?  If not, we
> could simply restrict alternates so they work with *all* visitors.  If
> we ever run into an actual need for alternates that don't, we'll be no
> worse off than now.
> 
> Let's review existing alternates outside tests:
> 
> * Qcow2OverlapChecks: struct + enum
> * BlockdevRef: struct + str
> * GuestFileWhence: int + enum (all enum members start with a letter)
> 
> Restricting alternates looks practical to me.  Eric, what do you think?

As in: we forbid the combination of a scalar (whether 'int', 'number',
'bool', and perhaps 'null') with a plain 'str' (since there's no way to
tell whether '1' should parse as an integer or the string "1"); and
combining a scalar with an 'enum' requires that all enum members be
distinct from what could otherwise be parsed as a scalar?  I can live
with such a restriction.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 05/04/2017 03:06 AM, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>>> On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
>>>> Eduardo Habkost <ehabkost@redhat.com> writes:
>>>>
>>>>> When parsing alternates from a string, there are some limitations in
>>>>> what we can do, but it is a valid use case in some situations. We can
>>>>> support booleans, integer types, and enums.
>> 
>> By the way, the same restrictions apply to the "keyval" variant of the
>> QObject input visitor.  It's a known problem stated here:
>> 
>>     Message-ID: <8737exuz6u.fsf@dusky.pond.sub.org>
>>     https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00046.html
>> 
>> However, I failed to document it properly in the source.
>> 
>
>>>> Begs the question what happens when you violate these restrictions.
>>>
>>> Right now, we don't detect those cases and behavior is undefined.
>>> I think it will be a good idea to give start_alternate() enough
>>> information to detect those cases (by adding a 'const char *const
>>> enum_table[]' parameter).
>> 
>> Alternate types that won't work with the string input visitor can be
>> detected at compile time (by qapi.py), but not their actual use.  Pity.
>> 
>> Do we actually use alternates that violate the restrictions?  If not, we
>> could simply restrict alternates so they work with *all* visitors.  If
>> we ever run into an actual need for alternates that don't, we'll be no
>> worse off than now.
>> 
>> Let's review existing alternates outside tests:
>> 
>> * Qcow2OverlapChecks: struct + enum
>> * BlockdevRef: struct + str
>> * GuestFileWhence: int + enum (all enum members start with a letter)
>> 
>> Restricting alternates looks practical to me.  Eric, what do you think?
>
> As in: we forbid the combination of a scalar (whether 'int', 'number',
> 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
> tell whether '1' should parse as an integer or the string "1"); and
> combining a scalar with an 'enum' requires that all enum members be
> distinct from what could otherwise be parsed as a scalar?

Exactly.

>                                                            I can live
> with such a restriction.

Then let's do it.

Eduardo, are you comfortable implementing this, or would you like me to
do it?

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eduardo Habkost]

On Thu, May 04, 2017 at 03:42:59PM +0200, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 05/04/2017 03:06 AM, Markus Armbruster wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >> 
> >>> On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
> >>>> Eduardo Habkost <ehabkost@redhat.com> writes:
> >>>>
> >>>>> When parsing alternates from a string, there are some limitations in
> >>>>> what we can do, but it is a valid use case in some situations. We can
> >>>>> support booleans, integer types, and enums.
> >> 
> >> By the way, the same restrictions apply to the "keyval" variant of the
> >> QObject input visitor.  It's a known problem stated here:
> >> 
> >>     Message-ID: <8737exuz6u.fsf@dusky.pond.sub.org>
> >>     https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00046.html
> >> 
> >> However, I failed to document it properly in the source.
> >> 
> >
> >>>> Begs the question what happens when you violate these restrictions.
> >>>
> >>> Right now, we don't detect those cases and behavior is undefined.
> >>> I think it will be a good idea to give start_alternate() enough
> >>> information to detect those cases (by adding a 'const char *const
> >>> enum_table[]' parameter).
> >> 
> >> Alternate types that won't work with the string input visitor can be
> >> detected at compile time (by qapi.py), but not their actual use.  Pity.
> >> 
> >> Do we actually use alternates that violate the restrictions?  If not, we
> >> could simply restrict alternates so they work with *all* visitors.  If
> >> we ever run into an actual need for alternates that don't, we'll be no
> >> worse off than now.
> >> 
> >> Let's review existing alternates outside tests:
> >> 
> >> * Qcow2OverlapChecks: struct + enum
> >> * BlockdevRef: struct + str
> >> * GuestFileWhence: int + enum (all enum members start with a letter)
> >> 
> >> Restricting alternates looks practical to me.  Eric, what do you think?
> >
> > As in: we forbid the combination of a scalar (whether 'int', 'number',
> > 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
> > tell whether '1' should parse as an integer or the string "1"); and
> > combining a scalar with an 'enum' requires that all enum members be
> > distinct from what could otherwise be parsed as a scalar?
> 
> Exactly.
> 
> >                                                            I can live
> > with such a restriction.
> 
> Then let's do it.
> 
> Eduardo, are you comfortable implementing this, or would you like me to
> do it?

I will give it a try and include it in the next version. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eduardo Habkost]

On Thu, May 04, 2017 at 11:10:56AM -0300, Eduardo Habkost wrote:
> On Thu, May 04, 2017 at 03:42:59PM +0200, Markus Armbruster wrote:
> > Eric Blake <eblake@redhat.com> writes:
> > 
> > > On 05/04/2017 03:06 AM, Markus Armbruster wrote:
> > >> Eduardo Habkost <ehabkost@redhat.com> writes:
> > >> 
> > >>> On Wed, May 03, 2017 at 06:07:43PM +0200, Markus Armbruster wrote:
> > >>>> Eduardo Habkost <ehabkost@redhat.com> writes:
> > >>>>
> > >>>>> When parsing alternates from a string, there are some limitations in
> > >>>>> what we can do, but it is a valid use case in some situations. We can
> > >>>>> support booleans, integer types, and enums.
> > >> 
> > >> By the way, the same restrictions apply to the "keyval" variant of the
> > >> QObject input visitor.  It's a known problem stated here:
> > >> 
> > >>     Message-ID: <8737exuz6u.fsf@dusky.pond.sub.org>
> > >>     https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00046.html
> > >> 
> > >> However, I failed to document it properly in the source.
> > >> 
> > >
> > >>>> Begs the question what happens when you violate these restrictions.
> > >>>
> > >>> Right now, we don't detect those cases and behavior is undefined.
> > >>> I think it will be a good idea to give start_alternate() enough
> > >>> information to detect those cases (by adding a 'const char *const
> > >>> enum_table[]' parameter).
> > >> 
> > >> Alternate types that won't work with the string input visitor can be
> > >> detected at compile time (by qapi.py), but not their actual use.  Pity.
> > >> 
> > >> Do we actually use alternates that violate the restrictions?  If not, we
> > >> could simply restrict alternates so they work with *all* visitors.  If
> > >> we ever run into an actual need for alternates that don't, we'll be no
> > >> worse off than now.
> > >> 
> > >> Let's review existing alternates outside tests:
> > >> 
> > >> * Qcow2OverlapChecks: struct + enum
> > >> * BlockdevRef: struct + str
> > >> * GuestFileWhence: int + enum (all enum members start with a letter)
> > >> 
> > >> Restricting alternates looks practical to me.  Eric, what do you think?
> > >
> > > As in: we forbid the combination of a scalar (whether 'int', 'number',
> > > 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
> > > tell whether '1' should parse as an integer or the string "1"); and
> > > combining a scalar with an 'enum' requires that all enum members be
> > > distinct from what could otherwise be parsed as a scalar?
> > 
> > Exactly.
> > 
> > >                                                            I can live
> > > with such a restriction.
> > 
> > Then let's do it.
> > 
> > Eduardo, are you comfortable implementing this, or would you like me to
> > do it?
> 
> I will give it a try and include it in the next version. Thanks!

So, I made qapi.py detect ambiguous alternates[1]. The bad news
is that lots of the alternates in qapi-schema-test.json already
break those rules. This will require changing the schema and
rewriting tests in test-clone-visitor and qobject-input-visitor.

I think there's a small risk we will want to support some of
those forbidden alternate combinations again in the future. If
that happens, detecting them at runtime in string-input-visitor
will keep us safe.

I plan to submit v2 with code to detect ambiguous alternates at
runtime, only, because it seems simpler than rewriting the test
code. After that, we can still make QAPI reject them at compile
time too, if we really want to.

[1] https://github.com/ehabkost/qemu-hacks/commit/cda70a2e1c30c8dadb36fd46095a4f6ee3d89737

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2767 bytes --]

On 05/04/2017 02:42 PM, Eduardo Habkost wrote:

>>>> As in: we forbid the combination of a scalar (whether 'int', 'number',
>>>> 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
>>>> tell whether '1' should parse as an integer or the string "1"); and
>>>> combining a scalar with an 'enum' requires that all enum members be
>>>> distinct from what could otherwise be parsed as a scalar?
>>>
>>> Exactly.
>>>
>>>>                                                            I can live
>>>> with such a restriction.
>>>
>>> Then let's do it.
>>>
>>> Eduardo, are you comfortable implementing this, or would you like me to
>>> do it?
>>
>> I will give it a try and include it in the next version. Thanks!
> 
> So, I made qapi.py detect ambiguous alternates[1]. The bad news
> is that lots of the alternates in qapi-schema-test.json already
> break those rules.

I'm not surprised, but if test code gets in the way of real life, I'm in
favor of simplifying test code (change what is currently positive tests
of "does this corner case work even though no one uses it" to instead be
negative tests "do we properly reject this alternate as ambiguous").

> This will require changing the schema and
> rewriting tests in test-clone-visitor and qobject-input-visitor.

Yes, and Markus or I could help do some of that work, if you don't feel
up to it.

> 
> I think there's a small risk we will want to support some of
> those forbidden alternate combinations again in the future.

Maybe, but 'git revert' is powerfully easy, and whoever adds the
(now-ambiguous) use case should be able to justify their need at the
time they (re-)add support.

> If
> that happens, detecting them at runtime in string-input-visitor
> will keep us safe.

I still much prefer compile-time rejections ("we don't support this, and
we're telling you up front") over runtime rejections ("it compiled, and
only if you get lucky will you discover that you had a problem").

> 
> I plan to submit v2 with code to detect ambiguous alternates at
> runtime, only, because it seems simpler than rewriting the test
> code.

Simpler, maybe. But harder to maintain, so it may STILL be worth going
the more complex way and updating the testsuite to comply with the new
rules.  Perhaps it can be done as followups (again, where Markus or I
may step in and do the rest on top of your initial work).

> After that, we can still make QAPI reject them at compile
> time too, if we really want to.
> 
> [1] https://github.com/ehabkost/qemu-hacks/commit/cda70a2e1c30c8dadb36fd46095a4f6ee3d89737
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eduardo Habkost]

On Thu, May 04, 2017 at 03:03:36PM -0500, Eric Blake wrote:
> On 05/04/2017 02:42 PM, Eduardo Habkost wrote:
> 
> >>>> As in: we forbid the combination of a scalar (whether 'int', 'number',
> >>>> 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
> >>>> tell whether '1' should parse as an integer or the string "1"); and
> >>>> combining a scalar with an 'enum' requires that all enum members be
> >>>> distinct from what could otherwise be parsed as a scalar?
> >>>
> >>> Exactly.
> >>>
> >>>>                                                            I can live
> >>>> with such a restriction.
> >>>
> >>> Then let's do it.
> >>>
> >>> Eduardo, are you comfortable implementing this, or would you like me to
> >>> do it?
> >>
> >> I will give it a try and include it in the next version. Thanks!
> > 
> > So, I made qapi.py detect ambiguous alternates[1]. The bad news
> > is that lots of the alternates in qapi-schema-test.json already
> > break those rules.
> 
> I'm not surprised, but if test code gets in the way of real life, I'm in
> favor of simplifying test code (change what is currently positive tests
> of "does this corner case work even though no one uses it" to instead be
> negative tests "do we properly reject this alternate as ambiguous").

In a few cases, I have the impression that the test cases aren't
testing a feature nobody uses, but it's testing a feature
everybody relies on (like QAPI_CLONE), but using an alternate
type definition that nobody uses.

> 
> > This will require changing the schema and
> > rewriting tests in test-clone-visitor and qobject-input-visitor.
> 
> Yes, and Markus or I could help do some of that work, if you don't feel
> up to it.

I plan to work on it after I send v2 of this series, but if
anybody wants to take over, please be my guest. :)

> 
> > 
> > I think there's a small risk we will want to support some of
> > those forbidden alternate combinations again in the future.
> 
> Maybe, but 'git revert' is powerfully easy, and whoever adds the
> (now-ambiguous) use case should be able to justify their need at the
> time they (re-)add support.
> 
> > If
> > that happens, detecting them at runtime in string-input-visitor
> > will keep us safe.
> 
> I still much prefer compile-time rejections ("we don't support this, and
> we're telling you up front") over runtime rejections ("it compiled, and
> only if you get lucky will you discover that you had a problem").

Yeah. I don't mean the runtime check would be a good replacement
for the compile-time checks, but that the runtime checks will be
useful in case we have to disable the compile-time checks one
day.

> 
> > 
> > I plan to submit v2 with code to detect ambiguous alternates at
> > runtime, only, because it seems simpler than rewriting the test
> > code.
> 
> Simpler, maybe. But harder to maintain, so it may STILL be worth going
> the more complex way and updating the testsuite to comply with the new
> rules.  Perhaps it can be done as followups (again, where Markus or I
> may step in and do the rest on top of your initial work).
> 
> > After that, we can still make QAPI reject them at compile
> > time too, if we really want to.
> > 
> > [1] https://github.com/ehabkost/qemu-hacks/commit/cda70a2e1c30c8dadb36fd46095a4f6ee3d89737
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, May 04, 2017 at 03:03:36PM -0500, Eric Blake wrote:
>> On 05/04/2017 02:42 PM, Eduardo Habkost wrote:
>> 
>> >>>> As in: we forbid the combination of a scalar (whether 'int', 'number',
>> >>>> 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
>> >>>> tell whether '1' should parse as an integer or the string "1"); and
>> >>>> combining a scalar with an 'enum' requires that all enum members be
>> >>>> distinct from what could otherwise be parsed as a scalar?
>> >>>
>> >>> Exactly.
>> >>>
>> >>>>                                                            I can live
>> >>>> with such a restriction.
>> >>>
>> >>> Then let's do it.
>> >>>
>> >>> Eduardo, are you comfortable implementing this, or would you like me to
>> >>> do it?
>> >>
>> >> I will give it a try and include it in the next version. Thanks!
>> > 
>> > So, I made qapi.py detect ambiguous alternates[1]. The bad news
>> > is that lots of the alternates in qapi-schema-test.json already
>> > break those rules.
>> 
>> I'm not surprised, but if test code gets in the way of real life, I'm in
>> favor of simplifying test code (change what is currently positive tests
>> of "does this corner case work even though no one uses it" to instead be
>> negative tests "do we properly reject this alternate as ambiguous").

Exercising corner cases that are supposed to work is as useful as
exercising corner cases that are supposed to be rejected.  Whether
they're actually used is unimportant.

> In a few cases, I have the impression that the test cases aren't
> testing a feature nobody uses, but it's testing a feature
> everybody relies on (like QAPI_CLONE), but using an alternate
> type definition that nobody uses.

The schema for positive tests tests/qapi-schema/qapi-schema-test.json is
intentionally decoupled from the real schema.  New tests tend to reuse
whatever types they can find there.  The result is not exactly pretty,
just useful.

>> > This will require changing the schema and
>> > rewriting tests in test-clone-visitor and qobject-input-visitor.
>> 
>> Yes, and Markus or I could help do some of that work, if you don't feel
>> up to it.
>
> I plan to work on it after I send v2 of this series, but if
> anybody wants to take over, please be my guest. :)

Let's coordinate after v2 is out.

>> > I think there's a small risk we will want to support some of
>> > those forbidden alternate combinations again in the future.
>> 
>> Maybe, but 'git revert' is powerfully easy, and whoever adds the
>> (now-ambiguous) use case should be able to justify their need at the
>> time they (re-)add support.
>> 
>> > If
>> > that happens, detecting them at runtime in string-input-visitor
>> > will keep us safe.
>> 
>> I still much prefer compile-time rejections ("we don't support this, and
>> we're telling you up front") over runtime rejections ("it compiled, and
>> only if you get lucky will you discover that you had a problem").
>
> Yeah. I don't mean the runtime check would be a good replacement
> for the compile-time checks, but that the runtime checks will be
> useful in case we have to disable the compile-time checks one
> day.

My preferred place for carrying code we might need one day is git
history.  It's out of my way there, and it rots no faster than in the
tree.

>> > I plan to submit v2 with code to detect ambiguous alternates at
>> > runtime, only, because it seems simpler than rewriting the test
>> > code.
>> 
>> Simpler, maybe. But harder to maintain, so it may STILL be worth going
>> the more complex way and updating the testsuite to comply with the new
>> rules.  Perhaps it can be done as followups (again, where Markus or I
>> may step in and do the rest on top of your initial work).
>> 
>> > After that, we can still make QAPI reject them at compile
>> > time too, if we really want to.
>> > 
>> > [1] https://github.com/ehabkost/qemu-hacks/commit/cda70a2e1c30c8dadb36fd46095a4f6ee3d89737
>> > 

[Qemu-devel] [PATCH 3/4] tests: Add [+-]feature and feature=on|off test cases

[Eduardo Habkost]

Add test code to ensure features are enabled/disabled correctly in the
command-line. The test case use the "feature-words" and
"filtered-features" properties to check if the features were
enabled/disabled correctly.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/test-x86-cpuid-compat.c | 107 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 79a2e69a28..00ee8a264f 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -1,6 +1,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qbool.h"
@@ -68,6 +69,9 @@ static void test_cpuid_prop(const void *data)
     g_free(path);
 }
 
+/* Add simple test that ensure that a given (integer) property has
+ * the right value after running QEMU using cmdline
+ */
 static void add_cpuid_test(const char *name, const char *cmdline,
                            const char *property, int64_t expected_value)
 {
@@ -78,6 +82,83 @@ static void add_cpuid_test(const char *name, const char *cmdline,
     qtest_add_data_func(name, args, test_cpuid_prop);
 }
 
+
+/* Parameters to a add_feature_test() test case */
+typedef struct FeatureTestArgs {
+    /* cmdline to start QEMU */
+    const char *cmdline;
+    /*
+     * cpuid-input-eax and cpuid-input-ecx values to look for,
+     * in "feature-words" and "filtered-features" properties.
+     */
+    uint32_t input_eax, input_ecx;
+    /* The register name to look for, in the X86CPUFeatureWordInfo array */
+    const char *reg;
+    /* The bit to check in X86CPUFeatureWordInfo.features */
+    int bitnr;
+    /* The expected value for the bit in (X86CPUFeatureWordInfo.features) */
+    bool expected_value;
+} FeatureTestArgs;
+
+/* Get the value for a feature word in a X86CPUFeatureWordInfo list */
+static uint32_t get_feature_word(QList *features, uint32_t eax, uint32_t ecx, const char *reg)
+{
+    const QListEntry *e;
+
+    for (e = qlist_first(features); e; e = qlist_next(e)) {
+        QDict *w = qobject_to_qdict(qlist_entry_obj(e));
+        if (eax == qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-eax")))
+            && (!qdict_haskey(w, "cpuid-input-ecx")
+                || ecx == qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-ecx"))))
+            && !strcmp(qstring_get_str(qobject_to_qstring(qdict_get(w, "cpuid-register"))), reg)) {
+            return qint_get_int(qobject_to_qint(qdict_get(w, "features")));
+        }
+    }
+    return 0;
+}
+
+static void test_feature_flag(const void *data)
+{
+    const FeatureTestArgs *args = data;
+    char *path;
+    QList *present, *filtered;
+    uint32_t value;
+
+    qtest_start(args->cmdline);
+    path = get_cpu0_qom_path();
+    present = qobject_to_qlist(qom_get(path, "feature-words"));
+    filtered = qobject_to_qlist(qom_get(path, "filtered-features"));
+    value = get_feature_word(present, args->input_eax, args->input_ecx, args->reg);
+    value |= get_feature_word(filtered, args->input_eax, args->input_ecx, args->reg);
+    qtest_end();
+
+    g_assert(!!(value & (1U << args->bitnr)) == args->expected_value);
+
+    QDECREF(present);
+    QDECREF(filtered);
+    g_free(path);
+}
+
+/* Add test case to ensure that a given feature flag is set in
+  * either "feature-words" or "filtered-features", when running QEMU
+  * using cmdline
+  */
+static FeatureTestArgs *add_feature_test(const char *name, const char *cmdline,
+                                         uint32_t eax, uint32_t ecx,
+                                         const char *reg, int bitnr,
+                                         bool expected_value)
+{
+    FeatureTestArgs *args = g_new0(FeatureTestArgs, 1);
+    args->cmdline = cmdline;
+    args->input_eax = eax;
+    args->input_ecx = ecx;
+    args->reg = reg;
+    args->bitnr = bitnr;
+    args->expected_value = expected_value;
+    qtest_add_data_func(name, args, test_feature_flag);
+    return args;
+}
+
 #ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
 static void test_plus_minus_subprocess(void)
 {
@@ -229,5 +310,31 @@ int main(int argc, char **argv)
                    "-machine pc-i440fx-2.7 -cpu 486,+xstore",
                    "xlevel2", 0);
 
+    /* Test feature parsing */
+    add_feature_test("x86/cpuid/features/plus",
+                     "-cpu 486,+arat",
+                     6, 0, "EAX", 2, true);
+    add_feature_test("x86/cpuid/features/minus",
+                     "-cpu pentium,-mmx",
+                     1, 0, "EDX", 23, false);
+    add_feature_test("x86/cpuid/features/on",
+                     "-cpu 486,arat=on",
+                     6, 0, "EAX", 2, true);
+    add_feature_test("x86/cpuid/features/off",
+                     "-cpu pentium,mmx=off",
+                     1, 0, "EDX", 23, false);
+    add_feature_test("x86/cpuid/features/max-plus-invtsc",
+                     "-machine accel=kvm:tcg -cpu max,+invtsc",
+                     0x80000007, 0, "EDX", 8, true);
+    add_feature_test("x86/cpuid/features/max-invtsc-on",
+                     "-machine accel=kvm:tcg -cpu max,invtsc=on",
+                     0x80000007, 0, "EDX", 8, true);
+    add_feature_test("x86/cpuid/features/max-minus-mmx",
+                     "-machine accel=kvm:tcg -cpu max,-mmx",
+                     1, 0, "EDX", 23, false);
+    add_feature_test("x86/cpuid/features/max-invtsc-on,mmx=off",
+                     "-machine accel=kvm:tcg -cpu max,mmx=off",
+                     1, 0, "EDX", 23, false);
+
     return g_test_run();
 }
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Eduardo Habkost]

Introduce a new CPUFeatureSetting QAPI data type, and use it to support
feature=force on -cpu.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qapi-schema.json              | 32 +++++++++++++++++++++++++
 target/i386/cpu.h             |  2 ++
 target/i386/cpu.c             | 55 +++++++++++++++++++++++++++++++++----------
 tests/test-x86-cpuid-compat.c | 14 ++++++++++-
 4 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 01b087fa16..d716409114 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4250,6 +4250,38 @@
 { 'command': 'query-machines', 'returns': ['MachineInfo'] }
 
 ##
+# @CPUFeatureSettingEnum:
+#
+# Additional valid values for a CPUFeatureSetting property.
+#
+# @force: Force feature to be enabled, even if the accelerator
+#         reports the feature as unavailable. Should be used only
+#         for testing or debugging purposes.
+#
+# Since: 2.10
+##
+{ 'enum': 'CPUFeatureSettingEnum',
+  'data': ['force'] }
+
+##
+# @CPUFeatureSetting:
+#
+# Values for a CPU feature property.
+#
+# @bool: If false, the feature is forcibly disabled.
+#        If true, QEMU will try to enable the feature. QEMU will
+#        refuse to start if the feature is unavailable and
+#        'enforce' mode is enabled in the CPU.
+#
+# @enum: See @CPUFeatureSettingEnum.
+#
+# Since: 2.10
+##
+{ 'alternate': 'CPUFeatureSetting',
+  'data': { 'bool': 'bool',
+            'enum': 'CPUFeatureSettingEnum' } }
+
+##
 # @CpuDefinitionInfo:
 #
 # Virtual CPU definition.
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index c4602ca80d..7a34998e0a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1149,6 +1149,8 @@ typedef struct CPUX86State {
     FeatureWordArray features;
     /* Features that were explicitly enabled/disabled */
     FeatureWordArray user_features;
+    /* Features set to 'force' */
+    FeatureWordArray forced_features;
     uint32_t cpuid_model[12];
 
     /* MTRRs */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 13c0985f11..6c24b92cee 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3463,7 +3463,7 @@ static int x86_cpu_filter_features(X86CPU *cpu)
         uint32_t host_feat =
             x86_cpu_get_supported_feature_word(w, false);
         uint32_t requested_features = env->features[w];
-        env->features[w] &= host_feat;
+        env->features[w] &= host_feat | env->forced_features[w];
         cpu->filtered_features[w] = requested_features & ~env->features[w];
         if (cpu->filtered_features[w]) {
             rv = 1;
@@ -3705,9 +3705,19 @@ static void x86_cpu_get_bit_prop(Object *obj, Visitor *v, const char *name,
 {
     X86CPU *cpu = X86_CPU(obj);
     BitProperty *fp = opaque;
-    uint32_t f = cpu->env.features[fp->w];
-    bool value = (f & fp->mask) == fp->mask;
-    visit_type_bool(v, name, &value, errp);
+    bool bvalue = (cpu->env.features[fp->w] & fp->mask) == fp->mask;
+    bool forced = (cpu->env.forced_features[fp->w] & fp->mask) == fp->mask;
+    CPUFeatureSetting *value = g_new0(CPUFeatureSetting, 1);
+
+    if (!forced) {
+        value->type = QTYPE_QBOOL;
+        value->u.q_bool = bvalue;
+    } else {
+        value->type = QTYPE_QSTRING;
+        value->u.q_enum = CPU_FEATURE_SETTING_ENUM_FORCE;
+    }
+    visit_type_CPUFeatureSetting(v, name, &value, errp);
+    qapi_free_CPUFeatureSetting(value);
 }
 
 static void x86_cpu_set_bit_prop(Object *obj, Visitor *v, const char *name,
@@ -3717,25 +3727,46 @@ static void x86_cpu_set_bit_prop(Object *obj, Visitor *v, const char *name,
     X86CPU *cpu = X86_CPU(obj);
     BitProperty *fp = opaque;
     Error *local_err = NULL;
-    bool value;
+    CPUFeatureSetting *value = NULL;
 
     if (dev->realized) {
         qdev_prop_set_after_realize(dev, name, errp);
         return;
     }
 
-    visit_type_bool(v, name, &value, &local_err);
+    visit_type_CPUFeatureSetting(v, name, &value, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
     }
 
-    if (value) {
-        cpu->env.features[fp->w] |= fp->mask;
-    } else {
-        cpu->env.features[fp->w] &= ~fp->mask;
+    switch (value->type) {
+    case QTYPE_QBOOL:
+        if (value->u.q_bool) {
+            cpu->env.features[fp->w] |= fp->mask;
+        } else {
+            cpu->env.features[fp->w] &= ~fp->mask;
+        }
+        cpu->env.forced_features[fp->w] &= ~fp->mask;
+        cpu->env.user_features[fp->w] |= fp->mask;
+    break;
+    case QTYPE_QSTRING:
+        switch (value->u.q_enum) {
+        case CPU_FEATURE_SETTING_ENUM_FORCE:
+            cpu->env.features[fp->w] |= fp->mask;
+            cpu->env.forced_features[fp->w] |= fp->mask;
+        break;
+        default:
+            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
+                       "CPUFeatureSetting");
+        }
+    break;
+    default:
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
+                   "CPUFeatureSetting");
     }
-    cpu->env.user_features[fp->w] |= fp->mask;
+
+    qapi_free_CPUFeatureSetting(value);
 }
 
 static void x86_cpu_release_bit_prop(Object *obj, const char *name,
@@ -3769,7 +3800,7 @@ static void x86_cpu_register_bit_prop(X86CPU *cpu,
         fp = g_new0(BitProperty, 1);
         fp->w = w;
         fp->mask = mask;
-        object_property_add(OBJECT(cpu), prop_name, "bool",
+        object_property_add(OBJECT(cpu), prop_name, "CPUFeatureSetting",
                             x86_cpu_get_bit_prop,
                             x86_cpu_set_bit_prop,
                             x86_cpu_release_bit_prop, fp, &error_abort);
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 00ee8a264f..9940f4de1c 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -98,6 +98,8 @@ typedef struct FeatureTestArgs {
     int bitnr;
     /* The expected value for the bit in (X86CPUFeatureWordInfo.features) */
     bool expected_value;
+    /* Don't look at filtered-features when checking feature value */
+    bool ignore_filtered_features;
 } FeatureTestArgs;
 
 /* Get the value for a feature word in a X86CPUFeatureWordInfo list */
@@ -129,7 +131,9 @@ static void test_feature_flag(const void *data)
     present = qobject_to_qlist(qom_get(path, "feature-words"));
     filtered = qobject_to_qlist(qom_get(path, "filtered-features"));
     value = get_feature_word(present, args->input_eax, args->input_ecx, args->reg);
-    value |= get_feature_word(filtered, args->input_eax, args->input_ecx, args->reg);
+    if (!args->ignore_filtered_features) {
+        value |= get_feature_word(filtered, args->input_eax, args->input_ecx, args->reg);
+    }
     qtest_end();
 
     g_assert(!!(value & (1U << args->bitnr)) == args->expected_value);
@@ -336,5 +340,13 @@ int main(int argc, char **argv)
                      "-machine accel=kvm:tcg -cpu max,mmx=off",
                      1, 0, "EDX", 23, false);
 
+    {
+    FeatureTestArgs *a;
+    a = add_feature_test("x86/cpuid/features/monitor-force",
+                         "-machine accel=kvm:tcg -cpu 486,monitor=force",
+                         1, 0, "ECX",  3, true);
+    a->ignore_filtered_features = true;
+    }
+
     return g_test_run();
 }
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH] fixup! tests: Add [+-]feature and feature=on|off test cases

[Eduardo Habkost]

On Tue, May 02, 2017 at 05:31:15PM -0300, Eduardo Habkost wrote:
> Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> feature=force on -cpu.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

I will include this in v2 to fix the coding style issues.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 tests/test-x86-cpuid-compat.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 9940f4de1c..02f56ad0e8 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -103,16 +103,24 @@ typedef struct FeatureTestArgs {
 } FeatureTestArgs;
 
 /* Get the value for a feature word in a X86CPUFeatureWordInfo list */
-static uint32_t get_feature_word(QList *features, uint32_t eax, uint32_t ecx, const char *reg)
+static uint32_t get_feature_word(QList *features, uint32_t eax, uint32_t ecx,
+                                 const char *reg)
 {
     const QListEntry *e;
 
     for (e = qlist_first(features); e; e = qlist_next(e)) {
         QDict *w = qobject_to_qdict(qlist_entry_obj(e));
-        if (eax == qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-eax")))
-            && (!qdict_haskey(w, "cpuid-input-ecx")
-                || ecx == qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-ecx"))))
-            && !strcmp(qstring_get_str(qobject_to_qstring(qdict_get(w, "cpuid-register"))), reg)) {
+        const char *rreg =
+            qstring_get_str(qobject_to_qstring(qdict_get(w, "cpuid-register")));
+        uint32_t reax =
+            qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-eax")));
+        bool has_ecx = qdict_haskey(w, "cpuid-input-ecx");
+        uint32_t recx = 0;
+
+        if (has_ecx) {
+            recx = qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-ecx")));
+        }
+        if (eax == reax && (!has_ecx || ecx == recx) && !strcmp(rreg, reg)) {
             return qint_get_int(qobject_to_qint(qdict_get(w, "features")));
         }
     }
-- 
2.11.0.259.g40922b1

-- 
Eduardo

Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3159 bytes --]

On 05/02/2017 03:31 PM, Eduardo Habkost wrote:
> Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> feature=force on -cpu.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qapi-schema.json              | 32 +++++++++++++++++++++++++
>  target/i386/cpu.h             |  2 ++
>  target/i386/cpu.c             | 55 +++++++++++++++++++++++++++++++++----------
>  tests/test-x86-cpuid-compat.c | 14 ++++++++++-
>  4 files changed, 90 insertions(+), 13 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 01b087fa16..d716409114 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4250,6 +4250,38 @@
>  { 'command': 'query-machines', 'returns': ['MachineInfo'] }
>  
>  ##
> +# @CPUFeatureSettingEnum:
> +#
> +# Additional valid values for a CPUFeatureSetting property.
> +#
> +# @force: Force feature to be enabled, even if the accelerator
> +#         reports the feature as unavailable. Should be used only
> +#         for testing or debugging purposes.
> +#
> +# Since: 2.10
> +##
> +{ 'enum': 'CPUFeatureSettingEnum',
> +  'data': ['force'] }
> +
> +##
> +# @CPUFeatureSetting:
> +#
> +# Values for a CPU feature property.
> +#
> +# @bool: If false, the feature is forcibly disabled.
> +#        If true, QEMU will try to enable the feature. QEMU will
> +#        refuse to start if the feature is unavailable and
> +#        'enforce' mode is enabled in the CPU.
> +#
> +# @enum: See @CPUFeatureSettingEnum.
> +#
> +# Since: 2.10
> +##
> +{ 'alternate': 'CPUFeatureSetting',
> +  'data': { 'bool': 'bool',
> +            'enum': 'CPUFeatureSettingEnum' } }
> +

Looks reasonable; I'm glad the suggestion for using an alternate worked.


> -    if (value) {
> -        cpu->env.features[fp->w] |= fp->mask;
> -    } else {
> -        cpu->env.features[fp->w] &= ~fp->mask;
> +    switch (value->type) {
> +    case QTYPE_QBOOL:
> +        if (value->u.q_bool) {
> +            cpu->env.features[fp->w] |= fp->mask;
> +        } else {
> +            cpu->env.features[fp->w] &= ~fp->mask;
> +        }
> +        cpu->env.forced_features[fp->w] &= ~fp->mask;
> +        cpu->env.user_features[fp->w] |= fp->mask;
> +    break;

Isn't the break supposed to be indented four more spaces?

> +    case QTYPE_QSTRING:
> +        switch (value->u.q_enum) {
> +        case CPU_FEATURE_SETTING_ENUM_FORCE:
> +            cpu->env.features[fp->w] |= fp->mask;
> +            cpu->env.forced_features[fp->w] |= fp->mask;
> +        break;

and again

> +        default:
> +            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
> +                       "CPUFeatureSetting");
> +        }
> +    break;

and again

> +    default:
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
> +                   "CPUFeatureSetting");
>      }
> -    cpu->env.user_features[fp->w] |= fp->mask;
> +
> +    qapi_free_CPUFeatureSetting(value);
>  }
>  

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Eduardo Habkost]

On Tue, May 02, 2017 at 04:42:02PM -0500, Eric Blake wrote:
[...]
> > -    if (value) {
> > -        cpu->env.features[fp->w] |= fp->mask;
> > -    } else {
> > -        cpu->env.features[fp->w] &= ~fp->mask;
> > +    switch (value->type) {
> > +    case QTYPE_QBOOL:
> > +        if (value->u.q_bool) {
> > +            cpu->env.features[fp->w] |= fp->mask;
> > +        } else {
> > +            cpu->env.features[fp->w] &= ~fp->mask;
> > +        }
> > +        cpu->env.forced_features[fp->w] &= ~fp->mask;
> > +        cpu->env.user_features[fp->w] |= fp->mask;
> > +    break;
> 
> Isn't the break supposed to be indented four more spaces?
> 
[...]

I will fix it in v2. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Igor Mammedov]

On Tue,  2 May 2017 17:31:15 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> feature=force on -cpu.
commit message lacks rationale why it's needed.
I suspect that it's to enable forced mwait.
It would be nice to put here answer to what
motivated to write this patch and reference commit ids
of kernel patches if there are/needed any to make it work.

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
[...]
> @@ -3717,25 +3727,46 @@ static void x86_cpu_set_bit_prop(Object *obj, Visitor *v, const char *name,
>      X86CPU *cpu = X86_CPU(obj);
>      BitProperty *fp = opaque;
>      Error *local_err = NULL;
> -    bool value;
> +    CPUFeatureSetting *value = NULL;
>  
>      if (dev->realized) {
>          qdev_prop_set_after_realize(dev, name, errp);
>          return;
>      }
>  
> -    visit_type_bool(v, name, &value, &local_err);
> +    visit_type_CPUFeatureSetting(v, name, &value, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
>      }
>  
> -    if (value) {
> -        cpu->env.features[fp->w] |= fp->mask;
> -    } else {
> -        cpu->env.features[fp->w] &= ~fp->mask;
> +    switch (value->type) {
> +    case QTYPE_QBOOL:
> +        if (value->u.q_bool) {
> +            cpu->env.features[fp->w] |= fp->mask;
> +        } else {
> +            cpu->env.features[fp->w] &= ~fp->mask;
> +        }
> +        cpu->env.forced_features[fp->w] &= ~fp->mask;
> +        cpu->env.user_features[fp->w] |= fp->mask;
> +    break;
> +    case QTYPE_QSTRING:
> +        switch (value->u.q_enum) {
> +        case CPU_FEATURE_SETTING_ENUM_FORCE:
> +            cpu->env.features[fp->w] |= fp->mask;
> +            cpu->env.forced_features[fp->w] |= fp->mask;
> +        break;
> +        default:
> +            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
> +                       "CPUFeatureSetting");
> +        }
> +    break;
> +    default:
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
> +                   "CPUFeatureSetting");
>      }
> -    cpu->env.user_features[fp->w] |= fp->mask;
that's the only place it was set and then used by x86_cpu_expand_features()
Is it correct to remove setter?


> +
> +    qapi_free_CPUFeatureSetting(value);
>  }
>  
>  static void x86_cpu_release_bit_prop(Object *obj, const char *name,

Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Eduardo Habkost]

On Thu, May 04, 2017 at 11:49:38AM +0200, Igor Mammedov wrote:
> On Tue,  2 May 2017 17:31:15 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> > feature=force on -cpu.
> commit message lacks rationale why it's needed.
> I suspect that it's to enable forced mwait.
> It would be nice to put here answer to what
> motivated to write this patch and reference commit ids
> of kernel patches if there are/needed any to make it work.

In this case I need to ask Alex for help writing the rationale,
as he is sent the original mwait CPUID override patch.

It's not clear to me yet why KVM doesn't return MONITOR on
GET_SUPPORTED_CPUID today, making this patch unnecessary.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Kashyap Chamarthy]

On Tue, May 02, 2017 at 05:31:15PM -0300, Eduardo Habkost wrote:

Hi,

Yet to try this series, a small question in-line.

> Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> feature=force on -cpu.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qapi-schema.json              | 32 +++++++++++++++++++++++++
>  target/i386/cpu.h             |  2 ++
>  target/i386/cpu.c             | 55 +++++++++++++++++++++++++++++++++----------
>  tests/test-x86-cpuid-compat.c | 14 ++++++++++-
>  4 files changed, 90 insertions(+), 13 deletions(-)

[...]

> --- a/tests/test-x86-cpuid-compat.c
> +++ b/tests/test-x86-cpuid-compat.c
> @@ -98,6 +98,8 @@ typedef struct FeatureTestArgs {
>      int bitnr;
>      /* The expected value for the bit in (X86CPUFeatureWordInfo.features) */
>      bool expected_value;
> +    /* Don't look at filtered-features when checking feature value */
> +    bool ignore_filtered_features;
>  } FeatureTestArgs;
>  
>  /* Get the value for a feature word in a X86CPUFeatureWordInfo list */
> @@ -129,7 +131,9 @@ static void test_feature_flag(const void *data)
>      present = qobject_to_qlist(qom_get(path, "feature-words"));
>      filtered = qobject_to_qlist(qom_get(path, "filtered-features"));
>      value = get_feature_word(present, args->input_eax, args->input_ecx, args->reg);
> -    value |= get_feature_word(filtered, args->input_eax, args->input_ecx, args->reg);
> +    if (!args->ignore_filtered_features) {
> +        value |= get_feature_word(filtered, args->input_eax, args->input_ecx, args->reg);
> +    }
>      qtest_end();
>  
>      g_assert(!!(value & (1U << args->bitnr)) == args->expected_value);
> @@ -336,5 +340,13 @@ int main(int argc, char **argv)
>                       "-machine accel=kvm:tcg -cpu max,mmx=off",
>                       1, 0, "EDX", 23, false);
>  
> +    {
> +    FeatureTestArgs *a;
> +    a = add_feature_test("x86/cpuid/features/monitor-force",
> +                         "-machine accel=kvm:tcg -cpu 486,monitor=force",

Following your above test example, should the 'force' boolean also work
(understood: only for testing / debugging) as below, for a recognized
CPUID flag, taking the example of 'INVPCID'?

  $ qemu-system-x86_64 [...] -cpu Haswell-noTSX,invpcid=force [...]

> +                         1, 0, "ECX",  3, true);
> +    a->ignore_filtered_features = true;
> +    }
> +
>      return g_test_run();
>  }
> -- 
> 2.11.0.259.g40922b1
> 
> 

-- 
/kashyap

Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Eduardo Habkost]

On Thu, May 04, 2017 at 12:16:38PM +0200, Kashyap Chamarthy wrote:
> On Tue, May 02, 2017 at 05:31:15PM -0300, Eduardo Habkost wrote:
> 
> Hi,
> 
> Yet to try this series, a small question in-line.
> 
> > Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> > feature=force on -cpu.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[...]
> > @@ -336,5 +340,13 @@ int main(int argc, char **argv)
> >                       "-machine accel=kvm:tcg -cpu max,mmx=off",
> >                       1, 0, "EDX", 23, false);
> >  
> > +    {
> > +    FeatureTestArgs *a;
> > +    a = add_feature_test("x86/cpuid/features/monitor-force",
> > +                         "-machine accel=kvm:tcg -cpu 486,monitor=force",
> 
> Following your above test example, should the 'force' boolean also work
> (understood: only for testing / debugging) as below, for a recognized
> CPUID flag, taking the example of 'INVPCID'?
> 
>   $ qemu-system-x86_64 [...] -cpu Haswell-noTSX,invpcid=force [...]

This would be valid syntax and would enable invpcid on CPUID,
yes. But on most cases this means you will get a broken VCPU
because the host won't virtualize the feature properly (if it
did, the feature would be reported as available in
GET_SUPPORTED_CPUID and "feature=on" would already work).

-- 
Eduardo

Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Kashyap Chamarthy]

On Fri, May 05, 2017 at 02:59:24PM -0300, Eduardo Habkost wrote:
> On Thu, May 04, 2017 at 12:16:38PM +0200, Kashyap Chamarthy wrote:
> > On Tue, May 02, 2017 at 05:31:15PM -0300, Eduardo Habkost wrote:
> > 
> > Hi,
> > 
> > Yet to try this series, a small question in-line.
> > 
> > > Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> > > feature=force on -cpu.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> [...]
> > > @@ -336,5 +340,13 @@ int main(int argc, char **argv)
> > >                       "-machine accel=kvm:tcg -cpu max,mmx=off",
> > >                       1, 0, "EDX", 23, false);
> > >  
> > > +    {
> > > +    FeatureTestArgs *a;
> > > +    a = add_feature_test("x86/cpuid/features/monitor-force",
> > > +                         "-machine accel=kvm:tcg -cpu 486,monitor=force",
> > 
> > Following your above test example, should the 'force' boolean also work
> > (understood: only for testing / debugging) as below, for a recognized
> > CPUID flag, taking the example of 'INVPCID'?
> > 
> >   $ qemu-system-x86_64 [...] -cpu Haswell-noTSX,invpcid=force [...]
> 
> This would be valid syntax and would enable invpcid on CPUID,
> yes. But on most cases this means you will get a broken VCPU
> because the host won't virtualize the feature properly (if it
> did, the feature would be reported as available in
> GET_SUPPORTED_CPUID and "feature=on" would already work).

Thanks for the clarification.  (/me today learnt that
GET_SUPPORTED_CPUID is a KVM ioctl(2).


-- 
/kashyap

Re: [Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force"

[no-reply]

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Subject: [Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force"
Message-id: 20170502203115.22233-1-ehabkost@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
make docker-test-quick@centos6
make docker-test-mingw@fedora
make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20170502203115.22233-1-ehabkost@redhat.com -> patchew/20170502203115.22233-1-ehabkost@redhat.com
Switched to a new branch 'test'
8ffc6d4 x86: Support feature=force on the command-line
9418a88 tests: Add [+-]feature and feature=on|off test cases
f6954a1 string-input-visitor: Support alternate types
ea7dc8d visitor: Add 'supported_qtypes' parameter to visit_start_alternate()

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-mqqq0zgw/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-mqqq0zgw/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY    RUNNER
    RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache     tar git make gcc g++     zlib-devel glib2-devel SDL-devel pixman-devel     epel-release
HOSTNAME=49d0c1635055
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix    /var/tmp/qemu-build/install
BIOS directory    /var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path       /tmp/qemu-test/src
C compiler        cc
Host C compiler   cc
C++ compiler      
Objective-C compiler cc
ARFLAGS           rv
CFLAGS            -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS       -I/usr/include/pixman-1   -I$(SRC_PATH)/dtc/libfdt -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all
LDFLAGS           -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make              make
install           install
python            python -B
smbd              /usr/sbin/smbd
module support    no
host CPU          x86_64
host big endian   no
target list       x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled     no
sparse enabled    no
strip binaries    yes
profiler          no
static build      no
pixman            system
SDL support       yes (1.2.14)
GTK support       no 
GTK GL support    no
VTE support       no 
TLS priority      NORMAL
GNUTLS support    no
GNUTLS rnd        no
libgcrypt         no
libgcrypt kdf     no
nettle            no 
nettle kdf        no
libtasn1          no
curses support    no
virgl support     no
curl support      no
mingw32 support   no
Audio drivers     oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS support    no
VNC support       yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support       no
brlapi support    no
bluez  support    no
Documentation     no
PIE               yes
vde support       no
netmap support    no
Linux AIO support no
ATTR/XATTR support yes
Install blobs     yes
KVM support       yes
HAX support       no
RDMA support      no
TCG interpreter   no
fdt support       yes
preadv support    yes
fdatasync         yes
madvise           yes
posix_madvise     yes
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backends    log
spice support     no 
rbd support       no
xfsctl support    no
smartcard support no
libusb            no
usb net redir     no
OpenGL support    no
OpenGL dmabufs    no
libiscsi support  no
libnfs support    no
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine pool    yes
debug stack usage no
GlusterFS support no
gcov              gcov
gcov enabled      no
TPM support       yes
libssh2 support   no
TPM passthrough   yes
QOM debugging     yes
lzo support       no
snappy support    no
bzip2 support     no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
replication support yes
VxHS block device no
mkdir -p dtc/libfdt
mkdir -p dtc/tests
  GEN     x86_64-softmmu/config-devices.mak.tmp
  GEN     config-host.h
  GEN     aarch64-softmmu/config-devices.mak.tmp
  GEN     qemu-options.def
  GEN     qapi-visit.h
  GEN     qmp-commands.h
  GEN     qapi-types.h
  GEN     qapi-event.h
  GEN     x86_64-softmmu/config-devices.mak
  GEN     aarch64-softmmu/config-devices.mak
  GEN     qmp-marshal.c
  GEN     qapi-types.c
  GEN     qapi-visit.c
  GEN     qapi-event.c
  GEN     qmp-introspect.h
  GEN     qmp-introspect.c
  GEN     trace/generated-tcg-tracers.h
  GEN     trace/generated-helpers-wrappers.h
  GEN     trace/generated-helpers.h
  GEN     trace/generated-helpers.c
  GEN     module_block.h
  GEN     tests/test-qapi-visit.h
  GEN     tests/test-qapi-types.h
  GEN     tests/test-qmp-commands.h
  GEN     tests/test-qapi-event.h
  GEN     tests/test-qmp-introspect.h
  GEN     trace-root.h
  GEN     util/trace.h
  GEN     crypto/trace.h
  GEN     io/trace.h
  GEN     migration/trace.h
  GEN     block/trace.h
  GEN     backends/trace.h
  GEN     hw/block/trace.h
  GEN     hw/block/dataplane/trace.h
  GEN     hw/char/trace.h
  GEN     hw/intc/trace.h
  GEN     hw/net/trace.h
  GEN     hw/virtio/trace.h
  GEN     hw/audio/trace.h
  GEN     hw/misc/trace.h
  GEN     hw/usb/trace.h
  GEN     hw/scsi/trace.h
  GEN     hw/nvram/trace.h
  GEN     hw/display/trace.h
  GEN     hw/input/trace.h
  GEN     hw/timer/trace.h
  GEN     hw/dma/trace.h
  GEN     hw/sparc/trace.h
  GEN     hw/sd/trace.h
  GEN     hw/isa/trace.h
  GEN     hw/mem/trace.h
  GEN     hw/i386/trace.h
  GEN     hw/i386/xen/trace.h
  GEN     hw/9pfs/trace.h
  GEN     hw/ppc/trace.h
  GEN     hw/pci/trace.h
  GEN     hw/s390x/trace.h
  GEN     hw/vfio/trace.h
  GEN     hw/acpi/trace.h
  GEN     hw/arm/trace.h
  GEN     hw/alpha/trace.h
  GEN     hw/xen/trace.h
  GEN     ui/trace.h
  GEN     audio/trace.h
  GEN     net/trace.h
  GEN     target/arm/trace.h
  GEN     target/i386/trace.h
  GEN     target/mips/trace.h
  GEN     target/sparc/trace.h
  GEN     target/s390x/trace.h
  GEN     target/ppc/trace.h
  GEN     qom/trace.h
  GEN     linux-user/trace.h
  GEN     qapi/trace.h
  GEN     trace-root.c
  GEN     util/trace.c
  GEN     crypto/trace.c
  GEN     io/trace.c
  GEN     migration/trace.c
  GEN     block/trace.c
  GEN     backends/trace.c
  GEN     hw/block/trace.c
  GEN     hw/block/dataplane/trace.c
  GEN     hw/char/trace.c
  GEN     hw/intc/trace.c
  GEN     hw/net/trace.c
  GEN     hw/virtio/trace.c
  GEN     hw/audio/trace.c
  GEN     hw/misc/trace.c
  GEN     hw/usb/trace.c
  GEN     hw/scsi/trace.c
  GEN     hw/nvram/trace.c
  GEN     hw/display/trace.c
  GEN     hw/input/trace.c
  GEN     hw/timer/trace.c
  GEN     hw/dma/trace.c
  GEN     hw/sparc/trace.c
  GEN     hw/sd/trace.c
  GEN     hw/isa/trace.c
  GEN     hw/mem/trace.c
  GEN     hw/i386/trace.c
  GEN     hw/i386/xen/trace.c
  GEN     hw/9pfs/trace.c
  GEN     hw/ppc/trace.c
  GEN     hw/pci/trace.c
  GEN     hw/s390x/trace.c
  GEN     hw/vfio/trace.c
  GEN     hw/acpi/trace.c
  GEN     hw/arm/trace.c
  GEN     hw/alpha/trace.c
  GEN     hw/xen/trace.c
  GEN     ui/trace.c
  GEN     audio/trace.c
  GEN     net/trace.c
  GEN     target/arm/trace.c
  GEN     target/i386/trace.c
  GEN     target/mips/trace.c
  GEN     target/sparc/trace.c
  GEN     target/s390x/trace.c
  GEN     target/ppc/trace.c
  GEN     qom/trace.c
  GEN     linux-user/trace.c
  GEN     qapi/trace.c
  GEN     config-all-devices.mak
	 DEP /tmp/qemu-test/src/dtc/tests/dumptrees.c
	 DEP /tmp/qemu-test/src/dtc/tests/trees.S
	 DEP /tmp/qemu-test/src/dtc/tests/testutils.c
	 DEP /tmp/qemu-test/src/dtc/tests/value-labels.c
	 DEP /tmp/qemu-test/src/dtc/tests/asm_tree_dump.c
	 DEP /tmp/qemu-test/src/dtc/tests/truncated_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/check_path.c
	 DEP /tmp/qemu-test/src/dtc/tests/overlay_bad_fixup.c
	 DEP /tmp/qemu-test/src/dtc/tests/overlay.c
	 DEP /tmp/qemu-test/src/dtc/tests/subnode_iterate.c
	 DEP /tmp/qemu-test/src/dtc/tests/property_iterate.c
	 DEP /tmp/qemu-test/src/dtc/tests/integer-expressions.c
	 DEP /tmp/qemu-test/src/dtc/tests/utilfdt_test.c
	 DEP /tmp/qemu-test/src/dtc/tests/path_offset_aliases.c
	 DEP /tmp/qemu-test/src/dtc/tests/add_subnode_with_nops.c
	 DEP /tmp/qemu-test/src/dtc/tests/dtbs_equal_unordered.c
	 DEP /tmp/qemu-test/src/dtc/tests/dtb_reverse.c
	 DEP /tmp/qemu-test/src/dtc/tests/dtbs_equal_ordered.c
	 DEP /tmp/qemu-test/src/dtc/tests/extra-terminating-null.c
	 DEP /tmp/qemu-test/src/dtc/tests/incbin.c
	 DEP /tmp/qemu-test/src/dtc/tests/boot-cpuid.c
	 DEP /tmp/qemu-test/src/dtc/tests/phandle_format.c
	 DEP /tmp/qemu-test/src/dtc/tests/path-references.c
	 DEP /tmp/qemu-test/src/dtc/tests/references.c
	 DEP /tmp/qemu-test/src/dtc/tests/string_escapes.c
	 DEP /tmp/qemu-test/src/dtc/tests/propname_escapes.c
	 DEP /tmp/qemu-test/src/dtc/tests/appendprop2.c
	 DEP /tmp/qemu-test/src/dtc/tests/appendprop1.c
	 DEP /tmp/qemu-test/src/dtc/tests/del_node.c
	 DEP /tmp/qemu-test/src/dtc/tests/del_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/setprop.c
	 DEP /tmp/qemu-test/src/dtc/tests/set_name.c
	 DEP /tmp/qemu-test/src/dtc/tests/rw_tree1.c
	 DEP /tmp/qemu-test/src/dtc/tests/open_pack.c
	 DEP /tmp/qemu-test/src/dtc/tests/nopulate.c
	 DEP /tmp/qemu-test/src/dtc/tests/mangle-layout.c
	 DEP /tmp/qemu-test/src/dtc/tests/move_and_save.c
	 DEP /tmp/qemu-test/src/dtc/tests/sw_tree1.c
	 DEP /tmp/qemu-test/src/dtc/tests/nop_node.c
	 DEP /tmp/qemu-test/src/dtc/tests/nop_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/setprop_inplace.c
	 DEP /tmp/qemu-test/src/dtc/tests/stringlist.c
	 DEP /tmp/qemu-test/src/dtc/tests/addr_size_cells.c
	 DEP /tmp/qemu-test/src/dtc/tests/notfound.c
	 DEP /tmp/qemu-test/src/dtc/tests/sized_cells.c
	 DEP /tmp/qemu-test/src/dtc/tests/char_literal.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_alias.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_offset_by_compatible.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_check_compatible.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_offset_by_phandle.c
	 DEP /tmp/qemu-test/src/dtc/tests/node_offset_by_prop_value.c
	 DEP /tmp/qemu-test/src/dtc/tests/parent_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/supernode_atdepth_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_path.c
	 DEP /tmp/qemu-test/src/dtc/tests/getprop.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_name.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_phandle.c
	 DEP /tmp/qemu-test/src/dtc/tests/path_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/subnode_offset.c
	 DEP /tmp/qemu-test/src/dtc/tests/find_property.c
	 DEP /tmp/qemu-test/src/dtc/tests/root_node.c
	 DEP /tmp/qemu-test/src/dtc/tests/get_mem_rsv.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_overlay.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_addresses.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_strerror.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_empty_tree.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_rw.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_sw.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_wip.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt_ro.c
	 DEP /tmp/qemu-test/src/dtc/libfdt/fdt.c
	 DEP /tmp/qemu-test/src/dtc/util.c
	 DEP /tmp/qemu-test/src/dtc/fdtput.c
	 DEP /tmp/qemu-test/src/dtc/fdtget.c
	 DEP /tmp/qemu-test/src/dtc/fdtdump.c
	 LEX convert-dtsv0-lexer.lex.c
make[1]: flex: Command not found
	 DEP /tmp/qemu-test/src/dtc/srcpos.c
	 DEP /tmp/qemu-test/src/dtc/treesource.c
	 BISON dtc-parser.tab.c
	 LEX dtc-lexer.lex.c
make[1]: bison: Command not found
make[1]: flex: Command not found
	 DEP /tmp/qemu-test/src/dtc/livetree.c
	 DEP /tmp/qemu-test/src/dtc/fstree.c
	 DEP /tmp/qemu-test/src/dtc/flattree.c
	 DEP /tmp/qemu-test/src/dtc/dtc.c
	 DEP /tmp/qemu-test/src/dtc/data.c
	 DEP /tmp/qemu-test/src/dtc/checks.c
	CHK version_gen.h
	 LEX convert-dtsv0-lexer.lex.c
	 BISON dtc-parser.tab.c
	 LEX dtc-lexer.lex.c
make[1]: flex: Command not found
make[1]: bison: Command not found
make[1]: flex: Command not found
	UPD version_gen.h
	 DEP /tmp/qemu-test/src/dtc/util.c
	 LEX convert-dtsv0-lexer.lex.c
	 BISON dtc-parser.tab.c
make[1]: flex: Command not found
make[1]: bison: Command not found
	 LEX dtc-lexer.lex.c
make[1]: flex: Command not found
	 CC libfdt/fdt.o
	 CC libfdt/fdt_ro.o
	 CC libfdt/fdt_wip.o
	 CC libfdt/fdt_sw.o
	 CC libfdt/fdt_empty_tree.o
	 CC libfdt/fdt_rw.o
	 CC libfdt/fdt_strerror.o
	 CC libfdt/fdt_addresses.o
	 CC libfdt/fdt_overlay.o
	 AR libfdt/libfdt.a
ar: creating libfdt/libfdt.a
a - libfdt/fdt.o
a - libfdt/fdt_ro.o
a - libfdt/fdt_wip.o
a - libfdt/fdt_sw.o
a - libfdt/fdt_rw.o
a - libfdt/fdt_strerror.o
a - libfdt/fdt_empty_tree.o
a - libfdt/fdt_addresses.o
a - libfdt/fdt_overlay.o
	 LEX convert-dtsv0-lexer.lex.c
	 LEX dtc-lexer.lex.c
	 BISON dtc-parser.tab.c
make[1]: flex: Command not found
make[1]: bison: Command not found
make[1]: flex: Command not found
  CC      tests/qemu-iotests/socket_scm_helper.o
  GEN     qga/qapi-generated/qga-qapi-types.h
  GEN     qga/qapi-generated/qga-qapi-visit.h
  GEN     qga/qapi-generated/qga-qapi-types.c
  GEN     qga/qapi-generated/qga-qmp-commands.h
  GEN     qga/qapi-generated/qga-qmp-marshal.c
  GEN     qga/qapi-generated/qga-qapi-visit.c
  CC      qmp-introspect.o
  CC      qapi-types.o
  CC      qapi-visit.o
  CC      qapi-event.o
  CC      qapi/qapi-visit-core.o
  CC      qapi/qapi-dealloc-visitor.o
  CC      qapi/qobject-input-visitor.o
  CC      qapi/qobject-output-visitor.o
  CC      qapi/qmp-registry.o
  CC      qapi/qmp-dispatch.o
  CC      qapi/string-input-visitor.o
  CC      qapi/string-output-visitor.o
  CC      qapi/opts-visitor.o
  CC      qapi/qapi-clone-visitor.o
  CC      qapi/qmp-event.o
  CC      qapi/qapi-util.o
  CC      qobject/qnull.o
  CC      qobject/qint.o
  CC      qobject/qstring.o
  CC      qobject/qdict.o
  CC      qobject/qlist.o
  CC      qobject/qfloat.o
  CC      qobject/qbool.o
  CC      qobject/qjson.o
  CC      qobject/qobject.o
  CC      qobject/json-lexer.o
  CC      qobject/json-streamer.o
  CC      qobject/json-parser.o
  CC      trace/control.o
  CC      trace/qmp.o
  CC      util/osdep.o
  CC      util/cutils.o
  CC      util/unicode.o
  CC      util/qemu-timer-common.o
  CC      util/bufferiszero.o
  CC      util/lockcnt.o
  CC      util/aiocb.o
  CC      util/async.o
  CC      util/thread-pool.o
  CC      util/qemu-timer.o
  CC      util/main-loop.o
  CC      util/iohandler.o
  CC      util/aio-posix.o
  CC      util/compatfd.o
  CC      util/event_notifier-posix.o
  CC      util/mmap-alloc.o
  CC      util/oslib-posix.o
  CC      util/qemu-openpty.o
  CC      util/qemu-thread-posix.o
  CC      util/memfd.o
  CC      util/envlist.o
  CC      util/path.o
  CC      util/module.o
  CC      util/host-utils.o
  CC      util/bitmap.o
  CC      util/bitops.o
  CC      util/hbitmap.o
  CC      util/acl.o
  CC      util/fifo8.o
  CC      util/error.o
  CC      util/qemu-error.o
  CC      util/id.o
  CC      util/iov.o
  CC      util/qemu-sockets.o
  CC      util/qemu-config.o
  CC      util/uri.o
  CC      util/notify.o
  CC      util/qemu-option.o
  CC      util/qemu-progress.o
  CC      util/keyval.o
  CC      util/hexdump.o
  CC      util/uuid.o
  CC      util/crc32c.o
  CC      util/readline.o
  CC      util/throttle.o
  CC      util/getauxval.o
  CC      util/rcu.o
  CC      util/qemu-coroutine.o
  CC      util/qemu-coroutine-lock.o
  CC      util/qemu-coroutine-io.o
  CC      util/qemu-coroutine-sleep.o
  CC      util/coroutine-ucontext.o
  CC      util/buffer.o
  CC      util/timed-average.o
  CC      util/base64.o
  CC      util/log.o
  CC      util/qdist.o
  CC      util/qht.o
  CC      util/range.o
  CC      util/systemd.o
  CC      trace-root.o
  CC      util/trace.o
  CC      crypto/trace.o
  CC      io/trace.o
  CC      migration/trace.o
  CC      block/trace.o
  CC      backends/trace.o
  CC      hw/block/trace.o
  CC      hw/block/dataplane/trace.o
  CC      hw/char/trace.o
  CC      hw/intc/trace.o
  CC      hw/net/trace.o
  CC      hw/virtio/trace.o
  CC      hw/audio/trace.o
  CC      hw/misc/trace.o
  CC      hw/usb/trace.o
  CC      hw/scsi/trace.o
  CC      hw/nvram/trace.o
  CC      hw/display/trace.o
  CC      hw/input/trace.o
  CC      hw/timer/trace.o
  CC      hw/dma/trace.o
  CC      hw/sparc/trace.o
  CC      hw/sd/trace.o
  CC      hw/isa/trace.o
  CC      hw/mem/trace.o
  CC      hw/i386/trace.o
  CC      hw/i386/xen/trace.o
  CC      hw/9pfs/trace.o
  CC      hw/ppc/trace.o
  CC      hw/pci/trace.o
  CC      hw/s390x/trace.o
  CC      hw/vfio/trace.o
  CC      hw/acpi/trace.o
  CC      hw/arm/trace.o
  CC      hw/alpha/trace.o
  CC      hw/xen/trace.o
  CC      audio/trace.o
  CC      ui/trace.o
  CC      net/trace.o
  CC      target/arm/trace.o
  CC      target/i386/trace.o
  CC      target/mips/trace.o
  CC      target/sparc/trace.o
  CC      target/s390x/trace.o
  CC      target/ppc/trace.o
  CC      qom/trace.o
  CC      linux-user/trace.o
  CC      qapi/trace.o
  CC      crypto/pbkdf-stub.o
  CC      stubs/arch-query-cpu-def.o
  CC      stubs/arch-query-cpu-model-expansion.o
  CC      stubs/arch-query-cpu-model-comparison.o
  CC      stubs/arch-query-cpu-model-baseline.o
  CC      stubs/bdrv-next-monitor-owned.o
  CC      stubs/blk-commit-all.o
  CC      stubs/clock-warp.o
  CC      stubs/blockdev-close-all-bdrv-states.o
  CC      stubs/cpu-get-clock.o
  CC      stubs/cpu-get-icount.o
  CC      stubs/dump.o
  CC      stubs/error-printf.o
  CC      stubs/fdset.o
  CC      stubs/gdbstub.o
  CC      stubs/get-vm-name.o
  CC      stubs/iothread.o
  CC      stubs/iothread-lock.o
  CC      stubs/is-daemonized.o
  CC      stubs/machine-init-done.o
  CC      stubs/migr-blocker.o
  CC      stubs/monitor.o
  CC      stubs/notify-event.o
  CC      stubs/qtest.o
  CC      stubs/replay.o
  CC      stubs/runstate-check.o
  CC      stubs/set-fd-handler.o
  CC      stubs/slirp.o
  CC      stubs/sysbus.o
  CC      stubs/trace-control.o
  CC      stubs/uuid.o
  CC      stubs/vm-stop.o
  CC      stubs/vmstate.o
  CC      stubs/qmp_pc_dimm_device_list.o
  CC      stubs/target-monitor-defs.o
  CC      stubs/target-get-monitor-def.o
  CC      stubs/pc_madt_cpu_entry.o
  CC      stubs/vmgenid.o
  CC      stubs/xen-common.o
  CC      stubs/xen-hvm.o
  CC      contrib/ivshmem-client/ivshmem-client.o
  CC      contrib/ivshmem-client/main.o
  CC      contrib/ivshmem-server/ivshmem-server.o
  CC      contrib/ivshmem-server/main.o
  CC      qemu-nbd.o
  CC      block.o
  CC      blockjob.o
  CC      qemu-io-cmds.o
  CC      replication.o
  CC      block/raw-format.o
  CC      block/qcow.o
  CC      block/vdi.o
  CC      block/vmdk.o
  CC      block/cloop.o
  CC      block/bochs.o
  CC      block/vpc.o
  CC      block/vvfat.o
  CC      block/dmg.o
  CC      block/qcow2.o
  CC      block/qcow2-refcount.o
  CC      block/qcow2-cluster.o
  CC      block/qcow2-snapshot.o
  CC      block/qcow2-cache.o
  CC      block/qed.o
  CC      block/qed-gencb.o
  CC      block/qed-l2-cache.o
  CC      block/qed-table.o
  CC      block/qed-cluster.o
  CC      block/qed-check.o
  CC      block/vhdx.o
  CC      block/vhdx-endian.o
  CC      block/vhdx-log.o
  CC      block/quorum.o
  CC      block/parallels.o
  CC      block/blkdebug.o
  CC      block/blkverify.o
  CC      block/blkreplay.o
  CC      block/block-backend.o
  CC      block/snapshot.o
  CC      block/qapi.o
  CC      block/file-posix.o
  CC      block/null.o
  CC      block/mirror.o
  CC      block/commit.o
  CC      block/io.o
  CC      block/throttle-groups.o
  CC      block/nbd.o
  CC      block/nbd-client.o
  CC      block/sheepdog.o
  CC      block/accounting.o
  CC      block/dirty-bitmap.o
  CC      block/write-threshold.o
  CC      block/backup.o
  CC      block/replication.o
  CC      block/crypto.o
  CC      nbd/server.o
  CC      nbd/client.o
  CC      nbd/common.o
  CC      crypto/init.o
  CC      crypto/hash.o
  CC      crypto/hash-glib.o
  CC      crypto/hmac.o
  CC      crypto/hmac-glib.o
  CC      crypto/aes.o
  CC      crypto/desrfb.o
  CC      crypto/cipher.o
  CC      crypto/tlscreds.o
  CC      crypto/tlscredsanon.o
  CC      crypto/tlscredsx509.o
  CC      crypto/tlssession.o
  CC      crypto/secret.o
  CC      crypto/random-platform.o
  CC      crypto/pbkdf.o
  CC      crypto/ivgen.o
  CC      crypto/ivgen-essiv.o
  CC      crypto/ivgen-plain.o
  CC      crypto/ivgen-plain64.o
  CC      crypto/afsplit.o
  CC      crypto/xts.o
  CC      crypto/block.o
  CC      crypto/block-qcow.o
  CC      crypto/block-luks.o
  CC      io/channel.o
  CC      io/channel-buffer.o
  CC      io/channel-command.o
  CC      io/channel-file.o
  CC      io/channel-socket.o
  CC      io/channel-tls.o
  CC      io/channel-watch.o
  CC      io/channel-websock.o
  CC      io/channel-util.o
  CC      io/dns-resolver.o
  CC      qom/object.o
  CC      io/task.o
  CC      qom/container.o
  CC      qom/qom-qobject.o
  CC      qom/object_interfaces.o
  GEN     qemu-img-cmds.h
  CC      qemu-io.o
  CC      qemu-bridge-helper.o
  CC      blockdev.o
  CC      blockdev-nbd.o
  CC      iothread.o
  CC      qdev-monitor.o
  CC      device-hotplug.o
  CC      os-posix.o
  CC      page_cache.o
  CC      accel.o
  CC      bt-host.o
  CC      bt-vhci.o
  CC      dma-helpers.o
  CC      vl.o
  CC      tpm.o
  CC      device_tree.o
  CC      qmp-marshal.o
  CC      hmp.o
  CC      qmp.o
  CC      cpus-common.o
  CC      audio/audio.o
  CC      audio/noaudio.o
  CC      audio/wavaudio.o
  CC      audio/mixeng.o
  CC      audio/sdlaudio.o
  CC      audio/ossaudio.o
  CC      audio/wavcapture.o
  CC      backends/rng.o
  CC      backends/rng-egd.o
  CC      backends/rng-random.o
  CC      backends/msmouse.o
  CC      backends/wctablet.o
  CC      backends/testdev.o
  CC      backends/tpm.o
  CC      backends/hostmem.o
  CC      backends/hostmem-ram.o
  CC      backends/hostmem-file.o
  CC      backends/cryptodev.o
  CC      backends/cryptodev-builtin.o
  CC      block/stream.o
  CC      disas/arm.o
  CC      disas/i386.o
  CC      fsdev/qemu-fsdev-dummy.o
  CC      fsdev/qemu-fsdev-opts.o
  CC      fsdev/qemu-fsdev-throttle.o
  CC      hw/acpi/core.o
  CC      hw/acpi/piix4.o
  CC      hw/acpi/pcihp.o
  CC      hw/acpi/ich9.o
  CC      hw/acpi/tco.o
  CC      hw/acpi/cpu_hotplug.o
  CC      hw/acpi/memory_hotplug.o
  CC      hw/acpi/cpu.o
  CC      hw/acpi/nvdimm.o
  CC      hw/acpi/vmgenid.o
  CC      hw/acpi/acpi_interface.o
  CC      hw/acpi/bios-linker-loader.o
  CC      hw/acpi/aml-build.o
  CC      hw/acpi/ipmi.o
  CC      hw/acpi/acpi-stub.o
  CC      hw/acpi/ipmi-stub.o
  CC      hw/audio/sb16.o
  CC      hw/audio/es1370.o
  CC      hw/audio/ac97.o
  CC      hw/audio/fmopl.o
  CC      hw/audio/adlib.o
  CC      hw/audio/gus.o
  CC      hw/audio/gusemu_hal.o
  CC      hw/audio/gusemu_mixer.o
  CC      hw/audio/cs4231a.o
  CC      hw/audio/intel-hda.o
  CC      hw/audio/hda-codec.o
  CC      hw/audio/pcspk.o
  CC      hw/audio/wm8750.o
  CC      hw/audio/pl041.o
  CC      hw/audio/lm4549.o
  CC      hw/audio/marvell_88w8618.o
  CC      hw/block/block.o
  CC      hw/block/cdrom.o
  CC      hw/block/hd-geometry.o
  CC      hw/block/fdc.o
  CC      hw/block/m25p80.o
  CC      hw/block/nand.o
  CC      hw/block/pflash_cfi01.o
  CC      hw/block/pflash_cfi02.o
  CC      hw/block/ecc.o
  CC      hw/block/onenand.o
  CC      hw/block/nvme.o
  CC      hw/bt/core.o
  CC      hw/bt/l2cap.o
  CC      hw/bt/sdp.o
  CC      hw/bt/hci.o
  CC      hw/bt/hid.o
  CC      hw/bt/hci-csr.o
  CC      hw/char/ipoctal232.o
  CC      hw/char/parallel.o
  CC      hw/char/pl011.o
  CC      hw/char/serial.o
  CC      hw/char/serial-isa.o
  CC      hw/char/serial-pci.o
  CC      hw/char/virtio-console.o
  CC      hw/char/cadence_uart.o
  CC      hw/char/debugcon.o
  CC      hw/char/imx_serial.o
  CC      hw/core/qdev.o
  CC      hw/core/qdev-properties.o
  CC      hw/core/bus.o
  CC      hw/core/reset.o
  CC      hw/core/fw-path-provider.o
  CC      hw/core/irq.o
  CC      hw/core/hotplug.o
  CC      hw/core/ptimer.o
  CC      hw/core/sysbus.o
  CC      hw/core/machine.o
  CC      hw/core/loader.o
  CC      hw/core/qdev-properties-system.o
  CC      hw/core/register.o
  CC      hw/core/or-irq.o
  CC      hw/core/platform-bus.o
  CC      hw/display/ads7846.o
  CC      hw/display/cirrus_vga.o
  CC      hw/display/pl110.o
  CC      hw/display/ssd0303.o
  CC      hw/display/ssd0323.o
  CC      hw/display/vga-pci.o
  CC      hw/display/vga-isa.o
  CC      hw/display/vmware_vga.o
  CC      hw/display/blizzard.o
  CC      hw/display/exynos4210_fimd.o
  CC      hw/display/framebuffer.o
  CC      hw/display/tc6393xb.o
  CC      hw/dma/pl080.o
  CC      hw/dma/pl330.o
  CC      hw/dma/i8257.o
  CC      hw/dma/xlnx-zynq-devcfg.o
  CC      hw/gpio/max7310.o
  CC      hw/gpio/pl061.o
  CC      hw/gpio/zaurus.o
  CC      hw/gpio/gpio_key.o
  CC      hw/i2c/core.o
  CC      hw/i2c/smbus.o
  CC      hw/i2c/smbus_eeprom.o
  CC      hw/i2c/i2c-ddc.o
  CC      hw/i2c/versatile_i2c.o
  CC      hw/i2c/smbus_ich9.o
  CC      hw/i2c/pm_smbus.o
  CC      hw/i2c/bitbang_i2c.o
  CC      hw/i2c/exynos4210_i2c.o
  CC      hw/i2c/imx_i2c.o
  CC      hw/i2c/aspeed_i2c.o
  CC      hw/ide/core.o
  CC      hw/ide/atapi.o
  CC      hw/ide/qdev.o
  CC      hw/ide/pci.o
  CC      hw/ide/isa.o
  CC      hw/ide/piix.o
  CC      hw/ide/microdrive.o
  CC      hw/ide/ahci.o
  CC      hw/ide/ich.o
  CC      hw/input/hid.o
  CC      hw/input/lm832x.o
  CC      hw/input/pckbd.o
  CC      hw/input/pl050.o
  CC      hw/input/ps2.o
  CC      hw/input/stellaris_input.o
  CC      hw/input/vmmouse.o
  CC      hw/input/tsc2005.o
  CC      hw/input/virtio-input.o
  CC      hw/input/virtio-input-hid.o
  CC      hw/input/virtio-input-host.o
  CC      hw/intc/i8259_common.o
  CC      hw/intc/i8259.o
  CC      hw/intc/pl190.o
  CC      hw/intc/imx_avic.o
  CC      hw/intc/realview_gic.o
  CC      hw/intc/ioapic_common.o
  CC      hw/intc/arm_gic_common.o
  CC      hw/intc/arm_gic.o
  CC      hw/intc/arm_gicv2m.o
  CC      hw/intc/arm_gicv3_common.o
  CC      hw/intc/arm_gicv3.o
  CC      hw/intc/arm_gicv3_dist.o
  CC      hw/intc/arm_gicv3_redist.o
  CC      hw/intc/arm_gicv3_its_common.o
  CC      hw/intc/intc.o
  CC      hw/ipack/ipack.o
  CC      hw/ipack/tpci200.o
  CC      hw/ipmi/ipmi.o
  CC      hw/ipmi/ipmi_bmc_sim.o
  CC      hw/ipmi/ipmi_bmc_extern.o
  CC      hw/ipmi/isa_ipmi_kcs.o
  CC      hw/ipmi/isa_ipmi_bt.o
  CC      hw/isa/isa-bus.o
  CC      hw/isa/apm.o
  CC      hw/mem/pc-dimm.o
  CC      hw/mem/nvdimm.o
  CC      hw/misc/applesmc.o
  CC      hw/misc/max111x.o
  CC      hw/misc/tmp105.o
  CC      hw/misc/debugexit.o
  CC      hw/misc/sga.o
  CC      hw/misc/pc-testdev.o
  CC      hw/misc/pci-testdev.o
  CC      hw/misc/unimp.o
  CC      hw/misc/arm_l2x0.o
  CC      hw/misc/arm_integrator_debug.o
  CC      hw/misc/a9scu.o
  CC      hw/misc/arm11scu.o
  CC      hw/net/ne2000.o
  CC      hw/net/eepro100.o
  CC      hw/net/pcnet-pci.o
  CC      hw/net/pcnet.o
  CC      hw/net/e1000.o
  CC      hw/net/e1000x_common.o
  CC      hw/net/net_tx_pkt.o
  CC      hw/net/net_rx_pkt.o
  CC      hw/net/e1000e.o
  CC      hw/net/e1000e_core.o
  CC      hw/net/rtl8139.o
  CC      hw/net/vmxnet3.o
  CC      hw/net/smc91c111.o
  CC      hw/net/lan9118.o
  CC      hw/net/ne2000-isa.o
  CC      hw/net/xgmac.o
  CC      hw/net/allwinner_emac.o
  CC      hw/net/imx_fec.o
  CC      hw/net/cadence_gem.o
  CC      hw/net/stellaris_enet.o
  CC      hw/net/ftgmac100.o
  CC      hw/net/rocker/rocker.o
  CC      hw/net/rocker/rocker_fp.o
  CC      hw/net/rocker/rocker_desc.o
  CC      hw/net/rocker/rocker_world.o
  CC      hw/net/rocker/rocker_of_dpa.o
  CC      hw/nvram/eeprom93xx.o
  CC      hw/nvram/fw_cfg.o
  CC      hw/nvram/chrp_nvram.o
  CC      hw/pci-bridge/pci_bridge_dev.o
  CC      hw/pci-bridge/pcie_root_port.o
  CC      hw/pci-bridge/gen_pcie_root_port.o
  CC      hw/pci-bridge/pci_expander_bridge.o
  CC      hw/pci-bridge/xio3130_upstream.o
  CC      hw/pci-bridge/xio3130_downstream.o
  CC      hw/pci-bridge/ioh3420.o
  CC      hw/pci-bridge/i82801b11.o
  CC      hw/pci-host/pam.o
  CC      hw/pci-host/versatile.o
  CC      hw/pci-host/piix.o
  CC      hw/pci-host/q35.o
  CC      hw/pci-host/gpex.o
  CC      hw/pci/pci.o
  CC      hw/pci/pci_bridge.o
  CC      hw/pci/msix.o
  CC      hw/pci/msi.o
  CC      hw/pci/shpc.o
  CC      hw/pci/pci_host.o
  CC      hw/pci/slotid_cap.o
  CC      hw/pci/pcie_host.o
  CC      hw/pci/pcie.o
  CC      hw/pci/pcie_aer.o
  CC      hw/pci/pcie_port.o
  CC      hw/pci/pci-stub.o
  CC      hw/pcmcia/pcmcia.o
  CC      hw/scsi/scsi-disk.o
  CC      hw/scsi/scsi-generic.o
  CC      hw/scsi/scsi-bus.o
  CC      hw/scsi/lsi53c895a.o
  CC      hw/scsi/mptsas.o
  CC      hw/scsi/mptconfig.o
  CC      hw/scsi/mptendian.o
  CC      hw/scsi/megasas.o
  CC      hw/scsi/vmw_pvscsi.o
  CC      hw/scsi/esp.o
  CC      hw/scsi/esp-pci.o
  CC      hw/sd/pl181.o
  CC      hw/sd/ssi-sd.o
  CC      hw/sd/sd.o
  CC      hw/sd/core.o
  CC      hw/sd/sdhci.o
  CC      hw/smbios/smbios.o
  CC      hw/smbios/smbios_type_38.o
  CC      hw/smbios/smbios-stub.o
  CC      hw/smbios/smbios_type_38-stub.o
  CC      hw/ssi/pl022.o
  CC      hw/ssi/ssi.o
  CC      hw/ssi/xilinx_spips.o
  CC      hw/ssi/aspeed_smc.o
  CC      hw/ssi/stm32f2xx_spi.o
  CC      hw/timer/arm_timer.o
  CC      hw/timer/arm_mptimer.o
  CC      hw/timer/armv7m_systick.o
  CC      hw/timer/a9gtimer.o
  CC      hw/timer/cadence_ttc.o
  CC      hw/timer/ds1338.o
  CC      hw/timer/hpet.o
  CC      hw/timer/i8254_common.o
  CC      hw/timer/i8254.o
  CC      hw/timer/pl031.o
  CC      hw/timer/twl92230.o
  CC      hw/timer/imx_epit.o
  CC      hw/timer/imx_gpt.o
  CC      hw/timer/stm32f2xx_timer.o
  CC      hw/timer/aspeed_timer.o
  CC      hw/tpm/tpm_tis.o
  CC      hw/tpm/tpm_passthrough.o
  CC      hw/tpm/tpm_util.o
  CC      hw/usb/combined-packet.o
  CC      hw/usb/core.o
  CC      hw/usb/bus.o
  CC      hw/usb/libhw.o
  CC      hw/usb/desc.o
  CC      hw/usb/desc-msos.o
  CC      hw/usb/hcd-uhci.o
  CC      hw/usb/hcd-ohci.o
  CC      hw/usb/hcd-ehci.o
  CC      hw/usb/hcd-ehci-pci.o
  CC      hw/usb/hcd-ehci-sysbus.o
  CC      hw/usb/hcd-xhci.o
  CC      hw/usb/hcd-musb.o
  CC      hw/usb/dev-hub.o
  CC      hw/usb/dev-hid.o
  CC      hw/usb/dev-wacom.o
  CC      hw/usb/dev-storage.o
  CC      hw/usb/dev-uas.o
  CC      hw/usb/dev-audio.o
  CC      hw/usb/dev-serial.o
  CC      hw/usb/dev-network.o
  CC      hw/usb/dev-bluetooth.o
  CC      hw/usb/dev-smartcard-reader.o
  CC      hw/usb/dev-mtp.o
  CC      hw/usb/host-stub.o
  CC      hw/virtio/virtio-rng.o
  CC      hw/virtio/virtio-pci.o
  CC      hw/virtio/virtio-bus.o
  CC      hw/virtio/virtio-mmio.o
  CC      hw/virtio/vhost-stub.o
  CC      hw/watchdog/watchdog.o
  CC      hw/watchdog/wdt_i6300esb.o
  CC      hw/watchdog/wdt_ib700.o
  CC      hw/watchdog/wdt_aspeed.o
  CC      migration/migration.o
  CC      migration/socket.o
  CC      migration/fd.o
  CC      migration/exec.o
  CC      migration/tls.o
  CC      migration/colo-comm.o
  CC      migration/colo.o
  CC      migration/colo-failover.o
  CC      migration/vmstate.o
  CC      migration/qemu-file.o
  CC      migration/qemu-file-channel.o
  CC      migration/xbzrle.o
  CC      migration/postcopy-ram.o
  CC      migration/qjson.o
  CC      migration/block.o
  CC      net/net.o
  CC      net/queue.o
  CC      net/checksum.o
  CC      net/util.o
  CC      net/hub.o
  CC      net/socket.o
  CC      net/dump.o
  CC      net/eth.o
  CC      net/l2tpv3.o
  CC      net/tap.o
  CC      net/vhost-user.o
  CC      net/tap-linux.o
  CC      net/slirp.o
  CC      net/filter.o
  CC      net/filter-buffer.o
  CC      net/filter-mirror.o
  CC      net/colo-compare.o
  CC      net/colo.o
  CC      net/filter-rewriter.o
  CC      net/filter-replay.o
  CC      qom/cpu.o
  CC      replay/replay.o
  CC      replay/replay-internal.o
  CC      replay/replay-events.o
  CC      replay/replay-time.o
  CC      replay/replay-input.o
  CC      replay/replay-char.o
  CC      replay/replay-snapshot.o
  CC      replay/replay-net.o
  CC      replay/replay-audio.o
  CC      slirp/cksum.o
  CC      slirp/if.o
  CC      slirp/ip_icmp.o
/tmp/qemu-test/src/replay/replay-internal.c: In function ‘replay_put_array’:
/tmp/qemu-test/src/replay/replay-internal.c:65: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result
  CC      slirp/ip6_icmp.o
  CC      slirp/ip6_input.o
  CC      slirp/ip6_output.o
  CC      slirp/ip_input.o
  CC      slirp/ip_output.o
  CC      slirp/dnssearch.o
  CC      slirp/dhcpv6.o
  CC      slirp/slirp.o
  CC      slirp/mbuf.o
  CC      slirp/misc.o
  CC      slirp/sbuf.o
  CC      slirp/socket.o
  CC      slirp/tcp_input.o
  CC      slirp/tcp_output.o
  CC      slirp/tcp_subr.o
  CC      slirp/tcp_timer.o
  CC      slirp/udp.o
  CC      slirp/udp6.o
  CC      slirp/bootp.o
  CC      slirp/tftp.o
  CC      slirp/arp_table.o
/tmp/qemu-test/src/slirp/tcp_input.c: In function ‘tcp_input’:
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_p’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_len’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_tos’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_id’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_off’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_ttl’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_sum’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_src.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_dst.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:220: warning: ‘save_ip6.ip_nh’ may be used uninitialized in this function
  CC      slirp/ndp_table.o
  CC      slirp/ncsi.o
  CC      ui/keymaps.o
  CC      ui/console.o
  CC      ui/cursor.o
  CC      ui/qemu-pixman.o
  CC      ui/input.o
  CC      ui/input-keymap.o
  CC      ui/input-legacy.o
  CC      ui/input-linux.o
  CC      ui/sdl.o
  CC      ui/sdl_zoom.o
  CC      ui/x_keymap.o
  CC      ui/vnc.o
  CC      ui/vnc-enc-zlib.o
  CC      ui/vnc-enc-hextile.o
  CC      ui/vnc-enc-tight.o
  CC      ui/vnc-palette.o
  CC      ui/vnc-enc-zrle.o
  CC      ui/vnc-auth-vencrypt.o
  CC      ui/vnc-ws.o
  CC      ui/vnc-jobs.o
  CC      chardev/char.o
  CC      chardev/char-fd.o
  CC      chardev/char-file.o
  CC      chardev/char-io.o
  CC      chardev/char-mux.o
  CC      chardev/char-null.o
  CC      chardev/char-parallel.o
  CC      chardev/char-pipe.o
  CC      chardev/char-pty.o
  CC      chardev/char-ringbuf.o
  CC      chardev/char-serial.o
  CC      chardev/char-socket.o
  CC      chardev/char-stdio.o
  CC      chardev/char-udp.o
  LINK    tests/qemu-iotests/socket_scm_helper
  CC      qga/commands.o
  AS      optionrom/multiboot.o
  AS      optionrom/linuxboot.o
  CC      optionrom/linuxboot_dma.o
cc: unrecognized option '-no-integrated-as'
cc: unrecognized option '-no-integrated-as'
  AS      optionrom/kvmvapic.o
  BUILD   optionrom/multiboot.img
  BUILD   optionrom/linuxboot.img
  BUILD   optionrom/linuxboot_dma.img
  BUILD   optionrom/kvmvapic.img
  CC      qga/guest-agent-command-state.o
  BUILD   optionrom/multiboot.raw
  BUILD   optionrom/linuxboot.raw
  BUILD   optionrom/linuxboot_dma.raw
  BUILD   optionrom/kvmvapic.raw
  SIGN    optionrom/multiboot.bin
  SIGN    optionrom/linuxboot.bin
  SIGN    optionrom/linuxboot_dma.bin
  SIGN    optionrom/kvmvapic.bin
  CC      qga/main.o
  CC      qga/commands-posix.o
  CC      qga/channel-posix.o
  CC      qga/qapi-generated/qga-qapi-types.o
  CC      qga/qapi-generated/qga-qapi-visit.o
  CC      qga/qapi-generated/qga-qmp-marshal.o
  AR      libqemuutil.a
  AR      libqemustub.a
  CC      qemu-img.o
  LINK    ivshmem-client
  LINK    ivshmem-server
  LINK    qemu-nbd
  LINK    qemu-io
  LINK    qemu-bridge-helper
  LINK    qemu-ga
  GEN     x86_64-softmmu/hmp-commands.h
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/config-target.h
  LINK    qemu-img
  GEN     aarch64-softmmu/hmp-commands-info.h
  GEN     aarch64-softmmu/config-target.h
  GEN     aarch64-softmmu/hmp-commands.h
  CC      x86_64-softmmu/exec.o
  CC      x86_64-softmmu/cpu-exec.o
  CC      x86_64-softmmu/translate-all.o
  CC      x86_64-softmmu/translate-common.o
  CC      x86_64-softmmu/cpu-exec-common.o
  CC      x86_64-softmmu/tcg/tcg.o
  CC      aarch64-softmmu/exec.o
  CC      aarch64-softmmu/translate-all.o
  CC      x86_64-softmmu/tcg/tcg-op.o
  CC      x86_64-softmmu/tcg/optimize.o
  CC      x86_64-softmmu/tcg/tcg-common.o
  CC      x86_64-softmmu/fpu/softfloat.o
  CC      x86_64-softmmu/disas.o
  CC      aarch64-softmmu/cpu-exec.o
  CC      x86_64-softmmu/tcg-runtime.o
  CC      x86_64-softmmu/hax-stub.o
  CC      x86_64-softmmu/arch_init.o
  CC      x86_64-softmmu/cpus.o
  CC      x86_64-softmmu/monitor.o
  CC      x86_64-softmmu/gdbstub.o
  CC      aarch64-softmmu/translate-common.o
  CC      x86_64-softmmu/balloon.o
  CC      x86_64-softmmu/ioport.o
  CC      aarch64-softmmu/cpu-exec-common.o
  CC      aarch64-softmmu/tcg/tcg.o
  CC      x86_64-softmmu/numa.o
  CC      x86_64-softmmu/qtest.o
  CC      aarch64-softmmu/tcg/tcg-op.o
  CC      aarch64-softmmu/tcg/optimize.o
  CC      aarch64-softmmu/tcg/tcg-common.o
  CC      x86_64-softmmu/bootdevice.o
  CC      aarch64-softmmu/fpu/softfloat.o
  CC      aarch64-softmmu/disas.o
  CC      x86_64-softmmu/kvm-all.o
  CC      aarch64-softmmu/tcg-runtime.o
  CC      x86_64-softmmu/memory.o
  CC      x86_64-softmmu/cputlb.o
  CC      x86_64-softmmu/memory_mapping.o
  GEN     aarch64-softmmu/gdbstub-xml.c
  CC      aarch64-softmmu/hax-stub.o
  CC      aarch64-softmmu/kvm-stub.o
  CC      aarch64-softmmu/arch_init.o
  CC      aarch64-softmmu/cpus.o
  CC      x86_64-softmmu/dump.o
  CC      x86_64-softmmu/migration/ram.o
  CC      aarch64-softmmu/monitor.o
  CC      x86_64-softmmu/migration/savevm.o
  CC      x86_64-softmmu/hw/block/virtio-blk.o
  CC      aarch64-softmmu/gdbstub.o
  CC      aarch64-softmmu/balloon.o
  CC      x86_64-softmmu/hw/block/dataplane/virtio-blk.o
  CC      x86_64-softmmu/hw/char/virtio-serial-bus.o
  CC      aarch64-softmmu/ioport.o
  CC      aarch64-softmmu/numa.o
  CC      x86_64-softmmu/hw/core/nmi.o
  CC      aarch64-softmmu/qtest.o
  CC      aarch64-softmmu/bootdevice.o
  CC      x86_64-softmmu/hw/core/generic-loader.o
  CC      x86_64-softmmu/hw/core/null-machine.o
  CC      aarch64-softmmu/memory.o
  CC      x86_64-softmmu/hw/cpu/core.o
  CC      aarch64-softmmu/cputlb.o
  CC      aarch64-softmmu/memory_mapping.o
  CC      x86_64-softmmu/hw/display/vga.o
  CC      aarch64-softmmu/dump.o
  CC      aarch64-softmmu/migration/ram.o
  CC      aarch64-softmmu/migration/savevm.o
  CC      aarch64-softmmu/hw/adc/stm32f2xx_adc.o
  CC      aarch64-softmmu/hw/block/virtio-blk.o
  CC      aarch64-softmmu/hw/block/dataplane/virtio-blk.o
  CC      aarch64-softmmu/hw/char/exynos4210_uart.o
  CC      x86_64-softmmu/hw/display/virtio-gpu.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-3d.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-pci.o
  CC      x86_64-softmmu/hw/display/virtio-vga.o
  CC      aarch64-softmmu/hw/char/omap_uart.o
  CC      aarch64-softmmu/hw/char/digic-uart.o
  CC      aarch64-softmmu/hw/char/stm32f2xx_usart.o
  CC      aarch64-softmmu/hw/char/bcm2835_aux.o
  CC      aarch64-softmmu/hw/char/virtio-serial-bus.o
  CC      x86_64-softmmu/hw/intc/apic.o
  CC      x86_64-softmmu/hw/intc/apic_common.o
  CC      x86_64-softmmu/hw/intc/ioapic.o
  CC      x86_64-softmmu/hw/isa/lpc_ich9.o
  CC      aarch64-softmmu/hw/core/nmi.o
  CC      x86_64-softmmu/hw/misc/vmport.o
  CC      x86_64-softmmu/hw/misc/ivshmem.o
  CC      aarch64-softmmu/hw/core/generic-loader.o
  CC      aarch64-softmmu/hw/core/null-machine.o
  CC      x86_64-softmmu/hw/misc/pvpanic.o
  CC      aarch64-softmmu/hw/cpu/arm11mpcore.o
  CC      aarch64-softmmu/hw/cpu/realview_mpcore.o
  CC      aarch64-softmmu/hw/cpu/a9mpcore.o
  CC      aarch64-softmmu/hw/cpu/a15mpcore.o
  CC      aarch64-softmmu/hw/cpu/core.o
  CC      aarch64-softmmu/hw/display/omap_dss.o
  CC      aarch64-softmmu/hw/display/omap_lcdc.o
  CC      aarch64-softmmu/hw/display/pxa2xx_lcd.o
  CC      aarch64-softmmu/hw/display/bcm2835_fb.o
  CC      aarch64-softmmu/hw/display/vga.o
  CC      aarch64-softmmu/hw/display/virtio-gpu.o
  CC      aarch64-softmmu/hw/display/virtio-gpu-3d.o
  CC      aarch64-softmmu/hw/display/virtio-gpu-pci.o
  CC      aarch64-softmmu/hw/display/dpcd.o
  CC      x86_64-softmmu/hw/misc/edu.o
  CC      aarch64-softmmu/hw/display/xlnx_dp.o
  CC      x86_64-softmmu/hw/misc/hyperv_testdev.o
  CC      aarch64-softmmu/hw/dma/xlnx_dpdma.o
  CC      aarch64-softmmu/hw/dma/omap_dma.o
  CC      aarch64-softmmu/hw/dma/soc_dma.o
  CC      aarch64-softmmu/hw/dma/pxa2xx_dma.o
  CC      aarch64-softmmu/hw/dma/bcm2835_dma.o
  CC      aarch64-softmmu/hw/gpio/omap_gpio.o
  CC      x86_64-softmmu/hw/net/virtio-net.o
  CC      x86_64-softmmu/hw/net/vhost_net.o
  CC      aarch64-softmmu/hw/gpio/imx_gpio.o
  CC      aarch64-softmmu/hw/gpio/bcm2835_gpio.o
  CC      x86_64-softmmu/hw/scsi/virtio-scsi.o
  CC      aarch64-softmmu/hw/i2c/omap_i2c.o
  CC      x86_64-softmmu/hw/scsi/virtio-scsi-dataplane.o
  CC      aarch64-softmmu/hw/input/pxa2xx_keypad.o
  CC      x86_64-softmmu/hw/scsi/vhost-scsi.o
  CC      aarch64-softmmu/hw/input/tsc210x.o
  CC      x86_64-softmmu/hw/timer/mc146818rtc.o
  CC      x86_64-softmmu/hw/vfio/common.o
  CC      aarch64-softmmu/hw/intc/armv7m_nvic.o
  CC      aarch64-softmmu/hw/intc/exynos4210_gic.o
  CC      aarch64-softmmu/hw/intc/exynos4210_combiner.o
  CC      x86_64-softmmu/hw/vfio/pci.o
  CC      aarch64-softmmu/hw/intc/omap_intc.o
  CC      x86_64-softmmu/hw/vfio/pci-quirks.o
  CC      aarch64-softmmu/hw/intc/bcm2835_ic.o
  CC      x86_64-softmmu/hw/vfio/platform.o
  CC      aarch64-softmmu/hw/intc/bcm2836_control.o
  CC      x86_64-softmmu/hw/vfio/spapr.o
  CC      x86_64-softmmu/hw/virtio/virtio.o
  CC      x86_64-softmmu/hw/virtio/virtio-balloon.o
  CC      aarch64-softmmu/hw/intc/allwinner-a10-pic.o
  CC      aarch64-softmmu/hw/intc/aspeed_vic.o
  CC      x86_64-softmmu/hw/virtio/vhost.o
  CC      x86_64-softmmu/hw/virtio/vhost-backend.o
  CC      aarch64-softmmu/hw/intc/arm_gicv3_cpuif.o
  CC      x86_64-softmmu/hw/virtio/vhost-user.o
  CC      x86_64-softmmu/hw/virtio/vhost-vsock.o
  CC      aarch64-softmmu/hw/misc/ivshmem.o
  CC      x86_64-softmmu/hw/virtio/virtio-crypto.o
  CC      aarch64-softmmu/hw/misc/arm_sysctl.o
  CC      aarch64-softmmu/hw/misc/cbus.o
  CC      aarch64-softmmu/hw/misc/exynos4210_pmu.o
  CC      aarch64-softmmu/hw/misc/exynos4210_clk.o
  CC      x86_64-softmmu/hw/virtio/virtio-crypto-pci.o
  CC      x86_64-softmmu/hw/i386/multiboot.o
  CC      x86_64-softmmu/hw/i386/pc.o
  CC      aarch64-softmmu/hw/misc/imx_ccm.o
  CC      aarch64-softmmu/hw/misc/imx31_ccm.o
  CC      aarch64-softmmu/hw/misc/imx25_ccm.o
  CC      aarch64-softmmu/hw/misc/imx6_ccm.o
  CC      x86_64-softmmu/hw/i386/pc_piix.o
  CC      aarch64-softmmu/hw/misc/imx6_src.o
  CC      aarch64-softmmu/hw/misc/mst_fpga.o
  CC      aarch64-softmmu/hw/misc/omap_clk.o
  CC      aarch64-softmmu/hw/misc/omap_gpmc.o
  CC      aarch64-softmmu/hw/misc/omap_l4.o
  CC      aarch64-softmmu/hw/misc/omap_sdrc.o
/tmp/qemu-test/src/hw/i386/pc_piix.c: In function ‘igd_passthrough_isa_bridge_create’:
/tmp/qemu-test/src/hw/i386/pc_piix.c:1055: warning: ‘pch_rev_id’ may be used uninitialized in this function
  CC      x86_64-softmmu/hw/i386/pc_q35.o
  CC      aarch64-softmmu/hw/misc/omap_tap.o
  CC      aarch64-softmmu/hw/misc/bcm2835_mbox.o
  CC      x86_64-softmmu/hw/i386/pc_sysfw.o
  CC      aarch64-softmmu/hw/misc/bcm2835_property.o
  CC      x86_64-softmmu/hw/i386/x86-iommu.o
  CC      x86_64-softmmu/hw/i386/intel_iommu.o
  CC      aarch64-softmmu/hw/misc/bcm2835_rng.o
  CC      aarch64-softmmu/hw/misc/zynq_slcr.o
  CC      aarch64-softmmu/hw/misc/zynq-xadc.o
  CC      aarch64-softmmu/hw/misc/stm32f2xx_syscfg.o
  CC      aarch64-softmmu/hw/misc/edu.o
  CC      aarch64-softmmu/hw/misc/auxbus.o
  CC      aarch64-softmmu/hw/misc/aspeed_scu.o
  CC      x86_64-softmmu/hw/i386/amd_iommu.o
  CC      x86_64-softmmu/hw/i386/kvmvapic.o
  CC      aarch64-softmmu/hw/misc/aspeed_sdmc.o
  CC      aarch64-softmmu/hw/net/virtio-net.o
  CC      aarch64-softmmu/hw/net/vhost_net.o
  CC      aarch64-softmmu/hw/pcmcia/pxa2xx.o
  CC      aarch64-softmmu/hw/scsi/virtio-scsi.o
  CC      aarch64-softmmu/hw/scsi/virtio-scsi-dataplane.o
  CC      x86_64-softmmu/hw/i386/acpi-build.o
  CC      aarch64-softmmu/hw/scsi/vhost-scsi.o
  CC      x86_64-softmmu/hw/i386/pci-assign-load-rom.o
  CC      x86_64-softmmu/hw/i386/kvm/clock.o
  CC      x86_64-softmmu/hw/i386/kvm/apic.o
  CC      aarch64-softmmu/hw/sd/omap_mmc.o
  CC      x86_64-softmmu/hw/i386/kvm/i8259.o
  CC      aarch64-softmmu/hw/sd/pxa2xx_mmci.o
  CC      aarch64-softmmu/hw/sd/bcm2835_sdhost.o
  CC      x86_64-softmmu/hw/i386/kvm/ioapic.o
  CC      x86_64-softmmu/hw/i386/kvm/i8254.o
  CC      x86_64-softmmu/hw/i386/kvm/pci-assign.o
  CC      aarch64-softmmu/hw/ssi/omap_spi.o
  CC      x86_64-softmmu/target/i386/translate.o
  CC      aarch64-softmmu/hw/ssi/imx_spi.o
  CC      aarch64-softmmu/hw/timer/exynos4210_mct.o
  CC      aarch64-softmmu/hw/timer/exynos4210_pwm.o
  CC      x86_64-softmmu/target/i386/helper.o
/tmp/qemu-test/src/hw/i386/acpi-build.c: In function ‘build_append_pci_bus_devices’:
/tmp/qemu-test/src/hw/i386/acpi-build.c:496: warning: ‘notify_method’ may be used uninitialized in this function
  CC      x86_64-softmmu/target/i386/cpu.o
  CC      aarch64-softmmu/hw/timer/exynos4210_rtc.o
  CC      aarch64-softmmu/hw/timer/omap_gptimer.o
  CC      aarch64-softmmu/hw/timer/omap_synctimer.o
  CC      x86_64-softmmu/target/i386/bpt_helper.o
  CC      aarch64-softmmu/hw/timer/pxa2xx_timer.o
  CC      aarch64-softmmu/hw/timer/digic-timer.o
  CC      x86_64-softmmu/target/i386/excp_helper.o
  CC      aarch64-softmmu/hw/timer/allwinner-a10-pit.o
  CC      x86_64-softmmu/target/i386/fpu_helper.o
  CC      x86_64-softmmu/target/i386/cc_helper.o
  CC      aarch64-softmmu/hw/usb/tusb6010.o
  CC      x86_64-softmmu/target/i386/int_helper.o
  CC      aarch64-softmmu/hw/vfio/common.o
  CC      x86_64-softmmu/target/i386/svm_helper.o
  CC      x86_64-softmmu/target/i386/smm_helper.o
  CC      aarch64-softmmu/hw/vfio/pci.o
  CC      aarch64-softmmu/hw/vfio/pci-quirks.o
  CC      aarch64-softmmu/hw/vfio/platform.o
  CC      x86_64-softmmu/target/i386/misc_helper.o
  CC      x86_64-softmmu/target/i386/mem_helper.o
  CC      aarch64-softmmu/hw/vfio/calxeda-xgmac.o
  CC      x86_64-softmmu/target/i386/seg_helper.o
  CC      aarch64-softmmu/hw/vfio/amd-xgbe.o
  CC      aarch64-softmmu/hw/vfio/spapr.o
  CC      aarch64-softmmu/hw/virtio/virtio.o
  CC      aarch64-softmmu/hw/virtio/virtio-balloon.o
  CC      aarch64-softmmu/hw/virtio/vhost.o
  CC      aarch64-softmmu/hw/virtio/vhost-backend.o
  CC      x86_64-softmmu/target/i386/mpx_helper.o
  CC      aarch64-softmmu/hw/virtio/vhost-user.o
  CC      aarch64-softmmu/hw/virtio/vhost-vsock.o
  CC      aarch64-softmmu/hw/virtio/virtio-crypto.o
  CC      aarch64-softmmu/hw/virtio/virtio-crypto-pci.o
  CC      x86_64-softmmu/target/i386/gdbstub.o
  CC      x86_64-softmmu/target/i386/machine.o
  CC      aarch64-softmmu/hw/arm/boot.o
  CC      aarch64-softmmu/hw/arm/collie.o
  CC      x86_64-softmmu/target/i386/arch_memory_mapping.o
  CC      aarch64-softmmu/hw/arm/exynos4_boards.o
  CC      aarch64-softmmu/hw/arm/gumstix.o
  CC      aarch64-softmmu/hw/arm/highbank.o
  CC      x86_64-softmmu/target/i386/arch_dump.o
  CC      x86_64-softmmu/target/i386/monitor.o
  CC      aarch64-softmmu/hw/arm/digic_boards.o
  CC      aarch64-softmmu/hw/arm/integratorcp.o
  CC      aarch64-softmmu/hw/arm/mainstone.o
  CC      x86_64-softmmu/target/i386/kvm.o
  CC      x86_64-softmmu/target/i386/hyperv.o
  GEN     trace/generated-helpers.c
  CC      aarch64-softmmu/hw/arm/musicpal.o
  CC      x86_64-softmmu/trace/control-target.o
  CC      aarch64-softmmu/hw/arm/nseries.o
  CC      aarch64-softmmu/hw/arm/omap_sx1.o
  CC      aarch64-softmmu/hw/arm/palm.o
  CC      x86_64-softmmu/trace/generated-helpers.o
  CC      aarch64-softmmu/hw/arm/realview.o
  CC      aarch64-softmmu/hw/arm/spitz.o
  CC      aarch64-softmmu/hw/arm/stellaris.o
  CC      aarch64-softmmu/hw/arm/tosa.o
  CC      aarch64-softmmu/hw/arm/versatilepb.o
  CC      aarch64-softmmu/hw/arm/vexpress.o
  CC      aarch64-softmmu/hw/arm/virt.o
  CC      aarch64-softmmu/hw/arm/xilinx_zynq.o
  CC      aarch64-softmmu/hw/arm/z2.o
  CC      aarch64-softmmu/hw/arm/virt-acpi-build.o
  CC      aarch64-softmmu/hw/arm/netduino2.o
  CC      aarch64-softmmu/hw/arm/sysbus-fdt.o
  CC      aarch64-softmmu/hw/arm/armv7m.o
  CC      aarch64-softmmu/hw/arm/exynos4210.o
  CC      aarch64-softmmu/hw/arm/pxa2xx.o
  CC      aarch64-softmmu/hw/arm/pxa2xx_gpio.o
  CC      aarch64-softmmu/hw/arm/pxa2xx_pic.o
  CC      aarch64-softmmu/hw/arm/digic.o
  CC      aarch64-softmmu/hw/arm/omap1.o
  CC      aarch64-softmmu/hw/arm/omap2.o
  CC      aarch64-softmmu/hw/arm/strongarm.o
  CC      aarch64-softmmu/hw/arm/allwinner-a10.o
  CC      aarch64-softmmu/hw/arm/cubieboard.o
  CC      aarch64-softmmu/hw/arm/bcm2835_peripherals.o
  CC      aarch64-softmmu/hw/arm/bcm2836.o
  CC      aarch64-softmmu/hw/arm/raspi.o
  CC      aarch64-softmmu/hw/arm/stm32f205_soc.o
  CC      aarch64-softmmu/hw/arm/xlnx-zynqmp.o
  CC      aarch64-softmmu/hw/arm/xlnx-ep108.o
  CC      aarch64-softmmu/hw/arm/fsl-imx25.o
  CC      aarch64-softmmu/hw/arm/imx25_pdk.o
  CC      aarch64-softmmu/hw/arm/fsl-imx31.o
  CC      aarch64-softmmu/hw/arm/kzm.o
  CC      aarch64-softmmu/hw/arm/fsl-imx6.o
  CC      aarch64-softmmu/hw/arm/sabrelite.o
  CC      aarch64-softmmu/hw/arm/aspeed_soc.o
  CC      aarch64-softmmu/hw/arm/aspeed.o
  CC      aarch64-softmmu/target/arm/arm-semi.o
  CC      aarch64-softmmu/target/arm/machine.o
  CC      aarch64-softmmu/target/arm/psci.o
  CC      aarch64-softmmu/target/arm/arch_dump.o
  CC      aarch64-softmmu/target/arm/monitor.o
  CC      aarch64-softmmu/target/arm/kvm-stub.o
  CC      aarch64-softmmu/target/arm/translate.o
  CC      aarch64-softmmu/target/arm/op_helper.o
  CC      aarch64-softmmu/target/arm/helper.o
  CC      aarch64-softmmu/target/arm/cpu.o
  CC      aarch64-softmmu/target/arm/neon_helper.o
  CC      aarch64-softmmu/target/arm/iwmmxt_helper.o
  CC      aarch64-softmmu/target/arm/gdbstub.o
  CC      aarch64-softmmu/target/arm/cpu64.o
  CC      aarch64-softmmu/target/arm/translate-a64.o
  CC      aarch64-softmmu/target/arm/helper-a64.o
  CC      aarch64-softmmu/target/arm/gdbstub64.o
  CC      aarch64-softmmu/target/arm/crypto_helper.o
  CC      aarch64-softmmu/target/arm/arm-powerctl.o
  GEN     trace/generated-helpers.c
  CC      aarch64-softmmu/trace/control-target.o
  CC      aarch64-softmmu/gdbstub-xml.o
  CC      aarch64-softmmu/trace/generated-helpers.o
/tmp/qemu-test/src/target/arm/translate-a64.c: In function ‘handle_shri_with_rndacc’:
/tmp/qemu-test/src/target/arm/translate-a64.c:6359: warning: ‘tcg_src_hi’ may be used uninitialized in this function
/tmp/qemu-test/src/target/arm/translate-a64.c: In function ‘disas_simd_scalar_two_reg_misc’:
/tmp/qemu-test/src/target/arm/translate-a64.c:8086: warning: ‘rmode’ may be used uninitialized in this function
  LINK    x86_64-softmmu/qemu-system-x86_64
  LINK    aarch64-softmmu/qemu-system-aarch64
	 LEX convert-dtsv0-lexer.lex.c
make[1]: flex: Command not found
	 BISON dtc-parser.tab.c
make[1]: bison: Command not found
	 LEX dtc-lexer.lex.c
make[1]: flex: Command not found
  TEST    tests/qapi-schema/alternate-array.out
  TEST    tests/qapi-schema/alternate-any.out
  TEST    tests/qapi-schema/alternate-clash.out
  TEST    tests/qapi-schema/alternate-base.out
  TEST    tests/qapi-schema/alternate-conflict-dict.out
  TEST    tests/qapi-schema/alternate-conflict-string.out
  TEST    tests/qapi-schema/alternate-empty.out
  TEST    tests/qapi-schema/alternate-nested.out
  TEST    tests/qapi-schema/alternate-unknown.out
  TEST    tests/qapi-schema/args-alternate.out
  TEST    tests/qapi-schema/args-any.out
  TEST    tests/qapi-schema/args-array-empty.out
  TEST    tests/qapi-schema/args-array-unknown.out
  TEST    tests/qapi-schema/args-bad-boxed.out
  TEST    tests/qapi-schema/args-boxed-anon.out
  TEST    tests/qapi-schema/args-boxed-empty.out
  TEST    tests/qapi-schema/args-boxed-string.out
  TEST    tests/qapi-schema/args-int.out
  TEST    tests/qapi-schema/args-invalid.out
  TEST    tests/qapi-schema/args-member-array-bad.out
  TEST    tests/qapi-schema/args-member-case.out
  TEST    tests/qapi-schema/args-member-unknown.out
  TEST    tests/qapi-schema/args-name-clash.out
  TEST    tests/qapi-schema/args-union.out
  TEST    tests/qapi-schema/args-unknown.out
  TEST    tests/qapi-schema/bad-base.out
  TEST    tests/qapi-schema/bad-data.out
  TEST    tests/qapi-schema/bad-ident.out
  TEST    tests/qapi-schema/bad-type-bool.out
  TEST    tests/qapi-schema/bad-type-dict.out
  TEST    tests/qapi-schema/bad-type-int.out
  TEST    tests/qapi-schema/base-cycle-direct.out
  TEST    tests/qapi-schema/base-cycle-indirect.out
  TEST    tests/qapi-schema/command-int.out
  TEST    tests/qapi-schema/comments.out
  TEST    tests/qapi-schema/doc-bad-alternate-member.out
  TEST    tests/qapi-schema/doc-bad-symbol.out
  TEST    tests/qapi-schema/doc-bad-command-arg.out
  TEST    tests/qapi-schema/doc-bad-union-member.out
  TEST    tests/qapi-schema/doc-before-include.out
  TEST    tests/qapi-schema/doc-before-pragma.out
  TEST    tests/qapi-schema/doc-duplicated-arg.out
  TEST    tests/qapi-schema/doc-duplicated-return.out
  TEST    tests/qapi-schema/doc-duplicated-since.out
  TEST    tests/qapi-schema/doc-empty-arg.out
  TEST    tests/qapi-schema/doc-empty-section.out
  TEST    tests/qapi-schema/doc-empty-symbol.out
  TEST    tests/qapi-schema/doc-good.out
  TEST    tests/qapi-schema/doc-interleaved-section.out
  TEST    tests/qapi-schema/doc-invalid-end.out
  TEST    tests/qapi-schema/doc-invalid-end2.out
  TEST    tests/qapi-schema/doc-invalid-return.out
  TEST    tests/qapi-schema/doc-invalid-section.out
  TEST    tests/qapi-schema/doc-invalid-start.out
  TEST    tests/qapi-schema/doc-missing.out
  TEST    tests/qapi-schema/doc-missing-colon.out
  TEST    tests/qapi-schema/doc-missing-expr.out
  TEST    tests/qapi-schema/doc-missing-space.out
  TEST    tests/qapi-schema/doc-no-symbol.out
  TEST    tests/qapi-schema/double-data.out
  TEST    tests/qapi-schema/double-type.out
  TEST    tests/qapi-schema/duplicate-key.out
  TEST    tests/qapi-schema/empty.out
  TEST    tests/qapi-schema/enum-bad-name.out
  TEST    tests/qapi-schema/enum-bad-prefix.out
  TEST    tests/qapi-schema/enum-clash-member.out
  TEST    tests/qapi-schema/enum-dict-member.out
  TEST    tests/qapi-schema/enum-int-member.out
  TEST    tests/qapi-schema/enum-member-case.out
  TEST    tests/qapi-schema/enum-missing-data.out
  TEST    tests/qapi-schema/enum-wrong-data.out
  TEST    tests/qapi-schema/escape-outside-string.out
  TEST    tests/qapi-schema/escape-too-big.out
  TEST    tests/qapi-schema/escape-too-short.out
  TEST    tests/qapi-schema/event-boxed-empty.out
  TEST    tests/qapi-schema/event-case.out
  TEST    tests/qapi-schema/event-nest-struct.out
  TEST    tests/qapi-schema/flat-union-array-branch.out
  TEST    tests/qapi-schema/flat-union-bad-base.out
  TEST    tests/qapi-schema/flat-union-bad-discriminator.out
  TEST    tests/qapi-schema/flat-union-base-any.out
  TEST    tests/qapi-schema/flat-union-base-union.out
  TEST    tests/qapi-schema/flat-union-clash-member.out
  TEST    tests/qapi-schema/flat-union-empty.out
  TEST    tests/qapi-schema/flat-union-incomplete-branch.out
  TEST    tests/qapi-schema/flat-union-inline.out
  TEST    tests/qapi-schema/flat-union-int-branch.out
  TEST    tests/qapi-schema/flat-union-invalid-branch-key.out
  TEST    tests/qapi-schema/flat-union-invalid-discriminator.out
  TEST    tests/qapi-schema/flat-union-no-base.out
  TEST    tests/qapi-schema/flat-union-optional-discriminator.out
  TEST    tests/qapi-schema/flat-union-string-discriminator.out
  TEST    tests/qapi-schema/funny-char.out
  TEST    tests/qapi-schema/ident-with-escape.out
  TEST    tests/qapi-schema/include-before-err.out
  TEST    tests/qapi-schema/include-cycle.out
  TEST    tests/qapi-schema/include-extra-junk.out
  TEST    tests/qapi-schema/include-format-err.out
  TEST    tests/qapi-schema/include-nested-err.out
  TEST    tests/qapi-schema/include-no-file.out
  TEST    tests/qapi-schema/include-non-file.out
  TEST    tests/qapi-schema/include-relpath.out
  TEST    tests/qapi-schema/include-repetition.out
  TEST    tests/qapi-schema/include-self-cycle.out
  TEST    tests/qapi-schema/include-simple.out
  TEST    tests/qapi-schema/indented-expr.out
  TEST    tests/qapi-schema/leading-comma-list.out
  TEST    tests/qapi-schema/leading-comma-object.out
  TEST    tests/qapi-schema/missing-colon.out
  TEST    tests/qapi-schema/missing-comma-list.out
  TEST    tests/qapi-schema/missing-comma-object.out
  TEST    tests/qapi-schema/missing-type.out
  TEST    tests/qapi-schema/nested-struct-data.out
  TEST    tests/qapi-schema/non-objects.out
  TEST    tests/qapi-schema/pragma-doc-required-crap.out
  TEST    tests/qapi-schema/pragma-extra-junk.out
  TEST    tests/qapi-schema/pragma-name-case-whitelist-crap.out
  TEST    tests/qapi-schema/pragma-non-dict.out
  TEST    tests/qapi-schema/pragma-returns-whitelist-crap.out
  TEST    tests/qapi-schema/qapi-schema-test.out
  TEST    tests/qapi-schema/quoted-structural-chars.out
  TEST    tests/qapi-schema/redefined-builtin.out
  TEST    tests/qapi-schema/redefined-command.out
Files /tmp/qemu-test/src/tests/qapi-schema/qapi-schema-test.out and tests/qapi-schema/qapi-schema-test.test.out differ
make: *** [check-tests/qapi-schema/qapi-schema-test.json] Error 1
make: *** Waiting for unfinished jobs....
tests/docker/Makefile.include:118: recipe for target 'docker-run' failed
make[1]: *** [docker-run] Error 2
make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-mqqq0zgw/src'
tests/docker/Makefile.include:149: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 2
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force"

[Eduardo Habkost]

On Tue, May 02, 2017 at 01:46:22PM -0700, no-reply@patchew.org wrote:
[...]
>   TEST    tests/qapi-schema/qapi-schema-test.out
>   TEST    tests/qapi-schema/quoted-structural-chars.out
>   TEST    tests/qapi-schema/redefined-builtin.out
>   TEST    tests/qapi-schema/redefined-command.out
> Files /tmp/qemu-test/src/tests/qapi-schema/qapi-schema-test.out and tests/qapi-schema/qapi-schema-test.test.out differ
> make: *** [check-tests/qapi-schema/qapi-schema-test.json] Error 1
> make: *** Waiting for unfinished jobs....

Oh, I didn't know we had a qapi-schema-test.out file that had to
be manually kept in sync. I will fix this in v2.

> tests/docker/Makefile.include:118: recipe for target 'docker-run' failed
> make[1]: *** [docker-run] Error 2
> make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-mqqq0zgw/src'
> tests/docker/Makefile.include:149: recipe for target 'docker-run-test-quick@centos6' failed
> make: *** [docker-run-test-quick@centos6] Error 2
> === OUTPUT END ===
> 
> Test command exited with code: 2
> 
> 
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@freelists.org

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force"

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH 0/4] x86: Support "-cpu feature=force"
Message-id: 20170502203115.22233-1-ehabkost@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
8ffc6d4 x86: Support feature=force on the command-line
9418a88 tests: Add [+-]feature and feature=on|off test cases
f6954a1 string-input-visitor: Support alternate types
ea7dc8d visitor: Add 'supported_qtypes' parameter to visit_start_alternate()

=== OUTPUT BEGIN ===
Checking PATCH 1/4: visitor: Add 'supported_qtypes' parameter to visit_start_alternate()...
Checking PATCH 2/4: string-input-visitor: Support alternate types...
Checking PATCH 3/4: tests: Add [+-]feature and feature=on|off test cases...
ERROR: line over 90 characters
#59: FILE: tests/test-x86-cpuid-compat.c:104:
+static uint32_t get_feature_word(QList *features, uint32_t eax, uint32_t ecx, const char *reg)

WARNING: line over 80 characters
#65: FILE: tests/test-x86-cpuid-compat.c:110:
+        if (eax == qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-eax")))

WARNING: line over 80 characters
#67: FILE: tests/test-x86-cpuid-compat.c:112:
+                || ecx == qint_get_int(qobject_to_qint(qdict_get(w, "cpuid-input-ecx"))))

ERROR: line over 90 characters
#68: FILE: tests/test-x86-cpuid-compat.c:113:
+            && !strcmp(qstring_get_str(qobject_to_qstring(qdict_get(w, "cpuid-register"))), reg)) {

WARNING: line over 80 characters
#86: FILE: tests/test-x86-cpuid-compat.c:131:
+    value = get_feature_word(present, args->input_eax, args->input_ecx, args->reg);

WARNING: line over 80 characters
#87: FILE: tests/test-x86-cpuid-compat.c:132:
+    value |= get_feature_word(filtered, args->input_eax, args->input_ecx, args->reg);

total: 2 errors, 4 warnings, 130 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/4: x86: Support feature=force on the command-line...
WARNING: line over 80 characters
#186: FILE: tests/test-x86-cpuid-compat.c:135:
+        value |= get_feature_word(filtered, args->input_eax, args->input_ecx, args->reg);

total: 0 errors, 1 warnings, 168 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org