Series comparison

-[PATCH v2 0/4] qapi: generalize special features
+[PATCH v3 0/4] qapi: generalize special features
 ...
 I've changed impl such that we expose all features to the code
 regardless of whether they are special, and don't require any pragma.
 I've split it from the QGA patches since it makes more sense to work
 on this bit in isolation.
+Changed in v3:
+ * "make -C python check-tox" is now clean
+ * Added test case for "too many features" error scenario
+ * Don't sort the features enum, just rely on ordered dict
+ * Add 'features' accessor on QAPISchema instead of touching
+   private data via 'schema._features.values()'
 Changed in v2:
  * Reference QapiSpecialFeature enums constants when defining
    QapiFeature enums constants for deprecated & unstable
 ...
   qapi: cope with  feature names containing a '-'
   qapi: change 'unsigned special_features' to 'uint64_t features'
   qapi: rename 'special_features' to 'features'
   qapi: expose all schema features to code
- include/qapi/compat-policy.h  |  2 +-
+ include/qapi/compat-policy.h             |  2 +-
- include/qapi/qmp/dispatch.h   |  4 +--
+ include/qapi/qmp/dispatch.h              |  4 +-
- include/qapi/util.h           |  2 +-
+ include/qapi/util.h                      |  2 +-
- include/qapi/visitor-impl.h   |  4 +--
+ include/qapi/visitor-impl.h              |  4 +-
- include/qapi/visitor.h        | 12 +++----
+ include/qapi/visitor.h                   | 12 +++---
- meson.build                   |  1 +
+ meson.build                              |  1 +
- qapi/qapi-forward-visitor.c   |  8 ++---
+ qapi/qapi-forward-visitor.c              |  8 ++--
- qapi/qapi-util.c              |  6 ++--
+ qapi/qapi-util.c                         |  6 +--
- qapi/qapi-visit-core.c        | 12 +++----
+ qapi/qapi-visit-core.c                   | 12 +++---
- qapi/qmp-dispatch.c           |  2 +-
+ qapi/qmp-dispatch.c                      |  2 +-
- qapi/qmp-registry.c           |  4 +--
+ qapi/qmp-registry.c                      |  4 +-
- qapi/qobject-input-visitor.c  |  4 +--
+ qapi/qobject-input-visitor.c             |  4 +-
- qapi/qobject-output-visitor.c |  6 ++--
+ qapi/qobject-output-visitor.c            |  6 +--
- scripts/qapi/commands.py      |  5 +--
+ scripts/qapi/commands.py                 |  5 ++-
- scripts/qapi/features.py      | 62 +++++++++++++++++++++++++++++++++++
+ scripts/qapi/features.py                 | 51 ++++++++++++++++++++++++
- scripts/qapi/gen.py           |  9 ++---
+ scripts/qapi/gen.py                      |  9 +++--
- scripts/qapi/main.py          |  2 ++
+ scripts/qapi/main.py                     |  2 +
- scripts/qapi/schema.py        | 19 ++++++++++-
+ scripts/qapi/schema.py                   | 30 +++++++++++++-
- scripts/qapi/types.py         | 18 +++++-----
+ scripts/qapi/types.py                    | 19 +++++----
- scripts/qapi/visit.py         | 17 +++++-----
+ scripts/qapi/visit.py                    | 17 ++++----
-files changed, 143 insertions(+), 56 deletions(-)
+ tests/meson.build                        |  2 +
  tests/qapi-schema/features-too-many.err  |  2 +
  tests/qapi-schema/features-too-many.json | 13 ++++++
  tests/qapi-schema/features-too-many.out  |  0
  tests/qapi-schema/meson.build            |  1 +
 files changed, 162 insertions(+), 56 deletions(-)
  create mode 100644 scripts/qapi/features.py
+ create mode 100644 tests/qapi-schema/features-too-many.err
+ create mode 100644 tests/qapi-schema/features-too-many.json
+ create mode 100644 tests/qapi-schema/features-too-many.out
 --
 .46.0

-[PATCH v2 1/4] qapi: cope with  feature names containing a '-'
+[PATCH v3 1/4] qapi: cope with  feature names containing a '-'
 When we shortly expose all feature names to code, it will be valid to
 include a '-', which must be translated to a '_' for the enum constants.
 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
 ---
  scripts/qapi/gen.py | 3 ++-
 file changed, 2 insertions(+), 1 deletion(-)
 diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
 index XXXXXXX..XXXXXXX 100644
 --- a/scripts/qapi/gen.py
 +++ b/scripts/qapi/gen.py
 @@ -XXX,XX +XXX,XX @@
  )
  from .common import (
 +    c_enum_const,
      c_fname,
      c_name,
      guardend,
 @@ -XXX,XX +XXX,XX @@
  def gen_special_features(features: Sequence[QAPISchemaFeature]) -> str:
 -    special_features = [f"1u << QAPI_{feat.name.upper()}"
 +    special_features = [f"1u << {c_enum_const('qapi', feat.name)}"
                          for feat in features if feat.is_special()]
      return ' | '.join(special_features) or '0'
 --
 .46.0

-[PATCH v2 2/4] qapi: change 'unsigned special_features' to 'uint64_t features'
+[PATCH v3 2/4] qapi: change 'unsigned special_features' to 'uint64_t features'
 ...
 This special casing of internal features is going to be removed, so
 prepare for that by renaming to 'features'. Using a fixed size type
 is also best practice for bit fields.
+Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
 ---
  include/qapi/compat-policy.h  |  2 +-
  include/qapi/qmp/dispatch.h   |  4 ++--
  include/qapi/util.h           |  2 +-
 ...

-[PATCH v2 3/4] qapi: rename 'special_features' to 'features'
+[PATCH v3 3/4] qapi: rename 'special_features' to 'features'
 This updates the QAPI code generation to refer to 'features' instead
 of 'special_features', in preparation for generalizing their exposure.
 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
 ---
  scripts/qapi/commands.py |  4 ++--
- scripts/qapi/gen.py      |  6 +++---
+ scripts/qapi/gen.py      |  8 ++++----
  scripts/qapi/types.py    | 10 +++++-----
  scripts/qapi/visit.py    | 14 +++++++-------
-files changed, 17 insertions(+), 17 deletions(-)
+files changed, 18 insertions(+), 18 deletions(-)
 diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
 index XXXXXXX..XXXXXXX 100644
 --- a/scripts/qapi/commands.py
 +++ b/scripts/qapi/commands.py
 ...
  from .source import QAPISourceInfo
 -def gen_special_features(features: Sequence[QAPISchemaFeature]) -> str:
 -    special_features = [f"1u << {c_enum_const('qapi', feat.name)}"
+-                        for feat in features if feat.is_special()]
+-    return ' | '.join(special_features) or '0'
 +def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
-+    features = [f"1u << {c_enum_const('qapi', feat.name)}"
++    featenum = [f"1u << {c_enum_const('qapi', feat.name)}"
-                         for feat in features if feat.is_special()]
++                for feat in features if feat.is_special()]
--    return ' | '.join(special_features) or '0'
++    return ' | '.join(featenum) or '0'
 +    return ' | '.join(features) or '0'
  class QAPIGen:
 diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
 index XXXXXXX..XXXXXXX 100644
 ...

-[PATCH v2 4/4] qapi: expose all schema features to code
+[PATCH v3 4/4] qapi: expose all schema features to code
 ...
 retains compatibility with common code that references the features
 via the QapiSpecialFeatures constants.
 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
 ---
- meson.build              |  1 +
+ meson.build                              |  1 +
- scripts/qapi/commands.py |  1 +
+ scripts/qapi/commands.py                 |  1 +
- scripts/qapi/features.py | 62 ++++++++++++++++++++++++++++++++++++++++
+ scripts/qapi/features.py                 | 51 ++++++++++++++++++++++++
- scripts/qapi/gen.py      |  4 +--
+ scripts/qapi/gen.py                      |  6 +--
- scripts/qapi/main.py     |  2 ++
+ scripts/qapi/main.py                     |  2 +
- scripts/qapi/schema.py   | 19 +++++++++++-
+ scripts/qapi/schema.py                   | 30 +++++++++++++-
- scripts/qapi/types.py    |  6 ++--
+ scripts/qapi/types.py                    |  7 +++-
- scripts/qapi/visit.py    |  3 +-
+ scripts/qapi/visit.py                    |  3 +-
-files changed, 92 insertions(+), 6 deletions(-)
+ tests/meson.build                        |  2 +
  tests/qapi-schema/features-too-many.err  |  2 +
  tests/qapi-schema/features-too-many.json | 13 ++++++
  tests/qapi-schema/features-too-many.out  |  0
  tests/qapi-schema/meson.build            |  1 +
 files changed, 112 insertions(+), 7 deletions(-)
  create mode 100644 scripts/qapi/features.py
+ create mode 100644 tests/qapi-schema/features-too-many.err
+ create mode 100644 tests/qapi-schema/features-too-many.json
+ create mode 100644 tests/qapi-schema/features-too-many.out
 diff --git a/meson.build b/meson.build
 index XXXXXXX..XXXXXXX 100644
 --- a/meson.build
 +++ b/meson.build
 ...
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/scripts/qapi/features.py
 @@ -XXX,XX +XXX,XX @@
 +"""
-+QAPI types generator
++QAPI features generator
 +
 +Copyright 2024 Red Hat
 +
 +This work is licensed under the terms of the GNU GPL, version 2.
 +# See the COPYING file in the top-level directory.
 +"""
 +
-+from typing import List, Optional
++from typing import Dict
 +
-+from .common import c_enum_const, mcgen, c_name
++from .common import c_enum_const, c_name
 +from .gen import QAPISchemaMonolithicCVisitor
 +from .schema import (
 +    QAPISchema,
 +    QAPISchemaFeature,
 +)
-+from .source import QAPISourceInfo
 +
 +
 +class QAPISchemaGenFeatureVisitor(QAPISchemaMonolithicCVisitor):
 +
 +    def __init__(self, prefix: str):
 +        super().__init__(
 +            prefix, 'qapi-features',
 +            ' * Schema-defined QAPI features',
 +            __doc__)
 +
-+        self.features = {}
++        self.features: Dict[str, QAPISchemaFeature] = {}
 +
-+    def visit_begin(self, schema: QAPISchema):
++    def visit_begin(self, schema: QAPISchema) -> None:
-+        self.features = schema._feature_dict
++        self.features = schema.features()
 +        self._genh.add("#include \"qapi/util.h\"\n\n")
 +
 +    def visit_end(self) -> None:
-+        features = [
-+            self.features[f]
-+            for f in QAPISchemaFeature.SPECIAL_NAMES
-+        ]
-+
-+        features.extend(
-+            sorted(
-+                filter(lambda f: not f.is_special(),
-+                       self.features.values()),
-+                key=lambda f: f.name)
-+        )
-+
 +        self._genh.add("typedef enum {\n")
-+        for f in features:
++        for f in self.features:
 +            self._genh.add(f"    {c_enum_const('qapi_feature', f.name)}")
 +            if f.name in QAPISchemaFeature.SPECIAL_NAMES:
-+                self._genh.add(f" = {c_enum_const('qapi', f.name)},\n" )
++                self._genh.add(f" = {c_enum_const('qapi', f.name)},\n")
 +            else:
 +                self._genh.add(",\n")
 +
 +        self._genh.add("} " + c_name('QapiFeature') + ";\n")
++
 +
 +def gen_features(schema: QAPISchema,
 +                 output_dir: str,
 +                 prefix: str) -> None:
 +    vis = QAPISchemaGenFeatureVisitor(prefix)
 ...
 +++ b/scripts/qapi/gen.py
 @@ -XXX,XX +XXX,XX @@
  def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
--    features = [f"1u << {c_enum_const('qapi', feat.name)}"
+-    featenum = [f"1u << {c_enum_const('qapi', feat.name)}"
--                        for feat in features if feat.is_special()]
+-                for feat in features if feat.is_special()]
-+    features = [f"1u << {c_enum_const('qapi_feature', feat.name)}"
+-    return ' | '.join(featenum) or '0'
-+                        for feat in features]
++    feats = [f"1u << {c_enum_const('qapi_feature', feat.name)}"
-     return ' | '.join(features) or '0'
++             for feat in features]
++    return ' | '.join(feats) or '0'
  class QAPIGen:
 diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
 index XXXXXXX..XXXXXXX 100644
 --- a/scripts/qapi/main.py
 +++ b/scripts/qapi/main.py
 @@ -XXX,XX +XXX,XX @@
 ...
  class QAPISchemaObjectTypeMember(QAPISchemaMember):
 @@ -XXX,XX +XXX,XX @@ def __init__(self, fname: str):
          self._entity_list: List[QAPISchemaEntity] = []
          self._entity_dict: Dict[str, QAPISchemaDefinition] = {}
          self._module_dict: Dict[str, QAPISchemaModule] = OrderedDict()
-+        self._feature_dict: Dict[str, QAPISchemaFeature] = {}
++        # NB, values in the dict will identify the first encountered
-+
++        #     usage of a named feature only
 +        self._feature_dict: Dict[str, QAPISchemaFeature] = OrderedDict()
 +
 +        # All schemas get the names defined in the QapiSpecialFeature enum.
 +        # Use of OrderedDict ensures they are emitted first when generating
 +        # the enum definition, thus matching QapiSpecialFeature.
 +        for f in QAPISchemaFeature.SPECIAL_NAMES:
-+            self._feature_dict[f] = QAPISchemaFeature(f, "special feature")
++            self._feature_dict[f] = QAPISchemaFeature(f, None)
 +
          self._schema_dir = os.path.dirname(fname)
          self._make_module(QAPISchemaModule.BUILTIN_MODULE_NAME)
          self._make_module(fname)
+@@ -XXX,XX +XXX,XX @@ def __init__(self, fname: str):
+         self._def_exprs(exprs)
+         self.check()
++    def features(self) -> List[QAPISchemaFeature]:
++        return self._feature_dict.values()
++
+     def _def_entity(self, ent: QAPISchemaEntity) -> None:
+         self._entity_list.append(ent)
 @@ -XXX,XX +XXX,XX @@ def _make_features(
      ) -> List[QAPISchemaFeature]:
          if features is None:
              return []
 +
 +        for f in features:
 +            feat = QAPISchemaFeature(f['name'], info)
 +            if feat.name not in self._feature_dict:
-+                if len(self._feature_dict) == 64:
-+                    raise Exception("Maximum of 64 schema features is permitted")
-+
 +                self._feature_dict[feat.name] = feat
 +
          return [QAPISchemaFeature(f['name'], info,
                                    QAPISchemaIfCond(f.get('if')))
                  for f in features]
+@@ -XXX,XX +XXX,XX @@ def check(self) -> None:
+         for doc in self.docs:
+             doc.check()
++        features = list(self._feature_dict.values())
++        if len(features) > 64:
++            raise QAPISemError(
++                features[64].info,
++                "Maximum of 64 schema features is permitted")
++
+     def visit(self, visitor: QAPISchemaVisitor) -> None:
+         visitor.visit_begin(self)
+         for mod in self._module_dict.values():
 diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
 index XXXXXXX..XXXXXXX 100644
 --- a/scripts/qapi/types.py
 +++ b/scripts/qapi/types.py
 @@ -XXX,XX +XXX,XX @@ def _begin_user_module(self, name: str) -> None:
  #include "qapi/dealloc-visitor.h"
  #include "%(types)s.h"
  #include "%(visit)s.h"
 +#include "%(prefix)sqapi-features.h"
  ''',
 -                                      types=types, visit=visit))
-+                                      types=types, visit=visit, prefix=self._prefix))
++                                      types=types, visit=visit,
 +                                      prefix=self._prefix))
          self._genh.preamble_add(mcgen('''
  #include "qapi/qapi-builtin-types.h"
 -'''))
 +''',
 +                                      prefix=self._prefix))
 ...
 -                                      visit=visit))
 +                                      visit=visit, prefix=self._prefix))
          self._genh.preamble_add(mcgen('''
  #include "qapi/qapi-builtin-visit.h"
  #include "%(types)s.h"
+diff --git a/tests/meson.build b/tests/meson.build
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/meson.build
++++ b/tests/meson.build
+@@ -XXX,XX +XXX,XX @@ test_qapi_outputs = [
+   'test-qapi-events-sub-sub-module.h',
+   'test-qapi-events.c',
+   'test-qapi-events.h',
++  'test-qapi-features.c',
++  'test-qapi-features.h',
+   'test-qapi-init-commands.c',
+   'test-qapi-init-commands.h',
+   'test-qapi-introspect.c',
+diff --git a/tests/qapi-schema/features-too-many.err b/tests/qapi-schema/features-too-many.err
+new file mode 100644
+index XXXXXXX..XXXXXXX
+--- /dev/null
++++ b/tests/qapi-schema/features-too-many.err
+@@ -XXX,XX +XXX,XX @@
++features-too-many.json: In command 'go-fish':
++features-too-many.json:2: Maximum of 64 schema features is permitted
+diff --git a/tests/qapi-schema/features-too-many.json b/tests/qapi-schema/features-too-many.json
+new file mode 100644
+index XXXXXXX..XXXXXXX
+--- /dev/null
++++ b/tests/qapi-schema/features-too-many.json
+@@ -XXX,XX +XXX,XX @@
++# Max 64 features, with 2 specials, so 63rd custom is invalid
++{ 'command': 'go-fish',
++  'features': [
++      'f00', 'f01', 'f02', 'f03', 'f04', 'f05', 'f06', 'f07',
++      'f08', 'f09', 'f0a', 'f0b', 'f0c', 'f0d', 'f0e', 'f0f',
++      'f10', 'f11', 'f12', 'f13', 'f14', 'f15', 'f16', 'f17',
++      'f18', 'f19', 'f1a', 'f1b', 'f1c', 'f1d', 'f1e', 'f1f',
++      'f20', 'f21', 'f22', 'f23', 'f24', 'f25', 'f26', 'f27',
++      'f28', 'f29', 'f2a', 'f2b', 'f2c', 'f2d', 'f2e', 'f2f',
++      'f30', 'f31', 'f32', 'f33', 'f34', 'f35', 'f36', 'f37',
++      'f38', 'f39', 'f3a', 'f3b', 'f3c', 'f3d', 'f3e'
++  ]
++}
+diff --git a/tests/qapi-schema/features-too-many.out b/tests/qapi-schema/features-too-many.out
+new file mode 100644
+index XXXXXXX..XXXXXXX
+diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/qapi-schema/meson.build
++++ b/tests/qapi-schema/meson.build
+@@ -XXX,XX +XXX,XX @@ schemas = [
+   'event-case.json',
+   'event-member-invalid-dict.json',
+   'event-nest-struct.json',
++  'features-too-many.json',
+   'features-bad-type.json',
+   'features-deprecated-type.json',
+   'features-duplicate-name.json',
 --
 .46.0

This series is a spin-off from

https://lists.nongnu.org/archive/html/qemu-devel/2024-06/msg00807.html

That series introduced a pragma allowing a schema to declare extra
features that would be exposed to code.

Following Markus' suggestion:

https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03765.html

I've changed impl such that we expose all features to the code
regardless of whether they are special, and don't require any pragma.

I've split it from the QGA patches since it makes more sense to work
on this bit in isolation.

Changed in v2:

* Reference QapiSpecialFeature enums constants when defining
   QapiFeature enums constants for deprecated & unstable

* Don't expose qapi-features.h in qapi-types.h, to avoid
   global namespace pollution, instead pull it into only the
   .c files that need it. This avoids the need to add a 'prefix'
   to enum constants

* Collect all features during parse time, rather than
   generate time, to allow earlier error reporting of the
   64 feature limit

Daniel P. Berrangé (4):
  qapi: cope with  feature names containing a '-'
  qapi: change 'unsigned special_features' to 'uint64_t features'
  qapi: rename 'special_features' to 'features'
  qapi: expose all schema features to code

-- 
2.46.0

The "special_features" field / parameter holds the subset of schema
features that are for internal code use. Specifically 'DEPRECATED'
and 'UNSTABLE'.

This special casing of internal features is going to be removed, so
prepare for that by renaming to 'features'. Using a fixed size type
is also best practice for bit fields.

diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qapi/compat-policy.h
+++ b/include/qapi/compat-policy.h
@@ -XXX,XX +XXX,XX @@
 
 extern CompatPolicy compat_policy;
 
-bool compat_policy_input_ok(unsigned special_features,
+bool compat_policy_input_ok(uint64_t features,
                             const CompatPolicy *policy,
                             ErrorClass error_class,
                             const char *kind, const char *name,
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -XXX,XX +XXX,XX @@ typedef struct QmpCommand
     /* Runs in coroutine context if QCO_COROUTINE is set */
     QmpCommandFunc *fn;
     QmpCommandOptions options;
-    unsigned special_features;
+    uint64_t features;
     QTAILQ_ENTRY(QmpCommand) node;
     bool enabled;
     const char *disable_reason;
@@ -XXX,XX +XXX,XX @@ typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
 
 void qmp_register_command(QmpCommandList *cmds, const char *name,
                           QmpCommandFunc *fn, QmpCommandOptions options,
-                          unsigned special_features);
+                          uint64_t features);
 const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
                                    const char *name);
 void qmp_disable_command(QmpCommandList *cmds, const char *name,
diff --git a/include/qapi/util.h b/include/qapi/util.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -XXX,XX +XXX,XX @@ typedef enum {
 
 typedef struct QEnumLookup {
     const char *const *array;
-    const unsigned char *const special_features;
+    const uint64_t *const features;
     const int size;
 } QEnumLookup;
 
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -XXX,XX +XXX,XX @@ struct Visitor
 
     /* Optional */
     bool (*policy_reject)(Visitor *v, const char *name,
-                          unsigned special_features, Error **errp);
+                          uint64_t features, Error **errp);
 
     /* Optional */
     bool (*policy_skip)(Visitor *v, const char *name,
-                        unsigned special_features);
+                        uint64_t features);
 
     /* Must be set */
     VisitorType type;
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -XXX,XX +XXX,XX @@ bool visit_optional(Visitor *v, const char *name, bool *present);
 /*
  * Should we reject member @name due to policy?
  *
- * @special_features is the member's special features encoded as a
- * bitset of QapiSpecialFeature.
+ * @features is the member's special features encoded as a
+ * bitset of QapiFeature.
  *
  * @name must not be NULL.  This function is only useful between
  * visit_start_struct() and visit_end_struct(), since only objects
  * have deprecated members.
  */
 bool visit_policy_reject(Visitor *v, const char *name,
-                         unsigned special_features, Error **errp);
+                         uint64_t features, Error **errp);
 
 /*
  *
  * Should we skip member @name due to policy?
  *
- * @special_features is the member's special features encoded as a
- * bitset of QapiSpecialFeature.
+ * @features is the member's special features encoded as a
+ * bitset of QapiFeature.
  *
  * @name must not be NULL.  This function is only useful between
  * visit_start_struct() and visit_end_struct(), since only objects
  * have deprecated members.
  */
 bool visit_policy_skip(Visitor *v, const char *name,
-                       unsigned special_features);
+                       uint64_t features);
 
 /*
  * Set policy for handling deprecated management interfaces.
diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qapi-forward-visitor.c
+++ b/qapi/qapi-forward-visitor.c
@@ -XXX,XX +XXX,XX @@ static void forward_field_optional(Visitor *v, const char *name, bool *present)
 }
 
 static bool forward_field_policy_reject(Visitor *v, const char *name,
-                                        unsigned special_features,
+                                        uint64_t features,
                                         Error **errp)
 {
     ForwardFieldVisitor *ffv = to_ffv(v);
@@ -XXX,XX +XXX,XX @@ static bool forward_field_policy_reject(Visitor *v, const char *name,
     if (!forward_field_translate_name(ffv, &name, errp)) {
         return true;
     }
-    return visit_policy_reject(ffv->target, name, special_features, errp);
+    return visit_policy_reject(ffv->target, name, features, errp);
 }
 
 static bool forward_field_policy_skip(Visitor *v, const char *name,
-                                      unsigned special_features)
+                                      uint64_t features)
 {
     ForwardFieldVisitor *ffv = to_ffv(v);
 
     if (!forward_field_translate_name(ffv, &name, NULL)) {
         return true;
     }
-    return visit_policy_skip(ffv->target, name, special_features);
+    return visit_policy_skip(ffv->target, name, features);
 }
 
 static void forward_field_complete(Visitor *v, void *opaque)
diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -XXX,XX +XXX,XX @@ static bool compat_policy_input_ok1(const char *adjective,
     }
 }
 
-bool compat_policy_input_ok(unsigned special_features,
+bool compat_policy_input_ok(uint64_t features,
                             const CompatPolicy *policy,
                             ErrorClass error_class,
                             const char *kind, const char *name,
                             Error **errp)
 {
-    if ((special_features & 1u << QAPI_DEPRECATED)
+    if ((features & 1u << QAPI_DEPRECATED)
         && !compat_policy_input_ok1("Deprecated",
                                     policy->deprecated_input,
                                     error_class, kind, name, errp)) {
         return false;
     }
-    if ((special_features & (1u << QAPI_UNSTABLE))
+    if ((features & (1u << QAPI_UNSTABLE))
         && !compat_policy_input_ok1("Unstable",
                                     policy->unstable_input,
                                     error_class, kind, name, errp)) {
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -XXX,XX +XXX,XX @@ bool visit_optional(Visitor *v, const char *name, bool *present)
 }
 
 bool visit_policy_reject(Visitor *v, const char *name,
-                         unsigned special_features, Error **errp)
+                         uint64_t features, Error **errp)
 {
     trace_visit_policy_reject(v, name);
     if (v->policy_reject) {
-        return v->policy_reject(v, name, special_features, errp);
+        return v->policy_reject(v, name, features, errp);
     }
     return false;
 }
 
 bool visit_policy_skip(Visitor *v, const char *name,
-                       unsigned special_features)
+                       uint64_t features)
 {
     trace_visit_policy_skip(v, name);
     if (v->policy_skip) {
-        return v->policy_skip(v, name, special_features);
+        return v->policy_skip(v, name, features);
     }
     return false;
 }
@@ -XXX,XX +XXX,XX @@ static bool input_type_enum(Visitor *v, const char *name, int *obj,
         return false;
     }
 
-    if (lookup->special_features
-        && !compat_policy_input_ok(lookup->special_features[value],
+    if (lookup->features
+        && !compat_policy_input_ok(lookup->features[value],
                                    &v->compat_policy,
                                    ERROR_CLASS_GENERIC_ERROR,
                                    "value", enum_str, errp)) {
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -XXX,XX +XXX,XX @@ QDict *coroutine_mixed_fn qmp_dispatch(const QmpCommandList *cmds, QObject *requ
                   "The command %s has not been found", command);
         goto out;
     }
-    if (!compat_policy_input_ok(cmd->special_features, &compat_policy,
+    if (!compat_policy_input_ok(cmd->features, &compat_policy,
                                 ERROR_CLASS_COMMAND_NOT_FOUND,
                                 "command", command, &err)) {
         goto out;
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -XXX,XX +XXX,XX @@
 
 void qmp_register_command(QmpCommandList *cmds, const char *name,
                           QmpCommandFunc *fn, QmpCommandOptions options,
-                          unsigned special_features)
+                          uint64_t features)
 {
     QmpCommand *cmd = g_malloc0(sizeof(*cmd));
 
@@ -XXX,XX +XXX,XX @@ void qmp_register_command(QmpCommandList *cmds, const char *name,
     cmd->fn = fn;
     cmd->enabled = true;
     cmd->options = options;
-    cmd->special_features = special_features;
+    cmd->features = features;
     QTAILQ_INSERT_TAIL(cmds, cmd, node);
 }
 
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -XXX,XX +XXX,XX @@ static void qobject_input_optional(Visitor *v, const char *name, bool *present)
 }
 
 static bool qobject_input_policy_reject(Visitor *v, const char *name,
-                                        unsigned special_features,
+                                        uint64_t features,
                                         Error **errp)
 {
-    return !compat_policy_input_ok(special_features, &v->compat_policy,
+    return !compat_policy_input_ok(features, &v->compat_policy,
                                    ERROR_CLASS_GENERIC_ERROR,
                                    "parameter", name, errp);
 }
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index XXXXXXX..XXXXXXX 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -XXX,XX +XXX,XX @@ static bool qobject_output_type_null(Visitor *v, const char *name,
 }
 
 static bool qobject_output_policy_skip(Visitor *v, const char *name,
-                                       unsigned special_features)
+                                       uint64_t features)
 {
     CompatPolicy *pol = &v->compat_policy;
 
-    return ((special_features & 1u << QAPI_DEPRECATED)
+    return ((features & 1u << QAPI_DEPRECATED)
             && pol->deprecated_output == COMPAT_POLICY_OUTPUT_HIDE)
-        || ((special_features & 1u << QAPI_UNSTABLE)
+        || ((features & 1u << QAPI_UNSTABLE)
             && pol->unstable_output == COMPAT_POLICY_OUTPUT_HIDE);
 }
 
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -XXX,XX +XXX,XX @@ def gen_enum_lookup(name: str,
     if feats:
         ret += mcgen('''
     },
-    .special_features = (const unsigned char[%(max_index)s]) {
+    .features = (const uint64_t[%(max_index)s]) {
 ''',
                      max_index=max_index)
         ret += feats
-- 
2.46.0

This updates the QAPI code generation to refer to 'features' instead
of 'special_features', in preparation for generalizing their exposure.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/qapi/commands.py |  4 ++--
 scripts/qapi/gen.py      |  6 +++---
 scripts/qapi/types.py    | 10 +++++-----
 scripts/qapi/visit.py    | 14 +++++++-------
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -XXX,XX +XXX,XX @@
     QAPIGenC,
     QAPISchemaModularCVisitor,
     build_params,
-    gen_special_features,
+    gen_features,
     ifcontext,
 )
 from .schema import (
@@ -XXX,XX +XXX,XX @@ def gen_register_command(name: str,
 ''',
                 name=name, c_name=c_name(name),
                 opts=' | '.join(options) or 0,
-                feats=gen_special_features(features))
+                feats=gen_features(features))
     return ret
 
 
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -XXX,XX +XXX,XX @@
 from .source import QAPISourceInfo
 
 
-def gen_special_features(features: Sequence[QAPISchemaFeature]) -> str:
-    special_features = [f"1u << {c_enum_const('qapi', feat.name)}"
+def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
+    features = [f"1u << {c_enum_const('qapi', feat.name)}"
                         for feat in features if feat.is_special()]
-    return ' | '.join(special_features) or '0'
+    return ' | '.join(features) or '0'
 
 
 class QAPIGen:
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -XXX,XX +XXX,XX @@
 from .common import c_enum_const, c_name, mcgen
 from .gen import (
     QAPISchemaModularCVisitor,
-    gen_special_features,
+    gen_features,
     ifcontext,
 )
 from .schema import (
@@ -XXX,XX +XXX,XX @@ def gen_enum_lookup(name: str,
                      index=index, name=memb.name)
         ret += memb.ifcond.gen_endif()
 
-        special_features = gen_special_features(memb.features)
-        if special_features != '0':
+        features = gen_features(memb.features)
+        if features != '0':
             feats += mcgen('''
-        [%(index)s] = %(special_features)s,
+        [%(index)s] = %(features)s,
 ''',
-                           index=index, special_features=special_features)
+                           index=index, features=features)
 
     if feats:
         ret += mcgen('''
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -XXX,XX +XXX,XX @@
 )
 from .gen import (
     QAPISchemaModularCVisitor,
-    gen_special_features,
+    gen_features,
     ifcontext,
 )
 from .schema import (
@@ -XXX,XX +XXX,XX @@ def gen_visit_object_members(name: str,
 ''',
                          name=memb.name, has=has)
             indent.increase()
-        special_features = gen_special_features(memb.features)
-        if special_features != '0':
+        features = gen_features(memb.features)
+        if features != '0':
             ret += mcgen('''
-    if (visit_policy_reject(v, "%(name)s", %(special_features)s, errp)) {
+    if (visit_policy_reject(v, "%(name)s", %(features)s, errp)) {
         return false;
     }
-    if (!visit_policy_skip(v, "%(name)s", %(special_features)s)) {
+    if (!visit_policy_skip(v, "%(name)s", %(features)s)) {
 ''',
-                         name=memb.name, special_features=special_features)
+                         name=memb.name, features=features)
             indent.increase()
         ret += mcgen('''
     if (!visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, errp)) {
@@ -XXX,XX +XXX,XX @@ def gen_visit_object_members(name: str,
 ''',
                      c_type=memb.type.c_name(), name=memb.name,
                      c_name=c_name(memb.name))
-        if special_features != '0':
+        if features != '0':
             indent.decrease()
             ret += mcgen('''
     }
-- 
2.46.0

This replaces use of the constants from the QapiSpecialFeatures
enum, with constants from the auto-generate QapiFeatures enum
in qapi-features.h

The 'deprecated' and 'unstable' features still have a little bit of
special handling, being force defined to be the 1st + 2nd features
in the enum, regardless of whether they're used in the schema. This
retains compatibility with common code that references the features
via the QapiSpecialFeatures constants.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build              |  1 +
 scripts/qapi/commands.py |  1 +
 scripts/qapi/features.py | 62 ++++++++++++++++++++++++++++++++++++++++
 scripts/qapi/gen.py      |  4 +--
 scripts/qapi/main.py     |  2 ++
 scripts/qapi/schema.py   | 19 +++++++++++-
 scripts/qapi/types.py    |  6 ++--
 scripts/qapi/visit.py    |  3 +-
 8 files changed, 92 insertions(+), 6 deletions(-)
 create mode 100644 scripts/qapi/features.py

diff --git a/meson.build b/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/meson.build
+++ b/meson.build
@@ -XXX,XX +XXX,XX @@ qapi_gen_depends = [ meson.current_source_dir() / 'scripts/qapi/__init__.py',
                      meson.current_source_dir() / 'scripts/qapi/schema.py',
                      meson.current_source_dir() / 'scripts/qapi/source.py',
                      meson.current_source_dir() / 'scripts/qapi/types.py',
+                     meson.current_source_dir() / 'scripts/qapi/features.py',
                      meson.current_source_dir() / 'scripts/qapi/visit.py',
                      meson.current_source_dir() / 'scripts/qapi-gen.py'
 ]
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -XXX,XX +XXX,XX @@ def visit_begin(self, schema: QAPISchema) -> None:
 #include "qemu/osdep.h"
 #include "%(prefix)sqapi-commands.h"
 #include "%(prefix)sqapi-init-commands.h"
+#include "%(prefix)sqapi-features.h"
 
 void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
 {
diff --git a/scripts/qapi/features.py b/scripts/qapi/features.py
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/scripts/qapi/features.py
@@ -XXX,XX +XXX,XX @@
+"""
+QAPI types generator
+
+Copyright 2024 Red Hat
+
+This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+"""
+
+from typing import List, Optional
+
+from .common import c_enum_const, mcgen, c_name
+from .gen import QAPISchemaMonolithicCVisitor
+from .schema import (
+    QAPISchema,
+    QAPISchemaFeature,
+)
+from .source import QAPISourceInfo
+
+
+class QAPISchemaGenFeatureVisitor(QAPISchemaMonolithicCVisitor):
+
+    def __init__(self, prefix: str):
+        super().__init__(
+            prefix, 'qapi-features',
+            ' * Schema-defined QAPI features',
+            __doc__)
+
+        self.features = {}
+
+    def visit_begin(self, schema: QAPISchema):
+        self.features = schema._feature_dict
+
+    def visit_end(self) -> None:
+        features = [
+            self.features[f]
+            for f in QAPISchemaFeature.SPECIAL_NAMES
+        ]
+
+        features.extend(
+            sorted(
+                filter(lambda f: not f.is_special(),
+                       self.features.values()),
+                key=lambda f: f.name)
+        )
+
+        self._genh.add("typedef enum {\n")
+        for f in features:
+            self._genh.add(f"    {c_enum_const('qapi_feature', f.name)}")
+            if f.name in QAPISchemaFeature.SPECIAL_NAMES:
+                self._genh.add(f" = {c_enum_const('qapi', f.name)},\n" )
+            else:
+                self._genh.add(",\n")
+
+        self._genh.add("} " + c_name('QapiFeature') + ";\n")
+
+def gen_features(schema: QAPISchema,
+                 output_dir: str,
+                 prefix: str) -> None:
+    vis = QAPISchemaGenFeatureVisitor(prefix)
+    schema.visit(vis)
+    vis.write(output_dir)
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -XXX,XX +XXX,XX @@
 
 
 def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
-    features = [f"1u << {c_enum_const('qapi', feat.name)}"
-                        for feat in features if feat.is_special()]
+    features = [f"1u << {c_enum_const('qapi_feature', feat.name)}"
+                        for feat in features]
     return ' | '.join(features) or '0'
 
 
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -XXX,XX +XXX,XX @@
 from .introspect import gen_introspect
 from .schema import QAPISchema
 from .types import gen_types
+from .features import gen_features
 from .visit import gen_visit
 
 
@@ -XXX,XX +XXX,XX @@ def generate(schema_file: str,
 
     schema = QAPISchema(schema_file)
     gen_types(schema, output_dir, prefix, builtins)
+    gen_features(schema, output_dir, prefix)
     gen_visit(schema, output_dir, prefix, builtins)
     gen_commands(schema, output_dir, prefix, gen_tracing)
     gen_events(schema, output_dir, prefix)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -XXX,XX +XXX,XX @@ def connect_doc(self, doc: Optional[QAPIDoc]) -> None:
 class QAPISchemaFeature(QAPISchemaMember):
     role = 'feature'
 
+    # Features which are standardized across all schemas
+    SPECIAL_NAMES = ['deprecated', 'unstable']
+
     def is_special(self) -> bool:
-        return self.name in ('deprecated', 'unstable')
+        return self.name in QAPISchemaFeature.SPECIAL_NAMES
 
 
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
@@ -XXX,XX +XXX,XX @@ def __init__(self, fname: str):
         self._entity_list: List[QAPISchemaEntity] = []
         self._entity_dict: Dict[str, QAPISchemaDefinition] = {}
         self._module_dict: Dict[str, QAPISchemaModule] = OrderedDict()
+        self._feature_dict: Dict[str, QAPISchemaFeature] = {}
+
+        for f in QAPISchemaFeature.SPECIAL_NAMES:
+            self._feature_dict[f] = QAPISchemaFeature(f, "special feature")
+
         self._schema_dir = os.path.dirname(fname)
         self._make_module(QAPISchemaModule.BUILTIN_MODULE_NAME)
         self._make_module(fname)
@@ -XXX,XX +XXX,XX @@ def _make_features(
     ) -> List[QAPISchemaFeature]:
         if features is None:
             return []
+
+        for f in features:
+            feat = QAPISchemaFeature(f['name'], info)
+            if feat.name not in self._feature_dict:
+                if len(self._feature_dict) == 64:
+                    raise Exception("Maximum of 64 schema features is permitted")
+
+                self._feature_dict[feat.name] = feat
+
         return [QAPISchemaFeature(f['name'], info,
                                   QAPISchemaIfCond(f.get('if')))
                 for f in features]
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -XXX,XX +XXX,XX @@ def _begin_user_module(self, name: str) -> None:
 #include "qapi/dealloc-visitor.h"
 #include "%(types)s.h"
 #include "%(visit)s.h"
+#include "%(prefix)sqapi-features.h"
 ''',
-                                      types=types, visit=visit))
+                                      types=types, visit=visit, prefix=self._prefix))
         self._genh.preamble_add(mcgen('''
 #include "qapi/qapi-builtin-types.h"
-'''))
+''',
+                                      prefix=self._prefix))
 
     def visit_begin(self, schema: QAPISchema) -> None:
         # gen_object() is recursive, ensure it doesn't visit the empty type
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -XXX,XX +XXX,XX @@ def _begin_user_module(self, name: str) -> None:
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "%(visit)s.h"
+#include "%(prefix)sqapi-features.h"
 ''',
-                                      visit=visit))
+                                      visit=visit, prefix=self._prefix))
         self._genh.preamble_add(mcgen('''
 #include "qapi/qapi-builtin-visit.h"
 #include "%(types)s.h"
-- 
2.46.0

This series is a spin-off from

https://lists.nongnu.org/archive/html/qemu-devel/2024-06/msg00807.html

That series introduced a pragma allowing a schema to declare extra
features that would be exposed to code.

Following Markus' suggestion:

https://lists.nongnu.org/archive/html/qemu-devel/2024-07/msg03765.html

I've changed impl such that we expose all features to the code
regardless of whether they are special, and don't require any pragma.

I've split it from the QGA patches since it makes more sense to work
on this bit in isolation.

Changed in v3:

* "make -C python check-tox" is now clean
 * Added test case for "too many features" error scenario
 * Don't sort the features enum, just rely on ordered dict
 * Add 'features' accessor on QAPISchema instead of touching
   private data via 'schema._features.values()'

Changed in v2:

* Reference QapiSpecialFeature enums constants when defining
   QapiFeature enums constants for deprecated & unstable

* Collect all features during parse time, rather than
   generate time, to allow earlier error reporting of the
   64 feature limit

include/qapi/compat-policy.h             |  2 +-
 include/qapi/qmp/dispatch.h              |  4 +-
 include/qapi/util.h                      |  2 +-
 include/qapi/visitor-impl.h              |  4 +-
 include/qapi/visitor.h                   | 12 +++---
 meson.build                              |  1 +
 qapi/qapi-forward-visitor.c              |  8 ++--
 qapi/qapi-util.c                         |  6 +--
 qapi/qapi-visit-core.c                   | 12 +++---
 qapi/qmp-dispatch.c                      |  2 +-
 qapi/qmp-registry.c                      |  4 +-
 qapi/qobject-input-visitor.c             |  4 +-
 qapi/qobject-output-visitor.c            |  6 +--
 scripts/qapi/commands.py                 |  5 ++-
 scripts/qapi/features.py                 | 51 ++++++++++++++++++++++++
 scripts/qapi/gen.py                      |  9 +++--
 scripts/qapi/main.py                     |  2 +
 scripts/qapi/schema.py                   | 30 +++++++++++++-
 scripts/qapi/types.py                    | 19 +++++----
 scripts/qapi/visit.py                    | 17 ++++----
 tests/meson.build                        |  2 +
 tests/qapi-schema/features-too-many.err  |  2 +
 tests/qapi-schema/features-too-many.json | 13 ++++++
 tests/qapi-schema/features-too-many.out  |  0
 tests/qapi-schema/meson.build            |  1 +
 25 files changed, 162 insertions(+), 56 deletions(-)
 create mode 100644 scripts/qapi/features.py
 create mode 100644 tests/qapi-schema/features-too-many.err
 create mode 100644 tests/qapi-schema/features-too-many.json
 create mode 100644 tests/qapi-schema/features-too-many.out

-- 
2.46.0

The "special_features" field / parameter holds the subset of schema
features that are for internal code use. Specifically 'DEPRECATED'
and 'UNSTABLE'.

This special casing of internal features is going to be removed, so
prepare for that by renaming to 'features'. Using a fixed size type
is also best practice for bit fields.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/qapi/compat-policy.h  |  2 +-
 include/qapi/qmp/dispatch.h   |  4 ++--
 include/qapi/util.h           |  2 +-
 include/qapi/visitor-impl.h   |  4 ++--
 include/qapi/visitor.h        | 12 ++++++------
 qapi/qapi-forward-visitor.c   |  8 ++++----
 qapi/qapi-util.c              |  6 +++---
 qapi/qapi-visit-core.c        | 12 ++++++------
 qapi/qmp-dispatch.c           |  2 +-
 qapi/qmp-registry.c           |  4 ++--
 qapi/qobject-input-visitor.c  |  4 ++--
 qapi/qobject-output-visitor.c |  6 +++---
 scripts/qapi/types.py         |  2 +-
 13 files changed, 34 insertions(+), 34 deletions(-)

This updates the QAPI code generation to refer to 'features' instead
of 'special_features', in preparation for generalizing their exposure.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 scripts/qapi/commands.py |  4 ++--
 scripts/qapi/gen.py      |  8 ++++----
 scripts/qapi/types.py    | 10 +++++-----
 scripts/qapi/visit.py    | 14 +++++++-------
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -XXX,XX +XXX,XX @@
     QAPIGenC,
     QAPISchemaModularCVisitor,
     build_params,
-    gen_special_features,
+    gen_features,
     ifcontext,
 )
 from .schema import (
@@ -XXX,XX +XXX,XX @@ def gen_register_command(name: str,
 ''',
                 name=name, c_name=c_name(name),
                 opts=' | '.join(options) or 0,
-                feats=gen_special_features(features))
+                feats=gen_features(features))
     return ret
 
 
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -XXX,XX +XXX,XX @@
 from .source import QAPISourceInfo
 
 
-def gen_special_features(features: Sequence[QAPISchemaFeature]) -> str:
-    special_features = [f"1u << {c_enum_const('qapi', feat.name)}"
-                        for feat in features if feat.is_special()]
-    return ' | '.join(special_features) or '0'
+def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
+    featenum = [f"1u << {c_enum_const('qapi', feat.name)}"
+                for feat in features if feat.is_special()]
+    return ' | '.join(featenum) or '0'
 
 
 class QAPIGen:
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -XXX,XX +XXX,XX @@
 from .common import c_enum_const, c_name, mcgen
 from .gen import (
     QAPISchemaModularCVisitor,
-    gen_special_features,
+    gen_features,
     ifcontext,
 )
 from .schema import (
@@ -XXX,XX +XXX,XX @@ def gen_enum_lookup(name: str,
                      index=index, name=memb.name)
         ret += memb.ifcond.gen_endif()
 
-        special_features = gen_special_features(memb.features)
-        if special_features != '0':
+        features = gen_features(memb.features)
+        if features != '0':
             feats += mcgen('''
-        [%(index)s] = %(special_features)s,
+        [%(index)s] = %(features)s,
 ''',
-                           index=index, special_features=special_features)
+                           index=index, features=features)
 
     if feats:
         ret += mcgen('''
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -XXX,XX +XXX,XX @@
 )
 from .gen import (
     QAPISchemaModularCVisitor,
-    gen_special_features,
+    gen_features,
     ifcontext,
 )
 from .schema import (
@@ -XXX,XX +XXX,XX @@ def gen_visit_object_members(name: str,
 ''',
                          name=memb.name, has=has)
             indent.increase()
-        special_features = gen_special_features(memb.features)
-        if special_features != '0':
+        features = gen_features(memb.features)
+        if features != '0':
             ret += mcgen('''
-    if (visit_policy_reject(v, "%(name)s", %(special_features)s, errp)) {
+    if (visit_policy_reject(v, "%(name)s", %(features)s, errp)) {
         return false;
     }
-    if (!visit_policy_skip(v, "%(name)s", %(special_features)s)) {
+    if (!visit_policy_skip(v, "%(name)s", %(features)s)) {
 ''',
-                         name=memb.name, special_features=special_features)
+                         name=memb.name, features=features)
             indent.increase()
         ret += mcgen('''
     if (!visit_type_%(c_type)s(v, "%(name)s", &obj->%(c_name)s, errp)) {
@@ -XXX,XX +XXX,XX @@ def gen_visit_object_members(name: str,
 ''',
                      c_type=memb.type.c_name(), name=memb.name,
                      c_name=c_name(memb.name))
-        if special_features != '0':
+        if features != '0':
             indent.decrease()
             ret += mcgen('''
     }
-- 
2.46.0

This replaces use of the constants from the QapiSpecialFeatures
enum, with constants from the auto-generate QapiFeatures enum
in qapi-features.h

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build                              |  1 +
 scripts/qapi/commands.py                 |  1 +
 scripts/qapi/features.py                 | 51 ++++++++++++++++++++++++
 scripts/qapi/gen.py                      |  6 +--
 scripts/qapi/main.py                     |  2 +
 scripts/qapi/schema.py                   | 30 +++++++++++++-
 scripts/qapi/types.py                    |  7 +++-
 scripts/qapi/visit.py                    |  3 +-
 tests/meson.build                        |  2 +
 tests/qapi-schema/features-too-many.err  |  2 +
 tests/qapi-schema/features-too-many.json | 13 ++++++
 tests/qapi-schema/features-too-many.out  |  0
 tests/qapi-schema/meson.build            |  1 +
 13 files changed, 112 insertions(+), 7 deletions(-)
 create mode 100644 scripts/qapi/features.py
 create mode 100644 tests/qapi-schema/features-too-many.err
 create mode 100644 tests/qapi-schema/features-too-many.json
 create mode 100644 tests/qapi-schema/features-too-many.out

diff --git a/meson.build b/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/meson.build
+++ b/meson.build
@@ -XXX,XX +XXX,XX @@ qapi_gen_depends = [ meson.current_source_dir() / 'scripts/qapi/__init__.py',
                      meson.current_source_dir() / 'scripts/qapi/schema.py',
                      meson.current_source_dir() / 'scripts/qapi/source.py',
                      meson.current_source_dir() / 'scripts/qapi/types.py',
+                     meson.current_source_dir() / 'scripts/qapi/features.py',
                      meson.current_source_dir() / 'scripts/qapi/visit.py',
                      meson.current_source_dir() / 'scripts/qapi-gen.py'
 ]
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -XXX,XX +XXX,XX @@ def visit_begin(self, schema: QAPISchema) -> None:
 #include "qemu/osdep.h"
 #include "%(prefix)sqapi-commands.h"
 #include "%(prefix)sqapi-init-commands.h"
+#include "%(prefix)sqapi-features.h"
 
 void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
 {
diff --git a/scripts/qapi/features.py b/scripts/qapi/features.py
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/scripts/qapi/features.py
@@ -XXX,XX +XXX,XX @@
+"""
+QAPI features generator
+
+Copyright 2024 Red Hat
+
+This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+"""
+
+from typing import Dict
+
+from .common import c_enum_const, c_name
+from .gen import QAPISchemaMonolithicCVisitor
+from .schema import (
+    QAPISchema,
+    QAPISchemaFeature,
+)
+
+
+class QAPISchemaGenFeatureVisitor(QAPISchemaMonolithicCVisitor):
+
+    def __init__(self, prefix: str):
+        super().__init__(
+            prefix, 'qapi-features',
+            ' * Schema-defined QAPI features',
+            __doc__)
+
+        self.features: Dict[str, QAPISchemaFeature] = {}
+
+    def visit_begin(self, schema: QAPISchema) -> None:
+        self.features = schema.features()
+        self._genh.add("#include \"qapi/util.h\"\n\n")
+
+    def visit_end(self) -> None:
+        self._genh.add("typedef enum {\n")
+        for f in self.features:
+            self._genh.add(f"    {c_enum_const('qapi_feature', f.name)}")
+            if f.name in QAPISchemaFeature.SPECIAL_NAMES:
+                self._genh.add(f" = {c_enum_const('qapi', f.name)},\n")
+            else:
+                self._genh.add(",\n")
+
+        self._genh.add("} " + c_name('QapiFeature') + ";\n")
+
+
+def gen_features(schema: QAPISchema,
+                 output_dir: str,
+                 prefix: str) -> None:
+    vis = QAPISchemaGenFeatureVisitor(prefix)
+    schema.visit(vis)
+    vis.write(output_dir)
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -XXX,XX +XXX,XX @@
 
 
 def gen_features(features: Sequence[QAPISchemaFeature]) -> str:
-    featenum = [f"1u << {c_enum_const('qapi', feat.name)}"
-                for feat in features if feat.is_special()]
-    return ' | '.join(featenum) or '0'
+    feats = [f"1u << {c_enum_const('qapi_feature', feat.name)}"
+             for feat in features]
+    return ' | '.join(feats) or '0'
 
 
 class QAPIGen:
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -XXX,XX +XXX,XX @@
 from .introspect import gen_introspect
 from .schema import QAPISchema
 from .types import gen_types
+from .features import gen_features
 from .visit import gen_visit
 
 
@@ -XXX,XX +XXX,XX @@ def generate(schema_file: str,
 
     schema = QAPISchema(schema_file)
     gen_types(schema, output_dir, prefix, builtins)
+    gen_features(schema, output_dir, prefix)
     gen_visit(schema, output_dir, prefix, builtins)
     gen_commands(schema, output_dir, prefix, gen_tracing)
     gen_events(schema, output_dir, prefix)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -XXX,XX +XXX,XX @@ def connect_doc(self, doc: Optional[QAPIDoc]) -> None:
 class QAPISchemaFeature(QAPISchemaMember):
     role = 'feature'
 
+    # Features which are standardized across all schemas
+    SPECIAL_NAMES = ['deprecated', 'unstable']
+
     def is_special(self) -> bool:
-        return self.name in ('deprecated', 'unstable')
+        return self.name in QAPISchemaFeature.SPECIAL_NAMES
 
 
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
@@ -XXX,XX +XXX,XX @@ def __init__(self, fname: str):
         self._entity_list: List[QAPISchemaEntity] = []
         self._entity_dict: Dict[str, QAPISchemaDefinition] = {}
         self._module_dict: Dict[str, QAPISchemaModule] = OrderedDict()
+        # NB, values in the dict will identify the first encountered
+        #     usage of a named feature only
+        self._feature_dict: Dict[str, QAPISchemaFeature] = OrderedDict()
+
+        # All schemas get the names defined in the QapiSpecialFeature enum.
+        # Use of OrderedDict ensures they are emitted first when generating
+        # the enum definition, thus matching QapiSpecialFeature.
+        for f in QAPISchemaFeature.SPECIAL_NAMES:
+            self._feature_dict[f] = QAPISchemaFeature(f, None)
+
         self._schema_dir = os.path.dirname(fname)
         self._make_module(QAPISchemaModule.BUILTIN_MODULE_NAME)
         self._make_module(fname)
@@ -XXX,XX +XXX,XX @@ def __init__(self, fname: str):
         self._def_exprs(exprs)
         self.check()
 
+    def features(self) -> List[QAPISchemaFeature]:
+        return self._feature_dict.values()
+
     def _def_entity(self, ent: QAPISchemaEntity) -> None:
         self._entity_list.append(ent)
 
@@ -XXX,XX +XXX,XX @@ def _make_features(
     ) -> List[QAPISchemaFeature]:
         if features is None:
             return []
+
+        for f in features:
+            feat = QAPISchemaFeature(f['name'], info)
+            if feat.name not in self._feature_dict:
+                self._feature_dict[feat.name] = feat
+
         return [QAPISchemaFeature(f['name'], info,
                                   QAPISchemaIfCond(f.get('if')))
                 for f in features]
@@ -XXX,XX +XXX,XX @@ def check(self) -> None:
         for doc in self.docs:
             doc.check()
 
+        features = list(self._feature_dict.values())
+        if len(features) > 64:
+            raise QAPISemError(
+                features[64].info,
+                "Maximum of 64 schema features is permitted")
+
     def visit(self, visitor: QAPISchemaVisitor) -> None:
         visitor.visit_begin(self)
         for mod in self._module_dict.values():
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -XXX,XX +XXX,XX @@ def _begin_user_module(self, name: str) -> None:
 #include "qapi/dealloc-visitor.h"
 #include "%(types)s.h"
 #include "%(visit)s.h"
+#include "%(prefix)sqapi-features.h"
 ''',
-                                      types=types, visit=visit))
+                                      types=types, visit=visit,
+                                      prefix=self._prefix))
         self._genh.preamble_add(mcgen('''
 #include "qapi/qapi-builtin-types.h"
-'''))
+''',
+                                      prefix=self._prefix))
 
     def visit_begin(self, schema: QAPISchema) -> None:
         # gen_object() is recursive, ensure it doesn't visit the empty type
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index XXXXXXX..XXXXXXX 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -XXX,XX +XXX,XX @@ def _begin_user_module(self, name: str) -> None:
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "%(visit)s.h"
+#include "%(prefix)sqapi-features.h"
 ''',
-                                      visit=visit))
+                                      visit=visit, prefix=self._prefix))
         self._genh.preamble_add(mcgen('''
 #include "qapi/qapi-builtin-visit.h"
 #include "%(types)s.h"
diff --git a/tests/meson.build b/tests/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -XXX,XX +XXX,XX @@ test_qapi_outputs = [
   'test-qapi-events-sub-sub-module.h',
   'test-qapi-events.c',
   'test-qapi-events.h',
+  'test-qapi-features.c',
+  'test-qapi-features.h',
   'test-qapi-init-commands.c',
   'test-qapi-init-commands.h',
   'test-qapi-introspect.c',
diff --git a/tests/qapi-schema/features-too-many.err b/tests/qapi-schema/features-too-many.err
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qapi-schema/features-too-many.err
@@ -XXX,XX +XXX,XX @@
+features-too-many.json: In command 'go-fish':
+features-too-many.json:2: Maximum of 64 schema features is permitted
diff --git a/tests/qapi-schema/features-too-many.json b/tests/qapi-schema/features-too-many.json
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qapi-schema/features-too-many.json
@@ -XXX,XX +XXX,XX @@
+# Max 64 features, with 2 specials, so 63rd custom is invalid
+{ 'command': 'go-fish',
+  'features': [
+      'f00', 'f01', 'f02', 'f03', 'f04', 'f05', 'f06', 'f07',
+      'f08', 'f09', 'f0a', 'f0b', 'f0c', 'f0d', 'f0e', 'f0f',
+      'f10', 'f11', 'f12', 'f13', 'f14', 'f15', 'f16', 'f17',
+      'f18', 'f19', 'f1a', 'f1b', 'f1c', 'f1d', 'f1e', 'f1f',
+      'f20', 'f21', 'f22', 'f23', 'f24', 'f25', 'f26', 'f27',
+      'f28', 'f29', 'f2a', 'f2b', 'f2c', 'f2d', 'f2e', 'f2f',
+      'f30', 'f31', 'f32', 'f33', 'f34', 'f35', 'f36', 'f37',
+      'f38', 'f39', 'f3a', 'f3b', 'f3c', 'f3d', 'f3e'
+  ]
+}
diff --git a/tests/qapi-schema/features-too-many.out b/tests/qapi-schema/features-too-many.out
new file mode 100644
index XXXXXXX..XXXXXXX
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index XXXXXXX..XXXXXXX 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -XXX,XX +XXX,XX @@ schemas = [
   'event-case.json',
   'event-member-invalid-dict.json',
   'event-nest-struct.json',
+  'features-too-many.json',
   'features-bad-type.json',
   'features-deprecated-type.json',
   'features-duplicate-name.json',
-- 
2.46.0