* [PATCH v4 1/4] qapi: cope with feature names containing a '-'
2025-02-05 12:35 [PATCH v4 0/4] qapi: generalize special features Daniel P. Berrangé
@ 2025-02-05 12:35 ` Daniel P. Berrangé
2025-02-05 12:35 ` [PATCH v4 2/4] qapi: change 'unsigned special_features' to 'uint64_t features' Daniel P. Berrangé
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-02-05 12:35 UTC (permalink / raw)
To: qemu-devel
Cc: Marc-André Lureau, Daniel P. Berrangé,
Markus Armbruster, Michael Roth, Philippe Mathieu-Daudé,
Paolo Bonzini
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 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 6a8abe0041..c53ca72950 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -24,6 +24,7 @@
)
from .common import (
+ c_enum_const,
c_fname,
c_name,
guardend,
@@ -41,7 +42,7 @@
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'
--
2.47.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 2/4] qapi: change 'unsigned special_features' to 'uint64_t features'
2025-02-05 12:35 [PATCH v4 0/4] qapi: generalize special features Daniel P. Berrangé
2025-02-05 12:35 ` [PATCH v4 1/4] qapi: cope with feature names containing a '-' Daniel P. Berrangé
@ 2025-02-05 12:35 ` Daniel P. Berrangé
2025-02-05 12:35 ` [PATCH v4 3/4] qapi: rename 'special_features' to 'features' Daniel P. Berrangé
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-02-05 12:35 UTC (permalink / raw)
To: qemu-devel
Cc: Marc-André Lureau, Daniel P. Berrangé,
Markus Armbruster, Michael Roth, Philippe Mathieu-Daudé,
Paolo Bonzini
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(-)
diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h
index 8b7b25c0b5..ea65e10744 100644
--- a/include/qapi/compat-policy.h
+++ b/include/qapi/compat-policy.h
@@ -18,7 +18,7 @@
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 f2e956813a..e0ee1ad3ac 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -33,7 +33,7 @@ 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;
@@ -43,7 +43,7 @@ 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 b8254247b8..29bc4eb865 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -18,7 +18,7 @@ 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 2badec5ba4..7beb0dbfa5 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -115,11 +115,11 @@ 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 27b85d4700..f6a9b0743f 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -463,29 +463,29 @@ 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 e36d9bc9ba..6e9a784a9f 100644
--- a/qapi/qapi-forward-visitor.c
+++ b/qapi/qapi-forward-visitor.c
@@ -246,7 +246,7 @@ 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);
@@ -254,18 +254,18 @@ 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 65a7d18437..3d849fe034 100644
--- a/qapi/qapi-util.c
+++ b/qapi/qapi-util.c
@@ -37,19 +37,19 @@ 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 6c13510a2b..706c61e026 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -141,21 +141,21 @@ 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;
}
@@ -409,8 +409,8 @@ 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 176b549473..d411eebf4e 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -173,7 +173,7 @@ 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 485bc5e6fc..bfcabec526 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -17,7 +17,7 @@
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));
@@ -28,7 +28,7 @@ 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 f110a804b2..ff9c726de3 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -664,10 +664,10 @@ 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 74770edd73..8902287caa 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -210,13 +210,13 @@ 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 0dd0b00ada..7bc3f8241f 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -71,7 +71,7 @@ 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.47.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4 3/4] qapi: rename 'special_features' to 'features'
2025-02-05 12:35 [PATCH v4 0/4] qapi: generalize special features Daniel P. Berrangé
2025-02-05 12:35 ` [PATCH v4 1/4] qapi: cope with feature names containing a '-' Daniel P. Berrangé
2025-02-05 12:35 ` [PATCH v4 2/4] qapi: change 'unsigned special_features' to 'uint64_t features' Daniel P. Berrangé
@ 2025-02-05 12:35 ` Daniel P. Berrangé
2025-02-07 11:43 ` Markus Armbruster
2025-02-05 12:35 ` [PATCH v4 4/4] qapi: expose all schema features to code Daniel P. Berrangé
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-02-05 12:35 UTC (permalink / raw)
To: qemu-devel
Cc: Marc-André Lureau, Daniel P. Berrangé,
Markus Armbruster, Michael Roth, Philippe Mathieu-Daudé,
Paolo Bonzini
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 79951a841f..d629d2d97e 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -25,7 +25,7 @@
QAPIGenC,
QAPISchemaModularCVisitor,
build_params,
- gen_special_features,
+ gen_features,
ifcontext,
)
from .schema import (
@@ -298,7 +298,7 @@ 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 c53ca72950..b51f8d955e 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -41,10 +41,10 @@
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 7bc3f8241f..ade6b7a3d7 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -18,7 +18,7 @@
from .common import c_enum_const, c_name, mcgen
from .gen import (
QAPISchemaModularCVisitor,
- gen_special_features,
+ gen_features,
ifcontext,
)
from .schema import (
@@ -61,12 +61,12 @@ 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 12f92e429f..8dbf4ef1c3 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -23,7 +23,7 @@
)
from .gen import (
QAPISchemaModularCVisitor,
- gen_special_features,
+ gen_features,
ifcontext,
)
from .schema import (
@@ -103,15 +103,15 @@ 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)) {
@@ -120,7 +120,7 @@ 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.47.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 3/4] qapi: rename 'special_features' to 'features'
2025-02-05 12:35 ` [PATCH v4 3/4] qapi: rename 'special_features' to 'features' Daniel P. Berrangé
@ 2025-02-07 11:43 ` Markus Armbruster
0 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2025-02-07 11:43 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Marc-André Lureau, Michael Roth,
Philippe Mathieu-Daudé, Paolo Bonzini, John Snow
Daniel P. Berrangé <berrange@redhat.com> writes:
> 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>
John pointed out that isort wants the appended fixup. Not thrilled
about the additional churn, but let's not fight our tools. Thanks,
John!
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index ade6b7a3d7..e4a1bb9f85 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -16,11 +16,7 @@
from typing import List, Optional
from .common import c_enum_const, c_name, mcgen
-from .gen import (
- QAPISchemaModularCVisitor,
- gen_features,
- ifcontext,
-)
+from .gen import QAPISchemaModularCVisitor, gen_features, ifcontext
from .schema import (
QAPISchema,
QAPISchemaAlternatives,
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 8dbf4ef1c3..928273b9bb 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -21,11 +21,7 @@
indent,
mcgen,
)
-from .gen import (
- QAPISchemaModularCVisitor,
- gen_features,
- ifcontext,
-)
+from .gen import QAPISchemaModularCVisitor, gen_features, ifcontext
from .schema import (
QAPISchema,
QAPISchemaAlternatives,
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 4/4] qapi: expose all schema features to code
2025-02-05 12:35 [PATCH v4 0/4] qapi: generalize special features Daniel P. Berrangé
` (2 preceding siblings ...)
2025-02-05 12:35 ` [PATCH v4 3/4] qapi: rename 'special_features' to 'features' Daniel P. Berrangé
@ 2025-02-05 12:35 ` Daniel P. Berrangé
2025-02-07 11:45 ` Markus Armbruster
2025-02-07 11:57 ` Markus Armbruster
2025-02-10 8:25 ` [PATCH v4 0/4] qapi: generalize special features Markus Armbruster
2025-02-10 8:37 ` Markus Armbruster
5 siblings, 2 replies; 12+ messages in thread
From: Daniel P. Berrangé @ 2025-02-05 12:35 UTC (permalink / raw)
To: qemu-devel
Cc: Marc-André Lureau, Daniel P. Berrangé,
Markus Armbruster, Michael Roth, Philippe Mathieu-Daudé,
Paolo Bonzini
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 | 51 ++++++++++++++++++++++++
scripts/qapi/gen.py | 6 +--
scripts/qapi/main.py | 2 +
scripts/qapi/schema.py | 31 +++++++++++++-
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, 113 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 2c9ac9cfe1..67759df257 100644
--- a/meson.build
+++ b/meson.build
@@ -3444,6 +3444,7 @@ 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 d629d2d97e..bf88bfc442 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -355,6 +355,7 @@ 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 0000000000..be3e5d03ff
--- /dev/null
+++ b/scripts/qapi/features.py
@@ -0,0 +1,51 @@
+"""
+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, ValuesView
+
+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: ValuesView[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 b51f8d955e..d3c56d45c8 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -42,9 +42,9 @@
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 316736b6a2..2b9a2c0c02 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -18,6 +18,7 @@
from .introspect import gen_introspect
from .schema import QAPISchema
from .types import gen_types
+from .features import gen_features
from .visit import gen_visit
@@ -49,6 +50,7 @@ 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 e97c978d38..7f70969c09 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -29,6 +29,7 @@
List,
Optional,
Union,
+ ValuesView,
cast,
)
@@ -933,8 +934,11 @@ 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):
@@ -1138,6 +1142,16 @@ 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.
+ # Rely on dict iteration order matching insertion order so that
+ # the special names are emitted first when generating code.
+ 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)
@@ -1147,6 +1161,9 @@ def __init__(self, fname: str):
self._def_exprs(exprs)
self.check()
+ def features(self) -> ValuesView[QAPISchemaFeature]:
+ return self._feature_dict.values()
+
def _def_entity(self, ent: QAPISchemaEntity) -> None:
self._entity_list.append(ent)
@@ -1258,6 +1275,12 @@ 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]
@@ -1485,6 +1508,12 @@ 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 ade6b7a3d7..5294e5ea3b 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -308,11 +308,14 @@ 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 8dbf4ef1c3..2d678c281d 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -360,8 +360,9 @@ 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 f96c1be574..c59619220f 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -16,6 +16,8 @@ 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 0000000000..bbbd6e5202
--- /dev/null
+++ b/tests/qapi-schema/features-too-many.err
@@ -0,0 +1,2 @@
+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 0000000000..aab0a0b5f1
--- /dev/null
+++ b/tests/qapi-schema/features-too-many.json
@@ -0,0 +1,13 @@
+# 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 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 0f479d9317..9577178b6f 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -105,6 +105,7 @@ 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.47.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 4/4] qapi: expose all schema features to code
2025-02-05 12:35 ` [PATCH v4 4/4] qapi: expose all schema features to code Daniel P. Berrangé
@ 2025-02-07 11:45 ` Markus Armbruster
2025-02-07 11:57 ` Markus Armbruster
1 sibling, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2025-02-07 11:45 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Marc-André Lureau, Michael Roth,
Philippe Mathieu-Daudé, Paolo Bonzini, John Snow
Daniel P. Berrangé <berrange@redhat.com> writes:
> 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>
John pointed out that isort wants the appended fixup. Not thrilled
about the additional churn in the first hunk, but let's not fight our
tools. Thanks, John!
diff --git a/scripts/qapi/features.py b/scripts/qapi/features.py
index be3e5d03ff..16ed26672b 100644
--- a/scripts/qapi/features.py
+++ b/scripts/qapi/features.py
@@ -11,10 +11,7 @@
from .common import c_enum_const, c_name
from .gen import QAPISchemaMonolithicCVisitor
-from .schema import (
- QAPISchema,
- QAPISchemaFeature,
-)
+from .schema import QAPISchema, QAPISchemaFeature
class QAPISchemaGenFeatureVisitor(QAPISchemaMonolithicCVisitor):
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 2b9a2c0c02..324081b9fc 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -15,10 +15,10 @@
from .common import must_match
from .error import QAPIError
from .events import gen_events
+from .features import gen_features
from .introspect import gen_introspect
from .schema import QAPISchema
from .types import gen_types
-from .features import gen_features
from .visit import gen_visit
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 4/4] qapi: expose all schema features to code
2025-02-05 12:35 ` [PATCH v4 4/4] qapi: expose all schema features to code Daniel P. Berrangé
2025-02-07 11:45 ` Markus Armbruster
@ 2025-02-07 11:57 ` Markus Armbruster
2025-02-07 18:56 ` John Snow
1 sibling, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2025-02-07 11:57 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Marc-André Lureau, Michael Roth,
Philippe Mathieu-Daudé, Paolo Bonzini, John Snow
Daniel P. Berrangé <berrange@redhat.com> writes:
> 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>
Daniel, feel free to ignore this at least for now. I'm trying to learn
some typing lore from John.
v3 made mypy unhappy. I asked John for advice, and also posted a
solution involving ValuesView I hacked up myself. Daniel took it for
v4.
John suggested to use List.
I now wonder whether could use Iterable.
I'll show the three solutions inline.
John, thoughts?
[...]
> diff --git a/scripts/qapi/features.py b/scripts/qapi/features.py
> new file mode 100644
> index 0000000000..be3e5d03ff
> --- /dev/null
> +++ b/scripts/qapi/features.py
> @@ -0,0 +1,51 @@
> +"""
> +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, ValuesView
> +
> +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: ValuesView[QAPISchemaFeature]
This is the ValuesView solution.
The List solution:
self.features: List[QAPISchemaFeature] = []
The Iterable solution:
self.features: Iterable[QAPISchemaFeature]
[...]
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index e97c978d38..7f70969c09 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
[...]
> @@ -1147,6 +1161,9 @@ def __init__(self, fname: str):
> self._def_exprs(exprs)
> self.check()
>
> + def features(self) -> ValuesView[QAPISchemaFeature]:
> + return self._feature_dict.values()
This is the ValuesView solution.
The List solution:
def features(self) -> List[QAPISchemaFeature]:
return list(self._feature_dict.values())
The Iterable solution:
def features(self) -> Iterable[QAPISchemaFeature]:
return self._feature_dict.values()
> +
> def _def_entity(self, ent: QAPISchemaEntity) -> None:
> self._entity_list.append(ent)
>
[...]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 4/4] qapi: expose all schema features to code
2025-02-07 11:57 ` Markus Armbruster
@ 2025-02-07 18:56 ` John Snow
2025-02-10 8:24 ` Markus Armbruster
0 siblings, 1 reply; 12+ messages in thread
From: John Snow @ 2025-02-07 18:56 UTC (permalink / raw)
To: Markus Armbruster
Cc: Daniel P. Berrangé, qemu-devel, Marc-André Lureau,
Michael Roth, Philippe Mathieu-Daudé, Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 4362 bytes --]
On Fri, Feb 7, 2025, 6:57 AM Markus Armbruster <armbru@redhat.com> wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > 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>
>
> Daniel, feel free to ignore this at least for now. I'm trying to learn
> some typing lore from John.
>
> v3 made mypy unhappy. I asked John for advice, and also posted a
> solution involving ValuesView I hacked up myself. Daniel took it for
> v4.
>
> John suggested to use List.
>
> I now wonder whether could use Iterable.
>
> I'll show the three solutions inline.
>
> John, thoughts?
>
ValuesView works just fine. It accurately describes what that function
returns. I only avoided it in my fixup because it's a more obscure type and
generally list is easier to work with as a first-class built in primitive
type to the language.
(read as: I didn't have to consult any docs to fix it up using List and I'm
lazy.)
Your solution describes precisely the type being returned (always good) and
avoids any re-copying of data.
Do be aware by caching the values view object in another object that you
are keeping a "live reference" to the list of dict values that I think can
change if the source dict changes. I doubt it matters, but you should know
about that.
The only design consideration you have now is what type you actually want
to return and why. I think it barely matters, and I'm always going to opt
for whatever is the least annoying for the patch author so I don't have to
bore/torture them with python minutiae.
As long as the tests pass (my first three patches in the dan-fixup branch I
posted based on v3) I'm more than content.
> [...]
>
> > diff --git a/scripts/qapi/features.py b/scripts/qapi/features.py
> > new file mode 100644
> > index 0000000000..be3e5d03ff
> > --- /dev/null
> > +++ b/scripts/qapi/features.py
> > @@ -0,0 +1,51 @@
> > +"""
> > +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, ValuesView
> > +
> > +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: ValuesView[QAPISchemaFeature]
>
> This is the ValuesView solution.
>
> The List solution:
>
> self.features: List[QAPISchemaFeature] = []
>
> The Iterable solution:
>
> self.features: Iterable[QAPISchemaFeature]
>
> [...]
>
>
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index e97c978d38..7f70969c09 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
>
> [...]
>
> > @@ -1147,6 +1161,9 @@ def __init__(self, fname: str):
> > self._def_exprs(exprs)
> > self.check()
> >
> > + def features(self) -> ValuesView[QAPISchemaFeature]:
> > + return self._feature_dict.values()
>
> This is the ValuesView solution.
>
> The List solution:
>
> def features(self) -> List[QAPISchemaFeature]:
> return list(self._feature_dict.values())
>
> The Iterable solution:
>
> def features(self) -> Iterable[QAPISchemaFeature]:
> return self._feature_dict.values()
>
>
> > +
> > def _def_entity(self, ent: QAPISchemaEntity) -> None:
> > self._entity_list.append(ent)
> >
>
> [...]
>
>
[-- Attachment #2: Type: text/html, Size: 5927 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 4/4] qapi: expose all schema features to code
2025-02-07 18:56 ` John Snow
@ 2025-02-10 8:24 ` Markus Armbruster
0 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2025-02-10 8:24 UTC (permalink / raw)
To: John Snow
Cc: Daniel P. Berrangé, qemu-devel, Marc-André Lureau,
Michael Roth, Philippe Mathieu-Daudé, Paolo Bonzini
John Snow <jsnow@redhat.com> writes:
> On Fri, Feb 7, 2025, 6:57 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>> > 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>
>>
>> Daniel, feel free to ignore this at least for now. I'm trying to learn
>> some typing lore from John.
>>
>> v3 made mypy unhappy. I asked John for advice, and also posted a
>> solution involving ValuesView I hacked up myself. Daniel took it for
>> v4.
>>
>> John suggested to use List.
>>
>> I now wonder whether could use Iterable.
>>
>> I'll show the three solutions inline.
>>
>> John, thoughts?
>>
>
> ValuesView works just fine. It accurately describes what that function
> returns. I only avoided it in my fixup because it's a more obscure type and
> generally list is easier to work with as a first-class built in primitive
> type to the language.
>
> (read as: I didn't have to consult any docs to fix it up using List and I'm
> lazy.)
Aside: John later shared a useful technique on IRC: "you can write
reveal_type(foo) to get mypy to spill the beans on what it thinks".
> Your solution describes precisely the type being returned (always good) and
> avoids any re-copying of data.
>
> Do be aware by caching the values view object in another object that you
> are keeping a "live reference" to the list of dict values that I think can
> change if the source dict changes.
Yes.
> I doubt it matters, but you should know
> about that.
I believe it's just fine.
> The only design consideration you have now is what type you actually want
> to return and why. I think it barely matters, and I'm always going to opt
> for whatever is the least annoying for the patch author so I don't have to
> bore/torture them with python minutiae.
Since the typing in Daniel's patch is fine, I'll refrain from messing
with it.
> As long as the tests pass (my first three patches in the dan-fixup branch I
> posted based on v3) I'm more than content.
Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 0/4] qapi: generalize special features
2025-02-05 12:35 [PATCH v4 0/4] qapi: generalize special features Daniel P. Berrangé
` (3 preceding siblings ...)
2025-02-05 12:35 ` [PATCH v4 4/4] qapi: expose all schema features to code Daniel P. Berrangé
@ 2025-02-10 8:25 ` Markus Armbruster
2025-02-10 8:37 ` Markus Armbruster
5 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2025-02-10 8:25 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Marc-André Lureau, Michael Roth,
Philippe Mathieu-Daudé, Paolo Bonzini
Daniel P. Berrangé <berrange@redhat.com> writes:
> 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.
Series
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/4] qapi: generalize special features
2025-02-05 12:35 [PATCH v4 0/4] qapi: generalize special features Daniel P. Berrangé
` (4 preceding siblings ...)
2025-02-10 8:25 ` [PATCH v4 0/4] qapi: generalize special features Markus Armbruster
@ 2025-02-10 8:37 ` Markus Armbruster
5 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2025-02-10 8:37 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Marc-André Lureau, Michael Roth,
Philippe Mathieu-Daudé, Paolo Bonzini
Queued with imports tidied up. Thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread