Re: [PATCH v7 02/12] accel: Introduce 'query-accels' QMP command

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
>> Introduce the 'query-accels' QMP command which returns a list
>> of built-in accelerator names.
>>
>> - Accelerator is a QAPI enum of all existing accelerators,
>>
>> - AcceleratorInfo is a QAPI structure providing accelerator
>>   specific information. Currently the common structure base
>>   provides the name of the accelerator, while the specific
>>   part is empty, but each accelerator can expand it.
>>
>> - 'query-accels' QMP command returns a list of @AcceleratorInfo
>>
>> For example on a KVM-only build we get:
>>
>>     { "execute": "query-accels" }
>>     {
>>         "return": [
>>             {
>>                 "name": "qtest"
>>             },
>>             {
>>                 "name": "kvm"
>>             }
>>         ]
>>     }
>>
>> Note that we can't make the enum values or union branches conditional

"union branches" existed in a previous iteration, but no more.

>> because of target-specific poisoning of accelerator definitions.
>
> I second Eduardo's plea to explain this more clearly.  It's important,
> because if a properly conditionalized enum is feasible, then query-accel
> isn't needed.

The appended incremental patch conditionalizes the enum.  It applies on
top of the series, and passes "make check" for me.  Seems to contradict
"we can't make the enum values conditional".


diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index e7811654b7..586a61b5d9 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -329,3 +329,57 @@
 ##
 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
   'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || 
defined(TARGET_S390X) || defined(TARGET_MIPS)' }
+
+##
+# @Accelerator:
+#
+# An enumeration of accelerator names.
+#
+# Since: 6.1
+##
+{ 'enum': 'Accelerator',
+  'data': [
+      { 'name': 'hax', 'if': 'defined(CONFIG_HAX)' },
+      { 'name': 'hvf', 'if': 'defined(CONFIG_HVF)' },
+      { 'name': 'kvm', 'if': 'defined(CONFIG_KVM)' },
+      { 'name': 'qtest' },
+      { 'name': 'tcg', 'if': 'defined(CONFIG_TCG)' },
+      { 'name': 'whpx', 'if': 'defined(CONFIG_WHPX)' },
+      { 'name': 'xen', 'if': 'defined(CONFIG_XEN_BACKEND)' } ] }
+
+##
+# @AcceleratorInfo:
+#
+# Accelerator information.
+#
+# @name: The accelerator name.
+#
+# Since: 6.1
+##
+{ 'struct': 'AcceleratorInfo',
+  'data': { 'name': 'Accelerator' } }
+
+##
+# @query-accels:
+#
+# Get a list of AcceleratorInfo for all built-in accelerators.
+#
+# Returns: a list of @AcceleratorInfo describing each accelerator.
+#
+# Since: 6.1
+#
+# Example:
+#
+# -> { "execute": "query-accels" }
+# <- { "return": [
+#        {
+#            "name": "qtest"
+#        },
+#        {
+#            "name": "kvm"
+#        }
+#    ] }
+#
+##
+{ 'command': 'query-accels',
+  'returns': ['AcceleratorInfo'] }
diff --git a/qapi/machine.json b/qapi/machine.json
index 79a0891793..58a9c86b36 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1274,50 +1274,3 @@
 ##
 { 'event': 'MEM_UNPLUG_ERROR',
   'data': { 'device': 'str', 'msg': 'str' } }
-
-##
-# @Accelerator:
-#
-# An enumeration of accelerator names.
-#
-# Since: 6.1
-##
-{ 'enum': 'Accelerator',
-  'data': [ 'hax', 'hvf', 'kvm', 'qtest', 'tcg', 'whpx', 'xen' ] }
-
-##
-# @AcceleratorInfo:
-#
-# Accelerator information.
-#
-# @name: The accelerator name.
-#
-# Since: 6.1
-##
-{ 'struct': 'AcceleratorInfo',
-  'data': { 'name': 'Accelerator' } }
-
-##
-# @query-accels:
-#
-# Get a list of AcceleratorInfo for all built-in accelerators.
-#
-# Returns: a list of @AcceleratorInfo describing each accelerator.
-#
-# Since: 6.1
-#
-# Example:
-#
-# -> { "execute": "query-accels" }
-# <- { "return": [
-#        {
-#            "name": "qtest"
-#        },
-#        {
-#            "name": "kvm"
-#        }
-#    ] }
-#
-##
-{ 'command': 'query-accels',
-  'returns': ['AcceleratorInfo'] }
diff --git a/accel/accel-qmp.c b/accel/accel-qmp.c
index 426737b3f9..aca90f8682 100644
--- a/accel/accel-qmp.c
+++ b/accel/accel-qmp.c
@@ -7,42 +7,18 @@
  */
 
 #include "qemu/osdep.h"
-#include "qapi/qapi-commands-machine.h"
-
-static const bool accel_builtin_list[ACCELERATOR__MAX] = {
-    [ACCELERATOR_QTEST] = true,
-#ifdef CONFIG_TCG
-    [ACCELERATOR_TCG] = true,
-#endif
-#ifdef CONFIG_KVM
-    [ACCELERATOR_KVM] = true,
-#endif
-#ifdef CONFIG_HAX
-    [ACCELERATOR_HAX] = true,
-#endif
-#ifdef CONFIG_HVF
-    [ACCELERATOR_HVF] = true,
-#endif
-#ifdef CONFIG_WHPX
-    [ACCELERATOR_WHPX] = true,
-#endif
-#ifdef CONFIG_XEN_BACKEND
-    [ACCELERATOR_XEN] = true,
-#endif
-};
+#include "qapi/qapi-commands-machine-target.h"
 
 AcceleratorInfoList *qmp_query_accels(Error **errp)
 {
     AcceleratorInfoList *list = NULL, **tail = &list;
 
     for (Accelerator accel = 0; accel < ACCELERATOR__MAX; accel++) {
-        if (accel_builtin_list[accel]) {
-            AcceleratorInfo *info = g_new0(AcceleratorInfo, 1);
+        AcceleratorInfo *info = g_new0(AcceleratorInfo, 1);
 
-            info->name = accel;
+        info->name = accel;
 
-            QAPI_LIST_APPEND(tail, info);
-        }
+        QAPI_LIST_APPEND(tail, info);
     }
 
     return list;