[PATCH v6 0/4] compare machine type compat_props

[PATCH v6 0/4] compare machine type compat_props

[Maksim Davydov]

This script can be used to choose the best machine type in the
appropriate cases. Also we have to check compat_props of the old MT
after changes to be sure that they haven't broken old the MT. For
example, pc_compat_3_1 of pc-q35-3.1 has Icelake-Client which was
removed.

v6 -> v5:
* add ability to compare different QEMU binaries
* replace abstract drivers by its implementations
* improve human-readable format
* code refactoring

v5 -> v4:
* minor fixes

v4 -> v3:
* increase read buffer limit to limit value in libvirt
* add caching of qmp requests to speed up the script 

v3 -> v2:
* simplify adding new methods for getting QEMU default values
* add typing
* change concept from fixed dictionaries to classes

v2 -> v1:
* fix script code style and descriptions
* reorder patches
 
v1 -> previous iteration:
* new default value print concept
* QEMU python library is used to collect qmp data
* remove auxiliary patches (that was used to fix `->get` sematics)
* print compat_props in the correct order
* delete `absract` field to reduce output JSON size

Maksim Davydov (4):
  qom: add default value
  qmp: add dump machine type compatible properties
  python: add binary
  scripts: add script to compare compatible properties

 hw/core/machine-qmp-cmds.c     |  23 +-
 python/qemu/machine/machine.py |   5 +
 qapi/machine.json              |  54 +++-
 qom/qom-qmp-cmds.c             |   1 +
 scripts/compare_mt.py          | 484 +++++++++++++++++++++++++++++++++
 tests/qtest/fuzz/qos_fuzz.c    |   2 +-
 6 files changed, 565 insertions(+), 4 deletions(-)
 create mode 100755 scripts/compare_mt.py

-- 
2.34.1

[PATCH v6 1/4] qom: add default value

[Maksim Davydov]

qmp_qom_list_properties can print default values if they are available
as qmp_device_list_properties does, because both of them use the
ObjectPropertyInfo structure with default_value field. This can be useful
when working with "not device" types (e.g. memory-backend).

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 qom/qom-qmp-cmds.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 7c087299de..e91a235347 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -212,6 +212,7 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
         info->name = g_strdup(prop->name);
         info->type = g_strdup(prop->type);
         info->description = g_strdup(prop->description);
+        info->default_value = qobject_ref(prop->defval);
 
         QAPI_LIST_PREPEND(prop_list, info);
     }
-- 
2.34.1

Re: [PATCH v6 1/4] qom: add default value

[Philippe Mathieu-Daudé]

On 8/11/23 16:38, Maksim Davydov wrote:
> qmp_qom_list_properties can print default values if they are available
> as qmp_device_list_properties does, because both of them use the
> ObjectPropertyInfo structure with default_value field. This can be useful
> when working with "not device" types (e.g. memory-backend).
> 
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>   qom/qom-qmp-cmds.c | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v6 1/4] qom: add default value

[Markus Armbruster]

Maksim Davydov <davydov-max@yandex-team.ru> writes:

> qmp_qom_list_properties can print default values if they are available
> as qmp_device_list_properties does, because both of them use the
> ObjectPropertyInfo structure with default_value field. This can be useful
> when working with "not device" types (e.g. memory-backend).
>
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  qom/qom-qmp-cmds.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
> index 7c087299de..e91a235347 100644
> --- a/qom/qom-qmp-cmds.c
> +++ b/qom/qom-qmp-cmds.c
> @@ -212,6 +212,7 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
>          info->name = g_strdup(prop->name);
>          info->type = g_strdup(prop->type);
>          info->description = g_strdup(prop->description);
> +        info->default_value = qobject_ref(prop->defval);
>  
>          QAPI_LIST_PREPEND(prop_list, info);
>      }

Reviewed-by: Markus Armbruster <armbru@redhat.com>

[PATCH v6 2/4] qmp: add dump machine type compatible properties

[Maksim Davydov]

To control that creating new machine type doesn't affect the previous
types (their compat_props) and to check complex compat_props inheritance
we need qmp command to print machine type compatible properties.
This patch adds the ability to get list of all the compat_props of the
corresponding supported machines for their comparison via new optional
argument of "query-machines" command.

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/core/machine-qmp-cmds.c  | 23 +++++++++++++++-
 qapi/machine.json           | 54 +++++++++++++++++++++++++++++++++++--
 tests/qtest/fuzz/qos_fuzz.c |  2 +-
 3 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 3860a50c3b..a49d0dc362 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -66,7 +66,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
     return head;
 }
 
-MachineInfoList *qmp_query_machines(Error **errp)
+MachineInfoList *qmp_query_machines(bool has_compat_props, bool compat_props,
+                                    Error **errp)
 {
     GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
     MachineInfoList *mach_list = NULL;
@@ -98,6 +99,26 @@ MachineInfoList *qmp_query_machines(Error **errp)
             info->default_ram_id = g_strdup(mc->default_ram_id);
         }
 
+        if (compat_props && mc->compat_props) {
+            int i;
+            info->compat_props = NULL;
+            CompatPropertyList **tail = &(info->compat_props);
+            info->has_compat_props = true;
+
+            for (i = 0; i < mc->compat_props->len; i++) {
+                GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props,
+                                                            i);
+                CompatProperty *prop;
+
+                prop = g_malloc0(sizeof(*prop));
+                prop->driver = g_strdup(mt_prop->driver);
+                prop->property = g_strdup(mt_prop->property);
+                prop->value = g_strdup(mt_prop->value);
+
+                QAPI_LIST_APPEND(tail, prop);
+            }
+        }
+
         QAPI_LIST_PREPEND(mach_list, info);
     }
 
diff --git a/qapi/machine.json b/qapi/machine.json
index b6d634b30d..8ca0c134a2 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -135,6 +135,25 @@
 ##
 { 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
 
+##
+# @CompatProperty:
+#
+# Machine type compatible property. It's based on GlobalProperty and created
+# for machine type compat properties (see scripts)
+#
+# @driver: name of the driver that has GlobalProperty
+#
+# @property: global property name
+#
+# @value: global property value
+#
+# Since: 8.2
+##
+{ 'struct': 'CompatProperty',
+  'data': { 'driver': 'str',
+            'property': 'str',
+            'value': 'str' } }
+
 ##
 # @MachineInfo:
 #
@@ -166,6 +185,9 @@
 #
 # @acpi: machine type supports ACPI (since 8.0)
 #
+# @compat-props: List of compatible properties that defines machine type
+#                (since 8.2)
+#
 # Since: 1.2
 ##
 { 'struct': 'MachineInfo',
@@ -173,18 +195,46 @@
             '*is-default': 'bool', 'cpu-max': 'int',
             'hotpluggable-cpus': 'bool',  'numa-mem-supported': 'bool',
             'deprecated': 'bool', '*default-cpu-type': 'str',
-            '*default-ram-id': 'str', 'acpi': 'bool' } }
+            '*default-ram-id': 'str', 'acpi': 'bool', '*compat-props': ['CompatProperty'] } }
 
 ##
 # @query-machines:
 #
 # Return a list of supported machines
 #
+# @compat-props: if true return will contain information about machine type
+#                compatible properties (since 8.2)
+#
 # Returns: a list of MachineInfo
 #
 # Since: 1.2
+#
+# Example:
+#
+# -> { "execute": "query-machines", "arguments": { "compat-props": true } }
+# <- { "return": [
+#          {
+#              "hotpluggable-cpus": true,
+#              "name": "pc-q35-6.2",
+#              "compat-props": [
+#                  {
+#                      "driver": "virtio-mem",
+#                      "property": "unplugged-inaccessible",
+#                      "value": "off"
+#                   }
+#               ],
+#               "numa-mem-supported": false,
+#               "default-cpu-type": "qemu64-x86_64-cpu",
+#               "cpu-max": 288,
+#               "deprecated": false,
+#               "default-ram-id": "pc.ram"
+#           },
+#           ...
+#    }
 ##
-{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+{ 'command': 'query-machines',
+  'data': { '*compat-props': 'bool' },
+  'returns': ['MachineInfo'] }
 
 ##
 # @CurrentMachineParams:
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index e403d373a0..b71e945c5f 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -46,7 +46,7 @@ static void qos_set_machines_devices_available(void)
     MachineInfoList *mach_info;
     ObjectTypeInfoList *type_info;
 
-    mach_info = qmp_query_machines(&error_abort);
+    mach_info = qmp_query_machines(false, false, &error_abort);
     machines_apply_to_node(mach_info);
     qapi_free_MachineInfoList(mach_info);
 
-- 
2.34.1

Re: [PATCH v6 2/4] qmp: add dump machine type compatible properties

[Markus Armbruster]

I apologize for the lateness of my review.

Maksim Davydov <davydov-max@yandex-team.ru> writes:

> To control that creating new machine type doesn't affect the previous
> types (their compat_props) and to check complex compat_props inheritance
> we need qmp command to print machine type compatible properties.
>
> This patch adds the ability to get list of all the compat_props of the
> corresponding supported machines for their comparison via new optional
> argument of "query-machines" command.

Sounds like this is to let developers prevent unwanted change.  Such
testing interfaces need not and should not be stable interfaces.
Thoughts?

> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  hw/core/machine-qmp-cmds.c  | 23 +++++++++++++++-
>  qapi/machine.json           | 54 +++++++++++++++++++++++++++++++++++--
>  tests/qtest/fuzz/qos_fuzz.c |  2 +-
>  3 files changed, 75 insertions(+), 4 deletions(-)
>
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 3860a50c3b..a49d0dc362 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> @@ -66,7 +66,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
>      return head;
>  }
>  
> -MachineInfoList *qmp_query_machines(Error **errp)
> +MachineInfoList *qmp_query_machines(bool has_compat_props, bool compat_props,
> +                                    Error **errp)
>  {
>      GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>      MachineInfoList *mach_list = NULL;
> @@ -98,6 +99,26 @@ MachineInfoList *qmp_query_machines(Error **errp)
>              info->default_ram_id = g_strdup(mc->default_ram_id);
>          }
>  
> +        if (compat_props && mc->compat_props) {
> +            int i;
> +            info->compat_props = NULL;
> +            CompatPropertyList **tail = &(info->compat_props);
> +            info->has_compat_props = true;
> +
> +            for (i = 0; i < mc->compat_props->len; i++) {
> +                GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props,
> +                                                            i);
> +                CompatProperty *prop;
> +
> +                prop = g_malloc0(sizeof(*prop));
> +                prop->driver = g_strdup(mt_prop->driver);
> +                prop->property = g_strdup(mt_prop->property);
> +                prop->value = g_strdup(mt_prop->value);
> +
> +                QAPI_LIST_APPEND(tail, prop);
> +            }
> +        }
> +
>          QAPI_LIST_PREPEND(mach_list, info);
>      }
>  
> diff --git a/qapi/machine.json b/qapi/machine.json
> index b6d634b30d..8ca0c134a2 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -135,6 +135,25 @@
>  ##
>  { 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
>  
> +##
> +# @CompatProperty:
> +#
> +# Machine type compatible property. It's based on GlobalProperty and created
> +# for machine type compat properties (see scripts)

"compatibility property"

Doc comments are user documentation.  Can we describe this without
referencing C data types like GlobalProperty?  I'd start with the
purpose: specify a default value for a certain property of certain kind
of device.

The reference (see scripts) needs to be more precise to be useful.

Limit line length to 70, please.

Two spaces between sentences for consistency, please.

> +#
> +# @driver: name of the driver that has GlobalProperty
> +#
> +# @property: global property name
> +#
> +# @value: global property value

I don't like these descriptions, but let's improve the paragraph above
them first.

> +#
> +# Since: 8.2

Going to be 9.0.  More of the same below.

> +##
> +{ 'struct': 'CompatProperty',
> +  'data': { 'driver': 'str',
> +            'property': 'str',
> +            'value': 'str' } }
> +
>  ##
>  # @MachineInfo:
>  #
> @@ -166,6 +185,9 @@
>  #
>  # @acpi: machine type supports ACPI (since 8.0)
>  #
> +# @compat-props: List of compatible properties that defines machine type

"The machine type's compatibility properties"

Leaves unsaid when the member is present.  Let's worry about that later.

> +#                (since 8.2)
> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'MachineInfo',
> @@ -173,18 +195,46 @@
>              '*is-default': 'bool', 'cpu-max': 'int',
>              'hotpluggable-cpus': 'bool',  'numa-mem-supported': 'bool',
>              'deprecated': 'bool', '*default-cpu-type': 'str',
> -            '*default-ram-id': 'str', 'acpi': 'bool' } }
> +            '*default-ram-id': 'str', 'acpi': 'bool', '*compat-props': ['CompatProperty'] } }

Long line, break it before '*compat-props', please.

To mark the interface unstable, do something like

               '*compat-props': { 'type': ['CompatProperty'],
                                  'features': [ 'unstable' ] },

You may want to rename it to x-compat-props, just to make "unstable"
obvious to human users.

>  
>  ##
>  # @query-machines:
>  #
>  # Return a list of supported machines
>  #
> +# @compat-props: if true return will contain information about machine type
> +#                compatible properties (since 8.2)

"compatibility properties"

Suppressing parts of the output makes sense only if it's fairly big.
How much additional compat-props output do you observe?

> +#
>  # Returns: a list of MachineInfo
>  #
>  # Since: 1.2
> +#
> +# Example:
> +#
> +# -> { "execute": "query-machines", "arguments": { "compat-props": true } }
> +# <- { "return": [
> +#          {
> +#              "hotpluggable-cpus": true,
> +#              "name": "pc-q35-6.2",
> +#              "compat-props": [
> +#                  {
> +#                      "driver": "virtio-mem",
> +#                      "property": "unplugged-inaccessible",
> +#                      "value": "off"
> +#                   }
> +#               ],
> +#               "numa-mem-supported": false,
> +#               "default-cpu-type": "qemu64-x86_64-cpu",
> +#               "cpu-max": 288,
> +#               "deprecated": false,
> +#               "default-ram-id": "pc.ram"
> +#           },
> +#           ...
> +#    }
>  ##
> -{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
> +{ 'command': 'query-machines',
> +  'data': { '*compat-props': 'bool' },
> +  'returns': ['MachineInfo'] }
>  
>  ##
>  # @CurrentMachineParams:
> diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
> index e403d373a0..b71e945c5f 100644
> --- a/tests/qtest/fuzz/qos_fuzz.c
> +++ b/tests/qtest/fuzz/qos_fuzz.c
> @@ -46,7 +46,7 @@ static void qos_set_machines_devices_available(void)
>      MachineInfoList *mach_info;
>      ObjectTypeInfoList *type_info;
>  
> -    mach_info = qmp_query_machines(&error_abort);
> +    mach_info = qmp_query_machines(false, false, &error_abort);
>      machines_apply_to_node(mach_info);
>      qapi_free_MachineInfoList(mach_info);

Re: [PATCH v6 2/4] qmp: add dump machine type compatible properties

[Maksim Davydov]

Thanks for reviewing
Sorry for replying late

On 12/1/23 12:49, Markus Armbruster wrote:
> I apologize for the lateness of my review.
>
> Maksim Davydov <davydov-max@yandex-team.ru> writes:
>
>> To control that creating new machine type doesn't affect the previous
>> types (their compat_props) and to check complex compat_props inheritance
>> we need qmp command to print machine type compatible properties.
>>
>> This patch adds the ability to get list of all the compat_props of the
>> corresponding supported machines for their comparison via new optional
>> argument of "query-machines" command.
> Sounds like this is to let developers prevent unwanted change.  Such
> testing interfaces need not and should not be stable interfaces.
> Thoughts?
I'm not sure that rightly understand your idea about unstable: do you mean
that we can allow this interface to be not robust (e.g. compat-props in
MachineInfo can be not presented) or that this is new thusit can be
unstable?
>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   hw/core/machine-qmp-cmds.c  | 23 +++++++++++++++-
>>   qapi/machine.json           | 54 +++++++++++++++++++++++++++++++++++--
>>   tests/qtest/fuzz/qos_fuzz.c |  2 +-
>>   3 files changed, 75 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
>> index 3860a50c3b..a49d0dc362 100644
>> --- a/hw/core/machine-qmp-cmds.c
>> +++ b/hw/core/machine-qmp-cmds.c
>> @@ -66,7 +66,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
>>       return head;
>>   }
>>   
>> -MachineInfoList *qmp_query_machines(Error **errp)
>> +MachineInfoList *qmp_query_machines(bool has_compat_props, bool compat_props,
>> +                                    Error **errp)
>>   {
>>       GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
>>       MachineInfoList *mach_list = NULL;
>> @@ -98,6 +99,26 @@ MachineInfoList *qmp_query_machines(Error **errp)
>>               info->default_ram_id = g_strdup(mc->default_ram_id);
>>           }
>>   
>> +        if (compat_props && mc->compat_props) {
>> +            int i;
>> +            info->compat_props = NULL;
>> +            CompatPropertyList **tail = &(info->compat_props);
>> +            info->has_compat_props = true;
>> +
>> +            for (i = 0; i < mc->compat_props->len; i++) {
>> +                GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props,
>> +                                                            i);
>> +                CompatProperty *prop;
>> +
>> +                prop = g_malloc0(sizeof(*prop));
>> +                prop->driver = g_strdup(mt_prop->driver);
>> +                prop->property = g_strdup(mt_prop->property);
>> +                prop->value = g_strdup(mt_prop->value);
>> +
>> +                QAPI_LIST_APPEND(tail, prop);
>> +            }
>> +        }
>> +
>>           QAPI_LIST_PREPEND(mach_list, info);
>>       }
>>   
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index b6d634b30d..8ca0c134a2 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -135,6 +135,25 @@
>>   ##
>>   { 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
>>   
>> +##
>> +# @CompatProperty:
>> +#
>> +# Machine type compatible property. It's based on GlobalProperty and created
>> +# for machine type compat properties (see scripts)
> "compatibility property"
>
> Doc comments are user documentation.  Can we describe this without
> referencing C data types like GlobalProperty?  I'd start with the
> purpose: specify a default value for a certain property of certain kind
> of device.
>
> The reference (see scripts) needs to be more precise to be useful.
>
> Limit line length to 70, please.
>
> Two spaces between sentences for consistency, please.
>
>> +#
>> +# @driver: name of the driver that has GlobalProperty
>> +#
>> +# @property: global property name
>> +#
>> +# @value: global property value
> I don't like these descriptions, but let's improve the paragraph above
> them first.
>
>> +#
>> +# Since: 8.2
> Going to be 9.0.  More of the same below.
>
>> +##
>> +{ 'struct': 'CompatProperty',
>> +  'data': { 'driver': 'str',
>> +            'property': 'str',
>> +            'value': 'str' } }
>> +
>>   ##
>>   # @MachineInfo:
>>   #
>> @@ -166,6 +185,9 @@
>>   #
>>   # @acpi: machine type supports ACPI (since 8.0)
>>   #
>> +# @compat-props: List of compatible properties that defines machine type
> "The machine type's compatibility properties"
>
> Leaves unsaid when the member is present.  Let's worry about that later.
>
>> +#                (since 8.2)
>> +#
>>   # Since: 1.2
>>   ##
>>   { 'struct': 'MachineInfo',
>> @@ -173,18 +195,46 @@
>>               '*is-default': 'bool', 'cpu-max': 'int',
>>               'hotpluggable-cpus': 'bool',  'numa-mem-supported': 'bool',
>>               'deprecated': 'bool', '*default-cpu-type': 'str',
>> -            '*default-ram-id': 'str', 'acpi': 'bool' } }
>> +            '*default-ram-id': 'str', 'acpi': 'bool', '*compat-props': ['CompatProperty'] } }
> Long line, break it before '*compat-props', please.
>
> To mark the interface unstable, do something like
>
>                 '*compat-props': { 'type': ['CompatProperty'],
>                                    'features': [ 'unstable' ] },
>
> You may want to rename it to x-compat-props, just to make "unstable"
> obvious to human users.
>
>>   
>>   ##
>>   # @query-machines:
>>   #
>>   # Return a list of supported machines
>>   #
>> +# @compat-props: if true return will contain information about machine type
>> +#                compatible properties (since 8.2)
> "compatibility properties"
>
> Suppressing parts of the output makes sense only if it's fairly big.
> How much additional compat-props output do you observe?
now, there are 61 machines types, so this qmp command generates a 450KB JSON
>> +#
>>   # Returns: a list of MachineInfo
>>   #
>>   # Since: 1.2
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "query-machines", "arguments": { "compat-props": true } }
>> +# <- { "return": [
>> +#          {
>> +#              "hotpluggable-cpus": true,
>> +#              "name": "pc-q35-6.2",
>> +#              "compat-props": [
>> +#                  {
>> +#                      "driver": "virtio-mem",
>> +#                      "property": "unplugged-inaccessible",
>> +#                      "value": "off"
>> +#                   }
>> +#               ],
>> +#               "numa-mem-supported": false,
>> +#               "default-cpu-type": "qemu64-x86_64-cpu",
>> +#               "cpu-max": 288,
>> +#               "deprecated": false,
>> +#               "default-ram-id": "pc.ram"
>> +#           },
>> +#           ...
>> +#    }
>>   ##
>> -{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
>> +{ 'command': 'query-machines',
>> +  'data': { '*compat-props': 'bool' },
>> +  'returns': ['MachineInfo'] }
>>   
>>   ##
>>   # @CurrentMachineParams:
>> diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
>> index e403d373a0..b71e945c5f 100644
>> --- a/tests/qtest/fuzz/qos_fuzz.c
>> +++ b/tests/qtest/fuzz/qos_fuzz.c
>> @@ -46,7 +46,7 @@ static void qos_set_machines_devices_available(void)
>>       MachineInfoList *mach_info;
>>       ObjectTypeInfoList *type_info;
>>   
>> -    mach_info = qmp_query_machines(&error_abort);
>> +    mach_info = qmp_query_machines(false, false, &error_abort);
>>       machines_apply_to_node(mach_info);
>>       qapi_free_MachineInfoList(mach_info);

-- 
Best regards,
Maksim Davydov

Re: [PATCH v6 2/4] qmp: add dump machine type compatible properties

[Markus Armbruster]

Maksim Davydov <davydov-max@yandex-team.ru> writes:

> Thanks for reviewing
> Sorry for replying late
>
> On 12/1/23 12:49, Markus Armbruster wrote:
>> I apologize for the lateness of my review.
>>
>> Maksim Davydov <davydov-max@yandex-team.ru> writes:
>>
>>> To control that creating new machine type doesn't affect the previous
>>> types (their compat_props) and to check complex compat_props inheritance
>>> we need qmp command to print machine type compatible properties.
>>>
>>> This patch adds the ability to get list of all the compat_props of the
>>> corresponding supported machines for their comparison via new optional
>>> argument of "query-machines" command.
>>
>> Sounds like this is to let developers prevent unwanted change.  Such
>> testing interfaces need not and should not be stable interfaces.
>> Thoughts?
>
> I'm not sure that rightly understand your idea about unstable: do you mean
> that we can allow this interface to be not robust (e.g. compat-props in
> MachineInfo can be not presented) or that this is new thusit can be
> unstable?

It's about external interface stability: incompatible change requires
deprecation and a grace period.  See docs/about/deprecated.rst first
section.

QMP is a stable external interface, except for parts marked unstable.

In my review, I wondered whether the QMP interface you add needs to be
stable in that sense.  Does it?

If no, I'll gladly show you how to mark it unstable.

>>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

[...]

>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>> index b6d634b30d..8ca0c134a2 100644
>>> --- a/qapi/machine.json
>>> +++ b/qapi/machine.json

[...]

>>>  ##
>>>  # @query-machines:
>>>  #
>>>  # Return a list of supported machines
>>>  #
>>> +# @compat-props: if true return will contain information about machine type
>>> +#                compatible properties (since 8.2)
>>
>> "compatibility properties"
>>
>> Suppressing parts of the output makes sense only if it's fairly big.
>> How much additional compat-props output do you observe?
>
> now, there are 61 machines types, so this qmp command generates a 450KB JSON

Uff.

Recommend to explain this briefly in the commit message.  Something like
"Since information on compatibility properties can increase the command
output by a factor of 40, add an argument to enable it, default off."

Does make me wonder whether we want a separate command, though.

[...]

[PATCH v6 3/4] python: add binary

[Maksim Davydov]

Add a supportive property to access the path to the qemu binary

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
---
 python/qemu/machine/machine.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 31cb9d617d..78436403b2 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -328,6 +328,11 @@ def args(self) -> List[str]:
         """Returns the list of arguments given to the QEMU binary."""
         return self._args
 
+    @property
+    def binary(self) -> str:
+        """Returns path to the qemu binary"""
+        return self._binary
+
     def _pre_launch(self) -> None:
         if self._qmp_set:
             if self._monitor_address is None:
-- 
2.34.1

Re: [PATCH v6 3/4] python: add binary

[Philippe Mathieu-Daudé]

Hi Maksim,

On 8/11/23 16:38, Maksim Davydov wrote:
> Add a supportive property to access the path to the qemu binary
> 
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> ---
>   python/qemu/machine/machine.py | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 31cb9d617d..78436403b2 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -328,6 +328,11 @@ def args(self) -> List[str]:
>           """Returns the list of arguments given to the QEMU binary."""
>           return self._args
>   
> +    @property
> +    def binary(self) -> str:
> +        """Returns path to the qemu binary"""

s/qemu/QEMU/ (like 2 lines earlier).

Otherwise,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        return self._binary
> +
>       def _pre_launch(self) -> None:
>           if self._qmp_set:
>               if self._monitor_address is None:

Re: [PATCH v6 3/4] python: add binary

[John Snow]

On Wed, Nov 8, 2023 at 10:39 AM Maksim Davydov
<davydov-max@yandex-team.ru> wrote:
>
> Add a supportive property to access the path to the qemu binary
>
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> ---
>  python/qemu/machine/machine.py | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 31cb9d617d..78436403b2 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -328,6 +328,11 @@ def args(self) -> List[str]:
>          """Returns the list of arguments given to the QEMU binary."""
>          return self._args
>
> +    @property
> +    def binary(self) -> str:
> +        """Returns path to the qemu binary"""
> +        return self._binary
> +
>      def _pre_launch(self) -> None:
>          if self._qmp_set:
>              if self._monitor_address is None:
> --
> 2.34.1
>
>

'kay.

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [PATCH v6 3/4] python: add binary

[Philippe Mathieu-Daudé]

On 8/11/23 16:38, Maksim Davydov wrote:
> Add a supportive property to access the path to the qemu binary
> 
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> ---
>   python/qemu/machine/machine.py | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 31cb9d617d..78436403b2 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -328,6 +328,11 @@ def args(self) -> List[str]:
>           """Returns the list of arguments given to the QEMU binary."""
>           return self._args
>   
> +    @property
> +    def binary(self) -> str:
> +        """Returns path to the qemu binary"""
> +        return self._binary
> +
>       def _pre_launch(self) -> None:
>           if self._qmp_set:
>               if self._monitor_address is None:

Better patch subject could be:
"python/qemu/machine: Add method to retrieve QEMUMachine::binary field"

Re: [PATCH v6 3/4] python: add binary

[Maksim Davydov]

Thanks for reviewing!
I'll change patch subject to more appropriate one and fix the docstring

On 11/10/23 10:03, Philippe Mathieu-Daudé wrote:
> On 8/11/23 16:38, Maksim Davydov wrote:
>> Add a supportive property to access the path to the qemu binary
>>
>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
>> ---
>>   python/qemu/machine/machine.py | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/python/qemu/machine/machine.py 
>> b/python/qemu/machine/machine.py
>> index 31cb9d617d..78436403b2 100644
>> --- a/python/qemu/machine/machine.py
>> +++ b/python/qemu/machine/machine.py
>> @@ -328,6 +328,11 @@ def args(self) -> List[str]:
>>           """Returns the list of arguments given to the QEMU binary."""
>>           return self._args
>>   +    @property
>> +    def binary(self) -> str:
>> +        """Returns path to the qemu binary"""
>> +        return self._binary
>> +
>>       def _pre_launch(self) -> None:
>>           if self._qmp_set:
>>               if self._monitor_address is None:
>
> Better patch subject could be:
> "python/qemu/machine: Add method to retrieve QEMUMachine::binary field"

-- 
Best regards,
Maksim Davydov

[PATCH v6 4/4] scripts: add script to compare compatible properties

[Maksim Davydov]

This script runs QEMU to obtain compat_props of machines and default
values of different types of drivers to produce comparison table. This
table can be used to compare machine types to choose the most suitable
machine or compare binaries to be sure that migration to the newer version
will save all device properties. Also the json or csv format of this
table can be used to check does a new machine affect the previous ones by
comparing tables with and without the new machine.

Default values (that will be used without machine compat_props) of
properties are needed to fill "holes" in the table (one machine has
the property but another machine not. For instance, 2.12 machine has
`{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
3.1 machine doesn't have it. Thus, to compare these machines we need to
get unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 machine. These
unknown values in the table are called "holes". To get values for these
"holes" the script uses list of appropriate methods.)

Notes:
* Some init values from the devices can't be available like properties
  from virtio-9p when configure has --disable-virtfs. This situations will
  be seen in the table as "unavailable driver".
* Default values can be obtained in an unobvious way, like x86 features.
  If the script doesn't know how to get property default value to compare
  one machine with another it fills "holes" with "unavailable method". This
  is done because script uses whitelist model to get default values of
  different types. It means that the method that can't be applied to a new
  type that can crash this script. It is better to get an "unavailable
  driver" when creating a new machine with new compatible properties than
  to break this script. So it turns out a more stable and generic script.
* If the default value can't be obtained because this property doesn't
  exist or because this property can't have default value, appropriate
  "hole" will be filled by "unknown property" or "no default value"
* If the property is applied to the abstract class, the script collects
  default values from all child classes (set of default values)
* Raw table (--raw flag) should be used with json/csv parameters for
  scripts and etc. Human-readable (default) format contains transformed
  and simplified values and it doesn't contain lines with the same values
  in columns

Example:
./scripts/compare_mt.py --mt pc-q35-6.2 pc-q35-7.1
╒══════════════════╤══════════════════════════╤════════════════════════════╤════════════════════════════╕
│      Driver      │         Property         │  build/qemu-system-x86_64  │  build/qemu-system-x86_64  │
│                  │                          │         pc-q35-6.2         │         pc-q35-7.1         │
╞══════════════════╪══════════════════════════╪════════════════════════════╪════════════════════════════╡
│     PIIX4_PM     │ x-not-migrate-acpi-index │            True            │           False            │
├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
│ arm-gicv3-common │     force-8-bit-prio     │            True            │     unavailable driver     │
├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
│     nvme-ns      │      eui64-default       │            True            │           False            │
├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
│    virtio-mem    │  unplugged-inaccessible  │           False            │            auto            │
╘══════════════════╧══════════════════════════╧════════════════════════════╧════════════════════════════╛

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Co-developed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 scripts/compare_mt.py | 484 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 484 insertions(+)
 create mode 100755 scripts/compare_mt.py

diff --git a/scripts/compare_mt.py b/scripts/compare_mt.py
new file mode 100755
index 0000000000..685bd9a336
--- /dev/null
+++ b/scripts/compare_mt.py
@@ -0,0 +1,484 @@
+#!/usr/bin/env python3
+#
+# Script to compare machine type compatible properties (include/hw/boards.h).
+# compat_props are applied to the driver during initialization to change
+# default values, for instance, to maintain compatibility.
+# This script constructs table with machines and values of their compat_props
+# to compare and to find places for improvements or places with bugs. If
+# during the comparison, some machine type doesn't have a property (it is in
+# the comparison table because another machine type has it), then the
+# appropriate method will be used to obtain the default value of this driver
+# property via qmp command (e.g. query-cpu-model-expansion for x86_64-cpu).
+# These methods are defined below in qemu_property_methods.
+#
+# Copyright (c) Yandex Technologies LLC, 2023
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, see <http://www.gnu.org/licenses/>.
+
+import sys
+from os import path
+from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
+import pandas as pd
+from contextlib import ExitStack
+from typing import Optional, List, Dict, Generator, Tuple, Union, Any, Set
+
+try:
+    qemu_dir = path.abspath(path.dirname(path.dirname(__file__)))
+    sys.path.append(path.join(qemu_dir, 'python'))
+    from qemu.machine import QEMUMachine
+except ModuleNotFoundError as exc:
+    print(f"Module '{exc.name}' not found.")
+    print("Try export PYTHONPATH=top-qemu-dir/python or run from top-qemu-dir")
+    sys.exit(1)
+
+
+default_qemu_args = '-enable-kvm -machine none'
+default_qemu_binary = 'build/qemu-system-x86_64'
+
+
+# Methods for gettig the right values of drivers properties
+#
+# Use these methods as a 'whitelist' and add entries only if necessary. It's
+# important to be stable and predictable in analysis and tests.
+# Be careful:
+# * Class must be inherited from 'QEMUObject' and used in new_driver()
+# * Class has to implement get_prop method in order to get values
+# * Specialization always wins (with the given classes for 'device' and
+#   'x86_64-cpu', method of 'x86_64-cpu' will be used for '486-x86_64-cpu')
+
+class Driver():
+    def __init__(self, vm: QEMUMachine, name: str, abstract: bool) -> None:
+        self.vm = vm
+        self.name = name
+        self.abstract = abstract
+        self.parent: Optional[Driver] = None
+        self.property_getter: Optional[Driver] = None
+
+    def get_prop(self, driver: str, prop: str) -> str:
+        if self.property_getter:
+            return self.property_getter.get_prop(driver, prop)
+        else:
+            return 'Unavailable method'
+
+    def is_child_of(self, parent: 'Driver') -> bool:
+        """Checks whether self is (recursive) child of @parent"""
+        cur_parent = self.parent
+        while cur_parent:
+            if cur_parent is parent:
+                return True
+            cur_parent = cur_parent.parent
+
+        return False
+
+    def set_implementations(self, implementations: List['Driver']) -> None:
+        self.implementations = implementations
+
+
+class QEMUObject(Driver):
+    def __init__(self, vm: QEMUMachine, name: str) -> None:
+        super().__init__(vm, name, True)
+
+    def set_implementations(self, implementations: List[Driver]) -> None:
+        self.implementations = implementations
+
+        # each implementation of the abstract driver has to use property getter
+        # of this abstract driver unless it has specialization. (e.g. having
+        # 'device' and 'x86_64-cpu', property getter of 'x86_64-cpu' will be
+        # used for '486-x86_64-cpu')
+        for impl in implementations:
+            if not impl.property_getter or\
+                    self.is_child_of(impl.property_getter):
+                impl.property_getter = self
+
+
+class QEMUDevice(QEMUObject):
+    def __init__(self, vm: QEMUMachine) -> None:
+        super().__init__(vm, 'device')
+        self.cached: Dict[str, List[Dict[str, Any]]] = {}
+
+    def get_prop(self, driver: str, prop_name: str) -> str:
+        if driver not in self.cached:
+            self.cached[driver] = self.vm.cmd('device-list-properties',
+                                              typename=driver)
+        for prop in self.cached[driver]:
+            if prop['name'] == prop_name:
+                return str(prop.get('default-value', 'No default value'))
+
+        return 'Unknown property'
+
+
+class QEMUx86CPU(QEMUObject):
+    def __init__(self, vm: QEMUMachine) -> None:
+        super().__init__(vm, 'x86_64-cpu')
+        self.cached: Dict[str, Dict[str, Any]] = {}
+
+    def get_prop(self, driver: str, prop_name: str) -> str:
+        if not driver.endswith('-x86_64-cpu'):
+            return 'Wrong x86_64-cpu name'
+
+        # crop last 11 chars '-x86_64-cpu'
+        name = driver[:-11]
+        if name not in self.cached:
+            self.cached[name] = self.vm.cmd(
+                'query-cpu-model-expansion', type='full',
+                model={'name': name})['model']['props']
+        return str(self.cached[name].get(prop_name, 'Unknown property'))
+
+
+# Now it's stub, because all memory_backend types don't have default values
+# but this behaviour can be changed
+class QEMUMemoryBackend(QEMUObject):
+    def __init__(self, vm: QEMUMachine) -> None:
+        super().__init__(vm, 'memory-backend')
+        self.cached: Dict[str, List[Dict[str, Any]]] = {}
+
+    def get_prop(self, driver: str, prop_name: str) -> str:
+        if driver not in self.cached:
+            self.cached[driver] = self.vm.cmd('qom-list-properties',
+                                              typename=driver)
+        for prop in self.cached[driver]:
+            if prop['name'] == prop_name:
+                return str(prop.get('default-value', 'No default value'))
+
+        return 'Unknown property'
+
+
+def new_driver(vm: QEMUMachine, name: str, is_abstr: bool) -> Driver:
+    if name == 'object':
+        return QEMUObject(vm, 'object')
+    elif name == 'device':
+        return QEMUDevice(vm)
+    elif name == 'x86_64-cpu':
+        return QEMUx86CPU(vm)
+    elif name == 'memory-backend':
+        return QEMUMemoryBackend(vm)
+    else:
+        return Driver(vm, name, is_abstr)
+# End of methods definition
+
+
+class VMPropertyGetter:
+    """It implements the relationship between drivers and how to get their
+    properties"""
+    def __init__(self, vm: QEMUMachine) -> None:
+        self.drivers: Dict[str, Driver] = {}
+
+        qom_all_types = vm.cmd('qom-list-types', abstract=True)
+        self.drivers = {t['name']: new_driver(vm, t['name'],
+                                              t.get('abstract', False))
+                        for t in qom_all_types}
+
+        for t in qom_all_types:
+            drv = self.drivers[t['name']]
+            if 'parent' in t:
+                drv.parent = self.drivers[t['parent']]
+
+        for drv in self.drivers.values():
+            imps = vm.cmd('qom-list-types', implements=drv.name)
+            # only implementations inherit property getter
+            drv.set_implementations([self.drivers[imp['name']]
+                                     for imp in imps])
+
+    def get_prop(self, driver: str, prop: str) -> str:
+        # wrong driver name or disabled in config driver
+        try:
+            drv = self.drivers[driver]
+        except KeyError:
+            return 'Unavailable driver'
+
+        assert not drv.abstract
+
+        return drv.get_prop(driver, prop)
+
+    def get_implementations(self, driver: str) -> List[str]:
+        return [impl.name for impl in self.drivers[driver].implementations]
+
+
+class Machine:
+    """A short QEMU machine type description. It contains only processed
+    compat_props (properties of abstract classes are applied to its
+    implementations)
+    """
+    # raw_mt_dict - dict produced by `query-machines`
+    def __init__(self, raw_mt_dict: Dict[str, Any],
+                 qemu_drivers: VMPropertyGetter) -> None:
+        self.name = raw_mt_dict['name']
+        self.compat_props: Dict[str, Any] = {}
+        # properties are applied sequentially and can rewrite values like in
+        # QEMU. Also it has to resolve class relationships to apply appropriate
+        # values from abstract class to all implementations
+        for prop in raw_mt_dict['compat-props']:
+            driver = prop['driver']
+            try:
+                # implementation adds only itself, abstract class adds
+                #  lementation (abstract classes are uninterestiong)
+                impls = qemu_drivers.get_implementations(driver)
+                for impl in impls:
+                    if impl not in self.compat_props:
+                        self.compat_props[impl] = {}
+                    self.compat_props[impl][prop['property']] = prop['value']
+            except KeyError:
+                # QEMU doesn't know this driver thus it has to be saved
+                if driver not in self.compat_props:
+                    self.compat_props[driver] = {}
+                self.compat_props[driver][prop['property']] = prop['value']
+
+
+class Configuration():
+    """Class contains all necessary components to generate table and is used
+    to compare different binaries"""
+    def __init__(self, vm: QEMUMachine,
+                 req_mt: List[str], all_mt: bool) -> None:
+        self._vm = vm
+        self._binary = vm.binary
+        self._qemu_args = args.qemu_args.split(' ')
+
+        self._qemu_drivers = VMPropertyGetter(vm)
+        self.req_mt = get_req_mt(self._qemu_drivers, vm, req_mt, all_mt)
+
+    def get_implementations(self, driver_name: str) -> List[str]:
+        return self._qemu_drivers.get_implementations(driver_name)
+
+    def get_table(self, req_props: List[Tuple[str, str]]) -> pd.DataFrame:
+        table: List[pd.DataFrame] = []
+        for mt in self.req_mt:
+            name = f'{self._binary}\n{mt.name}'
+            column = []
+            for driver, prop in req_props:
+                try:
+                    # values from QEMU machine type definitions
+                    column.append(mt.compat_props[driver][prop])
+                except KeyError:
+                    # values from QEMU type definitions
+                    column.append(self._qemu_drivers.get_prop(driver, prop))
+            table.append(pd.DataFrame({name: column}))
+
+        return pd.concat(table, axis=1)
+
+
+script_desc = """Script to compare machine types (their compat_props).
+
+Examples:
+* save info about all machines:  ./scripts/compare_mt.py --all --format csv \
+--raw > table.csv
+* compare machines: ./scripts/compare_mt.py --mt pc-q35-2.12 pc-q35-3.0
+* compare binaries and machines: ./scripts/compare_mt.py --mt pc-q35-6.2 \
+pc-q35-7.0 --qemu-binary build/qemu-system-x86_64 build/qemu-exp
+  ╒════════════╤══════════════════════════╤════════════════════════════\
+╤════════════════════════════╤══════════════════╤══════════════════╕
+  │   Driver   │         Property         │  build/qemu-system-x86_64  \
+│  build/qemu-system-x86_64  │  build/qemu-exp  │  build/qemu-exp  │
+  │            │                          │         pc-q35-6.2         \
+│         pc-q35-7.0         │    pc-q35-6.2    │    pc-q35-7.0    │
+  ╞════════════╪══════════════════════════╪════════════════════════════\
+╪════════════════════════════╪══════════════════╪══════════════════╡
+  │  PIIX4_PM  │ x-not-migrate-acpi-index │            True            \
+│           False            │      False       │      False       │
+  ├────────────┼──────────────────────────┼────────────────────────────\
+┼────────────────────────────┼──────────────────┼──────────────────┤
+  │ virtio-mem │  unplugged-inaccessible  │           False            \
+│            auto            │      False       │       auto       │
+  ╘════════════╧══════════════════════════╧════════════════════════════\
+╧════════════════════════════╧══════════════════╧══════════════════╛
+
+If a property from QEMU machine defintion applies to an abstract class (e.g. \
+x86_64-cpu) this script will compare all implementations of this class.
+
+"Unavailable method" - means that this script doesn't know how to get \
+default values of the driver. To add method use the construction described \
+at the top of the script.
+"Unavailable driver" - means that this script doesn't know this driver. \
+For instance, this can happen if you configure QEMU without this device or \
+if machine type definition has error.
+"No default value" - means that the appropriate method can't get the default \
+value and most likely that this property doesn't have it.
+"Unknown property" - means that the appropriate method can't find property \
+with this name."""
+
+
+def parse_args() -> Namespace:
+    parser = ArgumentParser(formatter_class=RawTextHelpFormatter,
+                            description=script_desc)
+    parser.add_argument('--format', choices=['human-readable', 'json', 'csv'],
+                        default='human-readable',
+                        help='returns table in json format')
+    parser.add_argument('--raw', action='store_true',
+                        help='prints ALL defined properties without value '
+                             'transformation. By default, only rows '
+                             'with different values will be printed and '
+                             'values will be transformed(e.g. "on" -> True)')
+    parser.add_argument('--qemu-args', default=default_qemu_args,
+                        help='command line to start qemu. '
+                             f'Default: "{default_qemu_args}"')
+    parser.add_argument('--qemu-binary', nargs="*", type=str,
+                        default=[default_qemu_binary],
+                        help='list of qemu binaries that will be compared. '
+                             f'Deafult: {default_qemu_binary}')
+
+    mt_args_group = parser.add_mutually_exclusive_group()
+    mt_args_group.add_argument('--all', action='store_true',
+                               help='prints all available machine types (list '
+                                    'of machine types will be ignored)')
+    mt_args_group.add_argument('--mt', nargs="*", type=str,
+                               help='list of Machine Types '
+                                    'that will be compared')
+
+    return parser.parse_args()
+
+
+def mt_comp(mt: Machine) -> Tuple[str, int, int, int]:
+    """Function to compare and sort machine by names.
+    It returns socket_name, major version, minor version, revision"""
+    # none, microvm, x-remote and etc.
+    if '-' not in mt.name or '.' not in mt.name:
+        return mt.name, 0, 0, 0
+
+    socket, ver = mt.name.rsplit('-', 1)
+    ver_list = list(map(int, ver.split('.', 2)))
+    ver_list += [0] * (3 - len(ver_list))
+    return socket, ver_list[0], ver_list[1], ver_list[2]
+
+
+def get_mt_definitions(qemu_drivers: VMPropertyGetter,
+                       vm: QEMUMachine) -> List[Machine]:
+    """Constructs list of machine definitions (primarily compat_props) via
+    info from QEMU"""
+    raw_mt_defs = vm.cmd('query-machines', compat_props=True)
+    mt_defs = []
+    for raw_mt in raw_mt_defs:
+        mt_defs.append(Machine(raw_mt, qemu_drivers))
+
+    mt_defs.sort(key=mt_comp)
+    return mt_defs
+
+
+def get_req_mt(qemu_drivers: VMPropertyGetter, vm: QEMUMachine,
+               req_mt: Optional[List[str]], all_mt: bool) -> List[Machine]:
+    """Returns list of requested by user machines"""
+    mt_defs = get_mt_definitions(qemu_drivers, vm)
+    if all_mt:
+        return mt_defs
+
+    if req_mt is None:
+        print('Enter machine types for comparision')
+        exit(0)
+
+    matched_mt = []
+    for mt in mt_defs:
+        if mt.name in req_mt:
+            matched_mt.append(mt)
+
+    return matched_mt
+
+
+def get_affected_props(configs: List[Configuration]) -> Generator[Tuple[str,
+                                                                        str],
+                                                                  None, None]:
+    """Helps to go through all affected in machine definitions drivers
+    and properties"""
+    driver_props: Dict[str, Set[Any]] = {}
+    for config in configs:
+        for mt in config.req_mt:
+            compat_props = mt.compat_props
+            for driver, prop in compat_props.items():
+                if driver not in driver_props:
+                    driver_props[driver] = set()
+                driver_props[driver].update(prop.keys())
+
+    for driver, props in sorted(driver_props.items()):
+        for prop in sorted(props):
+            yield driver, prop
+
+
+def transform_value(value: str) -> Union[str, bool]:
+    true_list = ['true', 'on']
+    false_list = ['false', 'off']
+
+    out = value.lower()
+
+    if out in true_list:
+        return True
+
+    if out in false_list:
+        return False
+
+    return value
+
+
+def simplify_table(table: pd.DataFrame) -> pd.DataFrame:
+    """transforms values to make it easier to compare it and drops rows
+    with the same values for all columns"""
+
+    table = table.map(transform_value)
+
+    return table[~table.iloc[:, 3:].eq(table.iloc[:, 2], axis=0).all(axis=1)]
+
+
+# constructs table in the format:
+#
+# Driver  | Property  | binary1  | binary1  | ...
+#         |           | machine1 | machine2 | ...
+# ------------------------------------------------------ ...
+# driver1 | property1 |  value1  |  value2  | ...
+# driver1 | property2 |  value3  |  value4  | ...
+# driver2 | property3 |  value5  |  value6  | ...
+#   ...   |    ...    |   ...    |   ...    | ...
+#
+def fill_prop_table(configs: List[Configuration],
+                    is_raw: bool) -> pd.DataFrame:
+    req_props = list(get_affected_props(configs))
+    if not req_props:
+        print('No drivers to compare. Check machine names')
+        exit(0)
+
+    driver_col, prop_col = tuple(zip(*req_props))
+    table = [pd.DataFrame({'Driver': driver_col}),
+             pd.DataFrame({'Property': prop_col})]
+
+    table.extend([config.get_table(req_props) for config in configs])
+
+    df_table = pd.concat(table, axis=1)
+
+    if is_raw:
+        return df_table
+
+    return simplify_table(df_table)
+
+
+def print_table(table: pd.DataFrame, table_format: str) -> None:
+    if table_format == 'json':
+        print(comp_table.to_json())
+    elif table_format == 'csv':
+        print(comp_table.to_csv())
+    else:
+        print(comp_table.to_markdown(index=False, stralign='center',
+                                     colalign=('center',), headers='keys',
+                                     tablefmt='fancy_grid',
+                                     disable_numparse=True))
+
+
+if __name__ == '__main__':
+    args = parse_args()
+    with ExitStack() as stack:
+        vms = [stack.enter_context(QEMUMachine(binary=binary, qmp_timer=15,
+               args=args.qemu_args.split(' '))) for binary in args.qemu_binary]
+
+        configurations = []
+        for vm in vms:
+            vm.launch()
+            configurations.append(Configuration(vm, args.mt, args.all))
+
+        comp_table = fill_prop_table(configurations, args.raw)
+        if not comp_table.empty:
+            print_table(comp_table, args.format)
-- 
2.34.1

Re: [PATCH v6 4/4] scripts: add script to compare compatible properties

[Markus Armbruster]

Review, anyone?

Maksim Davydov <davydov-max@yandex-team.ru> writes:

> This script runs QEMU to obtain compat_props of machines and default
> values of different types of drivers to produce comparison table. This
> table can be used to compare machine types to choose the most suitable
> machine or compare binaries to be sure that migration to the newer version
> will save all device properties. Also the json or csv format of this
> table can be used to check does a new machine affect the previous ones by
> comparing tables with and without the new machine.
>
> Default values (that will be used without machine compat_props) of
> properties are needed to fill "holes" in the table (one machine has
> the property but another machine not. For instance, 2.12 machine has
> `{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
> 3.1 machine doesn't have it. Thus, to compare these machines we need to
> get unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 machine. These
> unknown values in the table are called "holes". To get values for these
> "holes" the script uses list of appropriate methods.)
>
> Notes:
> * Some init values from the devices can't be available like properties
>   from virtio-9p when configure has --disable-virtfs. This situations will
>   be seen in the table as "unavailable driver".
> * Default values can be obtained in an unobvious way, like x86 features.
>   If the script doesn't know how to get property default value to compare
>   one machine with another it fills "holes" with "unavailable method". This
>   is done because script uses whitelist model to get default values of
>   different types. It means that the method that can't be applied to a new
>   type that can crash this script. It is better to get an "unavailable
>   driver" when creating a new machine with new compatible properties than
>   to break this script. So it turns out a more stable and generic script.
> * If the default value can't be obtained because this property doesn't
>   exist or because this property can't have default value, appropriate
>   "hole" will be filled by "unknown property" or "no default value"
> * If the property is applied to the abstract class, the script collects
>   default values from all child classes (set of default values)
> * Raw table (--raw flag) should be used with json/csv parameters for
>   scripts and etc. Human-readable (default) format contains transformed
>   and simplified values and it doesn't contain lines with the same values
>   in columns
>
> Example:
> ./scripts/compare_mt.py --mt pc-q35-6.2 pc-q35-7.1
> ╒══════════════════╤══════════════════════════╤════════════════════════════╤════════════════════════════╕
> │      Driver      │         Property         │  build/qemu-system-x86_64  │  build/qemu-system-x86_64  │
> │                  │                          │         pc-q35-6.2         │         pc-q35-7.1         │
> ╞══════════════════╪══════════════════════════╪════════════════════════════╪════════════════════════════╡
> │     PIIX4_PM     │ x-not-migrate-acpi-index │            True            │           False            │
> ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
> │ arm-gicv3-common │     force-8-bit-prio     │            True            │     unavailable driver     │
> ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
> │     nvme-ns      │      eui64-default       │            True            │           False            │
> ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
> │    virtio-mem    │  unplugged-inaccessible  │           False            │            auto            │
> ╘══════════════════╧══════════════════════════╧════════════════════════════╧════════════════════════════╛
>
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> Co-developed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  scripts/compare_mt.py | 484 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 484 insertions(+)
>  create mode 100755 scripts/compare_mt.py
>
> diff --git a/scripts/compare_mt.py b/scripts/compare_mt.py
> new file mode 100755
> index 0000000000..685bd9a336
> --- /dev/null
> +++ b/scripts/compare_mt.py
> @@ -0,0 +1,484 @@
> +#!/usr/bin/env python3
> +#
> +# Script to compare machine type compatible properties (include/hw/boards.h).
> +# compat_props are applied to the driver during initialization to change
> +# default values, for instance, to maintain compatibility.
> +# This script constructs table with machines and values of their compat_props
> +# to compare and to find places for improvements or places with bugs. If
> +# during the comparison, some machine type doesn't have a property (it is in
> +# the comparison table because another machine type has it), then the
> +# appropriate method will be used to obtain the default value of this driver
> +# property via qmp command (e.g. query-cpu-model-expansion for x86_64-cpu).
> +# These methods are defined below in qemu_property_methods.
> +#
> +# Copyright (c) Yandex Technologies LLC, 2023
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, see <http://www.gnu.org/licenses/>.
> +
> +import sys
> +from os import path
> +from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
> +import pandas as pd
> +from contextlib import ExitStack
> +from typing import Optional, List, Dict, Generator, Tuple, Union, Any, Set
> +
> +try:
> +    qemu_dir = path.abspath(path.dirname(path.dirname(__file__)))
> +    sys.path.append(path.join(qemu_dir, 'python'))
> +    from qemu.machine import QEMUMachine
> +except ModuleNotFoundError as exc:
> +    print(f"Module '{exc.name}' not found.")
> +    print("Try export PYTHONPATH=top-qemu-dir/python or run from top-qemu-dir")
> +    sys.exit(1)
> +
> +
> +default_qemu_args = '-enable-kvm -machine none'
> +default_qemu_binary = 'build/qemu-system-x86_64'
> +
> +
> +# Methods for gettig the right values of drivers properties
> +#
> +# Use these methods as a 'whitelist' and add entries only if necessary. It's
> +# important to be stable and predictable in analysis and tests.
> +# Be careful:
> +# * Class must be inherited from 'QEMUObject' and used in new_driver()
> +# * Class has to implement get_prop method in order to get values
> +# * Specialization always wins (with the given classes for 'device' and
> +#   'x86_64-cpu', method of 'x86_64-cpu' will be used for '486-x86_64-cpu')
> +
> +class Driver():
> +    def __init__(self, vm: QEMUMachine, name: str, abstract: bool) -> None:
> +        self.vm = vm
> +        self.name = name
> +        self.abstract = abstract
> +        self.parent: Optional[Driver] = None
> +        self.property_getter: Optional[Driver] = None
> +
> +    def get_prop(self, driver: str, prop: str) -> str:
> +        if self.property_getter:
> +            return self.property_getter.get_prop(driver, prop)
> +        else:
> +            return 'Unavailable method'
> +
> +    def is_child_of(self, parent: 'Driver') -> bool:
> +        """Checks whether self is (recursive) child of @parent"""
> +        cur_parent = self.parent
> +        while cur_parent:
> +            if cur_parent is parent:
> +                return True
> +            cur_parent = cur_parent.parent
> +
> +        return False
> +
> +    def set_implementations(self, implementations: List['Driver']) -> None:
> +        self.implementations = implementations
> +
> +
> +class QEMUObject(Driver):
> +    def __init__(self, vm: QEMUMachine, name: str) -> None:
> +        super().__init__(vm, name, True)
> +
> +    def set_implementations(self, implementations: List[Driver]) -> None:
> +        self.implementations = implementations
> +
> +        # each implementation of the abstract driver has to use property getter
> +        # of this abstract driver unless it has specialization. (e.g. having
> +        # 'device' and 'x86_64-cpu', property getter of 'x86_64-cpu' will be
> +        # used for '486-x86_64-cpu')
> +        for impl in implementations:
> +            if not impl.property_getter or\
> +                    self.is_child_of(impl.property_getter):
> +                impl.property_getter = self
> +
> +
> +class QEMUDevice(QEMUObject):
> +    def __init__(self, vm: QEMUMachine) -> None:
> +        super().__init__(vm, 'device')
> +        self.cached: Dict[str, List[Dict[str, Any]]] = {}
> +
> +    def get_prop(self, driver: str, prop_name: str) -> str:
> +        if driver not in self.cached:
> +            self.cached[driver] = self.vm.cmd('device-list-properties',
> +                                              typename=driver)
> +        for prop in self.cached[driver]:
> +            if prop['name'] == prop_name:
> +                return str(prop.get('default-value', 'No default value'))
> +
> +        return 'Unknown property'
> +
> +
> +class QEMUx86CPU(QEMUObject):
> +    def __init__(self, vm: QEMUMachine) -> None:
> +        super().__init__(vm, 'x86_64-cpu')
> +        self.cached: Dict[str, Dict[str, Any]] = {}
> +
> +    def get_prop(self, driver: str, prop_name: str) -> str:
> +        if not driver.endswith('-x86_64-cpu'):
> +            return 'Wrong x86_64-cpu name'
> +
> +        # crop last 11 chars '-x86_64-cpu'
> +        name = driver[:-11]
> +        if name not in self.cached:
> +            self.cached[name] = self.vm.cmd(
> +                'query-cpu-model-expansion', type='full',
> +                model={'name': name})['model']['props']
> +        return str(self.cached[name].get(prop_name, 'Unknown property'))
> +
> +
> +# Now it's stub, because all memory_backend types don't have default values
> +# but this behaviour can be changed
> +class QEMUMemoryBackend(QEMUObject):
> +    def __init__(self, vm: QEMUMachine) -> None:
> +        super().__init__(vm, 'memory-backend')
> +        self.cached: Dict[str, List[Dict[str, Any]]] = {}
> +
> +    def get_prop(self, driver: str, prop_name: str) -> str:
> +        if driver not in self.cached:
> +            self.cached[driver] = self.vm.cmd('qom-list-properties',
> +                                              typename=driver)
> +        for prop in self.cached[driver]:
> +            if prop['name'] == prop_name:
> +                return str(prop.get('default-value', 'No default value'))
> +
> +        return 'Unknown property'
> +
> +
> +def new_driver(vm: QEMUMachine, name: str, is_abstr: bool) -> Driver:
> +    if name == 'object':
> +        return QEMUObject(vm, 'object')
> +    elif name == 'device':
> +        return QEMUDevice(vm)
> +    elif name == 'x86_64-cpu':
> +        return QEMUx86CPU(vm)
> +    elif name == 'memory-backend':
> +        return QEMUMemoryBackend(vm)
> +    else:
> +        return Driver(vm, name, is_abstr)
> +# End of methods definition
> +
> +
> +class VMPropertyGetter:
> +    """It implements the relationship between drivers and how to get their
> +    properties"""
> +    def __init__(self, vm: QEMUMachine) -> None:
> +        self.drivers: Dict[str, Driver] = {}
> +
> +        qom_all_types = vm.cmd('qom-list-types', abstract=True)
> +        self.drivers = {t['name']: new_driver(vm, t['name'],
> +                                              t.get('abstract', False))
> +                        for t in qom_all_types}
> +
> +        for t in qom_all_types:
> +            drv = self.drivers[t['name']]
> +            if 'parent' in t:
> +                drv.parent = self.drivers[t['parent']]
> +
> +        for drv in self.drivers.values():
> +            imps = vm.cmd('qom-list-types', implements=drv.name)
> +            # only implementations inherit property getter
> +            drv.set_implementations([self.drivers[imp['name']]
> +                                     for imp in imps])
> +
> +    def get_prop(self, driver: str, prop: str) -> str:
> +        # wrong driver name or disabled in config driver
> +        try:
> +            drv = self.drivers[driver]
> +        except KeyError:
> +            return 'Unavailable driver'
> +
> +        assert not drv.abstract
> +
> +        return drv.get_prop(driver, prop)
> +
> +    def get_implementations(self, driver: str) -> List[str]:
> +        return [impl.name for impl in self.drivers[driver].implementations]
> +
> +
> +class Machine:
> +    """A short QEMU machine type description. It contains only processed
> +    compat_props (properties of abstract classes are applied to its
> +    implementations)
> +    """
> +    # raw_mt_dict - dict produced by `query-machines`
> +    def __init__(self, raw_mt_dict: Dict[str, Any],
> +                 qemu_drivers: VMPropertyGetter) -> None:
> +        self.name = raw_mt_dict['name']
> +        self.compat_props: Dict[str, Any] = {}
> +        # properties are applied sequentially and can rewrite values like in
> +        # QEMU. Also it has to resolve class relationships to apply appropriate
> +        # values from abstract class to all implementations
> +        for prop in raw_mt_dict['compat-props']:
> +            driver = prop['driver']
> +            try:
> +                # implementation adds only itself, abstract class adds
> +                #  lementation (abstract classes are uninterestiong)
> +                impls = qemu_drivers.get_implementations(driver)
> +                for impl in impls:
> +                    if impl not in self.compat_props:
> +                        self.compat_props[impl] = {}
> +                    self.compat_props[impl][prop['property']] = prop['value']
> +            except KeyError:
> +                # QEMU doesn't know this driver thus it has to be saved
> +                if driver not in self.compat_props:
> +                    self.compat_props[driver] = {}
> +                self.compat_props[driver][prop['property']] = prop['value']
> +
> +
> +class Configuration():
> +    """Class contains all necessary components to generate table and is used
> +    to compare different binaries"""
> +    def __init__(self, vm: QEMUMachine,
> +                 req_mt: List[str], all_mt: bool) -> None:
> +        self._vm = vm
> +        self._binary = vm.binary
> +        self._qemu_args = args.qemu_args.split(' ')
> +
> +        self._qemu_drivers = VMPropertyGetter(vm)
> +        self.req_mt = get_req_mt(self._qemu_drivers, vm, req_mt, all_mt)
> +
> +    def get_implementations(self, driver_name: str) -> List[str]:
> +        return self._qemu_drivers.get_implementations(driver_name)
> +
> +    def get_table(self, req_props: List[Tuple[str, str]]) -> pd.DataFrame:
> +        table: List[pd.DataFrame] = []
> +        for mt in self.req_mt:
> +            name = f'{self._binary}\n{mt.name}'
> +            column = []
> +            for driver, prop in req_props:
> +                try:
> +                    # values from QEMU machine type definitions
> +                    column.append(mt.compat_props[driver][prop])
> +                except KeyError:
> +                    # values from QEMU type definitions
> +                    column.append(self._qemu_drivers.get_prop(driver, prop))
> +            table.append(pd.DataFrame({name: column}))
> +
> +        return pd.concat(table, axis=1)
> +
> +
> +script_desc = """Script to compare machine types (their compat_props).
> +
> +Examples:
> +* save info about all machines:  ./scripts/compare_mt.py --all --format csv \
> +--raw > table.csv
> +* compare machines: ./scripts/compare_mt.py --mt pc-q35-2.12 pc-q35-3.0
> +* compare binaries and machines: ./scripts/compare_mt.py --mt pc-q35-6.2 \
> +pc-q35-7.0 --qemu-binary build/qemu-system-x86_64 build/qemu-exp
> +  ╒════════════╤══════════════════════════╤════════════════════════════\
> +╤════════════════════════════╤══════════════════╤══════════════════╕
> +  │   Driver   │         Property         │  build/qemu-system-x86_64  \
> +│  build/qemu-system-x86_64  │  build/qemu-exp  │  build/qemu-exp  │
> +  │            │                          │         pc-q35-6.2         \
> +│         pc-q35-7.0         │    pc-q35-6.2    │    pc-q35-7.0    │
> +  ╞════════════╪══════════════════════════╪════════════════════════════\
> +╪════════════════════════════╪══════════════════╪══════════════════╡
> +  │  PIIX4_PM  │ x-not-migrate-acpi-index │            True            \
> +│           False            │      False       │      False       │
> +  ├────────────┼──────────────────────────┼────────────────────────────\
> +┼────────────────────────────┼──────────────────┼──────────────────┤
> +  │ virtio-mem │  unplugged-inaccessible  │           False            \
> +│            auto            │      False       │       auto       │
> +  ╘════════════╧══════════════════════════╧════════════════════════════\
> +╧════════════════════════════╧══════════════════╧══════════════════╛
> +
> +If a property from QEMU machine defintion applies to an abstract class (e.g. \
> +x86_64-cpu) this script will compare all implementations of this class.
> +
> +"Unavailable method" - means that this script doesn't know how to get \
> +default values of the driver. To add method use the construction described \
> +at the top of the script.
> +"Unavailable driver" - means that this script doesn't know this driver. \
> +For instance, this can happen if you configure QEMU without this device or \
> +if machine type definition has error.
> +"No default value" - means that the appropriate method can't get the default \
> +value and most likely that this property doesn't have it.
> +"Unknown property" - means that the appropriate method can't find property \
> +with this name."""
> +
> +
> +def parse_args() -> Namespace:
> +    parser = ArgumentParser(formatter_class=RawTextHelpFormatter,
> +                            description=script_desc)
> +    parser.add_argument('--format', choices=['human-readable', 'json', 'csv'],
> +                        default='human-readable',
> +                        help='returns table in json format')
> +    parser.add_argument('--raw', action='store_true',
> +                        help='prints ALL defined properties without value '
> +                             'transformation. By default, only rows '
> +                             'with different values will be printed and '
> +                             'values will be transformed(e.g. "on" -> True)')
> +    parser.add_argument('--qemu-args', default=default_qemu_args,
> +                        help='command line to start qemu. '
> +                             f'Default: "{default_qemu_args}"')
> +    parser.add_argument('--qemu-binary', nargs="*", type=str,
> +                        default=[default_qemu_binary],
> +                        help='list of qemu binaries that will be compared. '
> +                             f'Deafult: {default_qemu_binary}')
> +
> +    mt_args_group = parser.add_mutually_exclusive_group()
> +    mt_args_group.add_argument('--all', action='store_true',
> +                               help='prints all available machine types (list '
> +                                    'of machine types will be ignored)')
> +    mt_args_group.add_argument('--mt', nargs="*", type=str,
> +                               help='list of Machine Types '
> +                                    'that will be compared')
> +
> +    return parser.parse_args()
> +
> +
> +def mt_comp(mt: Machine) -> Tuple[str, int, int, int]:
> +    """Function to compare and sort machine by names.
> +    It returns socket_name, major version, minor version, revision"""
> +    # none, microvm, x-remote and etc.
> +    if '-' not in mt.name or '.' not in mt.name:
> +        return mt.name, 0, 0, 0
> +
> +    socket, ver = mt.name.rsplit('-', 1)
> +    ver_list = list(map(int, ver.split('.', 2)))
> +    ver_list += [0] * (3 - len(ver_list))
> +    return socket, ver_list[0], ver_list[1], ver_list[2]
> +
> +
> +def get_mt_definitions(qemu_drivers: VMPropertyGetter,
> +                       vm: QEMUMachine) -> List[Machine]:
> +    """Constructs list of machine definitions (primarily compat_props) via
> +    info from QEMU"""
> +    raw_mt_defs = vm.cmd('query-machines', compat_props=True)
> +    mt_defs = []
> +    for raw_mt in raw_mt_defs:
> +        mt_defs.append(Machine(raw_mt, qemu_drivers))
> +
> +    mt_defs.sort(key=mt_comp)
> +    return mt_defs
> +
> +
> +def get_req_mt(qemu_drivers: VMPropertyGetter, vm: QEMUMachine,
> +               req_mt: Optional[List[str]], all_mt: bool) -> List[Machine]:
> +    """Returns list of requested by user machines"""
> +    mt_defs = get_mt_definitions(qemu_drivers, vm)
> +    if all_mt:
> +        return mt_defs
> +
> +    if req_mt is None:
> +        print('Enter machine types for comparision')
> +        exit(0)
> +
> +    matched_mt = []
> +    for mt in mt_defs:
> +        if mt.name in req_mt:
> +            matched_mt.append(mt)
> +
> +    return matched_mt
> +
> +
> +def get_affected_props(configs: List[Configuration]) -> Generator[Tuple[str,
> +                                                                        str],
> +                                                                  None, None]:
> +    """Helps to go through all affected in machine definitions drivers
> +    and properties"""
> +    driver_props: Dict[str, Set[Any]] = {}
> +    for config in configs:
> +        for mt in config.req_mt:
> +            compat_props = mt.compat_props
> +            for driver, prop in compat_props.items():
> +                if driver not in driver_props:
> +                    driver_props[driver] = set()
> +                driver_props[driver].update(prop.keys())
> +
> +    for driver, props in sorted(driver_props.items()):
> +        for prop in sorted(props):
> +            yield driver, prop
> +
> +
> +def transform_value(value: str) -> Union[str, bool]:
> +    true_list = ['true', 'on']
> +    false_list = ['false', 'off']
> +
> +    out = value.lower()
> +
> +    if out in true_list:
> +        return True
> +
> +    if out in false_list:
> +        return False
> +
> +    return value
> +
> +
> +def simplify_table(table: pd.DataFrame) -> pd.DataFrame:
> +    """transforms values to make it easier to compare it and drops rows
> +    with the same values for all columns"""
> +
> +    table = table.map(transform_value)
> +
> +    return table[~table.iloc[:, 3:].eq(table.iloc[:, 2], axis=0).all(axis=1)]
> +
> +
> +# constructs table in the format:
> +#
> +# Driver  | Property  | binary1  | binary1  | ...
> +#         |           | machine1 | machine2 | ...
> +# ------------------------------------------------------ ...
> +# driver1 | property1 |  value1  |  value2  | ...
> +# driver1 | property2 |  value3  |  value4  | ...
> +# driver2 | property3 |  value5  |  value6  | ...
> +#   ...   |    ...    |   ...    |   ...    | ...
> +#
> +def fill_prop_table(configs: List[Configuration],
> +                    is_raw: bool) -> pd.DataFrame:
> +    req_props = list(get_affected_props(configs))
> +    if not req_props:
> +        print('No drivers to compare. Check machine names')
> +        exit(0)
> +
> +    driver_col, prop_col = tuple(zip(*req_props))
> +    table = [pd.DataFrame({'Driver': driver_col}),
> +             pd.DataFrame({'Property': prop_col})]
> +
> +    table.extend([config.get_table(req_props) for config in configs])
> +
> +    df_table = pd.concat(table, axis=1)
> +
> +    if is_raw:
> +        return df_table
> +
> +    return simplify_table(df_table)
> +
> +
> +def print_table(table: pd.DataFrame, table_format: str) -> None:
> +    if table_format == 'json':
> +        print(comp_table.to_json())
> +    elif table_format == 'csv':
> +        print(comp_table.to_csv())
> +    else:
> +        print(comp_table.to_markdown(index=False, stralign='center',
> +                                     colalign=('center',), headers='keys',
> +                                     tablefmt='fancy_grid',
> +                                     disable_numparse=True))
> +
> +
> +if __name__ == '__main__':
> +    args = parse_args()
> +    with ExitStack() as stack:
> +        vms = [stack.enter_context(QEMUMachine(binary=binary, qmp_timer=15,
> +               args=args.qemu_args.split(' '))) for binary in args.qemu_binary]
> +
> +        configurations = []
> +        for vm in vms:
> +            vm.launch()
> +            configurations.append(Configuration(vm, args.mt, args.all))
> +
> +        comp_table = fill_prop_table(configurations, args.raw)
> +        if not comp_table.empty:
> +            print_table(comp_table, args.format)

Re: [PATCH v6 4/4] scripts: add script to compare compatible properties

[Maksim Davydov]

On 12/1/23 12:51, Markus Armbruster wrote:
> Review, anyone?

Only Vladimir

>
> Maksim Davydov <davydov-max@yandex-team.ru> writes:
>
>> This script runs QEMU to obtain compat_props of machines and default
>> values of different types of drivers to produce comparison table. This
>> table can be used to compare machine types to choose the most suitable
>> machine or compare binaries to be sure that migration to the newer version
>> will save all device properties. Also the json or csv format of this
>> table can be used to check does a new machine affect the previous ones by
>> comparing tables with and without the new machine.
>>
>> Default values (that will be used without machine compat_props) of
>> properties are needed to fill "holes" in the table (one machine has
>> the property but another machine not. For instance, 2.12 machine has
>> `{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
>> 3.1 machine doesn't have it. Thus, to compare these machines we need to
>> get unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 machine. These
>> unknown values in the table are called "holes". To get values for these
>> "holes" the script uses list of appropriate methods.)
>>
>> Notes:
>> * Some init values from the devices can't be available like properties
>>    from virtio-9p when configure has --disable-virtfs. This situations will
>>    be seen in the table as "unavailable driver".
>> * Default values can be obtained in an unobvious way, like x86 features.
>>    If the script doesn't know how to get property default value to compare
>>    one machine with another it fills "holes" with "unavailable method". This
>>    is done because script uses whitelist model to get default values of
>>    different types. It means that the method that can't be applied to a new
>>    type that can crash this script. It is better to get an "unavailable
>>    driver" when creating a new machine with new compatible properties than
>>    to break this script. So it turns out a more stable and generic script.
>> * If the default value can't be obtained because this property doesn't
>>    exist or because this property can't have default value, appropriate
>>    "hole" will be filled by "unknown property" or "no default value"
>> * If the property is applied to the abstract class, the script collects
>>    default values from all child classes (set of default values)
>> * Raw table (--raw flag) should be used with json/csv parameters for
>>    scripts and etc. Human-readable (default) format contains transformed
>>    and simplified values and it doesn't contain lines with the same values
>>    in columns
>>
>> Example:
>> ./scripts/compare_mt.py --mt pc-q35-6.2 pc-q35-7.1
>> ╒══════════════════╤══════════════════════════╤════════════════════════════╤════════════════════════════╕
>> │      Driver      │         Property         │  build/qemu-system-x86_64  │  build/qemu-system-x86_64  │
>> │                  │                          │         pc-q35-6.2         │         pc-q35-7.1         │
>> ╞══════════════════╪══════════════════════════╪════════════════════════════╪════════════════════════════╡
>> │     PIIX4_PM     │ x-not-migrate-acpi-index │            True            │           False            │
>> ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
>> │ arm-gicv3-common │     force-8-bit-prio     │            True            │     unavailable driver     │
>> ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
>> │     nvme-ns      │      eui64-default       │            True            │           False            │
>> ├──────────────────┼──────────────────────────┼────────────────────────────┼────────────────────────────┤
>> │    virtio-mem    │  unplugged-inaccessible  │           False            │            auto            │
>> ╘══════════════════╧══════════════════════════╧════════════════════════════╧════════════════════════════╛
>>
>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
>> Co-developed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   scripts/compare_mt.py | 484 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 484 insertions(+)
>>   create mode 100755 scripts/compare_mt.py
>>
>> diff --git a/scripts/compare_mt.py b/scripts/compare_mt.py
>> new file mode 100755
>> index 0000000000..685bd9a336
>> --- /dev/null
>> +++ b/scripts/compare_mt.py
>> @@ -0,0 +1,484 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Script to compare machine type compatible properties (include/hw/boards.h).
>> +# compat_props are applied to the driver during initialization to change
>> +# default values, for instance, to maintain compatibility.
>> +# This script constructs table with machines and values of their compat_props
>> +# to compare and to find places for improvements or places with bugs. If
>> +# during the comparison, some machine type doesn't have a property (it is in
>> +# the comparison table because another machine type has it), then the
>> +# appropriate method will be used to obtain the default value of this driver
>> +# property via qmp command (e.g. query-cpu-model-expansion for x86_64-cpu).
>> +# These methods are defined below in qemu_property_methods.
>> +#
>> +# Copyright (c) Yandex Technologies LLC, 2023
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, see <http://www.gnu.org/licenses/>.
>> +
>> +import sys
>> +from os import path
>> +from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
>> +import pandas as pd
>> +from contextlib import ExitStack
>> +from typing import Optional, List, Dict, Generator, Tuple, Union, Any, Set
>> +
>> +try:
>> +    qemu_dir = path.abspath(path.dirname(path.dirname(__file__)))
>> +    sys.path.append(path.join(qemu_dir, 'python'))
>> +    from qemu.machine import QEMUMachine
>> +except ModuleNotFoundError as exc:
>> +    print(f"Module '{exc.name}' not found.")
>> +    print("Try export PYTHONPATH=top-qemu-dir/python or run from top-qemu-dir")
>> +    sys.exit(1)
>> +
>> +
>> +default_qemu_args = '-enable-kvm -machine none'
>> +default_qemu_binary = 'build/qemu-system-x86_64'
>> +
>> +
>> +# Methods for gettig the right values of drivers properties
>> +#
>> +# Use these methods as a 'whitelist' and add entries only if necessary. It's
>> +# important to be stable and predictable in analysis and tests.
>> +# Be careful:
>> +# * Class must be inherited from 'QEMUObject' and used in new_driver()
>> +# * Class has to implement get_prop method in order to get values
>> +# * Specialization always wins (with the given classes for 'device' and
>> +#   'x86_64-cpu', method of 'x86_64-cpu' will be used for '486-x86_64-cpu')
>> +
>> +class Driver():
>> +    def __init__(self, vm: QEMUMachine, name: str, abstract: bool) -> None:
>> +        self.vm = vm
>> +        self.name = name
>> +        self.abstract = abstract
>> +        self.parent: Optional[Driver] = None
>> +        self.property_getter: Optional[Driver] = None
>> +
>> +    def get_prop(self, driver: str, prop: str) -> str:
>> +        if self.property_getter:
>> +            return self.property_getter.get_prop(driver, prop)
>> +        else:
>> +            return 'Unavailable method'
>> +
>> +    def is_child_of(self, parent: 'Driver') -> bool:
>> +        """Checks whether self is (recursive) child of @parent"""
>> +        cur_parent = self.parent
>> +        while cur_parent:
>> +            if cur_parent is parent:
>> +                return True
>> +            cur_parent = cur_parent.parent
>> +
>> +        return False
>> +
>> +    def set_implementations(self, implementations: List['Driver']) -> None:
>> +        self.implementations = implementations
>> +
>> +
>> +class QEMUObject(Driver):
>> +    def __init__(self, vm: QEMUMachine, name: str) -> None:
>> +        super().__init__(vm, name, True)
>> +
>> +    def set_implementations(self, implementations: List[Driver]) -> None:
>> +        self.implementations = implementations
>> +
>> +        # each implementation of the abstract driver has to use property getter
>> +        # of this abstract driver unless it has specialization. (e.g. having
>> +        # 'device' and 'x86_64-cpu', property getter of 'x86_64-cpu' will be
>> +        # used for '486-x86_64-cpu')
>> +        for impl in implementations:
>> +            if not impl.property_getter or\
>> +                    self.is_child_of(impl.property_getter):
>> +                impl.property_getter = self
>> +
>> +
>> +class QEMUDevice(QEMUObject):
>> +    def __init__(self, vm: QEMUMachine) -> None:
>> +        super().__init__(vm, 'device')
>> +        self.cached: Dict[str, List[Dict[str, Any]]] = {}
>> +
>> +    def get_prop(self, driver: str, prop_name: str) -> str:
>> +        if driver not in self.cached:
>> +            self.cached[driver] = self.vm.cmd('device-list-properties',
>> +                                              typename=driver)
>> +        for prop in self.cached[driver]:
>> +            if prop['name'] == prop_name:
>> +                return str(prop.get('default-value', 'No default value'))
>> +
>> +        return 'Unknown property'
>> +
>> +
>> +class QEMUx86CPU(QEMUObject):
>> +    def __init__(self, vm: QEMUMachine) -> None:
>> +        super().__init__(vm, 'x86_64-cpu')
>> +        self.cached: Dict[str, Dict[str, Any]] = {}
>> +
>> +    def get_prop(self, driver: str, prop_name: str) -> str:
>> +        if not driver.endswith('-x86_64-cpu'):
>> +            return 'Wrong x86_64-cpu name'
>> +
>> +        # crop last 11 chars '-x86_64-cpu'
>> +        name = driver[:-11]
>> +        if name not in self.cached:
>> +            self.cached[name] = self.vm.cmd(
>> +                'query-cpu-model-expansion', type='full',
>> +                model={'name': name})['model']['props']
>> +        return str(self.cached[name].get(prop_name, 'Unknown property'))
>> +
>> +
>> +# Now it's stub, because all memory_backend types don't have default values
>> +# but this behaviour can be changed
>> +class QEMUMemoryBackend(QEMUObject):
>> +    def __init__(self, vm: QEMUMachine) -> None:
>> +        super().__init__(vm, 'memory-backend')
>> +        self.cached: Dict[str, List[Dict[str, Any]]] = {}
>> +
>> +    def get_prop(self, driver: str, prop_name: str) -> str:
>> +        if driver not in self.cached:
>> +            self.cached[driver] = self.vm.cmd('qom-list-properties',
>> +                                              typename=driver)
>> +        for prop in self.cached[driver]:
>> +            if prop['name'] == prop_name:
>> +                return str(prop.get('default-value', 'No default value'))
>> +
>> +        return 'Unknown property'
>> +
>> +
>> +def new_driver(vm: QEMUMachine, name: str, is_abstr: bool) -> Driver:
>> +    if name == 'object':
>> +        return QEMUObject(vm, 'object')
>> +    elif name == 'device':
>> +        return QEMUDevice(vm)
>> +    elif name == 'x86_64-cpu':
>> +        return QEMUx86CPU(vm)
>> +    elif name == 'memory-backend':
>> +        return QEMUMemoryBackend(vm)
>> +    else:
>> +        return Driver(vm, name, is_abstr)
>> +# End of methods definition
>> +
>> +
>> +class VMPropertyGetter:
>> +    """It implements the relationship between drivers and how to get their
>> +    properties"""
>> +    def __init__(self, vm: QEMUMachine) -> None:
>> +        self.drivers: Dict[str, Driver] = {}
>> +
>> +        qom_all_types = vm.cmd('qom-list-types', abstract=True)
>> +        self.drivers = {t['name']: new_driver(vm, t['name'],
>> +                                              t.get('abstract', False))
>> +                        for t in qom_all_types}
>> +
>> +        for t in qom_all_types:
>> +            drv = self.drivers[t['name']]
>> +            if 'parent' in t:
>> +                drv.parent = self.drivers[t['parent']]
>> +
>> +        for drv in self.drivers.values():
>> +            imps = vm.cmd('qom-list-types', implements=drv.name)
>> +            # only implementations inherit property getter
>> +            drv.set_implementations([self.drivers[imp['name']]
>> +                                     for imp in imps])
>> +
>> +    def get_prop(self, driver: str, prop: str) -> str:
>> +        # wrong driver name or disabled in config driver
>> +        try:
>> +            drv = self.drivers[driver]
>> +        except KeyError:
>> +            return 'Unavailable driver'
>> +
>> +        assert not drv.abstract
>> +
>> +        return drv.get_prop(driver, prop)
>> +
>> +    def get_implementations(self, driver: str) -> List[str]:
>> +        return [impl.name for impl in self.drivers[driver].implementations]
>> +
>> +
>> +class Machine:
>> +    """A short QEMU machine type description. It contains only processed
>> +    compat_props (properties of abstract classes are applied to its
>> +    implementations)
>> +    """
>> +    # raw_mt_dict - dict produced by `query-machines`
>> +    def __init__(self, raw_mt_dict: Dict[str, Any],
>> +                 qemu_drivers: VMPropertyGetter) -> None:
>> +        self.name = raw_mt_dict['name']
>> +        self.compat_props: Dict[str, Any] = {}
>> +        # properties are applied sequentially and can rewrite values like in
>> +        # QEMU. Also it has to resolve class relationships to apply appropriate
>> +        # values from abstract class to all implementations
>> +        for prop in raw_mt_dict['compat-props']:
>> +            driver = prop['driver']
>> +            try:
>> +                # implementation adds only itself, abstract class adds
>> +                #  lementation (abstract classes are uninterestiong)
>> +                impls = qemu_drivers.get_implementations(driver)
>> +                for impl in impls:
>> +                    if impl not in self.compat_props:
>> +                        self.compat_props[impl] = {}
>> +                    self.compat_props[impl][prop['property']] = prop['value']
>> +            except KeyError:
>> +                # QEMU doesn't know this driver thus it has to be saved
>> +                if driver not in self.compat_props:
>> +                    self.compat_props[driver] = {}
>> +                self.compat_props[driver][prop['property']] = prop['value']
>> +
>> +
>> +class Configuration():
>> +    """Class contains all necessary components to generate table and is used
>> +    to compare different binaries"""
>> +    def __init__(self, vm: QEMUMachine,
>> +                 req_mt: List[str], all_mt: bool) -> None:
>> +        self._vm = vm
>> +        self._binary = vm.binary
>> +        self._qemu_args = args.qemu_args.split(' ')
>> +
>> +        self._qemu_drivers = VMPropertyGetter(vm)
>> +        self.req_mt = get_req_mt(self._qemu_drivers, vm, req_mt, all_mt)
>> +
>> +    def get_implementations(self, driver_name: str) -> List[str]:
>> +        return self._qemu_drivers.get_implementations(driver_name)
>> +
>> +    def get_table(self, req_props: List[Tuple[str, str]]) -> pd.DataFrame:
>> +        table: List[pd.DataFrame] = []
>> +        for mt in self.req_mt:
>> +            name = f'{self._binary}\n{mt.name}'
>> +            column = []
>> +            for driver, prop in req_props:
>> +                try:
>> +                    # values from QEMU machine type definitions
>> +                    column.append(mt.compat_props[driver][prop])
>> +                except KeyError:
>> +                    # values from QEMU type definitions
>> +                    column.append(self._qemu_drivers.get_prop(driver, prop))
>> +            table.append(pd.DataFrame({name: column}))
>> +
>> +        return pd.concat(table, axis=1)
>> +
>> +
>> +script_desc = """Script to compare machine types (their compat_props).
>> +
>> +Examples:
>> +* save info about all machines:  ./scripts/compare_mt.py --all --format csv \
>> +--raw > table.csv
>> +* compare machines: ./scripts/compare_mt.py --mt pc-q35-2.12 pc-q35-3.0
>> +* compare binaries and machines: ./scripts/compare_mt.py --mt pc-q35-6.2 \
>> +pc-q35-7.0 --qemu-binary build/qemu-system-x86_64 build/qemu-exp
>> +  ╒════════════╤══════════════════════════╤════════════════════════════\
>> +╤════════════════════════════╤══════════════════╤══════════════════╕
>> +  │   Driver   │         Property         │  build/qemu-system-x86_64  \
>> +│  build/qemu-system-x86_64  │  build/qemu-exp  │  build/qemu-exp  │
>> +  │            │                          │         pc-q35-6.2         \
>> +│         pc-q35-7.0         │    pc-q35-6.2    │    pc-q35-7.0    │
>> +  ╞════════════╪══════════════════════════╪════════════════════════════\
>> +╪════════════════════════════╪══════════════════╪══════════════════╡
>> +  │  PIIX4_PM  │ x-not-migrate-acpi-index │            True            \
>> +│           False            │      False       │      False       │
>> +  ├────────────┼──────────────────────────┼────────────────────────────\
>> +┼────────────────────────────┼──────────────────┼──────────────────┤
>> +  │ virtio-mem │  unplugged-inaccessible  │           False            \
>> +│            auto            │      False       │       auto       │
>> +  ╘════════════╧══════════════════════════╧════════════════════════════\
>> +╧════════════════════════════╧══════════════════╧══════════════════╛
>> +
>> +If a property from QEMU machine defintion applies to an abstract class (e.g. \
>> +x86_64-cpu) this script will compare all implementations of this class.
>> +
>> +"Unavailable method" - means that this script doesn't know how to get \
>> +default values of the driver. To add method use the construction described \
>> +at the top of the script.
>> +"Unavailable driver" - means that this script doesn't know this driver. \
>> +For instance, this can happen if you configure QEMU without this device or \
>> +if machine type definition has error.
>> +"No default value" - means that the appropriate method can't get the default \
>> +value and most likely that this property doesn't have it.
>> +"Unknown property" - means that the appropriate method can't find property \
>> +with this name."""
>> +
>> +
>> +def parse_args() -> Namespace:
>> +    parser = ArgumentParser(formatter_class=RawTextHelpFormatter,
>> +                            description=script_desc)
>> +    parser.add_argument('--format', choices=['human-readable', 'json', 'csv'],
>> +                        default='human-readable',
>> +                        help='returns table in json format')
>> +    parser.add_argument('--raw', action='store_true',
>> +                        help='prints ALL defined properties without value '
>> +                             'transformation. By default, only rows '
>> +                             'with different values will be printed and '
>> +                             'values will be transformed(e.g. "on" -> True)')
>> +    parser.add_argument('--qemu-args', default=default_qemu_args,
>> +                        help='command line to start qemu. '
>> +                             f'Default: "{default_qemu_args}"')
>> +    parser.add_argument('--qemu-binary', nargs="*", type=str,
>> +                        default=[default_qemu_binary],
>> +                        help='list of qemu binaries that will be compared. '
>> +                             f'Deafult: {default_qemu_binary}')
>> +
>> +    mt_args_group = parser.add_mutually_exclusive_group()
>> +    mt_args_group.add_argument('--all', action='store_true',
>> +                               help='prints all available machine types (list '
>> +                                    'of machine types will be ignored)')
>> +    mt_args_group.add_argument('--mt', nargs="*", type=str,
>> +                               help='list of Machine Types '
>> +                                    'that will be compared')
>> +
>> +    return parser.parse_args()
>> +
>> +
>> +def mt_comp(mt: Machine) -> Tuple[str, int, int, int]:
>> +    """Function to compare and sort machine by names.
>> +    It returns socket_name, major version, minor version, revision"""
>> +    # none, microvm, x-remote and etc.
>> +    if '-' not in mt.name or '.' not in mt.name:
>> +        return mt.name, 0, 0, 0
>> +
>> +    socket, ver = mt.name.rsplit('-', 1)
>> +    ver_list = list(map(int, ver.split('.', 2)))
>> +    ver_list += [0] * (3 - len(ver_list))
>> +    return socket, ver_list[0], ver_list[1], ver_list[2]
>> +
>> +
>> +def get_mt_definitions(qemu_drivers: VMPropertyGetter,
>> +                       vm: QEMUMachine) -> List[Machine]:
>> +    """Constructs list of machine definitions (primarily compat_props) via
>> +    info from QEMU"""
>> +    raw_mt_defs = vm.cmd('query-machines', compat_props=True)
>> +    mt_defs = []
>> +    for raw_mt in raw_mt_defs:
>> +        mt_defs.append(Machine(raw_mt, qemu_drivers))
>> +
>> +    mt_defs.sort(key=mt_comp)
>> +    return mt_defs
>> +
>> +
>> +def get_req_mt(qemu_drivers: VMPropertyGetter, vm: QEMUMachine,
>> +               req_mt: Optional[List[str]], all_mt: bool) -> List[Machine]:
>> +    """Returns list of requested by user machines"""
>> +    mt_defs = get_mt_definitions(qemu_drivers, vm)
>> +    if all_mt:
>> +        return mt_defs
>> +
>> +    if req_mt is None:
>> +        print('Enter machine types for comparision')
>> +        exit(0)
>> +
>> +    matched_mt = []
>> +    for mt in mt_defs:
>> +        if mt.name in req_mt:
>> +            matched_mt.append(mt)
>> +
>> +    return matched_mt
>> +
>> +
>> +def get_affected_props(configs: List[Configuration]) -> Generator[Tuple[str,
>> +                                                                        str],
>> +                                                                  None, None]:
>> +    """Helps to go through all affected in machine definitions drivers
>> +    and properties"""
>> +    driver_props: Dict[str, Set[Any]] = {}
>> +    for config in configs:
>> +        for mt in config.req_mt:
>> +            compat_props = mt.compat_props
>> +            for driver, prop in compat_props.items():
>> +                if driver not in driver_props:
>> +                    driver_props[driver] = set()
>> +                driver_props[driver].update(prop.keys())
>> +
>> +    for driver, props in sorted(driver_props.items()):
>> +        for prop in sorted(props):
>> +            yield driver, prop
>> +
>> +
>> +def transform_value(value: str) -> Union[str, bool]:
>> +    true_list = ['true', 'on']
>> +    false_list = ['false', 'off']
>> +
>> +    out = value.lower()
>> +
>> +    if out in true_list:
>> +        return True
>> +
>> +    if out in false_list:
>> +        return False
>> +
>> +    return value
>> +
>> +
>> +def simplify_table(table: pd.DataFrame) -> pd.DataFrame:
>> +    """transforms values to make it easier to compare it and drops rows
>> +    with the same values for all columns"""
>> +
>> +    table = table.map(transform_value)
>> +
>> +    return table[~table.iloc[:, 3:].eq(table.iloc[:, 2], axis=0).all(axis=1)]
>> +
>> +
>> +# constructs table in the format:
>> +#
>> +# Driver  | Property  | binary1  | binary1  | ...
>> +#         |           | machine1 | machine2 | ...
>> +# ------------------------------------------------------ ...
>> +# driver1 | property1 |  value1  |  value2  | ...
>> +# driver1 | property2 |  value3  |  value4  | ...
>> +# driver2 | property3 |  value5  |  value6  | ...
>> +#   ...   |    ...    |   ...    |   ...    | ...
>> +#
>> +def fill_prop_table(configs: List[Configuration],
>> +                    is_raw: bool) -> pd.DataFrame:
>> +    req_props = list(get_affected_props(configs))
>> +    if not req_props:
>> +        print('No drivers to compare. Check machine names')
>> +        exit(0)
>> +
>> +    driver_col, prop_col = tuple(zip(*req_props))
>> +    table = [pd.DataFrame({'Driver': driver_col}),
>> +             pd.DataFrame({'Property': prop_col})]
>> +
>> +    table.extend([config.get_table(req_props) for config in configs])
>> +
>> +    df_table = pd.concat(table, axis=1)
>> +
>> +    if is_raw:
>> +        return df_table
>> +
>> +    return simplify_table(df_table)
>> +
>> +
>> +def print_table(table: pd.DataFrame, table_format: str) -> None:
>> +    if table_format == 'json':
>> +        print(comp_table.to_json())
>> +    elif table_format == 'csv':
>> +        print(comp_table.to_csv())
>> +    else:
>> +        print(comp_table.to_markdown(index=False, stralign='center',
>> +                                     colalign=('center',), headers='keys',
>> +                                     tablefmt='fancy_grid',
>> +                                     disable_numparse=True))
>> +
>> +
>> +if __name__ == '__main__':
>> +    args = parse_args()
>> +    with ExitStack() as stack:
>> +        vms = [stack.enter_context(QEMUMachine(binary=binary, qmp_timer=15,
>> +               args=args.qemu_args.split(' '))) for binary in args.qemu_binary]
>> +
>> +        configurations = []
>> +        for vm in vms:
>> +            vm.launch()
>> +            configurations.append(Configuration(vm, args.mt, args.all))
>> +
>> +        comp_table = fill_prop_table(configurations, args.raw)
>> +        if not comp_table.empty:
>> +            print_table(comp_table, args.format)

-- 
Best regards,
Maksim Davydov

Re: [PATCH v6 4/4] scripts: add script to compare compatible properties

[Markus Armbruster]

Maksim Davydov <davydov-max@yandex-team.ru> writes:

> On 12/1/23 12:51, Markus Armbruster wrote:
>> Review, anyone?
>
> Only Vladimir

To be clear: I'm soliciting a second review.

[...]

Re: [PATCH v6 4/4] scripts: add script to compare compatible properties

[John Snow]

On Mon, Dec 18, 2023 at 8:20 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Maksim Davydov <davydov-max@yandex-team.ru> writes:
>
> > On 12/1/23 12:51, Markus Armbruster wrote:
> >> Review, anyone?
> >
> > Only Vladimir
>
> To be clear: I'm soliciting a second review.
>
> [...]
>

I volunteer to review it from the Python maintenance POV, but please
rebase and resend to fix the patchew desync. We still want review from
a more holistic perspective, though ... but if it's not part of the
build or test infrastructure, it doesn't have to be perfect.

--js

Re: [PATCH v6 0/4] compare machine type compat_props

[Philippe Mathieu-Daudé]

Hi Maksim,

On 8/11/23 16:38, Maksim Davydov wrote:
> This script can be used to choose the best machine type in the
> appropriate cases. Also we have to check compat_props of the old MT
> after changes to be sure that they haven't broken old the MT. For
> example, pc_compat_3_1 of pc-q35-3.1 has Icelake-Client which was
> removed.

> Maksim Davydov (4):
>    qom: add default value
>    qmp: add dump machine type compatible properties
>    python: add binary
>    scripts: add script to compare compatible properties
> 
>   hw/core/machine-qmp-cmds.c     |  23 +-
>   python/qemu/machine/machine.py |   5 +
>   qapi/machine.json              |  54 +++-
>   qom/qom-qmp-cmds.c             |   1 +
>   scripts/compare_mt.py          | 484 +++++++++++++++++++++++++++++++++

"compare_machine_types.py" name is more meaningful.

>   tests/qtest/fuzz/qos_fuzz.c    |   2 +-
>   6 files changed, 565 insertions(+), 4 deletions(-)
>   create mode 100755 scripts/compare_mt.py
> 

Re: [PATCH v6 0/4] compare machine type compat_props

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Hi Maksim,
>
> On 8/11/23 16:38, Maksim Davydov wrote:
>> This script can be used to choose the best machine type in the
>> appropriate cases. Also we have to check compat_props of the old MT
>> after changes to be sure that they haven't broken old the MT. For
>> example, pc_compat_3_1 of pc-q35-3.1 has Icelake-Client which was
>> removed.
>
>> Maksim Davydov (4):
>>    qom: add default value
>>    qmp: add dump machine type compatible properties
>>    python: add binary
>>    scripts: add script to compare compatible properties
>>   hw/core/machine-qmp-cmds.c     |  23 +-
>>   python/qemu/machine/machine.py |   5 +
>>   qapi/machine.json              |  54 +++-
>>   qom/qom-qmp-cmds.c             |   1 +
>>   scripts/compare_mt.py          | 484 +++++++++++++++++++++++++++++++++
>
> "compare_machine_types.py" name is more meaningful.

Please use '-' instead of '_' in program names.

>>   tests/qtest/fuzz/qos_fuzz.c    |   2 +-
>>   6 files changed, 565 insertions(+), 4 deletions(-)
>>   create mode 100755 scripts/compare_mt.py