[PULL 0/4] machine development tool

[PULL 0/4] machine development tool

[Maksim Davydov]

The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:

  Merge tag 'pull-request-2024-03-01' of https://gitlab.com/thuth/qemu into staging (2024-03-01 10:14:32 +0000)

are available in the Git repository at:

  https://gitlab.com/davydov-max/qemu.git tags/pull-compare-mt-2024-03-04

for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:

  scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)

----------------------------------------------------------------
Please note. This is the first pull request from me.
My public GPG key is available here
https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4

----------------------------------------------------------------
scripts: add a new script for machine development

----------------------------------------------------------------

Maksim Davydov (4):
  qom: add default value
  qmp: add dump machine type compatibility properties
  python/qemu/machine: add method to retrieve QEMUMachine::binary field
  scripts: add script to compare compatibility properties

 MAINTAINERS                      |   5 +
 hw/core/machine-qmp-cmds.c       |  23 +-
 python/qemu/machine/machine.py   |   5 +
 qapi/machine.json                |  69 ++++-
 qom/qom-qmp-cmds.c               |   1 +
 scripts/compare-machine-types.py | 486 +++++++++++++++++++++++++++++++
 tests/qtest/fuzz/qos_fuzz.c      |   2 +-
 7 files changed, 586 insertions(+), 5 deletions(-)
 create mode 100755 scripts/compare-machine-types.py

-- 
2.34.1

[PULL 1/4] qom: add default value

[Maksim Davydov]

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

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 qom/qom-qmp-cmds.c | 1 +
 1 file changed, 1 insertion(+)

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

[PULL 2/4] qmp: add dump machine type compatibility 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 compatibility 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. Since information on compatibility
properties can increase the command output by a factor of 40, add an
argument to enable it, default off.

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
 hw/core/machine-qmp-cmds.c  | 23 ++++++++++++-
 qapi/machine.json           | 69 +++++++++++++++++++++++++++++++++++--
 tests/qtest/fuzz/qos_fuzz.c |  2 +-
 3 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 3860a50c3b..4e908e12d3 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -66,7 +66,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
     return head;
 }
 
-MachineInfoList *qmp_query_machines(Error **errp)
+MachineInfoList *qmp_query_machines(bool has_compat_props, bool compat_props,
+                                    Error **errp)
 {
     GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
     MachineInfoList *mach_list = NULL;
@@ -98,6 +99,26 @@ MachineInfoList *qmp_query_machines(Error **errp)
             info->default_ram_id = g_strdup(mc->default_ram_id);
         }
 
+        if (compat_props && mc->compat_props) {
+            int i;
+            info->compat_props = NULL;
+            CompatPropertyList **tail = &(info->compat_props);
+            info->has_compat_props = true;
+
+            for (i = 0; i < mc->compat_props->len; i++) {
+                GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props,
+                                                            i);
+                CompatProperty *prop;
+
+                prop = g_malloc0(sizeof(*prop));
+                prop->qom_type = 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 93b4677286..4850ef212e 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -135,6 +135,26 @@
 ##
 { 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
 
+##
+# @CompatProperty:
+#
+# Property default values specific to a machine type, for use by
+# scripts/compare-machine-types.
+#
+# @qom-type: name of the QOM type to which the default applies
+#
+# @property: name of its property to which the default applies
+#
+# @value: the default value (machine-specific default can overwrite
+#     the "default" default, to avoid this use -machine none)
+#
+# Since: 9.0
+##
+{ 'struct': 'CompatProperty',
+  'data': { 'qom-type': 'str',
+            'property': 'str',
+            'value': 'str' } }
+
 ##
 # @MachineInfo:
 #
@@ -166,6 +186,14 @@
 #
 # @acpi: machine type supports ACPI (since 8.0)
 #
+# @compat-props: The machine type's compatibility properties.  Only
+#     present when query-machines argument @compat-props is true.
+#     (since 9.0)
+#
+# Features:
+#
+# @unstable: Member @compat-props is experimental.
+#
 # Since: 1.2
 ##
 { 'struct': 'MachineInfo',
@@ -173,18 +201,53 @@
             '*is-default': 'bool', 'cpu-max': 'int',
             'hotpluggable-cpus': 'bool',  'numa-mem-supported': 'bool',
             'deprecated': 'bool', '*default-cpu-type': 'str',
-            '*default-ram-id': 'str', 'acpi': 'bool' } }
+            '*default-ram-id': 'str', 'acpi': 'bool',
+            '*compat-props': { 'type': ['CompatProperty'],
+                               'features': ['unstable'] } } }
 
 ##
 # @query-machines:
 #
 # Return a list of supported machines
 #
+# @compat-props: if true, also return compatibility properties.
+#     (default: false) (since 9.0)
+#
+# Features:
+#
+# @unstable: Argument @compat-props is experimental.
+#
 # Returns: a list of MachineInfo
 #
 # Since: 1.2
-##
-{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+#
+# Example:
+#
+# -> { "execute": "query-machines", "arguments": { "compat-props": true } }
+# <- { "return": [
+#          {
+#              "hotpluggable-cpus": true,
+#              "name": "pc-q35-6.2",
+#              "compat-props": [
+#                  {
+#                      "qom-type": "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',
+  'data': { '*compat-props': { 'type': 'bool',
+                               'features': [ 'unstable' ] } },
+  'returns': ['MachineInfo'] }
 
 ##
 # @CurrentMachineParams:
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index e403d373a0..b71e945c5f 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -46,7 +46,7 @@ static void qos_set_machines_devices_available(void)
     MachineInfoList *mach_info;
     ObjectTypeInfoList *type_info;
 
-    mach_info = qmp_query_machines(&error_abort);
+    mach_info = qmp_query_machines(false, false, &error_abort);
     machines_apply_to_node(mach_info);
     qapi_free_MachineInfoList(mach_info);
 
-- 
2.34.1

[PULL 3/4] python/qemu/machine: add method to retrieve QEMUMachine::binary field

[Maksim Davydov]

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

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 python/qemu/machine/machine.py | 5 +++++
 1 file changed, 5 insertions(+)

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

[PULL 4/4] scripts: add script to compare compatibility properties

[Maksim Davydov]

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

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

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

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

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 MAINTAINERS                      |   5 +
 scripts/compare-machine-types.py | 486 +++++++++++++++++++++++++++++++
 2 files changed, 491 insertions(+)
 create mode 100755 scripts/compare-machine-types.py

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

Re: [PULL 0/4] machine development tool

[Peter Maydell]

On Mon, 4 Mar 2024 at 13:52, Maksim Davydov <davydov-max@yandex-team.ru> wrote:
>
> The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
>
>   Merge tag 'pull-request-2024-03-01' of https://gitlab.com/thuth/qemu into staging (2024-03-01 10:14:32 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/davydov-max/qemu.git tags/pull-compare-mt-2024-03-04
>
> for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
>
>   scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
>
> ----------------------------------------------------------------
> Please note. This is the first pull request from me.
> My public GPG key is available here
> https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
>
> ----------------------------------------------------------------
> scripts: add a new script for machine development
>
> ----------------------------------------------------------------

Hi; I would prefer this to go through some existing submaintainer
tree if possible, please.

thanks
-- PMM

Re: [PULL 0/4] machine development tool

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 4 Mar 2024 at 13:52, Maksim Davydov <davydov-max@yandex-team.ru> wrote:
>>
>> The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
>>
>>   Merge tag 'pull-request-2024-03-01' of https://gitlab.com/thuth/qemu into staging (2024-03-01 10:14:32 +0000)
>>
>> are available in the Git repository at:
>>
>>   https://gitlab.com/davydov-max/qemu.git tags/pull-compare-mt-2024-03-04
>>
>> for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
>>
>>   scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
>>
>> ----------------------------------------------------------------
>> Please note. This is the first pull request from me.
>> My public GPG key is available here
>> https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
>>
>> ----------------------------------------------------------------
>> scripts: add a new script for machine development
>>
>> ----------------------------------------------------------------
>
> Hi; I would prefer this to go through some existing submaintainer
> tree if possible, please.

Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.

Re: [PULL 0/4] machine development tool

[Peter Xu]

On Tue, Mar 05, 2024 at 03:43:41PM +0100, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
> > On Mon, 4 Mar 2024 at 13:52, Maksim Davydov <davydov-max@yandex-team.ru> wrote:
> >>
> >> The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
> >>
> >>   Merge tag 'pull-request-2024-03-01' of https://gitlab.com/thuth/qemu into staging (2024-03-01 10:14:32 +0000)
> >>
> >> are available in the Git repository at:
> >>
> >>   https://gitlab.com/davydov-max/qemu.git tags/pull-compare-mt-2024-03-04
> >>
> >> for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
> >>
> >>   scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
> >>
> >> ----------------------------------------------------------------
> >> Please note. This is the first pull request from me.
> >> My public GPG key is available here
> >> https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
> >>
> >> ----------------------------------------------------------------
> >> scripts: add a new script for machine development
> >>
> >> ----------------------------------------------------------------
> >
> > Hi; I would prefer this to go through some existing submaintainer
> > tree if possible, please.
> 
> Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.

Yeah this seems like migration relevant.. however now I'm slightly confused
on when this script should be useful.

According to:

https://lore.kernel.org/qemu-devel/20240222153912.646053-5-davydov-max@yandex-team.ru/

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

In regards to "choose the most suitable machine": why do you need to choose
a machine?

If it's pretty standalone setup, shouldn't we always try to use the latest
machine type if possible (as normally compat props are only used to keep
compatible with old machine types, and the default should always be
preferred). If it's a cluster setup, IMHO it should depend on the oldest
QEMU version that plans to be supported.  I don't see how such comparison
helps yet in either of the cases..

In regards to "compare binaries to be sure that migration to the newer
version will save all device properties": do we even support migrating
_between_ machine types??

Sololy relying on compat properties to detect device compatibility is also
not reliable.  For example, see VMStateField.field_exists() or similarly,
VMStateDescription.needed(), which are hooks that device can provide to
dynamically decide what device state to be saved/loaded.  Such things are
not reflected in compat properties, so even if compat properties of all
devices are the same between two machine types, it may not mean that the
migration stream will always be compatible.

Thanks,

-- 
Peter Xu

Re: [PULL 0/4] machine development tool

[Maksim Davydov]

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


On 3/6/24 04:57, Peter Xu wrote:
> On Tue, Mar 05, 2024 at 03:43:41PM +0100, Markus Armbruster wrote:
>> Peter Maydell<peter.maydell@linaro.org>  writes:
>>
>>> On Mon, 4 Mar 2024 at 13:52, Maksim Davydov<davydov-max@yandex-team.ru>  wrote:
>>>> The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
>>>>
>>>>    Merge tag 'pull-request-2024-03-01' ofhttps://gitlab.com/thuth/qemu  into staging (2024-03-01 10:14:32 +0000)
>>>>
>>>> are available in the Git repository at:
>>>>
>>>>    https://gitlab.com/davydov-max/qemu.git  tags/pull-compare-mt-2024-03-04
>>>>
>>>> for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
>>>>
>>>>    scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
>>>>
>>>> ----------------------------------------------------------------
>>>> Please note. This is the first pull request from me.
>>>> My public GPG key is available here
>>>> https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
>>>>
>>>> ----------------------------------------------------------------
>>>> scripts: add a new script for machine development
>>>>
>>>> ----------------------------------------------------------------
>>> Hi; I would prefer this to go through some existing submaintainer
>>> tree if possible, please.
>> Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.
> Yeah this seems like migration relevant.. however now I'm slightly confused
> on when this script should be useful.
>
> According to:
>
> https://lore.kernel.org/qemu-devel/20240222153912.646053-5-davydov-max@yandex-team.ru/
>
>          This script runs QEMU to obtain compat_props of machines and
>          default values of different types of drivers to produce comparison
>          table. This table can be used to compare machine types to choose
>          the most suitable machine or compare binaries to be sure that
>          migration to the newer version will save all device
>          properties. Also the json or csv format of this table can be used
>          to check does a new machine affect the previous ones by comparing
>          tables with and without the new machine.
>
> In regards to "choose the most suitable machine": why do you need to choose
> a machine?
>
> If it's pretty standalone setup, shouldn't we always try to use the latest
> machine type if possible (as normally compat props are only used to keep
> compatible with old machine types, and the default should always be
> preferred). If it's a cluster setup, IMHO it should depend on the oldest
> QEMU version that plans to be supported.  I don't see how such comparison
> helps yet in either of the cases..
>
> In regards to "compare binaries to be sure that migration to the newer
> version will save all device properties": do we even support migrating
> _between_ machine types??
>
> Sololy relying on compat properties to detect device compatibility is also
> not reliable.  For example, see VMStateField.field_exists() or similarly,
> VMStateDescription.needed(), which are hooks that device can provide to
> dynamically decide what device state to be saved/loaded.  Such things are
> not reflected in compat properties, so even if compat properties of all
> devices are the same between two machine types, it may not mean that the
> migration stream will always be compatible.
>
> Thanks,

In fact, the last commit describes the meaning of this series best. Perhaps
it should have been moved to the cover letter:
Often, many teams have their own "machines" inherited from "upstream" to
manage default values of devices. This is done because of the limitations
imposed by the compatibility requirements or the desire to help their
customers better configure their devices. And since machine type has
a hard-to-read structure, it is very easy to make a mistake when 
transferring
default values from one machine to another. For example, when some property
is set for the entire abstract class x86_64-cpu (which will be applied 
to all
models), and then rolled back for a specific model. The situation is about
the same with changing the default values of devices: if the value changes
in the description of the device itself, then you need to make sure that
nothing changes when using the current machine.
Therefore, there was a desire to make a dev tool that will help quickly 
expand
the definition of a machine or compare several machines from different 
binary
files. It can be used when rebasing to a new version of qemu and/or for
automatic tests.

-- 
Best regards,
Maksim Davydov

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

Re: [PULL 0/4] machine development tool

[Peter Xu]

On Thu, Mar 07, 2024 at 12:06:59PM +0300, Maksim Davydov wrote:
> 
> On 3/6/24 04:57, Peter Xu wrote:
> > On Tue, Mar 05, 2024 at 03:43:41PM +0100, Markus Armbruster wrote:
> > > Peter Maydell<peter.maydell@linaro.org>  writes:
> > > 
> > > > On Mon, 4 Mar 2024 at 13:52, Maksim Davydov<davydov-max@yandex-team.ru>  wrote:
> > > > > The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
> > > > > 
> > > > >    Merge tag 'pull-request-2024-03-01' ofhttps://gitlab.com/thuth/qemu  into staging (2024-03-01 10:14:32 +0000)
> > > > > 
> > > > > are available in the Git repository at:
> > > > > 
> > > > >    https://gitlab.com/davydov-max/qemu.git  tags/pull-compare-mt-2024-03-04
> > > > > 
> > > > > for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
> > > > > 
> > > > >    scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
> > > > > 
> > > > > ----------------------------------------------------------------
> > > > > Please note. This is the first pull request from me.
> > > > > My public GPG key is available here
> > > > > https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
> > > > > 
> > > > > ----------------------------------------------------------------
> > > > > scripts: add a new script for machine development
> > > > > 
> > > > > ----------------------------------------------------------------
> > > > Hi; I would prefer this to go through some existing submaintainer
> > > > tree if possible, please.
> > > Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.
> > Yeah this seems like migration relevant.. however now I'm slightly confused
> > on when this script should be useful.
> > 
> > According to:
> > 
> > https://lore.kernel.org/qemu-devel/20240222153912.646053-5-davydov-max@yandex-team.ru/
> > 
> >          This script runs QEMU to obtain compat_props of machines and
> >          default values of different types of drivers to produce comparison
> >          table. This table can be used to compare machine types to choose
> >          the most suitable machine or compare binaries to be sure that
> >          migration to the newer version will save all device
> >          properties. Also the json or csv format of this table can be used
> >          to check does a new machine affect the previous ones by comparing
> >          tables with and without the new machine.
> > 
> > In regards to "choose the most suitable machine": why do you need to choose
> > a machine?
> > 
> > If it's pretty standalone setup, shouldn't we always try to use the latest
> > machine type if possible (as normally compat props are only used to keep
> > compatible with old machine types, and the default should always be
> > preferred). If it's a cluster setup, IMHO it should depend on the oldest
> > QEMU version that plans to be supported.  I don't see how such comparison
> > helps yet in either of the cases..
> > 
> > In regards to "compare binaries to be sure that migration to the newer
> > version will save all device properties": do we even support migrating
> > _between_ machine types??
> > 
> > Sololy relying on compat properties to detect device compatibility is also
> > not reliable.  For example, see VMStateField.field_exists() or similarly,
> > VMStateDescription.needed(), which are hooks that device can provide to
> > dynamically decide what device state to be saved/loaded.  Such things are
> > not reflected in compat properties, so even if compat properties of all
> > devices are the same between two machine types, it may not mean that the
> > migration stream will always be compatible.
> > 
> > Thanks,
> 
> In fact, the last commit describes the meaning of this series best. Perhaps
> it should have been moved to the cover letter:
> Often, many teams have their own "machines" inherited from "upstream" to
> manage default values of devices. This is done because of the limitations
> imposed by the compatibility requirements or the desire to help their
> customers better configure their devices. And since machine type has
> a hard-to-read structure, it is very easy to make a mistake when
> transferring
> default values from one machine to another. For example, when some property
> is set for the entire abstract class x86_64-cpu (which will be applied to
> all
> models), and then rolled back for a specific model. The situation is about
> the same with changing the default values of devices: if the value changes
> in the description of the device itself, then you need to make sure that
> nothing changes when using the current machine.
> Therefore, there was a desire to make a dev tool that will help quickly
> expand
> the definition of a machine or compare several machines from different
> binary
> files. It can be used when rebasing to a new version of qemu and/or for
> automatic tests.

OK, thanks.

So is it a migration compatibility issue that you care (migrating VMs from
your old downstream binary to new downstream binary should always succeed),
or perhaps you care more on making sure the features you wanted, i.e., you
want to make sure some specific devices that you care will have the
properties that you expect?

I think compat properties are mostly used for migration purposes, but
indeed it can also be used to keep old behaviors of devices, even if the
migration could succed with/without such a compat property entry.

If it's about migration, I'd like to know whether vmstate-static-checker.py
could also help your case (under scripts/), perhaps in a better way,
because it directly observes the VMSD structures (which is the ultimate
form on wire, after all these compat properties applied to the devices).

If it's not about migration, then maybe it's more QOM-relevant, and if so I
don't have a strong opinion. It seems still make some sense to have a tool
simply dump the QOM tree for a machine type with all properties and compare
them between machines with some binaries.  For that I'll leave that to
Markus to decide.

Btw, I tried to apply the patches and build, but failed:

In file included from ../qapi/qapi-schema.json:70:
../qapi/machine.json:224: text required after 'Example:'
[40/2810] Generating trace/trace-hw_ide.h with a custom command
[41/2810] Generating trace/trace-hw_isa.h with a custom command
[42/2810] Generating trace/trace-hw_intc.c with a custom command
[43/2810] Generating trace/trace-hw_mem.h with a custom command
[44/2810] Generating trace/trace-hw_isa.c with a custom command
[45/2810] Generating trace/trace-hw_intc.h with a custom command
[46/2810] Generating trace/trace-hw_mem.c with a custom command
ninja: build stopped: subcommand failed.
make: *** [Makefile:162: run-ninja] Error 1

There also seems to have an assumption that QEMU is built under "build/" in
the script.

+default_qemu_binary = 'build/qemu-system-x86_64'

-- 
Peter Xu

Re: [PULL 0/4] machine development tool

[Vladimir Sementsov-Ogievskiy]

On 08.03.24 06:47, Peter Xu wrote:
> On Thu, Mar 07, 2024 at 12:06:59PM +0300, Maksim Davydov wrote:
>>
>> On 3/6/24 04:57, Peter Xu wrote:
>>> On Tue, Mar 05, 2024 at 03:43:41PM +0100, Markus Armbruster wrote:
>>>> Peter Maydell<peter.maydell@linaro.org>  writes:
>>>>
>>>>> On Mon, 4 Mar 2024 at 13:52, Maksim Davydov<davydov-max@yandex-team.ru>  wrote:
>>>>>> The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
>>>>>>
>>>>>>     Merge tag 'pull-request-2024-03-01' ofhttps://gitlab.com/thuth/qemu  into staging (2024-03-01 10:14:32 +0000)
>>>>>>
>>>>>> are available in the Git repository at:
>>>>>>
>>>>>>     https://gitlab.com/davydov-max/qemu.git  tags/pull-compare-mt-2024-03-04
>>>>>>
>>>>>> for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
>>>>>>
>>>>>>     scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>>> Please note. This is the first pull request from me.
>>>>>> My public GPG key is available here
>>>>>> https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>>> scripts: add a new script for machine development
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>> Hi; I would prefer this to go through some existing submaintainer
>>>>> tree if possible, please.
>>>> Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.
>>> Yeah this seems like migration relevant.. however now I'm slightly confused
>>> on when this script should be useful.
>>>
>>> According to:
>>>
>>> https://lore.kernel.org/qemu-devel/20240222153912.646053-5-davydov-max@yandex-team.ru/
>>>
>>>           This script runs QEMU to obtain compat_props of machines and
>>>           default values of different types of drivers to produce comparison
>>>           table. This table can be used to compare machine types to choose
>>>           the most suitable machine or compare binaries to be sure that
>>>           migration to the newer version will save all device
>>>           properties. Also the json or csv format of this table can be used
>>>           to check does a new machine affect the previous ones by comparing
>>>           tables with and without the new machine.
>>>
>>> In regards to "choose the most suitable machine": why do you need to choose
>>> a machine?
>>>
>>> If it's pretty standalone setup, shouldn't we always try to use the latest
>>> machine type if possible (as normally compat props are only used to keep
>>> compatible with old machine types, and the default should always be
>>> preferred). If it's a cluster setup, IMHO it should depend on the oldest
>>> QEMU version that plans to be supported.  I don't see how such comparison
>>> helps yet in either of the cases..
>>>
>>> In regards to "compare binaries to be sure that migration to the newer
>>> version will save all device properties": do we even support migrating
>>> _between_ machine types??
>>>
>>> Sololy relying on compat properties to detect device compatibility is also
>>> not reliable.  For example, see VMStateField.field_exists() or similarly,
>>> VMStateDescription.needed(), which are hooks that device can provide to
>>> dynamically decide what device state to be saved/loaded.  Such things are
>>> not reflected in compat properties, so even if compat properties of all
>>> devices are the same between two machine types, it may not mean that the
>>> migration stream will always be compatible.
>>>
>>> Thanks,
>>
>> In fact, the last commit describes the meaning of this series best. Perhaps
>> it should have been moved to the cover letter:
>> Often, many teams have their own "machines" inherited from "upstream" to
>> manage default values of devices. This is done because of the limitations
>> imposed by the compatibility requirements or the desire to help their
>> customers better configure their devices. And since machine type has
>> a hard-to-read structure, it is very easy to make a mistake when
>> transferring
>> default values from one machine to another. For example, when some property
>> is set for the entire abstract class x86_64-cpu (which will be applied to
>> all
>> models), and then rolled back for a specific model. The situation is about
>> the same with changing the default values of devices: if the value changes
>> in the description of the device itself, then you need to make sure that
>> nothing changes when using the current machine.
>> Therefore, there was a desire to make a dev tool that will help quickly
>> expand
>> the definition of a machine or compare several machines from different
>> binary
>> files. It can be used when rebasing to a new version of qemu and/or for
>> automatic tests.
> 
> OK, thanks.
> 
> So is it a migration compatibility issue that you care (migrating VMs from
> your old downstream binary to new downstream binary should always succeed),
> or perhaps you care more on making sure the features you wanted, i.e., you
> want to make sure some specific devices that you care will have the
> properties that you expect?

Actually both things.

1. We need a tool to analyze, what exactly changes between MT-s. Do we want to move on new upstream MT or not, how much it is different from our downstream MT and so on.
2. It also could be used to check, that new MT is correctly defined (not breaking old MT's)

> 
> I think compat properties are mostly used for migration purposes, but
> indeed it can also be used to keep old behaviors of devices, even if the
> migration could succed with/without such a compat property entry.
> 
> If it's about migration, I'd like to know whether vmstate-static-checker.py
> could also help your case (under scripts/), perhaps in a better way,
> because it directly observes the VMSD structures (which is the ultimate
> form on wire, after all these compat properties applied to the devices).

Hmm, vmstate-static-checker.py checks a concrete device configuration. So it's a different thing.

> 
> If it's not about migration, then maybe it's more QOM-relevant, and if so I
> don't have a strong opinion. It seems still make some sense to have a tool
> simply dump the QOM tree for a machine type with all properties and compare
> them between machines with some binaries.  For that I'll leave that to
> Markus to decide.

Markus ACKed :)

> 
> Btw, I tried to apply the patches and build, but failed:
> 
> In file included from ../qapi/qapi-schema.json:70:
> ../qapi/machine.json:224: text required after 'Example:'
> [40/2810] Generating trace/trace-hw_ide.h with a custom command
> [41/2810] Generating trace/trace-hw_isa.h with a custom command
> [42/2810] Generating trace/trace-hw_intc.c with a custom command
> [43/2810] Generating trace/trace-hw_mem.h with a custom command
> [44/2810] Generating trace/trace-hw_isa.c with a custom command
> [45/2810] Generating trace/trace-hw_intc.h with a custom command
> [46/2810] Generating trace/trace-hw_mem.c with a custom command
> ninja: build stopped: subcommand failed.
> make: *** [Makefile:162: run-ninja] Error 1
> 

The series missed change in QAPI documentation requirements. I see, we need 4 spaces indentation for Examples text.
Max, could you fix and resend as patches again? We also have to replace "Since: 9.0" -> "Since: 9.1".


> There also seems to have an assumption that QEMU is built under "build/" in
> the script.
> 
> +default_qemu_binary = 'build/qemu-system-x86_64'
> 

I think it's not a problem for now. Could be changed later if needed.

-- 
Best regards,
Vladimir

Re: [PULL 0/4] machine development tool

[Maksim Davydov]

On 3/8/24 06:47, Peter Xu wrote:
> On Thu, Mar 07, 2024 at 12:06:59PM +0300, Maksim Davydov wrote:
>> On 3/6/24 04:57, Peter Xu wrote:
>>> On Tue, Mar 05, 2024 at 03:43:41PM +0100, Markus Armbruster wrote:
>>>> Peter Maydell<peter.maydell@linaro.org>  writes:
>>>>
>>>>> On Mon, 4 Mar 2024 at 13:52, Maksim Davydov<davydov-max@yandex-team.ru>  wrote:
>>>>>> The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
>>>>>>
>>>>>>     Merge tag 'pull-request-2024-03-01' ofhttps://gitlab.com/thuth/qemu  into staging (2024-03-01 10:14:32 +0000)
>>>>>>
>>>>>> are available in the Git repository at:
>>>>>>
>>>>>>     https://gitlab.com/davydov-max/qemu.git  tags/pull-compare-mt-2024-03-04
>>>>>>
>>>>>> for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
>>>>>>
>>>>>>     scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>>> Please note. This is the first pull request from me.
>>>>>> My public GPG key is available here
>>>>>> https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>>> scripts: add a new script for machine development
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>> Hi; I would prefer this to go through some existing submaintainer
>>>>> tree if possible, please.
>>>> Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.
>>> Yeah this seems like migration relevant.. however now I'm slightly confused
>>> on when this script should be useful.
>>>
>>> According to:
>>>
>>> https://lore.kernel.org/qemu-devel/20240222153912.646053-5-davydov-max@yandex-team.ru/
>>>
>>>           This script runs QEMU to obtain compat_props of machines and
>>>           default values of different types of drivers to produce comparison
>>>           table. This table can be used to compare machine types to choose
>>>           the most suitable machine or compare binaries to be sure that
>>>           migration to the newer version will save all device
>>>           properties. Also the json or csv format of this table can be used
>>>           to check does a new machine affect the previous ones by comparing
>>>           tables with and without the new machine.
>>>
>>> In regards to "choose the most suitable machine": why do you need to choose
>>> a machine?
>>>
>>> If it's pretty standalone setup, shouldn't we always try to use the latest
>>> machine type if possible (as normally compat props are only used to keep
>>> compatible with old machine types, and the default should always be
>>> preferred). If it's a cluster setup, IMHO it should depend on the oldest
>>> QEMU version that plans to be supported.  I don't see how such comparison
>>> helps yet in either of the cases..
>>>
>>> In regards to "compare binaries to be sure that migration to the newer
>>> version will save all device properties": do we even support migrating
>>> _between_ machine types??
>>>
>>> Sololy relying on compat properties to detect device compatibility is also
>>> not reliable.  For example, see VMStateField.field_exists() or similarly,
>>> VMStateDescription.needed(), which are hooks that device can provide to
>>> dynamically decide what device state to be saved/loaded.  Such things are
>>> not reflected in compat properties, so even if compat properties of all
>>> devices are the same between two machine types, it may not mean that the
>>> migration stream will always be compatible.
>>>
>>> Thanks,
>> In fact, the last commit describes the meaning of this series best. Perhaps
>> it should have been moved to the cover letter:
>> Often, many teams have their own "machines" inherited from "upstream" to
>> manage default values of devices. This is done because of the limitations
>> imposed by the compatibility requirements or the desire to help their
>> customers better configure their devices. And since machine type has
>> a hard-to-read structure, it is very easy to make a mistake when
>> transferring
>> default values from one machine to another. For example, when some property
>> is set for the entire abstract class x86_64-cpu (which will be applied to
>> all
>> models), and then rolled back for a specific model. The situation is about
>> the same with changing the default values of devices: if the value changes
>> in the description of the device itself, then you need to make sure that
>> nothing changes when using the current machine.
>> Therefore, there was a desire to make a dev tool that will help quickly
>> expand
>> the definition of a machine or compare several machines from different
>> binary
>> files. It can be used when rebasing to a new version of qemu and/or for
>> automatic tests.
> OK, thanks.
>
> So is it a migration compatibility issue that you care (migrating VMs from
> your old downstream binary to new downstream binary should always succeed),
> or perhaps you care more on making sure the features you wanted, i.e., you
> want to make sure some specific devices that you care will have the
> properties that you expect?
>
> I think compat properties are mostly used for migration purposes, but
> indeed it can also be used to keep old behaviors of devices, even if the
> migration could succed with/without such a compat property entry.
>
> If it's about migration, I'd like to know whether vmstate-static-checker.py
> could also help your case (under scripts/), perhaps in a better way,
> because it directly observes the VMSD structures (which is the ultimate
> form on wire, after all these compat properties applied to the devices).
>
> If it's not about migration, then maybe it's more QOM-relevant, and if so I
> don't have a strong opinion. It seems still make some sense to have a tool
> simply dump the QOM tree for a machine type with all properties and compare
> them between machines with some binaries.  For that I'll leave that to
> Markus to decide.
>
> Btw, I tried to apply the patches and build, but failed:
>
> In file included from ../qapi/qapi-schema.json:70:
> ../qapi/machine.json:224: text required after 'Example:'
> [40/2810] Generating trace/trace-hw_ide.h with a custom command
> [41/2810] Generating trace/trace-hw_isa.h with a custom command
> [42/2810] Generating trace/trace-hw_intc.c with a custom command
> [43/2810] Generating trace/trace-hw_mem.h with a custom command
> [44/2810] Generating trace/trace-hw_isa.c with a custom command
> [45/2810] Generating trace/trace-hw_intc.h with a custom command
> [46/2810] Generating trace/trace-hw_mem.c with a custom command
> ninja: build stopped: subcommand failed.
> make: *** [Makefile:162: run-ninja] Error 1
>
> There also seems to have an assumption that QEMU is built under "build/" in
> the script.
>
> +default_qemu_binary = 'build/qemu-system-x86_64'
>
Sorry for late response
This is the default value, the script has the option to redefine the path to
the binary `--qemu-binary`

-- 
Best regards,
Maksim Davydov

Re: [PULL 0/4] machine development tool

[Peter Xu]

On Mon, Mar 18, 2024 at 08:08:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 08.03.24 06:47, Peter Xu wrote:
> > On Thu, Mar 07, 2024 at 12:06:59PM +0300, Maksim Davydov wrote:
> > > 
> > > On 3/6/24 04:57, Peter Xu wrote:
> > > > On Tue, Mar 05, 2024 at 03:43:41PM +0100, Markus Armbruster wrote:
> > > > > Peter Maydell<peter.maydell@linaro.org>  writes:
> > > > > 
> > > > > > On Mon, 4 Mar 2024 at 13:52, Maksim Davydov<davydov-max@yandex-team.ru>  wrote:
> > > > > > > The following changes since commit e1007b6bab5cf97705bf4f2aaec1f607787355b8:
> > > > > > > 
> > > > > > >     Merge tag 'pull-request-2024-03-01' ofhttps://gitlab.com/thuth/qemu  into staging (2024-03-01 10:14:32 +0000)
> > > > > > > 
> > > > > > > are available in the Git repository at:
> > > > > > > 
> > > > > > >     https://gitlab.com/davydov-max/qemu.git  tags/pull-compare-mt-2024-03-04
> > > > > > > 
> > > > > > > for you to fetch changes up to 7693a2e8518811a907d73a85807ee71dac8fabcb:
> > > > > > > 
> > > > > > >     scripts: add script to compare compatibility properties (2024-03-04 14:10:53 +0300)
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > > Please note. This is the first pull request from me.
> > > > > > > My public GPG key is available here
> > > > > > > https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > > scripts: add a new script for machine development
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > Hi; I would prefer this to go through some existing submaintainer
> > > > > > tree if possible, please.
> > > > > Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.
> > > > Yeah this seems like migration relevant.. however now I'm slightly confused
> > > > on when this script should be useful.
> > > > 
> > > > According to:
> > > > 
> > > > https://lore.kernel.org/qemu-devel/20240222153912.646053-5-davydov-max@yandex-team.ru/
> > > > 
> > > >           This script runs QEMU to obtain compat_props of machines and
> > > >           default values of different types of drivers to produce comparison
> > > >           table. This table can be used to compare machine types to choose
> > > >           the most suitable machine or compare binaries to be sure that
> > > >           migration to the newer version will save all device
> > > >           properties. Also the json or csv format of this table can be used
> > > >           to check does a new machine affect the previous ones by comparing
> > > >           tables with and without the new machine.
> > > > 
> > > > In regards to "choose the most suitable machine": why do you need to choose
> > > > a machine?
> > > > 
> > > > If it's pretty standalone setup, shouldn't we always try to use the latest
> > > > machine type if possible (as normally compat props are only used to keep
> > > > compatible with old machine types, and the default should always be
> > > > preferred). If it's a cluster setup, IMHO it should depend on the oldest
> > > > QEMU version that plans to be supported.  I don't see how such comparison
> > > > helps yet in either of the cases..
> > > > 
> > > > In regards to "compare binaries to be sure that migration to the newer
> > > > version will save all device properties": do we even support migrating
> > > > _between_ machine types??
> > > > 
> > > > Sololy relying on compat properties to detect device compatibility is also
> > > > not reliable.  For example, see VMStateField.field_exists() or similarly,
> > > > VMStateDescription.needed(), which are hooks that device can provide to
> > > > dynamically decide what device state to be saved/loaded.  Such things are
> > > > not reflected in compat properties, so even if compat properties of all
> > > > devices are the same between two machine types, it may not mean that the
> > > > migration stream will always be compatible.
> > > > 
> > > > Thanks,
> > > 
> > > In fact, the last commit describes the meaning of this series best. Perhaps
> > > it should have been moved to the cover letter:
> > > Often, many teams have their own "machines" inherited from "upstream" to
> > > manage default values of devices. This is done because of the limitations
> > > imposed by the compatibility requirements or the desire to help their
> > > customers better configure their devices. And since machine type has
> > > a hard-to-read structure, it is very easy to make a mistake when
> > > transferring
> > > default values from one machine to another. For example, when some property
> > > is set for the entire abstract class x86_64-cpu (which will be applied to
> > > all
> > > models), and then rolled back for a specific model. The situation is about
> > > the same with changing the default values of devices: if the value changes
> > > in the description of the device itself, then you need to make sure that
> > > nothing changes when using the current machine.
> > > Therefore, there was a desire to make a dev tool that will help quickly
> > > expand
> > > the definition of a machine or compare several machines from different
> > > binary
> > > files. It can be used when rebasing to a new version of qemu and/or for
> > > automatic tests.
> > 
> > OK, thanks.
> > 
> > So is it a migration compatibility issue that you care (migrating VMs from
> > your old downstream binary to new downstream binary should always succeed),
> > or perhaps you care more on making sure the features you wanted, i.e., you
> > want to make sure some specific devices that you care will have the
> > properties that you expect?
> 
> Actually both things.
> 
> 1. We need a tool to analyze, what exactly changes between MT-s. Do we want to move on new upstream MT or not, how much it is different from our downstream MT and so on.
> 2. It also could be used to check, that new MT is correctly defined (not breaking old MT's)
> 
> > 
> > I think compat properties are mostly used for migration purposes, but
> > indeed it can also be used to keep old behaviors of devices, even if the
> > migration could succed with/without such a compat property entry.
> > 
> > If it's about migration, I'd like to know whether vmstate-static-checker.py
> > could also help your case (under scripts/), perhaps in a better way,
> > because it directly observes the VMSD structures (which is the ultimate
> > form on wire, after all these compat properties applied to the devices).
> 
> Hmm, vmstate-static-checker.py checks a concrete device configuration. So it's a different thing.

I don't think so - 'qemu -dump-vmstate' should dump all device states that
it ever supports.  Feel free to have a look at dump_vmstate_json_to_file(),
or just try give it a dump.

> 
> > 
> > If it's not about migration, then maybe it's more QOM-relevant, and if so I
> > don't have a strong opinion. It seems still make some sense to have a tool
> > simply dump the QOM tree for a machine type with all properties and compare
> > them between machines with some binaries.  For that I'll leave that to
> > Markus to decide.
> 
> Markus ACKed :)

I didn't see Markus acked all the patches yet, but if so that's okay then.
Even if so, I think what Peter Maydell suggested is then this series should
go through the QOM tree, rather than a separate pull.

Thanks,

-- 
Peter Xu