[PATCH 0/3] fuzz: Probably there is a better way to do this

[PATCH 0/3] fuzz: Probably there is a better way to do this

[Markus Armbruster]

... the comment next to qos_set_machines_devices_available() wonders.
Yup, there is!

Markus Armbruster (3):
  Makefile: Drop unused, broken target recurse-fuzz
  fuzz: Simplify how we compute available machines and types
  libqos: Give get_machine_allocator() internal linkage

 Makefile                          |  1 -
 tests/qtest/libqos/qos_external.h | 10 +++--
 tests/qtest/fuzz/qos_fuzz.c       | 34 ++++-----------
 tests/qtest/libqos/qos_external.c | 72 +++++++++++--------------------
 tests/qtest/qos-test.c            | 29 +++++++++----
 5 files changed, 60 insertions(+), 86 deletions(-)

-- 
2.21.1

[PATCH 1/3] Makefile: Drop unused, broken target recurse-fuzz

[Markus Armbruster]

Target recurse-fuzz depends on pc-bios/optionrom/fuzz, which can't be
made.  It's not used anywhere.  Added in commit c621dc3e01c, looks
like cargo cult.  Delete.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8a9113e666..34275f57c9 100644
--- a/Makefile
+++ b/Makefile
@@ -582,7 +582,6 @@ $(ROM_DIRS_RULES):
 
 .PHONY: recurse-all recurse-clean recurse-install
 recurse-all: $(addsuffix /all, $(TARGET_DIRS) $(ROM_DIRS))
-recurse-fuzz: $(addsuffix /fuzz, $(TARGET_DIRS) $(ROM_DIRS))
 recurse-clean: $(addsuffix /clean, $(TARGET_DIRS) $(ROM_DIRS))
 recurse-install: $(addsuffix /install, $(TARGET_DIRS))
 $(addsuffix /install, $(TARGET_DIRS)): all
-- 
2.21.1

[PATCH 2/3] fuzz: Simplify how we compute available machines and types

[Markus Armbruster]

apply_to_qlist(), apply_to_node() work with QObjects.  This is
designed for use by tests/qtest/qos-test.c, which gets the data in
that form via QMP.  Goes back to commit fc281c8020 "tests: qgraph API
for the qtest driver framework".

Commit 275ab39d86 "fuzz: add support for qos-assisted fuzz targets"
added another user: qtest/fuzz/qos_fuzz.c.  To get the data as
QObjects, it uses qmp_marshal_query_machines() and
qmp_marshal_qom_list_types().

All this code is rather cumbersome.  Switch to working with generated
QAPI types instead:

* Replace apply_to_qlist() & friends by machines_apply_to_node() and
  types_apply_to_node().

* Have qos_fuzz.c use qmp_query_machines() and qmp_qom_list_types()
  instead.

* Have qos_test.c convert from QObject to the QAPI types.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qtest/libqos/qos_external.h |  8 +++-
 tests/qtest/fuzz/qos_fuzz.c       | 34 ++++-----------
 tests/qtest/libqos/qos_external.c | 70 +++++++++++--------------------
 tests/qtest/qos-test.c            | 29 +++++++++----
 4 files changed, 59 insertions(+), 82 deletions(-)

diff --git a/tests/qtest/libqos/qos_external.h b/tests/qtest/libqos/qos_external.h
index 7b44930c55..f63388cb30 100644
--- a/tests/qtest/libqos/qos_external.h
+++ b/tests/qtest/libqos/qos_external.h
@@ -20,8 +20,12 @@
 #define QOS_EXTERNAL_H
 #include "libqos/qgraph.h"
 
-void apply_to_node(const char *name, bool is_machine, bool is_abstract);
-void apply_to_qlist(QList *list, bool is_machine);
+#include "libqos/malloc.h"
+#include "qapi/qapi-types-machine.h"
+#include "qapi/qapi-types-qom.h"
+
+void machines_apply_to_node(MachineInfoList *mach_info);
+void types_apply_to_node(ObjectTypeInfoList *type_info);
 QGuestAllocator *get_machine_allocator(QOSGraphObject *obj);
 void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc);
 
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index af28c92866..87eadb0889 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -36,7 +36,6 @@
 
 #include "qapi/qapi-commands-machine.h"
 #include "qapi/qapi-commands-qom.h"
-#include "qapi/qmp/qlist.h"
 
 
 void *fuzz_qos_obj;
@@ -45,34 +44,19 @@ QGuestAllocator *fuzz_qos_alloc;
 static const char *fuzz_target_name;
 static char **fuzz_path_vec;
 
-/*
- * Replaced the qmp commands with direct qmp_marshal calls.
- * Probably there is a better way to do this
- */
 static void qos_set_machines_devices_available(void)
 {
-    QDict *req = qdict_new();
-    QObject *response;
-    QDict *args = qdict_new();
-    QList *lst;
+    MachineInfoList *mach_info;
+    ObjectTypeInfoList *type_info;
 
-    qmp_marshal_query_machines(NULL, &response, &error_abort);
-    lst = qobject_to(QList, response);
-    apply_to_qlist(lst, true);
+    mach_info = qmp_query_machines(&error_abort);
+    machines_apply_to_node(mach_info);
+    qapi_free_MachineInfoList(mach_info);
 
-    qobject_unref(response);
-
-
-    qdict_put_str(req, "execute", "qom-list-types");
-    qdict_put_str(args, "implements", "device");
-    qdict_put_bool(args, "abstract", true);
-    qdict_put_obj(req, "arguments", (QObject *) args);
-
-    qmp_marshal_qom_list_types(args, &response, &error_abort);
-    lst = qobject_to(QList, response);
-    apply_to_qlist(lst, false);
-    qobject_unref(response);
-    qobject_unref(req);
+    type_info = qmp_qom_list_types(true, "device", true, true,
+                                   &error_abort);
+    types_apply_to_node(type_info);
+    qapi_free_ObjectTypeInfoList(type_info);
 }
 
 static char **current_path;
diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
index 398556dde0..c707dac3b9 100644
--- a/tests/qtest/libqos/qos_external.c
+++ b/tests/qtest/libqos/qos_external.c
@@ -29,62 +29,40 @@
 #include "libqos/qgraph_internal.h"
 #include "libqos/qos_external.h"
 
-
-
-void apply_to_node(const char *name, bool is_machine, bool is_abstract)
+static void machine_apply_to_node(const char *name)
 {
-    char *machine_name = NULL;
-    if (is_machine) {
-        const char *arch = qtest_get_arch();
-        machine_name = g_strconcat(arch, "/", name, NULL);
-        name = machine_name;
+    char *machine_name = g_strconcat(qtest_get_arch(), "/", name, NULL);
+
+    qos_graph_node_set_availability(machine_name, true);
+    g_free(machine_name);
+}
+
+void machines_apply_to_node(MachineInfoList *mach_info)
+{
+    MachineInfoList *tail;
+
+    for (tail = mach_info; tail; tail = tail->next) {
+        machine_apply_to_node(tail->value->name);
+        if (tail->value->alias) {
+            machine_apply_to_node(tail->value->alias);
+        }
     }
+}
+
+static void type_apply_to_node(const char *name, bool is_abstract)
+{
     qos_graph_node_set_availability(name, true);
     if (is_abstract) {
         qos_delete_cmd_line(name);
     }
-    g_free(machine_name);
 }
 
-/**
- * apply_to_qlist(): using QMP queries QEMU for a list of
- * machines and devices available, and sets the respective node
- * as true. If a node is found, also all its produced and contained
- * child are marked available.
- *
- * See qos_graph_node_set_availability() for more info
- */
-void apply_to_qlist(QList *list, bool is_machine)
+void types_apply_to_node(ObjectTypeInfoList *type_info)
 {
-    const QListEntry *p;
-    const char *name;
-    bool abstract;
-    QDict *minfo;
-    QObject *qobj;
-    QString *qstr;
-    QBool *qbool;
+    ObjectTypeInfoList *tail;
 
-    for (p = qlist_first(list); p; p = qlist_next(p)) {
-        minfo = qobject_to(QDict, qlist_entry_obj(p));
-        qobj = qdict_get(minfo, "name");
-        qstr = qobject_to(QString, qobj);
-        name = qstring_get_str(qstr);
-
-        qobj = qdict_get(minfo, "abstract");
-        if (qobj) {
-            qbool = qobject_to(QBool, qobj);
-            abstract = qbool_get_bool(qbool);
-        } else {
-            abstract = false;
-        }
-
-        apply_to_node(name, is_machine, abstract);
-        qobj = qdict_get(minfo, "alias");
-        if (qobj) {
-            qstr = qobject_to(QString, qobj);
-            name = qstring_get_str(qstr);
-            apply_to_node(name, is_machine, abstract);
-        }
+    for (tail = type_info; tail; tail = tail->next) {
+        type_apply_to_node(tail->value->name, tail->value->abstract);
     }
 }
 
diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
index ad193f43a5..3062a13557 100644
--- a/tests/qtest/qos-test.c
+++ b/tests/qtest/qos-test.c
@@ -19,11 +19,12 @@
 #include "qemu/osdep.h"
 #include <getopt.h>
 #include "libqtest-single.h"
+#include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qbool.h"
-#include "qapi/qmp/qstring.h"
 #include "qemu/module.h"
-#include "qapi/qmp/qlist.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-machine.h"
+#include "qapi/qapi-visit-qom.h"
 #include "libqos/malloc.h"
 #include "libqos/qgraph.h"
 #include "libqos/qgraph_internal.h"
@@ -51,13 +52,20 @@ static void qos_set_machines_devices_available(void)
 {
     QDict *response;
     QDict *args = qdict_new();
-    QList *list;
+    QObject *ret;
+    Visitor *v;
+    MachineInfoList *mach_info;
+    ObjectTypeInfoList *type_info;
 
     qtest_start("-machine none");
     response = qmp("{ 'execute': 'query-machines' }");
-    list = qdict_get_qlist(response, "return");
+    ret = qdict_get(response, "return");
 
-    apply_to_qlist(list, true);
+    v = qobject_input_visitor_new(ret);
+    visit_type_MachineInfoList(v, NULL, &mach_info, &error_abort);
+    visit_free(v);
+    machines_apply_to_node(mach_info);
+    qapi_free_MachineInfoList(mach_info);
 
     qobject_unref(response);
 
@@ -66,10 +74,13 @@ static void qos_set_machines_devices_available(void)
 
     response = qmp("{'execute': 'qom-list-types',"
                    " 'arguments': %p }", args);
-    g_assert(qdict_haskey(response, "return"));
-    list = qdict_get_qlist(response, "return");
+    ret = qdict_get(response, "return");
 
-    apply_to_qlist(list, false);
+    v = qobject_input_visitor_new(ret);
+    visit_type_ObjectTypeInfoList(v, NULL, &type_info, &error_abort);
+    visit_free(v);
+    types_apply_to_node(type_info);
+    qapi_free_ObjectTypeInfoList(type_info);
 
     qtest_end();
     qobject_unref(response);
-- 
2.21.1

[PATCH 3/3] libqos: Give get_machine_allocator() internal linkage

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qtest/libqos/qos_external.h | 2 --
 tests/qtest/libqos/qos_external.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qtest/libqos/qos_external.h b/tests/qtest/libqos/qos_external.h
index f63388cb30..72d7f91707 100644
--- a/tests/qtest/libqos/qos_external.h
+++ b/tests/qtest/libqos/qos_external.h
@@ -18,7 +18,6 @@
 
 #ifndef QOS_EXTERNAL_H
 #define QOS_EXTERNAL_H
-#include "libqos/qgraph.h"
 
 #include "libqos/malloc.h"
 #include "qapi/qapi-types-machine.h"
@@ -26,7 +25,6 @@
 
 void machines_apply_to_node(MachineInfoList *mach_info);
 void types_apply_to_node(ObjectTypeInfoList *type_info);
-QGuestAllocator *get_machine_allocator(QOSGraphObject *obj);
 void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc);
 
 #endif
diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
index c707dac3b9..9f5180e18d 100644
--- a/tests/qtest/libqos/qos_external.c
+++ b/tests/qtest/libqos/qos_external.c
@@ -66,7 +66,7 @@ void types_apply_to_node(ObjectTypeInfoList *type_info)
     }
 }
 
-QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
+static QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
 {
     return obj->get_driver(obj, "memory");
 }
-- 
2.21.1

Re: [PATCH 3/3] libqos: Give get_machine_allocator() internal linkage

[Philippe Mathieu-Daudé]

On 4/24/20 9:11 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   tests/qtest/libqos/qos_external.h | 2 --
>   tests/qtest/libqos/qos_external.c | 2 +-
>   2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/qtest/libqos/qos_external.h b/tests/qtest/libqos/qos_external.h
> index f63388cb30..72d7f91707 100644
> --- a/tests/qtest/libqos/qos_external.h
> +++ b/tests/qtest/libqos/qos_external.h
> @@ -18,7 +18,6 @@
>   
>   #ifndef QOS_EXTERNAL_H
>   #define QOS_EXTERNAL_H
> -#include "libqos/qgraph.h"
>   
>   #include "libqos/malloc.h"
>   #include "qapi/qapi-types-machine.h"
> @@ -26,7 +25,6 @@
>   
>   void machines_apply_to_node(MachineInfoList *mach_info);
>   void types_apply_to_node(ObjectTypeInfoList *type_info);
> -QGuestAllocator *get_machine_allocator(QOSGraphObject *obj);
>   void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc);
>   
>   #endif
> diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
> index c707dac3b9..9f5180e18d 100644
> --- a/tests/qtest/libqos/qos_external.c
> +++ b/tests/qtest/libqos/qos_external.c
> @@ -66,7 +66,7 @@ void types_apply_to_node(ObjectTypeInfoList *type_info)
>       }
>   }
>   
> -QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
> +static QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
>   {
>       return obj->get_driver(obj, "memory");
>   }
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 2/3] fuzz: Simplify how we compute available machines and types

[Philippe Mathieu-Daudé]

On 4/24/20 9:11 AM, Markus Armbruster wrote:
> apply_to_qlist(), apply_to_node() work with QObjects.  This is
> designed for use by tests/qtest/qos-test.c, which gets the data in
> that form via QMP.  Goes back to commit fc281c8020 "tests: qgraph API
> for the qtest driver framework".
> 
> Commit 275ab39d86 "fuzz: add support for qos-assisted fuzz targets"
> added another user: qtest/fuzz/qos_fuzz.c.  To get the data as
> QObjects, it uses qmp_marshal_query_machines() and
> qmp_marshal_qom_list_types().
> 
> All this code is rather cumbersome.  Switch to working with generated
> QAPI types instead:
> 
> * Replace apply_to_qlist() & friends by machines_apply_to_node() and
>    types_apply_to_node().
> 
> * Have qos_fuzz.c use qmp_query_machines() and qmp_qom_list_types()
>    instead.
> 
> * Have qos_test.c convert from QObject to the QAPI types.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   tests/qtest/libqos/qos_external.h |  8 +++-
>   tests/qtest/fuzz/qos_fuzz.c       | 34 ++++-----------
>   tests/qtest/libqos/qos_external.c | 70 +++++++++++--------------------
>   tests/qtest/qos-test.c            | 29 +++++++++----
>   4 files changed, 59 insertions(+), 82 deletions(-)
> 
> diff --git a/tests/qtest/libqos/qos_external.h b/tests/qtest/libqos/qos_external.h
> index 7b44930c55..f63388cb30 100644
> --- a/tests/qtest/libqos/qos_external.h
> +++ b/tests/qtest/libqos/qos_external.h
> @@ -20,8 +20,12 @@
>   #define QOS_EXTERNAL_H
>   #include "libqos/qgraph.h"
>   
> -void apply_to_node(const char *name, bool is_machine, bool is_abstract);
> -void apply_to_qlist(QList *list, bool is_machine);
> +#include "libqos/malloc.h"
> +#include "qapi/qapi-types-machine.h"
> +#include "qapi/qapi-types-qom.h"
> +
> +void machines_apply_to_node(MachineInfoList *mach_info);
> +void types_apply_to_node(ObjectTypeInfoList *type_info);
>   QGuestAllocator *get_machine_allocator(QOSGraphObject *obj);
>   void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc);
>   
> diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
> index af28c92866..87eadb0889 100644
> --- a/tests/qtest/fuzz/qos_fuzz.c
> +++ b/tests/qtest/fuzz/qos_fuzz.c
> @@ -36,7 +36,6 @@
>   
>   #include "qapi/qapi-commands-machine.h"
>   #include "qapi/qapi-commands-qom.h"
> -#include "qapi/qmp/qlist.h"
>   
>   
>   void *fuzz_qos_obj;
> @@ -45,34 +44,19 @@ QGuestAllocator *fuzz_qos_alloc;
>   static const char *fuzz_target_name;
>   static char **fuzz_path_vec;
>   
> -/*
> - * Replaced the qmp commands with direct qmp_marshal calls.
> - * Probably there is a better way to do this
> - */
>   static void qos_set_machines_devices_available(void)
>   {
> -    QDict *req = qdict_new();
> -    QObject *response;
> -    QDict *args = qdict_new();
> -    QList *lst;
> +    MachineInfoList *mach_info;
> +    ObjectTypeInfoList *type_info;
>   
> -    qmp_marshal_query_machines(NULL, &response, &error_abort);
> -    lst = qobject_to(QList, response);
> -    apply_to_qlist(lst, true);
> +    mach_info = qmp_query_machines(&error_abort);
> +    machines_apply_to_node(mach_info);
> +    qapi_free_MachineInfoList(mach_info);
>   
> -    qobject_unref(response);
> -
> -
> -    qdict_put_str(req, "execute", "qom-list-types");
> -    qdict_put_str(args, "implements", "device");
> -    qdict_put_bool(args, "abstract", true);
> -    qdict_put_obj(req, "arguments", (QObject *) args);
> -
> -    qmp_marshal_qom_list_types(args, &response, &error_abort);
> -    lst = qobject_to(QList, response);
> -    apply_to_qlist(lst, false);
> -    qobject_unref(response);
> -    qobject_unref(req);
> +    type_info = qmp_qom_list_types(true, "device", true, true,
> +                                   &error_abort);

Glad you had interest in cleaning this.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

I tried to do a similar cleanup on the Python side, but didn't feel much 
support and ended up disabling s390x/ppc tests instead.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675074.html

> +    types_apply_to_node(type_info);
> +    qapi_free_ObjectTypeInfoList(type_info);
>   }
>   
>   static char **current_path;
> diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
> index 398556dde0..c707dac3b9 100644
> --- a/tests/qtest/libqos/qos_external.c
> +++ b/tests/qtest/libqos/qos_external.c
> @@ -29,62 +29,40 @@
>   #include "libqos/qgraph_internal.h"
>   #include "libqos/qos_external.h"
>   
> -
> -
> -void apply_to_node(const char *name, bool is_machine, bool is_abstract)
> +static void machine_apply_to_node(const char *name)
>   {
> -    char *machine_name = NULL;
> -    if (is_machine) {
> -        const char *arch = qtest_get_arch();
> -        machine_name = g_strconcat(arch, "/", name, NULL);
> -        name = machine_name;
> +    char *machine_name = g_strconcat(qtest_get_arch(), "/", name, NULL);
> +
> +    qos_graph_node_set_availability(machine_name, true);
> +    g_free(machine_name);
> +}
> +
> +void machines_apply_to_node(MachineInfoList *mach_info)
> +{
> +    MachineInfoList *tail;
> +
> +    for (tail = mach_info; tail; tail = tail->next) {
> +        machine_apply_to_node(tail->value->name);
> +        if (tail->value->alias) {
> +            machine_apply_to_node(tail->value->alias);
> +        }
>       }
> +}
> +
> +static void type_apply_to_node(const char *name, bool is_abstract)
> +{
>       qos_graph_node_set_availability(name, true);
>       if (is_abstract) {
>           qos_delete_cmd_line(name);
>       }
> -    g_free(machine_name);
>   }
>   
> -/**
> - * apply_to_qlist(): using QMP queries QEMU for a list of
> - * machines and devices available, and sets the respective node
> - * as true. If a node is found, also all its produced and contained
> - * child are marked available.
> - *
> - * See qos_graph_node_set_availability() for more info
> - */
> -void apply_to_qlist(QList *list, bool is_machine)
> +void types_apply_to_node(ObjectTypeInfoList *type_info)
>   {
> -    const QListEntry *p;
> -    const char *name;
> -    bool abstract;
> -    QDict *minfo;
> -    QObject *qobj;
> -    QString *qstr;
> -    QBool *qbool;
> +    ObjectTypeInfoList *tail;
>   
> -    for (p = qlist_first(list); p; p = qlist_next(p)) {
> -        minfo = qobject_to(QDict, qlist_entry_obj(p));
> -        qobj = qdict_get(minfo, "name");
> -        qstr = qobject_to(QString, qobj);
> -        name = qstring_get_str(qstr);
> -
> -        qobj = qdict_get(minfo, "abstract");
> -        if (qobj) {
> -            qbool = qobject_to(QBool, qobj);
> -            abstract = qbool_get_bool(qbool);
> -        } else {
> -            abstract = false;
> -        }
> -
> -        apply_to_node(name, is_machine, abstract);
> -        qobj = qdict_get(minfo, "alias");
> -        if (qobj) {
> -            qstr = qobject_to(QString, qobj);
> -            name = qstring_get_str(qstr);
> -            apply_to_node(name, is_machine, abstract);
> -        }
> +    for (tail = type_info; tail; tail = tail->next) {
> +        type_apply_to_node(tail->value->name, tail->value->abstract);
>       }
>   }
>   
> diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
> index ad193f43a5..3062a13557 100644
> --- a/tests/qtest/qos-test.c
> +++ b/tests/qtest/qos-test.c
> @@ -19,11 +19,12 @@
>   #include "qemu/osdep.h"
>   #include <getopt.h>
>   #include "libqtest-single.h"
> +#include "qapi/error.h"
>   #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qbool.h"
> -#include "qapi/qmp/qstring.h"
>   #include "qemu/module.h"
> -#include "qapi/qmp/qlist.h"
> +#include "qapi/qobject-input-visitor.h"
> +#include "qapi/qapi-visit-machine.h"
> +#include "qapi/qapi-visit-qom.h"
>   #include "libqos/malloc.h"
>   #include "libqos/qgraph.h"
>   #include "libqos/qgraph_internal.h"
> @@ -51,13 +52,20 @@ static void qos_set_machines_devices_available(void)
>   {
>       QDict *response;
>       QDict *args = qdict_new();
> -    QList *list;
> +    QObject *ret;
> +    Visitor *v;
> +    MachineInfoList *mach_info;
> +    ObjectTypeInfoList *type_info;
>   
>       qtest_start("-machine none");
>       response = qmp("{ 'execute': 'query-machines' }");
> -    list = qdict_get_qlist(response, "return");
> +    ret = qdict_get(response, "return");
>   
> -    apply_to_qlist(list, true);
> +    v = qobject_input_visitor_new(ret);
> +    visit_type_MachineInfoList(v, NULL, &mach_info, &error_abort);
> +    visit_free(v);
> +    machines_apply_to_node(mach_info);
> +    qapi_free_MachineInfoList(mach_info);
>   
>       qobject_unref(response);
>   
> @@ -66,10 +74,13 @@ static void qos_set_machines_devices_available(void)
>   
>       response = qmp("{'execute': 'qom-list-types',"
>                      " 'arguments': %p }", args);
> -    g_assert(qdict_haskey(response, "return"));
> -    list = qdict_get_qlist(response, "return");
> +    ret = qdict_get(response, "return");
>   
> -    apply_to_qlist(list, false);
> +    v = qobject_input_visitor_new(ret);
> +    visit_type_ObjectTypeInfoList(v, NULL, &type_info, &error_abort);
> +    visit_free(v);
> +    types_apply_to_node(type_info);
> +    qapi_free_ObjectTypeInfoList(type_info);
>   
>       qtest_end();
>       qobject_unref(response);
> 

Re: [PATCH 2/3] fuzz: Simplify how we compute available machines and types

[Alexander Bulekov]

On 200424 0911, Markus Armbruster wrote:
> apply_to_qlist(), apply_to_node() work with QObjects.  This is
> designed for use by tests/qtest/qos-test.c, which gets the data in
> that form via QMP.  Goes back to commit fc281c8020 "tests: qgraph API
> for the qtest driver framework".
> 
> Commit 275ab39d86 "fuzz: add support for qos-assisted fuzz targets"
> added another user: qtest/fuzz/qos_fuzz.c.  To get the data as
> QObjects, it uses qmp_marshal_query_machines() and
> qmp_marshal_qom_list_types().
> 
> All this code is rather cumbersome.  Switch to working with generated
> QAPI types instead:
> 
> * Replace apply_to_qlist() & friends by machines_apply_to_node() and
>   types_apply_to_node().
> 
> * Have qos_fuzz.c use qmp_query_machines() and qmp_qom_list_types()
>   instead.
> 
> * Have qos_test.c convert from QObject to the QAPI types.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Thank you for looking at this!
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>

> ---
>  tests/qtest/libqos/qos_external.h |  8 +++-
>  tests/qtest/fuzz/qos_fuzz.c       | 34 ++++-----------
>  tests/qtest/libqos/qos_external.c | 70 +++++++++++--------------------
>  tests/qtest/qos-test.c            | 29 +++++++++----
>  4 files changed, 59 insertions(+), 82 deletions(-)
> 
> diff --git a/tests/qtest/libqos/qos_external.h b/tests/qtest/libqos/qos_external.h
> index 7b44930c55..f63388cb30 100644
> --- a/tests/qtest/libqos/qos_external.h
> +++ b/tests/qtest/libqos/qos_external.h
> @@ -20,8 +20,12 @@
>  #define QOS_EXTERNAL_H
>  #include "libqos/qgraph.h"
>  
> -void apply_to_node(const char *name, bool is_machine, bool is_abstract);
> -void apply_to_qlist(QList *list, bool is_machine);
> +#include "libqos/malloc.h"
> +#include "qapi/qapi-types-machine.h"
> +#include "qapi/qapi-types-qom.h"
> +
> +void machines_apply_to_node(MachineInfoList *mach_info);
> +void types_apply_to_node(ObjectTypeInfoList *type_info);
>  QGuestAllocator *get_machine_allocator(QOSGraphObject *obj);
>  void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc);
>  
> diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
> index af28c92866..87eadb0889 100644
> --- a/tests/qtest/fuzz/qos_fuzz.c
> +++ b/tests/qtest/fuzz/qos_fuzz.c
> @@ -36,7 +36,6 @@
>  
>  #include "qapi/qapi-commands-machine.h"
>  #include "qapi/qapi-commands-qom.h"
> -#include "qapi/qmp/qlist.h"
>  
>  
>  void *fuzz_qos_obj;
> @@ -45,34 +44,19 @@ QGuestAllocator *fuzz_qos_alloc;
>  static const char *fuzz_target_name;
>  static char **fuzz_path_vec;
>  
> -/*
> - * Replaced the qmp commands with direct qmp_marshal calls.
> - * Probably there is a better way to do this
> - */
>  static void qos_set_machines_devices_available(void)
>  {
> -    QDict *req = qdict_new();
> -    QObject *response;
> -    QDict *args = qdict_new();
> -    QList *lst;
> +    MachineInfoList *mach_info;
> +    ObjectTypeInfoList *type_info;
>  
> -    qmp_marshal_query_machines(NULL, &response, &error_abort);
> -    lst = qobject_to(QList, response);
> -    apply_to_qlist(lst, true);
> +    mach_info = qmp_query_machines(&error_abort);
> +    machines_apply_to_node(mach_info);
> +    qapi_free_MachineInfoList(mach_info);
>  
> -    qobject_unref(response);
> -
> -
> -    qdict_put_str(req, "execute", "qom-list-types");
> -    qdict_put_str(args, "implements", "device");
> -    qdict_put_bool(args, "abstract", true);
> -    qdict_put_obj(req, "arguments", (QObject *) args);
> -
> -    qmp_marshal_qom_list_types(args, &response, &error_abort);
> -    lst = qobject_to(QList, response);
> -    apply_to_qlist(lst, false);
> -    qobject_unref(response);
> -    qobject_unref(req);
> +    type_info = qmp_qom_list_types(true, "device", true, true,
> +                                   &error_abort);
> +    types_apply_to_node(type_info);
> +    qapi_free_ObjectTypeInfoList(type_info);
>  }
>  
>  static char **current_path;
> diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
> index 398556dde0..c707dac3b9 100644
> --- a/tests/qtest/libqos/qos_external.c
> +++ b/tests/qtest/libqos/qos_external.c
> @@ -29,62 +29,40 @@
>  #include "libqos/qgraph_internal.h"
>  #include "libqos/qos_external.h"
>  
> -
> -
> -void apply_to_node(const char *name, bool is_machine, bool is_abstract)
> +static void machine_apply_to_node(const char *name)
>  {
> -    char *machine_name = NULL;
> -    if (is_machine) {
> -        const char *arch = qtest_get_arch();
> -        machine_name = g_strconcat(arch, "/", name, NULL);
> -        name = machine_name;
> +    char *machine_name = g_strconcat(qtest_get_arch(), "/", name, NULL);
> +
> +    qos_graph_node_set_availability(machine_name, true);
> +    g_free(machine_name);
> +}
> +
> +void machines_apply_to_node(MachineInfoList *mach_info)
> +{
> +    MachineInfoList *tail;
> +
> +    for (tail = mach_info; tail; tail = tail->next) {
> +        machine_apply_to_node(tail->value->name);
> +        if (tail->value->alias) {
> +            machine_apply_to_node(tail->value->alias);
> +        }
>      }
> +}
> +
> +static void type_apply_to_node(const char *name, bool is_abstract)
> +{
>      qos_graph_node_set_availability(name, true);
>      if (is_abstract) {
>          qos_delete_cmd_line(name);
>      }
> -    g_free(machine_name);
>  }
>  
> -/**
> - * apply_to_qlist(): using QMP queries QEMU for a list of
> - * machines and devices available, and sets the respective node
> - * as true. If a node is found, also all its produced and contained
> - * child are marked available.
> - *
> - * See qos_graph_node_set_availability() for more info
> - */
> -void apply_to_qlist(QList *list, bool is_machine)
> +void types_apply_to_node(ObjectTypeInfoList *type_info)
>  {
> -    const QListEntry *p;
> -    const char *name;
> -    bool abstract;
> -    QDict *minfo;
> -    QObject *qobj;
> -    QString *qstr;
> -    QBool *qbool;
> +    ObjectTypeInfoList *tail;
>  
> -    for (p = qlist_first(list); p; p = qlist_next(p)) {
> -        minfo = qobject_to(QDict, qlist_entry_obj(p));
> -        qobj = qdict_get(minfo, "name");
> -        qstr = qobject_to(QString, qobj);
> -        name = qstring_get_str(qstr);
> -
> -        qobj = qdict_get(minfo, "abstract");
> -        if (qobj) {
> -            qbool = qobject_to(QBool, qobj);
> -            abstract = qbool_get_bool(qbool);
> -        } else {
> -            abstract = false;
> -        }
> -
> -        apply_to_node(name, is_machine, abstract);
> -        qobj = qdict_get(minfo, "alias");
> -        if (qobj) {
> -            qstr = qobject_to(QString, qobj);
> -            name = qstring_get_str(qstr);
> -            apply_to_node(name, is_machine, abstract);
> -        }
> +    for (tail = type_info; tail; tail = tail->next) {
> +        type_apply_to_node(tail->value->name, tail->value->abstract);
>      }
>  }
>  
> diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
> index ad193f43a5..3062a13557 100644
> --- a/tests/qtest/qos-test.c
> +++ b/tests/qtest/qos-test.c
> @@ -19,11 +19,12 @@
>  #include "qemu/osdep.h"
>  #include <getopt.h>
>  #include "libqtest-single.h"
> +#include "qapi/error.h"
>  #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qbool.h"
> -#include "qapi/qmp/qstring.h"
>  #include "qemu/module.h"
> -#include "qapi/qmp/qlist.h"
> +#include "qapi/qobject-input-visitor.h"
> +#include "qapi/qapi-visit-machine.h"
> +#include "qapi/qapi-visit-qom.h"
>  #include "libqos/malloc.h"
>  #include "libqos/qgraph.h"
>  #include "libqos/qgraph_internal.h"
> @@ -51,13 +52,20 @@ static void qos_set_machines_devices_available(void)
>  {
>      QDict *response;
>      QDict *args = qdict_new();
> -    QList *list;
> +    QObject *ret;
> +    Visitor *v;
> +    MachineInfoList *mach_info;
> +    ObjectTypeInfoList *type_info;
>  
>      qtest_start("-machine none");
>      response = qmp("{ 'execute': 'query-machines' }");
> -    list = qdict_get_qlist(response, "return");
> +    ret = qdict_get(response, "return");
>  
> -    apply_to_qlist(list, true);
> +    v = qobject_input_visitor_new(ret);
> +    visit_type_MachineInfoList(v, NULL, &mach_info, &error_abort);
> +    visit_free(v);
> +    machines_apply_to_node(mach_info);
> +    qapi_free_MachineInfoList(mach_info);
>  
>      qobject_unref(response);
>  
> @@ -66,10 +74,13 @@ static void qos_set_machines_devices_available(void)
>  
>      response = qmp("{'execute': 'qom-list-types',"
>                     " 'arguments': %p }", args);
> -    g_assert(qdict_haskey(response, "return"));
> -    list = qdict_get_qlist(response, "return");
> +    ret = qdict_get(response, "return");
>  
> -    apply_to_qlist(list, false);
> +    v = qobject_input_visitor_new(ret);
> +    visit_type_ObjectTypeInfoList(v, NULL, &type_info, &error_abort);
> +    visit_free(v);
> +    types_apply_to_node(type_info);
> +    qapi_free_ObjectTypeInfoList(type_info);
>  
>      qtest_end();
>      qobject_unref(response);
> -- 
> 2.21.1
> 

Re: [PATCH 1/3] Makefile: Drop unused, broken target recurse-fuzz

[Alexander Bulekov]

On 200424 0911, Markus Armbruster wrote:
> Target recurse-fuzz depends on pc-bios/optionrom/fuzz, which can't be
> made.  It's not used anywhere.  Added in commit c621dc3e01c, looks
> like cargo cult.  Delete.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>

> ---
>  Makefile | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 8a9113e666..34275f57c9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -582,7 +582,6 @@ $(ROM_DIRS_RULES):
>  
>  .PHONY: recurse-all recurse-clean recurse-install
>  recurse-all: $(addsuffix /all, $(TARGET_DIRS) $(ROM_DIRS))
> -recurse-fuzz: $(addsuffix /fuzz, $(TARGET_DIRS) $(ROM_DIRS))
>  recurse-clean: $(addsuffix /clean, $(TARGET_DIRS) $(ROM_DIRS))
>  recurse-install: $(addsuffix /install, $(TARGET_DIRS))
>  $(addsuffix /install, $(TARGET_DIRS)): all
> -- 
> 2.21.1
> 

Re: [PATCH 0/3] fuzz: Probably there is a better way to do this

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> ... the comment next to qos_set_machines_devices_available() wonders.
> Yup, there is!

Queued.