[PATCH v3 0/4] compare machine type compat_props

[PATCH v3 0/4] compare machine type compat_props

[Maksim Davydov]

This script is necessary 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 in March.

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
  python/qmp: increase read buffer size
  qmp: add dump machine type compatible properties
  scripts: add script to compare compatible properties

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

-- 
2.25.1

[PATCH v3 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.

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

diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 2e63a4c184..1d7867dc19 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -217,6 +217,8 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
         info->type = g_strdup(prop->type);
         info->has_description = !!prop->description;
         info->description = g_strdup(prop->description);
+        info->default_value = qobject_ref(prop->defval);
+        info->has_default_value = !!info->default_value;
 
         QAPI_LIST_PREPEND(prop_list, info);
     }
-- 
2.25.1

[PATCH v3 2/4] python/qmp: increase read buffer size

[Maksim Davydov]

After modification of "query-machines" command the buffer size should be
more than 452kB to contain output with compat-props.

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 python/qemu/qmp/qmp_client.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/qemu/qmp/qmp_client.py b/python/qemu/qmp/qmp_client.py
index 5dcda04a75..659fe4d98c 100644
--- a/python/qemu/qmp/qmp_client.py
+++ b/python/qemu/qmp/qmp_client.py
@@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'):
     #: Logger object used for debugging messages.
     logger = logging.getLogger(__name__)
 
-    # Read buffer limit; large enough to accept query-qmp-schema
-    _limit = (256 * 1024)
+    # Read buffer limit; large enough to accept query-machines
+    _limit = (512 * 1024)
 
     # Type alias for pending execute() result items
     _PendingT = Union[Message, ExecInterruptedError]
-- 
2.25.1

[PATCH v3 3/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  | 22 ++++++++++++++-
 qapi/machine.json           | 54 +++++++++++++++++++++++++++++++++++--
 tests/qtest/fuzz/qos_fuzz.c |  2 +-
 3 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 4f4ab30f8c..a6fc387439 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -74,7 +74,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;
@@ -107,6 +108,25 @@ MachineInfoList *qmp_query_machines(Error **errp)
             info->default_ram_id = g_strdup(mc->default_ram_id);
             info->has_default_ram_id = true;
         }
+        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 b9228a5e46..07d3b01358 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -127,6 +127,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: 7.2
+##
+{ 'struct': 'CompatProperty',
+  'data': { 'driver': 'str',
+            'property': 'str',
+            'value': 'str' } }
+
 ##
 # @MachineInfo:
 #
@@ -155,6 +174,9 @@
 #
 # @default-ram-id: the default ID of initial RAM memory backend (since 5.2)
 #
+# @compat-props: List of compatible properties that defines machine type
+#                (since 7.2)
+#
 # Since: 1.2
 ##
 { 'struct': 'MachineInfo',
@@ -162,18 +184,46 @@
             '*is-default': 'bool', 'cpu-max': 'int',
             'hotpluggable-cpus': 'bool',  'numa-mem-supported': 'bool',
             'deprecated': 'bool', '*default-cpu-type': 'str',
-            '*default-ram-id': 'str' } }
+            '*default-ram-id': 'str', '*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 7.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 3a3d9c16dd..9420f950fd 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.25.1

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

[Maksim Davydov]

This script run QEMU to obtain compat_props of machines and default
values of different types and produce appropriate table. This table
can be used to compare machine types to choose the most suitable
machine. Also this table in json or csv format should be used to check that
new machine doesn't affect previous ones by comparing tables with and
without new machine.
Default values of properties are needed to fill "holes" in the table (one
machine has these properties and another not. For instance, 2.12 mt has
`{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
3.1 mt doesn't have it. So, to compare these machines we need to fill
unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 mt. This unknown value
in the table I called "hole". To get values (default 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)

Example:

./scripts/compare_mt.py --mt pc-q35-3.1 pc-q35-4.0

╒═══════════════════════════════════════════════════════════╤══════════════╤════════════════════╕
│                                                           │  pc-q35-3.1  │     pc-q35-4.0     │
╞═══════════════════════════════════════════════════════════╪══════════════╪════════════════════╡
│ Cascadelake-Server-x86_64-cpu:mpx                         │     True     │       False        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Cascadelake-Server-x86_64-cpu:stepping                    │      5       │         6          │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Icelake-Client-x86_64-cpu:mpx                             │     True     │ unavailable driver │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Icelake-Server-x86_64-cpu:mpx                             │     True     │       False        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Opteron_G3-x86_64-cpu:rdtscp                              │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Opteron_G4-x86_64-cpu:rdtscp                              │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Opteron_G5-x86_64-cpu:rdtscp                              │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Skylake-Client-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Skylake-Client-x86_64-cpu:mpx                             │     True     │       False        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Skylake-Server-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ Skylake-Server-x86_64-cpu:mpx                             │     True     │       False        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ intel-iommu:dma-drain                                     │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ memory-backend-file:x-use-canonical-path-for-ramblock-id  │     True     │  no default value  │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ memory-backend-memfd:x-use-canonical-path-for-ramblock-id │     True     │  no default value  │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ pcie-root-port:x-speed                                    │     2_5      │         16         │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ pcie-root-port:x-width                                    │      1       │         32         │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ pcie-root-port-base:disable-acs                           │     True     │       False        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ tpm-crb:ppi                                               │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ tpm-tis:ppi                                               │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ usb-kbd:serial                                            │      42      │  no default value  │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ usb-mouse:serial                                          │      42      │  no default value  │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ usb-tablet:serial                                         │      42      │  no default value  │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ virtio-balloon-device:qemu-4-0-config-size                │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ virtio-blk-device:discard                                 │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ virtio-blk-device:write-zeroes                            │    False     │        True        │
├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
│ x86_64-cpu:x-intel-pt-auto-level                          │    False     │        True        │
╘═══════════════════════════════════════════════════════════╧══════════════╧════════════════════╛

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
---
 scripts/compare_mt.py | 440 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 440 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..31ac86dddd
--- /dev/null
+++ b/scripts/compare_mt.py
@@ -0,0 +1,440 @@
+#!/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 comparision, some machine type doesn't have a property (it is in
+# the comparision 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_propery_methods.
+#
+# Copyright (c) Yandex Technologies LLC, 2022
+#
+# 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/>.
+
+from tabulate import tabulate
+import sys
+from os import path
+from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
+import pandas as pd
+from typing import Callable, List, Dict, Set, Generator, Tuple, Union, Any
+
+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_cmd_line = 'build/qemu-system-x86_64 -enable-kvm -machine none'
+
+
+# Methods to get right values of drivers props
+#
+# 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:
+# * Names should be in qom-list-types format (486-x86_64-cpu, not 486)
+# * Specialization always wins (from 'device' and 'x86_64-cpu', 'x86_64-cpu'
+#   will be used for '486-x86_64-cpu')
+
+# It's default stub for all undefined in property_methods drivers because all
+# QEMU types are inherited from Object
+def get_object_prop(vm: QEMUMachine, device: str, prop_name: str):
+    return 'Unavailable method'
+
+
+def get_device_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
+    device_props = vm.command('device-list-properties', typename=device)
+    for prop in device_props:
+        if prop['name'] == prop_name:
+            return str(prop.get('default-value', 'No default value'))
+
+    return 'Unknown property'
+
+
+def get_x86_64_cpu_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
+    # crop last 11 chars '-x86_64-cpu'
+    props = vm.command('query-cpu-model-expansion', type='full',
+                       model={'name': device[:-11]})['model']['props']
+    return str(props.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
+def get_memory_backend_prop(vm: QEMUMachine, driver: str,
+                            prop_name: str) -> str:
+    memory_backend_props = vm.command('qom-list-properties', typename=driver)
+    for prop in memory_backend_props:
+        if prop['name'] == prop_name:
+            return str(prop.get('default-value', 'No default value'))
+
+    return 'Unknown property'
+
+
+class GetPropMethod:
+    def __init__(self, driver_name: str,
+                 method: Callable[[QEMUMachine, str, str], str]) -> None:
+        self.name = driver_name
+        self.get_prop = method
+
+
+qemu_property_methods = [
+        GetPropMethod('device', get_device_prop),
+        GetPropMethod('x86_64-cpu', get_x86_64_cpu_prop),
+        GetPropMethod('memory-backend', get_memory_backend_prop)
+]
+
+# all types in QEMU are inherited from Object
+qemu_default_method = GetPropMethod('object', get_object_prop)
+# End of methods definition
+
+
+class Driver:
+    def __init__(self, driver_defs: Dict, driver_name: str, parent_name: str,
+                 is_abstr: bool, list_of_children: List[str],
+                 get_prop_method: GetPropMethod) -> None:
+        self.driver_defs = driver_defs
+        self.name = driver_name
+        self.parent = parent_name
+        self.abstract = is_abstr
+        self.children = list_of_children
+        self.method = get_prop_method
+
+
+    def is_parent(self, driver_name: str) -> bool:
+        if driver_name not in self.driver_defs:
+            return False
+
+        cur_parent = self.parent
+        while cur_parent:
+            if driver_name == cur_parent:
+                return True
+            cur_parent = self.driver_defs[cur_parent].parent
+
+        return False
+
+
+    def set_prop_method(self, prop_method: GetPropMethod) -> None:
+        if prop_method.name != self.name:
+            return
+
+        self.method = prop_method
+        if not self.abstract:
+            return
+
+        for child in self.children:
+            # specialization always wins
+            if self.is_parent(self.driver_defs[child].method.name):
+                self.driver_defs[child].method = prop_method
+
+
+class DriverDefinitions:
+    def __init__(self, vm: QEMUMachine, default_method: GetPropMethod,
+                 property_methods: List[GetPropMethod]) -> None:
+        self.driver_defs: Dict[str, Driver] = {}
+        self.default_method = default_method
+        self.property_methods = property_methods
+        self.vm = vm
+
+        qom_all_types = vm.command('qom-list-types', abstract=True)
+        for obj_type in qom_all_types:
+            # parent of Object is None
+            parent = obj_type.get('parent', None)
+            abstr = obj_type.get('abstract', False)
+            name = obj_type['name']
+            if abstr:
+                list_child_objs = vm.command('qom-list-types', implements=name,
+                                             abstract=True)
+                child_list = [child['name'] for child in list_child_objs]
+                self.driver_defs[name] = Driver(self.driver_defs, name, parent,
+                                                abstr, child_list,
+                                                default_method)
+            else:
+                self.driver_defs[name] = Driver(self.driver_defs, name, parent,
+                                                abstr, [], default_method)
+
+        for prop_method in property_methods:
+            # skipping other architectures and etc
+            if prop_method.name not in self.driver_defs:
+                continue
+            self.driver_defs[prop_method.name].set_prop_method(prop_method)
+
+
+    def add_prop_value(self, driver: str, prop: str, prop_list: list) -> None:
+        # wrong driver name or disabled in config driver
+        if driver not in self.driver_defs:
+            prop_list.append('Unavailable driver')
+            return
+
+        if not self.driver_defs[driver].abstract:
+            prop_list.append(self.driver_defs[driver].method.get_prop(self.vm,
+                                                                      driver,
+                                                                      prop))
+            return
+
+        # if abstract we need to collect default values from all children
+        values = set()
+        for child in self.driver_defs[driver].children:
+            if self.driver_defs[child].abstract:
+                continue
+
+            values.add(self.driver_defs[child].method.get_prop(self.vm, child,
+                                                               prop))
+
+        prop_list.append(list(values))
+
+
+class Machine:
+    # raw_mt_dict - dict produced by `query-machines`
+    def __init__(self, raw_mt_dict: dict) -> None:
+        self.name = raw_mt_dict['name']
+        self.compat_props: Dict[str, Dict[str, str]] = {}
+        # properties are applied sequentially and can rewrite values as in QEMU
+        for prop in raw_mt_dict['compat-props']:
+            if prop['driver'] not in self.compat_props:
+                self.compat_props[prop['driver']] = {}
+            self.compat_props[prop['driver']][prop['property']] = prop['value']
+
+
+script_desc="""Script to compare machine types (their compat_props).
+
+If a property applies to an abstract class this script collects default \
+values of all child classes and prints them as a set.
+
+"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 properties '
+                             'with different values will be printed and with '
+                             'value transformation(e.g. "on" -> True)')
+    parser.add_argument('--cmd-line', default=default_cmd_line,
+                        help='command line to start qemu. '
+                             f'Default: "{default_cmd_line}"')
+
+    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). '
+                                    'Execution may take several minutes!')
+    mt_args_group.add_argument('--mt', nargs="*", type=str,
+                               help='list of Machine Types '
+                                    'that will be compared')
+
+    return parser.parse_args()
+
+
+# return socket_name, major version, minor version, revision
+def mt_comp(mt: Machine) -> Tuple[str, int, int, int]:
+    # 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]
+
+
+# construct list of machine type definitions (primarily compat_props) from QEMU
+def get_mt_definitions(vm: QEMUMachine) -> List[Machine]:
+    raw_mt_defs = vm.command('query-machines', compat_props=True)
+    mt_defs: List[Machine] = []
+    for raw_mt in raw_mt_defs:
+        mt_defs.append(Machine(raw_mt))
+
+    mt_defs.sort(key=mt_comp)
+    return mt_defs
+
+
+def get_req_mt(vm: QEMUMachine, args: Namespace) -> List[Machine]:
+    mt_defs = get_mt_definitions(vm)
+    if args.all:
+        return mt_defs
+
+    list_mt = [mt.name for mt in mt_defs]
+
+    if args.mt is None:
+                print('Enter machine types for comparision or use --help')
+                print('List of available machine types:')
+                print(*list_mt, sep='\n')
+                sys.exit(1)
+
+    for mt in args.mt:
+        if mt not in list_mt:
+            print('Wrong machine type name')
+            print('List of available machine types:')
+            print(*list_mt, sep='\n')
+            sys.exit(1)
+
+    requested_mt = []
+    for mt in mt_defs:
+        if mt.name in args.mt:
+            requested_mt.append(mt)
+
+    return requested_mt
+
+
+# method to iterate through all requested properties in machine definitions
+def get_req_props(mt_defs: list) -> Generator[Tuple[str, str], None, None]:
+    driver_props: Dict[str, Set[str]] = {}
+    for mt in mt_defs:
+        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 out
+
+
+def transform_number(value: str) -> Union[int, None]:
+    try:
+        # C doesn't work with underscore ('2_5' != 25)
+        if '_' in value:
+            raise ValueError
+
+        return int(value, 0)
+
+    except ValueError:
+        return None
+
+
+# delete rows with the same values for all mt and transform values to make it
+# easier to compare
+def transform_table(table: Dict, mt_names: List[str]) -> pd.DataFrame:
+    new_table: Dict[str, List] = {}
+    for full_prop_name, prop_values in table.items():
+        new_row: List[Any] = []
+        all_values = set()
+        # original number format if not all values are the same in the row
+        numeric_values = set()
+        for mt_prop_val in prop_values:
+            if type(mt_prop_val) is list:
+                transformed_val_list = list(map(transform_value, mt_prop_val))
+                if len(transformed_val_list) == 1:
+                    new_row.append(transformed_val_list[0])
+                else:
+                    new_row.append(transformed_val_list)
+
+                numeric_values.update(set(map(transform_number, mt_prop_val)))
+                all_values.update(set(transformed_val_list))
+            else:
+                transformed_val = transform_value(mt_prop_val)
+                new_row.append(transformed_val)
+                numeric_values.add(transform_number(mt_prop_val))
+                all_values.add(transformed_val)
+
+        if len(mt_names) > 1:
+            if len(all_values) == 1:
+                continue
+
+            if None not in numeric_values and len(numeric_values) == 1:
+                continue
+
+        new_table[full_prop_name] = new_row
+
+    return pd.DataFrame.from_dict(new_table, orient='index', columns=mt_names)
+
+
+def fill_prop_table(mt_list: List[Machine],
+                    qemu_drivers: DriverDefinitions,
+                    is_raw: bool) -> pd.DataFrame:
+    table: Dict[str, List] = {}
+    for driver, prop in get_req_props(mt_list):
+        name = f'{driver}:{prop}'
+        table[name] = []
+        for mt in mt_list:
+            if driver in mt.compat_props:
+                # values from QEMU machine type definitions
+                if prop in mt.compat_props[driver]:
+                    table[name].append(mt.compat_props[driver][prop])
+                    continue
+
+            # values from QEMU type definitions
+            qemu_drivers.add_prop_value(driver, prop, table[name])
+
+    headers = [mt.name for mt in mt_list]
+
+    if is_raw:
+        return pd.DataFrame.from_dict(table, orient='index', columns=headers)
+
+    return transform_table(table, headers)
+
+
+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(tabulate(comp_table, showindex=True, stralign='center',
+                       colalign=('left',), tablefmt='fancy_grid',
+                       headers='keys', disable_numparse=True))
+
+
+if __name__ == '__main__':
+    args = parse_args()
+    qemu_arg_list = args.cmd_line.split(' ')
+    with QEMUMachine(binary=qemu_arg_list[0],
+                     qmp_timer=15, args=qemu_arg_list[1:]) as vm:
+        vm.launch()
+
+        req_mt = get_req_mt(vm, args)
+        qemu_drivers = DriverDefinitions(vm, qemu_default_method,
+                                         qemu_property_methods)
+        comp_table = fill_prop_table(req_mt, qemu_drivers, args.raw)
+        print_table(comp_table, args.format)
+
+        vm.shutdown()
-- 
2.25.1

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

[Vladimir Sementsov-Ogievskiy]

On 11/3/22 13:27, Maksim Davydov wrote:
> This script run QEMU to obtain compat_props of machines and default
> values of different types and produce appropriate table. This table
> can be used to compare machine types to choose the most suitable
> machine. Also this table in json or csv format should be used to check that
> new machine doesn't affect previous ones by comparing tables with and
> without new machine.
> Default values of properties are needed to fill "holes" in the table (one
> machine has these properties and another not. For instance, 2.12 mt has
> `{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
> 3.1 mt doesn't have it. So, to compare these machines we need to fill
> unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 mt. This unknown value
> in the table I called "hole". To get values (default 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)
> 
> Example:
> 
> ./scripts/compare_mt.py --mt pc-q35-3.1 pc-q35-4.0
> 
> ╒═══════════════════════════════════════════════════════════╤══════════════╤════════════════════╕
> │                                                           │  pc-q35-3.1  │     pc-q35-4.0     │
> ╞═══════════════════════════════════════════════════════════╪══════════════╪════════════════════╡
> │ Cascadelake-Server-x86_64-cpu:mpx                         │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Cascadelake-Server-x86_64-cpu:stepping                    │      5       │         6          │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Icelake-Client-x86_64-cpu:mpx                             │     True     │ unavailable driver │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Icelake-Server-x86_64-cpu:mpx                             │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Opteron_G3-x86_64-cpu:rdtscp                              │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Opteron_G4-x86_64-cpu:rdtscp                              │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Opteron_G5-x86_64-cpu:rdtscp                              │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Client-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Client-x86_64-cpu:mpx                             │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Server-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Server-x86_64-cpu:mpx                             │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ intel-iommu:dma-drain                                     │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ memory-backend-file:x-use-canonical-path-for-ramblock-id  │     True     │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ memory-backend-memfd:x-use-canonical-path-for-ramblock-id │     True     │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ pcie-root-port:x-speed                                    │     2_5      │         16         │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ pcie-root-port:x-width                                    │      1       │         32         │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ pcie-root-port-base:disable-acs                           │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ tpm-crb:ppi                                               │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ tpm-tis:ppi                                               │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ usb-kbd:serial                                            │      42      │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ usb-mouse:serial                                          │      42      │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ usb-tablet:serial                                         │      42      │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ virtio-balloon-device:qemu-4-0-config-size                │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ virtio-blk-device:discard                                 │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ virtio-blk-device:write-zeroes                            │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ x86_64-cpu:x-intel-pt-auto-level                          │    False     │        True        │
> ╘═══════════════════════════════════════════════════════════╧══════════════╧════════════════════╛
> 
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> ---
>   scripts/compare_mt.py | 440 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 440 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..31ac86dddd
> --- /dev/null
> +++ b/scripts/compare_mt.py
> @@ -0,0 +1,440 @@
> +#!/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 comparision, some machine type doesn't have a property (it is in
> +# the comparision 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_propery_methods.
> +#
> +# Copyright (c) Yandex Technologies LLC, 2022
> +#
> +# 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/>.
> +
> +from tabulate import tabulate
> +import sys
> +from os import path
> +from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
> +import pandas as pd
> +from typing import Callable, List, Dict, Set, Generator, Tuple, Union, Any
> +
> +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_cmd_line = 'build/qemu-system-x86_64 -enable-kvm -machine none'
> +
> +
> +# Methods to get right values of drivers props
> +#
> +# 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:
> +# * Names should be in qom-list-types format (486-x86_64-cpu, not 486)
> +# * Specialization always wins (from 'device' and 'x86_64-cpu', 'x86_64-cpu'
> +#   will be used for '486-x86_64-cpu')
> +
> +# It's default stub for all undefined in property_methods drivers because all
> +# QEMU types are inherited from Object
> +def get_object_prop(vm: QEMUMachine, device: str, prop_name: str):

missed "-> str" ?

> +    return 'Unavailable method'
> +
> +
> +def get_device_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
> +    device_props = vm.command('device-list-properties', typename=device)

Seems, would be good to cache the result of device-list-properties to not call it for each property. May be done separately.

> +    for prop in device_props:
> +        if prop['name'] == prop_name:
> +            return str(prop.get('default-value', 'No default value'))
> +
> +    return 'Unknown property'
> +
> +
> +def get_x86_64_cpu_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
> +    # crop last 11 chars '-x86_64-cpu'

seems assert(device.endswith('-x86_64-cpu') will not hurt

> +    props = vm.command('query-cpu-model-expansion', type='full',
> +                       model={'name': device[:-11]})['model']['props']
> +    return str(props.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
> +def get_memory_backend_prop(vm: QEMUMachine, driver: str,
> +                            prop_name: str) -> str:
> +    memory_backend_props = vm.command('qom-list-properties', typename=driver)
> +    for prop in memory_backend_props:
> +        if prop['name'] == prop_name:
> +            return str(prop.get('default-value', 'No default value'))
> +
> +    return 'Unknown property'
> +
> +
> +class GetPropMethod:
> +    def __init__(self, driver_name: str,
> +                 method: Callable[[QEMUMachine, str, str], str]) -> None:
> +        self.name = driver_name
> +        self.get_prop = method
> +
> +
> +qemu_property_methods = [
> +        GetPropMethod('device', get_device_prop),
> +        GetPropMethod('x86_64-cpu', get_x86_64_cpu_prop),
> +        GetPropMethod('memory-backend', get_memory_backend_prop)
> +]
> +
> +# all types in QEMU are inherited from Object

Hmm, inherited..

What do you think about the following:

class QEMUObject:
    ... (your class Driver, without set_prop_method() method)
    def get_prop(...):
       return 'Unavailable method'

class QEMUDevice(QEMUObject):
    def get_prop(...):
        <it's yours get_device_prop>

class QEMUx86Cpu(QEMUObject):
    def get_prop(...):
        <it's yours get_x86_64_cpu_prop>

class QEMUMemoryBackend(QEMUObject):
    ...


Than, a helper function to create an object of correct class:

def new_driver_obj(name, *args, **kwargs):
    if name == 'device':
        return QEMUDevice(name, *args, **kwargs)
    elif name == 'x86_64-cpu':
       ...
    else
       return QEMUObject(...)  # the default


-- 
Best regards,
Vladimir

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

[Maksim Davydov]

On 11/8/22 18:37, Vladimir Sementsov-Ogievskiy wrote:
> On 11/3/22 13:27, Maksim Davydov wrote:
>> This script run QEMU to obtain compat_props of machines and default
>> values of different types and produce appropriate table. This table
>> can be used to compare machine types to choose the most suitable
>> machine. Also this table in json or csv format should be used to 
>> check that
>> new machine doesn't affect previous ones by comparing tables with and
>> without new machine.
>> Default values of properties are needed to fill "holes" in the table 
>> (one
>> machine has these properties and another not. For instance, 2.12 mt has
>> `{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
>> 3.1 mt doesn't have it. So, to compare these machines we need to fill
>> unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 mt. This unknown value
>> in the table I called "hole". To get values (default 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)
>>
>> Example:
>>
>> ./scripts/compare_mt.py --mt pc-q35-3.1 pc-q35-4.0
>>
>> ╒═══════════════════════════════════════════════════════════╤══════════════╤════════════════════╕ 
>>
>> │                                                           │ 
>> pc-q35-3.1  │     pc-q35-4.0     │
>> ╞═══════════════════════════════════════════════════════════╪══════════════╪════════════════════╡ 
>>
>> │ Cascadelake-Server-x86_64-cpu:mpx │     True     │       
>> False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Cascadelake-Server-x86_64-cpu:stepping │      5       │         
>> 6          │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Icelake-Client-x86_64-cpu:mpx │     True     │ unavailable driver │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Icelake-Server-x86_64-cpu:mpx │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Opteron_G3-x86_64-cpu:rdtscp                              │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Opteron_G4-x86_64-cpu:rdtscp                              │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Opteron_G5-x86_64-cpu:rdtscp                              │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Skylake-Client-IBRS-x86_64-cpu:mpx │     True     │       
>> False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Skylake-Client-x86_64-cpu:mpx │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Skylake-Server-IBRS-x86_64-cpu:mpx │     True     │       
>> False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ Skylake-Server-x86_64-cpu:mpx │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ intel-iommu:dma-drain                                     │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ memory-backend-file:x-use-canonical-path-for-ramblock-id │     
>> True     │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ memory-backend-memfd:x-use-canonical-path-for-ramblock-id │     
>> True     │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ pcie-root-port:x-speed │     2_5      │         16         │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ pcie-root-port:x-width │      1       │         32         │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ pcie-root-port-base:disable-acs │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ tpm-crb:ppi                                               │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ tpm-tis:ppi                                               │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ usb-kbd:serial │      42      │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ usb-mouse:serial │      42      │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ usb-tablet:serial │      42      │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ virtio-balloon-device:qemu-4-0-config-size                │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ virtio-blk-device:discard                                 │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ virtio-blk-device:write-zeroes                            │ 
>> False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤ 
>>
>> │ x86_64-cpu:x-intel-pt-auto-level                          │ 
>> False     │        True        │
>> ╘═══════════════════════════════════════════════════════════╧══════════════╧════════════════════╛ 
>>
>>
>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
>> ---
>>   scripts/compare_mt.py | 440 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 440 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..31ac86dddd
>> --- /dev/null
>> +++ b/scripts/compare_mt.py
>> @@ -0,0 +1,440 @@
>> +#!/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 comparision, some machine type doesn't have a property 
>> (it is in
>> +# the comparision 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_propery_methods.
>> +#
>> +# Copyright (c) Yandex Technologies LLC, 2022
>> +#
>> +# 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/>.
>> +
>> +from tabulate import tabulate
>> +import sys
>> +from os import path
>> +from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
>> +import pandas as pd
>> +from typing import Callable, List, Dict, Set, Generator, Tuple, 
>> Union, Any
>> +
>> +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_cmd_line = 'build/qemu-system-x86_64 -enable-kvm -machine none'
>> +
>> +
>> +# Methods to get right values of drivers props
>> +#
>> +# 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:
>> +# * Names should be in qom-list-types format (486-x86_64-cpu, not 486)
>> +# * Specialization always wins (from 'device' and 'x86_64-cpu', 
>> 'x86_64-cpu'
>> +#   will be used for '486-x86_64-cpu')
>> +
>> +# It's default stub for all undefined in property_methods drivers 
>> because all
>> +# QEMU types are inherited from Object
>> +def get_object_prop(vm: QEMUMachine, device: str, prop_name: str):
>
> missed "-> str" ?
>
Yes, my fault
>> +    return 'Unavailable method'
>> +
>> +
>> +def get_device_prop(vm: QEMUMachine, device: str, prop_name: str) -> 
>> str:
>> +    device_props = vm.command('device-list-properties', 
>> typename=device)
>
> Seems, would be good to cache the result of device-list-properties to 
> not call it for each property. May be done separately.
Good idea! I'll add it to reduce the number of the same qmp requests
>> +    for prop in device_props:
>> +        if prop['name'] == prop_name:
>> +            return str(prop.get('default-value', 'No default value'))
>> +
>> +    return 'Unknown property'
>> +
>> +
>> +def get_x86_64_cpu_prop(vm: QEMUMachine, device: str, prop_name: 
>> str) -> str:
>> +    # crop last 11 chars '-x86_64-cpu'
>
> seems assert(device.endswith('-x86_64-cpu') will not hurt
>
>> +    props = vm.command('query-cpu-model-expansion', type='full',
>> +                       model={'name': device[:-11]})['model']['props']
>> +    return str(props.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
>> +def get_memory_backend_prop(vm: QEMUMachine, driver: str,
>> +                            prop_name: str) -> str:
>> +    memory_backend_props = vm.command('qom-list-properties', 
>> typename=driver)
>> +    for prop in memory_backend_props:
>> +        if prop['name'] == prop_name:
>> +            return str(prop.get('default-value', 'No default value'))
>> +
>> +    return 'Unknown property'
>> +
>> +
>> +class GetPropMethod:
>> +    def __init__(self, driver_name: str,
>> +                 method: Callable[[QEMUMachine, str, str], str]) -> 
>> None:
>> +        self.name = driver_name
>> +        self.get_prop = method
>> +
>> +
>> +qemu_property_methods = [
>> +        GetPropMethod('device', get_device_prop),
>> +        GetPropMethod('x86_64-cpu', get_x86_64_cpu_prop),
>> +        GetPropMethod('memory-backend', get_memory_backend_prop)
>> +]
>> +
>> +# all types in QEMU are inherited from Object
>
> Hmm, inherited..
>
> What do you think about the following:
>
> class QEMUObject:
>    ... (your class Driver, without set_prop_method() method)
>    def get_prop(...):
>       return 'Unavailable method'
>
> class QEMUDevice(QEMUObject):
>    def get_prop(...):
>        <it's yours get_device_prop>
>
> class QEMUx86Cpu(QEMUObject):
>    def get_prop(...):
>        <it's yours get_x86_64_cpu_prop>
>
> class QEMUMemoryBackend(QEMUObject):
>    ...
>
I'll try to improve this part of the script
>
> Than, a helper function to create an object of correct class:
>
> def new_driver_obj(name, *args, **kwargs):
>    if name == 'device':
>        return QEMUDevice(name, *args, **kwargs)
>    elif name == 'x86_64-cpu':
>       ...
>    else
>       return QEMUObject(...)  # the default
>
>
Thanks for reviewing!

-- 
Best regards,
Maksim Davydov

Re: [PATCH v3 2/4] python/qmp: increase read buffer size

[John Snow]

On Thu, Nov 3, 2022 at 6:29 AM Maksim Davydov
<davydov-max@yandex-team.ru> wrote:
>
> After modification of "query-machines" command the buffer size should be
> more than 452kB to contain output with compat-props.
>
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  python/qemu/qmp/qmp_client.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/python/qemu/qmp/qmp_client.py b/python/qemu/qmp/qmp_client.py
> index 5dcda04a75..659fe4d98c 100644
> --- a/python/qemu/qmp/qmp_client.py
> +++ b/python/qemu/qmp/qmp_client.py
> @@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'):
>      #: Logger object used for debugging messages.
>      logger = logging.getLogger(__name__)
>
> -    # Read buffer limit; large enough to accept query-qmp-schema
> -    _limit = (256 * 1024)
> +    # Read buffer limit; large enough to accept query-machines
> +    _limit = (512 * 1024)

wow :)

>
>      # Type alias for pending execute() result items
>      _PendingT = Union[Message, ExecInterruptedError]
> --
> 2.25.1
>

If you would please submit this to
https://gitlab.com/qemu-project/python-qemu-qmp I can get it merged
there quickly, then backport it to qemu.git.
Or, if you don't have a gitlab account (and do not want one), please
let me know and I'll port it there myself so you don't have to.

thanks,
--js

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

[John Snow]

On Thu, Nov 3, 2022 at 6:29 AM Maksim Davydov
<davydov-max@yandex-team.ru> wrote:
>
> This script run QEMU to obtain compat_props of machines and default
> values of different types and produce appropriate table. This table
> can be used to compare machine types to choose the most suitable
> machine. Also this table in json or csv format should be used to check that
> new machine doesn't affect previous ones by comparing tables with and
> without new machine.
> Default values of properties are needed to fill "holes" in the table (one
> machine has these properties and another not. For instance, 2.12 mt has
> `{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
> 3.1 mt doesn't have it. So, to compare these machines we need to fill
> unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 mt. This unknown value
> in the table I called "hole". To get values (default 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)
>
> Example:
>
> ./scripts/compare_mt.py --mt pc-q35-3.1 pc-q35-4.0
>
> ╒═══════════════════════════════════════════════════════════╤══════════════╤════════════════════╕
> │                                                           │  pc-q35-3.1  │     pc-q35-4.0     │
> ╞═══════════════════════════════════════════════════════════╪══════════════╪════════════════════╡
> │ Cascadelake-Server-x86_64-cpu:mpx                         │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Cascadelake-Server-x86_64-cpu:stepping                    │      5       │         6          │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Icelake-Client-x86_64-cpu:mpx                             │     True     │ unavailable driver │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Icelake-Server-x86_64-cpu:mpx                             │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Opteron_G3-x86_64-cpu:rdtscp                              │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Opteron_G4-x86_64-cpu:rdtscp                              │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Opteron_G5-x86_64-cpu:rdtscp                              │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Client-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Client-x86_64-cpu:mpx                             │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Server-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ Skylake-Server-x86_64-cpu:mpx                             │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ intel-iommu:dma-drain                                     │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ memory-backend-file:x-use-canonical-path-for-ramblock-id  │     True     │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ memory-backend-memfd:x-use-canonical-path-for-ramblock-id │     True     │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ pcie-root-port:x-speed                                    │     2_5      │         16         │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ pcie-root-port:x-width                                    │      1       │         32         │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ pcie-root-port-base:disable-acs                           │     True     │       False        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ tpm-crb:ppi                                               │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ tpm-tis:ppi                                               │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ usb-kbd:serial                                            │      42      │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ usb-mouse:serial                                          │      42      │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ usb-tablet:serial                                         │      42      │  no default value  │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ virtio-balloon-device:qemu-4-0-config-size                │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ virtio-blk-device:discard                                 │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ virtio-blk-device:write-zeroes                            │    False     │        True        │
> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
> │ x86_64-cpu:x-intel-pt-auto-level                          │    False     │        True        │
> ╘═══════════════════════════════════════════════════════════╧══════════════╧════════════════════╛
>
> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>

Do you expect this script to be used by any other portion of the
build, or is this merely an interactive tool for developers to
consult? You have the option of moving it into python/qemu/utils/, but
the linting system will be a lot more stringent there. For your
troubles, I'll be able to protect it from bitrot more proactively.
It's up to you -- but it looks like you did try to statically type it
already, so maybe a lot of the pain and gruntwork has already been
done.

> ---
>  scripts/compare_mt.py | 440 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 440 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..31ac86dddd
> --- /dev/null
> +++ b/scripts/compare_mt.py
> @@ -0,0 +1,440 @@
> +#!/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 comparision, some machine type doesn't have a property (it is in
> +# the comparision table because another machine type has it), then the

"comparison", not "comparision", previous two lines.

> +# 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_propery_methods.
> +#
> +# Copyright (c) Yandex Technologies LLC, 2022
> +#
> +# 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/>.
> +
> +from tabulate import tabulate
> +import sys
> +from os import path
> +from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
> +import pandas as pd
> +from typing import Callable, List, Dict, Set, Generator, Tuple, Union, Any
> +
> +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_cmd_line = 'build/qemu-system-x86_64 -enable-kvm -machine none'
> +
> +
> +# Methods to get right values of drivers props
> +#
> +# 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:
> +# * Names should be in qom-list-types format (486-x86_64-cpu, not 486)
> +# * Specialization always wins (from 'device' and 'x86_64-cpu', 'x86_64-cpu'
> +#   will be used for '486-x86_64-cpu')
> +
> +# It's default stub for all undefined in property_methods drivers because all
> +# QEMU types are inherited from Object
> +def get_object_prop(vm: QEMUMachine, device: str, prop_name: str):
> +    return 'Unavailable method'
> +
> +
> +def get_device_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
> +    device_props = vm.command('device-list-properties', typename=device)
> +    for prop in device_props:
> +        if prop['name'] == prop_name:
> +            return str(prop.get('default-value', 'No default value'))
> +
> +    return 'Unknown property'
> +
> +
> +def get_x86_64_cpu_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
> +    # crop last 11 chars '-x86_64-cpu'
> +    props = vm.command('query-cpu-model-expansion', type='full',
> +                       model={'name': device[:-11]})['model']['props']
> +    return str(props.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
> +def get_memory_backend_prop(vm: QEMUMachine, driver: str,
> +                            prop_name: str) -> str:
> +    memory_backend_props = vm.command('qom-list-properties', typename=driver)
> +    for prop in memory_backend_props:
> +        if prop['name'] == prop_name:
> +            return str(prop.get('default-value', 'No default value'))
> +
> +    return 'Unknown property'
> +
> +
> +class GetPropMethod:
> +    def __init__(self, driver_name: str,
> +                 method: Callable[[QEMUMachine, str, str], str]) -> None:
> +        self.name = driver_name
> +        self.get_prop = method
> +
> +
> +qemu_property_methods = [
> +        GetPropMethod('device', get_device_prop),
> +        GetPropMethod('x86_64-cpu', get_x86_64_cpu_prop),
> +        GetPropMethod('memory-backend', get_memory_backend_prop)
> +]
> +
> +# all types in QEMU are inherited from Object
> +qemu_default_method = GetPropMethod('object', get_object_prop)
> +# End of methods definition
> +
> +
> +class Driver:
> +    def __init__(self, driver_defs: Dict, driver_name: str, parent_name: str,
> +                 is_abstr: bool, list_of_children: List[str],
> +                 get_prop_method: GetPropMethod) -> None:
> +        self.driver_defs = driver_defs
> +        self.name = driver_name
> +        self.parent = parent_name
> +        self.abstract = is_abstr
> +        self.children = list_of_children
> +        self.method = get_prop_method
> +
> +
> +    def is_parent(self, driver_name: str) -> bool:
> +        if driver_name not in self.driver_defs:
> +            return False
> +
> +        cur_parent = self.parent
> +        while cur_parent:
> +            if driver_name == cur_parent:
> +                return True
> +            cur_parent = self.driver_defs[cur_parent].parent
> +
> +        return False
> +
> +
> +    def set_prop_method(self, prop_method: GetPropMethod) -> None:
> +        if prop_method.name != self.name:
> +            return
> +
> +        self.method = prop_method
> +        if not self.abstract:
> +            return
> +
> +        for child in self.children:
> +            # specialization always wins
> +            if self.is_parent(self.driver_defs[child].method.name):
> +                self.driver_defs[child].method = prop_method
> +
> +
> +class DriverDefinitions:
> +    def __init__(self, vm: QEMUMachine, default_method: GetPropMethod,
> +                 property_methods: List[GetPropMethod]) -> None:
> +        self.driver_defs: Dict[str, Driver] = {}
> +        self.default_method = default_method
> +        self.property_methods = property_methods
> +        self.vm = vm
> +
> +        qom_all_types = vm.command('qom-list-types', abstract=True)
> +        for obj_type in qom_all_types:
> +            # parent of Object is None
> +            parent = obj_type.get('parent', None)
> +            abstr = obj_type.get('abstract', False)
> +            name = obj_type['name']
> +            if abstr:
> +                list_child_objs = vm.command('qom-list-types', implements=name,
> +                                             abstract=True)
> +                child_list = [child['name'] for child in list_child_objs]
> +                self.driver_defs[name] = Driver(self.driver_defs, name, parent,
> +                                                abstr, child_list,
> +                                                default_method)
> +            else:
> +                self.driver_defs[name] = Driver(self.driver_defs, name, parent,
> +                                                abstr, [], default_method)
> +
> +        for prop_method in property_methods:
> +            # skipping other architectures and etc
> +            if prop_method.name not in self.driver_defs:
> +                continue
> +            self.driver_defs[prop_method.name].set_prop_method(prop_method)
> +
> +
> +    def add_prop_value(self, driver: str, prop: str, prop_list: list) -> None:
> +        # wrong driver name or disabled in config driver
> +        if driver not in self.driver_defs:
> +            prop_list.append('Unavailable driver')
> +            return
> +
> +        if not self.driver_defs[driver].abstract:
> +            prop_list.append(self.driver_defs[driver].method.get_prop(self.vm,
> +                                                                      driver,
> +                                                                      prop))
> +            return
> +
> +        # if abstract we need to collect default values from all children
> +        values = set()
> +        for child in self.driver_defs[driver].children:
> +            if self.driver_defs[child].abstract:
> +                continue
> +
> +            values.add(self.driver_defs[child].method.get_prop(self.vm, child,
> +                                                               prop))
> +
> +        prop_list.append(list(values))
> +
> +
> +class Machine:
> +    # raw_mt_dict - dict produced by `query-machines`
> +    def __init__(self, raw_mt_dict: dict) -> None:
> +        self.name = raw_mt_dict['name']
> +        self.compat_props: Dict[str, Dict[str, str]] = {}
> +        # properties are applied sequentially and can rewrite values as in QEMU
> +        for prop in raw_mt_dict['compat-props']:
> +            if prop['driver'] not in self.compat_props:
> +                self.compat_props[prop['driver']] = {}
> +            self.compat_props[prop['driver']][prop['property']] = prop['value']
> +
> +
> +script_desc="""Script to compare machine types (their compat_props).
> +
> +If a property applies to an abstract class this script collects default \
> +values of all child classes and prints them as a set.
> +
> +"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 properties '
> +                             'with different values will be printed and with '
> +                             'value transformation(e.g. "on" -> True)')
> +    parser.add_argument('--cmd-line', default=default_cmd_line,
> +                        help='command line to start qemu. '
> +                             f'Default: "{default_cmd_line}"')
> +
> +    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). '
> +                                    'Execution may take several minutes!')
> +    mt_args_group.add_argument('--mt', nargs="*", type=str,
> +                               help='list of Machine Types '
> +                                    'that will be compared')
> +
> +    return parser.parse_args()
> +
> +
> +# return socket_name, major version, minor version, revision
> +def mt_comp(mt: Machine) -> Tuple[str, int, int, int]:
> +    # 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]
> +
> +
> +# construct list of machine type definitions (primarily compat_props) from QEMU
> +def get_mt_definitions(vm: QEMUMachine) -> List[Machine]:
> +    raw_mt_defs = vm.command('query-machines', compat_props=True)
> +    mt_defs: List[Machine] = []
> +    for raw_mt in raw_mt_defs:
> +        mt_defs.append(Machine(raw_mt))
> +
> +    mt_defs.sort(key=mt_comp)
> +    return mt_defs
> +
> +
> +def get_req_mt(vm: QEMUMachine, args: Namespace) -> List[Machine]:
> +    mt_defs = get_mt_definitions(vm)
> +    if args.all:
> +        return mt_defs
> +
> +    list_mt = [mt.name for mt in mt_defs]
> +
> +    if args.mt is None:
> +                print('Enter machine types for comparision or use --help')
> +                print('List of available machine types:')
> +                print(*list_mt, sep='\n')
> +                sys.exit(1)
> +
> +    for mt in args.mt:
> +        if mt not in list_mt:
> +            print('Wrong machine type name')
> +            print('List of available machine types:')
> +            print(*list_mt, sep='\n')
> +            sys.exit(1)
> +
> +    requested_mt = []
> +    for mt in mt_defs:
> +        if mt.name in args.mt:
> +            requested_mt.append(mt)
> +
> +    return requested_mt
> +
> +
> +# method to iterate through all requested properties in machine definitions
> +def get_req_props(mt_defs: list) -> Generator[Tuple[str, str], None, None]:
> +    driver_props: Dict[str, Set[str]] = {}
> +    for mt in mt_defs:
> +        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 out
> +
> +
> +def transform_number(value: str) -> Union[int, None]:
> +    try:
> +        # C doesn't work with underscore ('2_5' != 25)
> +        if '_' in value:
> +            raise ValueError
> +
> +        return int(value, 0)
> +
> +    except ValueError:
> +        return None
> +
> +
> +# delete rows with the same values for all mt and transform values to make it
> +# easier to compare
> +def transform_table(table: Dict, mt_names: List[str]) -> pd.DataFrame:
> +    new_table: Dict[str, List] = {}
> +    for full_prop_name, prop_values in table.items():
> +        new_row: List[Any] = []
> +        all_values = set()
> +        # original number format if not all values are the same in the row
> +        numeric_values = set()
> +        for mt_prop_val in prop_values:
> +            if type(mt_prop_val) is list:
> +                transformed_val_list = list(map(transform_value, mt_prop_val))
> +                if len(transformed_val_list) == 1:
> +                    new_row.append(transformed_val_list[0])
> +                else:
> +                    new_row.append(transformed_val_list)
> +
> +                numeric_values.update(set(map(transform_number, mt_prop_val)))
> +                all_values.update(set(transformed_val_list))
> +            else:
> +                transformed_val = transform_value(mt_prop_val)
> +                new_row.append(transformed_val)
> +                numeric_values.add(transform_number(mt_prop_val))
> +                all_values.add(transformed_val)
> +
> +        if len(mt_names) > 1:
> +            if len(all_values) == 1:
> +                continue
> +
> +            if None not in numeric_values and len(numeric_values) == 1:
> +                continue
> +
> +        new_table[full_prop_name] = new_row
> +
> +    return pd.DataFrame.from_dict(new_table, orient='index', columns=mt_names)
> +
> +
> +def fill_prop_table(mt_list: List[Machine],
> +                    qemu_drivers: DriverDefinitions,
> +                    is_raw: bool) -> pd.DataFrame:
> +    table: Dict[str, List] = {}
> +    for driver, prop in get_req_props(mt_list):
> +        name = f'{driver}:{prop}'
> +        table[name] = []
> +        for mt in mt_list:
> +            if driver in mt.compat_props:
> +                # values from QEMU machine type definitions
> +                if prop in mt.compat_props[driver]:
> +                    table[name].append(mt.compat_props[driver][prop])
> +                    continue
> +
> +            # values from QEMU type definitions
> +            qemu_drivers.add_prop_value(driver, prop, table[name])
> +
> +    headers = [mt.name for mt in mt_list]
> +
> +    if is_raw:
> +        return pd.DataFrame.from_dict(table, orient='index', columns=headers)
> +
> +    return transform_table(table, headers)
> +
> +
> +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(tabulate(comp_table, showindex=True, stralign='center',
> +                       colalign=('left',), tablefmt='fancy_grid',
> +                       headers='keys', disable_numparse=True))
> +
> +
> +if __name__ == '__main__':
> +    args = parse_args()
> +    qemu_arg_list = args.cmd_line.split(' ')
> +    with QEMUMachine(binary=qemu_arg_list[0],
> +                     qmp_timer=15, args=qemu_arg_list[1:]) as vm:
> +        vm.launch()
> +
> +        req_mt = get_req_mt(vm, args)
> +        qemu_drivers = DriverDefinitions(vm, qemu_default_method,
> +                                         qemu_property_methods)
> +        comp_table = fill_prop_table(req_mt, qemu_drivers, args.raw)
> +        print_table(comp_table, args.format)
> +
> +        vm.shutdown()
> --
> 2.25.1
>

Re: [PATCH v3 2/4] python/qmp: increase read buffer size

[Daniel P. Berrangé]

On Tue, Nov 08, 2022 at 03:38:21PM -0500, John Snow wrote:
> On Thu, Nov 3, 2022 at 6:29 AM Maksim Davydov
> <davydov-max@yandex-team.ru> wrote:
> >
> > After modification of "query-machines" command the buffer size should be
> > more than 452kB to contain output with compat-props.
> >
> > Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > ---
> >  python/qemu/qmp/qmp_client.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/qemu/qmp/qmp_client.py b/python/qemu/qmp/qmp_client.py
> > index 5dcda04a75..659fe4d98c 100644
> > --- a/python/qemu/qmp/qmp_client.py
> > +++ b/python/qemu/qmp/qmp_client.py
> > @@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'):
> >      #: Logger object used for debugging messages.
> >      logger = logging.getLogger(__name__)
> >
> > -    # Read buffer limit; large enough to accept query-qmp-schema
> > -    _limit = (256 * 1024)
> > +    # Read buffer limit; large enough to accept query-machines
> > +    _limit = (512 * 1024)
> 
> wow :)

Meanwhile over in python/qemu/qmp/protocol.py the read buffer limit is
set to just 64 kb.

If the current output of a particular command is known to 450 kb, then
setting this limit to 512 kb is waaaaaaay to conservative, and we'll
inevitably have to change it again when someone finds the next command
that overflows.

Recall this thread

   https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01060.html

In fact, let me be the someone who demonstrates a real case where 512kb
is not enough....

Create a 200 deep chain of qcow2 files

   qemu-img create -f qcow2 demo0.img 1G
   j=0
   for i in `seq 1 200`
   do
     qemu-img create -f qcow2 -o backing_file=demo$j.qcow2 \
                              -o backing_fmt=qcow2 demo$i.qcow2
     j=$i
   done

Launch qemu with that, and then run

  "query-named-block-nodes"

and (many minutes later) you'll get an 86 MB  QMP reply.

Now, a bare no-arg "query-named-block-nodes" is known to be a bad
command as it returns data which is a combinatorial expansion of
the number of block nodes.  Essentially it is broken by design,
and no one should use it, without setting flat=true.


If we repeat it with flat=true, then we can get a 2.7 MB QMP
reply.  This is large, but a valid reply. Basically 13 KB of
data per qcow2 file.

Libvirt caps the backing chain depth it is willing to accept at 200
qcow2 files, but allows multiple disks. So with 4 disks, each with
200 deep chain, you'll get 10.8 MB of json back. Opps....

...Libvirt's QMP reply limit is 10 MB, so even libvirt will break
quite quickly. Libvirt can cope with around 787 qcow2 files in
at 13 kb per file. NB I'm assuming paths under the dir
/var/lib/libvirt/images/, shorter paths will allow for more.


The 64 KB limit will only cope with 4 qcow2 files before breaking,
while a 512 KB limit will only cope with 39 files before breaking.
Neither is anywhere near sufficient.

I'd suggest following libvirt here and setting the read limits to
10 MB.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 2/4] python/qmp: increase read buffer size

[Daniel P. Berrangé]

On Wed, Nov 09, 2022 at 09:39:14AM +0000, Daniel P. Berrangé wrote:
> On Tue, Nov 08, 2022 at 03:38:21PM -0500, John Snow wrote:
> > On Thu, Nov 3, 2022 at 6:29 AM Maksim Davydov
> > <davydov-max@yandex-team.ru> wrote:
> > >
> > > After modification of "query-machines" command the buffer size should be
> > > more than 452kB to contain output with compat-props.
> > >
> > > Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> > > ---
> > >  python/qemu/qmp/qmp_client.py | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/python/qemu/qmp/qmp_client.py b/python/qemu/qmp/qmp_client.py
> > > index 5dcda04a75..659fe4d98c 100644
> > > --- a/python/qemu/qmp/qmp_client.py
> > > +++ b/python/qemu/qmp/qmp_client.py
> > > @@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'):
> > >      #: Logger object used for debugging messages.
> > >      logger = logging.getLogger(__name__)
> > >
> > > -    # Read buffer limit; large enough to accept query-qmp-schema
> > > -    _limit = (256 * 1024)
> > > +    # Read buffer limit; large enough to accept query-machines
> > > +    _limit = (512 * 1024)
> > 
> > wow :)
> 
> Meanwhile over in python/qemu/qmp/protocol.py the read buffer limit is
> set to just 64 kb.
> 
> If the current output of a particular command is known to 450 kb, then
> setting this limit to 512 kb is waaaaaaay to conservative, and we'll
> inevitably have to change it again when someone finds the next command
> that overflows.
> 
> Recall this thread
> 
>    https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01060.html
> 
> In fact, let me be the someone who demonstrates a real case where 512kb
> is not enough....

Another example...

Create a guest with 255 vCPUs (current RHEL downstream vCPU limit),
and run

  {"execute":"query-stats","arguments":{"target": "vcpu"}}

it'll get back a 0.38 MB  QMP reply.  RHEL raised the limit to 710
vCPUs, giving a little over 1 MB QMP reply. There is a strong desire
to go even higher. With 4096 vCPUs it'd get an ~6 MB QMP reply.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 2/4] python/qmp: increase read buffer size

[John Snow]

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

On Wed, Nov 9, 2022, 6:00 AM Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Nov 09, 2022 at 09:39:14AM +0000, Daniel P. Berrangé wrote:
> > On Tue, Nov 08, 2022 at 03:38:21PM -0500, John Snow wrote:
> > > On Thu, Nov 3, 2022 at 6:29 AM Maksim Davydov
> > > <davydov-max@yandex-team.ru> wrote:
> > > >
> > > > After modification of "query-machines" command the buffer size
> should be
> > > > more than 452kB to contain output with compat-props.
> > > >
> > > > Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru
> >
> > > > ---
> > > >  python/qemu/qmp/qmp_client.py | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/python/qemu/qmp/qmp_client.py
> b/python/qemu/qmp/qmp_client.py
> > > > index 5dcda04a75..659fe4d98c 100644
> > > > --- a/python/qemu/qmp/qmp_client.py
> > > > +++ b/python/qemu/qmp/qmp_client.py
> > > > @@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'):
> > > >      #: Logger object used for debugging messages.
> > > >      logger = logging.getLogger(__name__)
> > > >
> > > > -    # Read buffer limit; large enough to accept query-qmp-schema
> > > > -    _limit = (256 * 1024)
> > > > +    # Read buffer limit; large enough to accept query-machines
> > > > +    _limit = (512 * 1024)
> > >
> > > wow :)
> >
> > Meanwhile over in python/qemu/qmp/protocol.py the read buffer limit is
> > set to just 64 kb.
>

This one will override the other - the protocol limit is for any arbitrary
full-duplex message-based protocol. It can stay at the lower limit.

(I used protocol.py to implement a qtest client as well, though I didn't
upstream that piece. If there's interest, I will.)

>
> > If the current output of a particular command is known to 450 kb, then
> > setting this limit to 512 kb is waaaaaaay to conservative, and we'll
> > inevitably have to change it again when someone finds the next command
> > that overflows.
> >
> > Recall this thread
> >
> >    https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg01060.html
> >
> > In fact, let me be the someone who demonstrates a real case where 512kb
> > is not enough....
>
> Another example...
>
> Create a guest with 255 vCPUs (current RHEL downstream vCPU limit),
> and run
>
>   {"execute":"query-stats","arguments":{"target": "vcpu"}}
>
> it'll get back a 0.38 MB  QMP reply.  RHEL raised the limit to 710
> vCPUs, giving a little over 1 MB QMP reply. There is a strong desire
> to go even higher. With 4096 vCPUs it'd get an ~6 MB QMP reply.
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>

You're right, of course. I recalled the thread but I was being lazy about
it. (Sorry.) I thought, naively, that it was better to speed Maksim along
for now.

I recall you (Daniel) stating that libvirt used a default of 10MB (iirc).
I'd be happy to adopt that default as well, if only for parity.

Maksim, can I trouble you to send a revised patch as an MR to
gitlab.com/qemu-project/python-qemu-qmp ? If not, a revised patch to the
mailing list here is fine and with your permission I'll forward-port it
over to the standalone repo myself. (Or I can just handle it entirely
myself, if you'd prefer - just let me know.)

Sorry for the fuss, and thanks for helping to improve QMP testing and
tooling

--js

>

[-- Attachment #2: Type: text/html, Size: 6100 bytes --]

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

[Maksim Davydov]

On 11/9/22 00:16, John Snow wrote:
> On Thu, Nov 3, 2022 at 6:29 AM Maksim Davydov
> <davydov-max@yandex-team.ru> wrote:
>> This script run QEMU to obtain compat_props of machines and default
>> values of different types and produce appropriate table. This table
>> can be used to compare machine types to choose the most suitable
>> machine. Also this table in json or csv format should be used to check that
>> new machine doesn't affect previous ones by comparing tables with and
>> without new machine.
>> Default values of properties are needed to fill "holes" in the table (one
>> machine has these properties and another not. For instance, 2.12 mt has
>> `{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x8000000a" }`, but compat_pros of
>> 3.1 mt doesn't have it. So, to compare these machines we need to fill
>> unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 mt. This unknown value
>> in the table I called "hole". To get values (default 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)
>>
>> Example:
>>
>> ./scripts/compare_mt.py --mt pc-q35-3.1 pc-q35-4.0
>>
>> ╒═══════════════════════════════════════════════════════════╤══════════════╤════════════════════╕
>> │                                                           │  pc-q35-3.1  │     pc-q35-4.0     │
>> ╞═══════════════════════════════════════════════════════════╪══════════════╪════════════════════╡
>> │ Cascadelake-Server-x86_64-cpu:mpx                         │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Cascadelake-Server-x86_64-cpu:stepping                    │      5       │         6          │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Icelake-Client-x86_64-cpu:mpx                             │     True     │ unavailable driver │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Icelake-Server-x86_64-cpu:mpx                             │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Opteron_G3-x86_64-cpu:rdtscp                              │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Opteron_G4-x86_64-cpu:rdtscp                              │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Opteron_G5-x86_64-cpu:rdtscp                              │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Skylake-Client-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Skylake-Client-x86_64-cpu:mpx                             │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Skylake-Server-IBRS-x86_64-cpu:mpx                        │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ Skylake-Server-x86_64-cpu:mpx                             │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ intel-iommu:dma-drain                                     │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ memory-backend-file:x-use-canonical-path-for-ramblock-id  │     True     │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ memory-backend-memfd:x-use-canonical-path-for-ramblock-id │     True     │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ pcie-root-port:x-speed                                    │     2_5      │         16         │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ pcie-root-port:x-width                                    │      1       │         32         │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ pcie-root-port-base:disable-acs                           │     True     │       False        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ tpm-crb:ppi                                               │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ tpm-tis:ppi                                               │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ usb-kbd:serial                                            │      42      │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ usb-mouse:serial                                          │      42      │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ usb-tablet:serial                                         │      42      │  no default value  │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ virtio-balloon-device:qemu-4-0-config-size                │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ virtio-blk-device:discard                                 │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ virtio-blk-device:write-zeroes                            │    False     │        True        │
>> ├───────────────────────────────────────────────────────────┼──────────────┼────────────────────┤
>> │ x86_64-cpu:x-intel-pt-auto-level                          │    False     │        True        │
>> ╘═══════════════════════════════════════════════════════════╧══════════════╧════════════════════╛
>>
>> Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
> Do you expect this script to be used by any other portion of the
> build, or is this merely an interactive tool for developers to
> consult? You have the option of moving it into python/qemu/utils/, but
> the linting system will be a lot more stringent there. For your
> troubles, I'll be able to protect it from bitrot more proactively.
> It's up to you -- but it looks like you did try to statically type it
> already, so maybe a lot of the pain and gruntwork has already been
> done.
>
I apologize for replying late
The purposes of this script were the following:
1) help to choose an appropriate machine for the case by answering the
    question "how does the first_machine differ from the second_machine?"
2) add build check to be sure that other machines are not corrupted after
    the modification of one machine
If it's more correct, then I'll move the script into python/qemu/utils/
>> ---
>>   scripts/compare_mt.py | 440 ++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 440 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..31ac86dddd
>> --- /dev/null
>> +++ b/scripts/compare_mt.py
>> @@ -0,0 +1,440 @@
>> +#!/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 comparision, some machine type doesn't have a property (it is in
>> +# the comparision table because another machine type has it), then the
> "comparison", not "comparision", previous two lines.
>
>> +# 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_propery_methods.
>> +#
>> +# Copyright (c) Yandex Technologies LLC, 2022
>> +#
>> +# 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/>.
>> +
>> +from tabulate import tabulate
>> +import sys
>> +from os import path
>> +from argparse import ArgumentParser, RawTextHelpFormatter, Namespace
>> +import pandas as pd
>> +from typing import Callable, List, Dict, Set, Generator, Tuple, Union, Any
>> +
>> +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_cmd_line = 'build/qemu-system-x86_64 -enable-kvm -machine none'
>> +
>> +
>> +# Methods to get right values of drivers props
>> +#
>> +# 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:
>> +# * Names should be in qom-list-types format (486-x86_64-cpu, not 486)
>> +# * Specialization always wins (from 'device' and 'x86_64-cpu', 'x86_64-cpu'
>> +#   will be used for '486-x86_64-cpu')
>> +
>> +# It's default stub for all undefined in property_methods drivers because all
>> +# QEMU types are inherited from Object
>> +def get_object_prop(vm: QEMUMachine, device: str, prop_name: str):
>> +    return 'Unavailable method'
>> +
>> +
>> +def get_device_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
>> +    device_props = vm.command('device-list-properties', typename=device)
>> +    for prop in device_props:
>> +        if prop['name'] == prop_name:
>> +            return str(prop.get('default-value', 'No default value'))
>> +
>> +    return 'Unknown property'
>> +
>> +
>> +def get_x86_64_cpu_prop(vm: QEMUMachine, device: str, prop_name: str) -> str:
>> +    # crop last 11 chars '-x86_64-cpu'
>> +    props = vm.command('query-cpu-model-expansion', type='full',
>> +                       model={'name': device[:-11]})['model']['props']
>> +    return str(props.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
>> +def get_memory_backend_prop(vm: QEMUMachine, driver: str,
>> +                            prop_name: str) -> str:
>> +    memory_backend_props = vm.command('qom-list-properties', typename=driver)
>> +    for prop in memory_backend_props:
>> +        if prop['name'] == prop_name:
>> +            return str(prop.get('default-value', 'No default value'))
>> +
>> +    return 'Unknown property'
>> +
>> +
>> +class GetPropMethod:
>> +    def __init__(self, driver_name: str,
>> +                 method: Callable[[QEMUMachine, str, str], str]) -> None:
>> +        self.name = driver_name
>> +        self.get_prop = method
>> +
>> +
>> +qemu_property_methods = [
>> +        GetPropMethod('device', get_device_prop),
>> +        GetPropMethod('x86_64-cpu', get_x86_64_cpu_prop),
>> +        GetPropMethod('memory-backend', get_memory_backend_prop)
>> +]
>> +
>> +# all types in QEMU are inherited from Object
>> +qemu_default_method = GetPropMethod('object', get_object_prop)
>> +# End of methods definition
>> +
>> +
>> +class Driver:
>> +    def __init__(self, driver_defs: Dict, driver_name: str, parent_name: str,
>> +                 is_abstr: bool, list_of_children: List[str],
>> +                 get_prop_method: GetPropMethod) -> None:
>> +        self.driver_defs = driver_defs
>> +        self.name = driver_name
>> +        self.parent = parent_name
>> +        self.abstract = is_abstr
>> +        self.children = list_of_children
>> +        self.method = get_prop_method
>> +
>> +
>> +    def is_parent(self, driver_name: str) -> bool:
>> +        if driver_name not in self.driver_defs:
>> +            return False
>> +
>> +        cur_parent = self.parent
>> +        while cur_parent:
>> +            if driver_name == cur_parent:
>> +                return True
>> +            cur_parent = self.driver_defs[cur_parent].parent
>> +
>> +        return False
>> +
>> +
>> +    def set_prop_method(self, prop_method: GetPropMethod) -> None:
>> +        if prop_method.name != self.name:
>> +            return
>> +
>> +        self.method = prop_method
>> +        if not self.abstract:
>> +            return
>> +
>> +        for child in self.children:
>> +            # specialization always wins
>> +            if self.is_parent(self.driver_defs[child].method.name):
>> +                self.driver_defs[child].method = prop_method
>> +
>> +
>> +class DriverDefinitions:
>> +    def __init__(self, vm: QEMUMachine, default_method: GetPropMethod,
>> +                 property_methods: List[GetPropMethod]) -> None:
>> +        self.driver_defs: Dict[str, Driver] = {}
>> +        self.default_method = default_method
>> +        self.property_methods = property_methods
>> +        self.vm = vm
>> +
>> +        qom_all_types = vm.command('qom-list-types', abstract=True)
>> +        for obj_type in qom_all_types:
>> +            # parent of Object is None
>> +            parent = obj_type.get('parent', None)
>> +            abstr = obj_type.get('abstract', False)
>> +            name = obj_type['name']
>> +            if abstr:
>> +                list_child_objs = vm.command('qom-list-types', implements=name,
>> +                                             abstract=True)
>> +                child_list = [child['name'] for child in list_child_objs]
>> +                self.driver_defs[name] = Driver(self.driver_defs, name, parent,
>> +                                                abstr, child_list,
>> +                                                default_method)
>> +            else:
>> +                self.driver_defs[name] = Driver(self.driver_defs, name, parent,
>> +                                                abstr, [], default_method)
>> +
>> +        for prop_method in property_methods:
>> +            # skipping other architectures and etc
>> +            if prop_method.name not in self.driver_defs:
>> +                continue
>> +            self.driver_defs[prop_method.name].set_prop_method(prop_method)
>> +
>> +
>> +    def add_prop_value(self, driver: str, prop: str, prop_list: list) -> None:
>> +        # wrong driver name or disabled in config driver
>> +        if driver not in self.driver_defs:
>> +            prop_list.append('Unavailable driver')
>> +            return
>> +
>> +        if not self.driver_defs[driver].abstract:
>> +            prop_list.append(self.driver_defs[driver].method.get_prop(self.vm,
>> +                                                                      driver,
>> +                                                                      prop))
>> +            return
>> +
>> +        # if abstract we need to collect default values from all children
>> +        values = set()
>> +        for child in self.driver_defs[driver].children:
>> +            if self.driver_defs[child].abstract:
>> +                continue
>> +
>> +            values.add(self.driver_defs[child].method.get_prop(self.vm, child,
>> +                                                               prop))
>> +
>> +        prop_list.append(list(values))
>> +
>> +
>> +class Machine:
>> +    # raw_mt_dict - dict produced by `query-machines`
>> +    def __init__(self, raw_mt_dict: dict) -> None:
>> +        self.name = raw_mt_dict['name']
>> +        self.compat_props: Dict[str, Dict[str, str]] = {}
>> +        # properties are applied sequentially and can rewrite values as in QEMU
>> +        for prop in raw_mt_dict['compat-props']:
>> +            if prop['driver'] not in self.compat_props:
>> +                self.compat_props[prop['driver']] = {}
>> +            self.compat_props[prop['driver']][prop['property']] = prop['value']
>> +
>> +
>> +script_desc="""Script to compare machine types (their compat_props).
>> +
>> +If a property applies to an abstract class this script collects default \
>> +values of all child classes and prints them as a set.
>> +
>> +"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 properties '
>> +                             'with different values will be printed and with '
>> +                             'value transformation(e.g. "on" -> True)')
>> +    parser.add_argument('--cmd-line', default=default_cmd_line,
>> +                        help='command line to start qemu. '
>> +                             f'Default: "{default_cmd_line}"')
>> +
>> +    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). '
>> +                                    'Execution may take several minutes!')
>> +    mt_args_group.add_argument('--mt', nargs="*", type=str,
>> +                               help='list of Machine Types '
>> +                                    'that will be compared')
>> +
>> +    return parser.parse_args()
>> +
>> +
>> +# return socket_name, major version, minor version, revision
>> +def mt_comp(mt: Machine) -> Tuple[str, int, int, int]:
>> +    # 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]
>> +
>> +
>> +# construct list of machine type definitions (primarily compat_props) from QEMU
>> +def get_mt_definitions(vm: QEMUMachine) -> List[Machine]:
>> +    raw_mt_defs = vm.command('query-machines', compat_props=True)
>> +    mt_defs: List[Machine] = []
>> +    for raw_mt in raw_mt_defs:
>> +        mt_defs.append(Machine(raw_mt))
>> +
>> +    mt_defs.sort(key=mt_comp)
>> +    return mt_defs
>> +
>> +
>> +def get_req_mt(vm: QEMUMachine, args: Namespace) -> List[Machine]:
>> +    mt_defs = get_mt_definitions(vm)
>> +    if args.all:
>> +        return mt_defs
>> +
>> +    list_mt = [mt.name for mt in mt_defs]
>> +
>> +    if args.mt is None:
>> +                print('Enter machine types for comparision or use --help')
>> +                print('List of available machine types:')
>> +                print(*list_mt, sep='\n')
>> +                sys.exit(1)
>> +
>> +    for mt in args.mt:
>> +        if mt not in list_mt:
>> +            print('Wrong machine type name')
>> +            print('List of available machine types:')
>> +            print(*list_mt, sep='\n')
>> +            sys.exit(1)
>> +
>> +    requested_mt = []
>> +    for mt in mt_defs:
>> +        if mt.name in args.mt:
>> +            requested_mt.append(mt)
>> +
>> +    return requested_mt
>> +
>> +
>> +# method to iterate through all requested properties in machine definitions
>> +def get_req_props(mt_defs: list) -> Generator[Tuple[str, str], None, None]:
>> +    driver_props: Dict[str, Set[str]] = {}
>> +    for mt in mt_defs:
>> +        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 out
>> +
>> +
>> +def transform_number(value: str) -> Union[int, None]:
>> +    try:
>> +        # C doesn't work with underscore ('2_5' != 25)
>> +        if '_' in value:
>> +            raise ValueError
>> +
>> +        return int(value, 0)
>> +
>> +    except ValueError:
>> +        return None
>> +
>> +
>> +# delete rows with the same values for all mt and transform values to make it
>> +# easier to compare
>> +def transform_table(table: Dict, mt_names: List[str]) -> pd.DataFrame:
>> +    new_table: Dict[str, List] = {}
>> +    for full_prop_name, prop_values in table.items():
>> +        new_row: List[Any] = []
>> +        all_values = set()
>> +        # original number format if not all values are the same in the row
>> +        numeric_values = set()
>> +        for mt_prop_val in prop_values:
>> +            if type(mt_prop_val) is list:
>> +                transformed_val_list = list(map(transform_value, mt_prop_val))
>> +                if len(transformed_val_list) == 1:
>> +                    new_row.append(transformed_val_list[0])
>> +                else:
>> +                    new_row.append(transformed_val_list)
>> +
>> +                numeric_values.update(set(map(transform_number, mt_prop_val)))
>> +                all_values.update(set(transformed_val_list))
>> +            else:
>> +                transformed_val = transform_value(mt_prop_val)
>> +                new_row.append(transformed_val)
>> +                numeric_values.add(transform_number(mt_prop_val))
>> +                all_values.add(transformed_val)
>> +
>> +        if len(mt_names) > 1:
>> +            if len(all_values) == 1:
>> +                continue
>> +
>> +            if None not in numeric_values and len(numeric_values) == 1:
>> +                continue
>> +
>> +        new_table[full_prop_name] = new_row
>> +
>> +    return pd.DataFrame.from_dict(new_table, orient='index', columns=mt_names)
>> +
>> +
>> +def fill_prop_table(mt_list: List[Machine],
>> +                    qemu_drivers: DriverDefinitions,
>> +                    is_raw: bool) -> pd.DataFrame:
>> +    table: Dict[str, List] = {}
>> +    for driver, prop in get_req_props(mt_list):
>> +        name = f'{driver}:{prop}'
>> +        table[name] = []
>> +        for mt in mt_list:
>> +            if driver in mt.compat_props:
>> +                # values from QEMU machine type definitions
>> +                if prop in mt.compat_props[driver]:
>> +                    table[name].append(mt.compat_props[driver][prop])
>> +                    continue
>> +
>> +            # values from QEMU type definitions
>> +            qemu_drivers.add_prop_value(driver, prop, table[name])
>> +
>> +    headers = [mt.name for mt in mt_list]
>> +
>> +    if is_raw:
>> +        return pd.DataFrame.from_dict(table, orient='index', columns=headers)
>> +
>> +    return transform_table(table, headers)
>> +
>> +
>> +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(tabulate(comp_table, showindex=True, stralign='center',
>> +                       colalign=('left',), tablefmt='fancy_grid',
>> +                       headers='keys', disable_numparse=True))
>> +
>> +
>> +if __name__ == '__main__':
>> +    args = parse_args()
>> +    qemu_arg_list = args.cmd_line.split(' ')
>> +    with QEMUMachine(binary=qemu_arg_list[0],
>> +                     qmp_timer=15, args=qemu_arg_list[1:]) as vm:
>> +        vm.launch()
>> +
>> +        req_mt = get_req_mt(vm, args)
>> +        qemu_drivers = DriverDefinitions(vm, qemu_default_method,
>> +                                         qemu_property_methods)
>> +        comp_table = fill_prop_table(req_mt, qemu_drivers, args.raw)
>> +        print_table(comp_table, args.format)
>> +
>> +        vm.shutdown()
>> --
>> 2.25.1
>>
-- 
Best regards,
Maksim Davydov