[RFC PATCH 0/7] qemu: QAPI-schema: Add support for enum value 'features'
This series is based on Markus' effort to add 'feature' flags to enum values so that e.g. they can be deprecated: https://listman.redhat.com/archives/libvir-list/2021-September/msg00453.html This series adapts both the schema query language to support querying for arbitrary feature flags and also the schema validator to reject deprecated enum values. Libvirt didn't use any deprecated value for now so to see that this really works you can fetch this series including patches which show it's working (tests fail): git fetch https://gitlab.com/pipo.sk/libvirt.git qemu-deprecate-enum The above branch also demonstrates that parsing from the new arrays produces identical results as it has updated qemu capabilities. This series is RFC as it's waiting for the qemu additions first. Peter Krempa (7): virQEMUQAPISchemaTraverseEnum: Move helper variables into loop virQEMUQAPISchemaTraverseEnum: Use the modern 'members' array virQEMUQAPISchemaTraverseEnum: Allow query of enume type features testQEMUSchemaValidateEnum: Refactor logic to simplify switching to new QMP schema format testQEMUSchemaValidateEnum: Use new 'members' for 'enum' meta type testQEMUSchemaValidateDeprecated: Move to the top testQEMUSchemaValidateEnum: Validate deprecated members src/qemu/qemu_qapi.c | 46 +++++++++++-- tests/testutilsqemuschema.c | 130 +++++++++++++++++++++--------------- 2 files changed, 117 insertions(+), 59 deletions(-) -- 2.31.1
Move them closer to where they are actually used. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_qapi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c index 36b184b226..165ecf1180 100644 --- a/src/qemu/qemu_qapi.c +++ b/src/qemu/qemu_qapi.c @@ -243,8 +243,6 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur, { const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt); virJSONValue *values; - virJSONValue *enumval; - const char *value; size_t i; if (query[0] != '^') @@ -259,6 +257,9 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur, return -2; for (i = 0; i < virJSONValueArraySize(values); i++) { + virJSONValue *enumval; + const char *value; + if (!(enumval = virJSONValueArrayGet(values, i)) || !(value = virJSONValueGetString(enumval))) continue; -- 2.31.1
Starting from QEMU-6.2 enum members are reported as an array of objects under new name "values" so that extra data can be reported for each member. Modify the code so that we prefer 'members' and skip 'values' completely if we've used 'members'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_qapi.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c index 165ecf1180..790f7c0fee 100644 --- a/src/qemu/qemu_qapi.c +++ b/src/qemu/qemu_qapi.c @@ -243,6 +243,7 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur, { const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt); virJSONValue *values; + virJSONValue *members; size_t i; if (query[0] != '^') @@ -253,6 +254,22 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur, query++; + /* qemu-6.2 added a "members" array superseding "values" */ + if ((members = virJSONValueObjectGetArray(cur, "members"))) { + for (i = 0; i < virJSONValueArraySize(members); i++) { + virJSONValue *member = virJSONValueArrayGet(members, i); + const char *name; + + if (!member || !(name = virJSONValueObjectGetString(member, "name"))) + return -2; + + if (STREQ(name, query)) + return 1; + } + + return 0; + } + if (!(values = virJSONValueObjectGetArray(cur, "values"))) return -2; -- 2.31.1
QEMU-6.2 added feature flags for enum types. Add support for querying them into our QMP schema query language. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_qapi.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_qapi.c b/src/qemu/qemu_qapi.c index 790f7c0fee..426db8d30d 100644 --- a/src/qemu/qemu_qapi.c +++ b/src/qemu/qemu_qapi.c @@ -242,6 +242,7 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur, struct virQEMUQAPISchemaTraverseContext *ctxt) { const char *query = virQEMUQAPISchemaTraverseContextNextQuery(ctxt); + const char *featurequery = NULL; virJSONValue *values; virJSONValue *members; size_t i; @@ -249,8 +250,16 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur, if (query[0] != '^') return 0; - if (virQEMUQAPISchemaTraverseContextHasNextQuery(ctxt)) - return -3; + if (virQEMUQAPISchemaTraverseContextHasNextQuery(ctxt)) { + /* we might have a query for a feature flag of an enum value */ + featurequery = virQEMUQAPISchemaTraverseContextNextQuery(ctxt); + + if (*featurequery != '$' || + virQEMUQAPISchemaTraverseContextHasNextQuery(ctxt)) + return -3; + + featurequery++; + } query++; @@ -263,13 +272,21 @@ virQEMUQAPISchemaTraverseEnum(virJSONValue *cur, if (!member || !(name = virJSONValueObjectGetString(member, "name"))) return -2; - if (STREQ(name, query)) + if (STREQ(name, query)) { + if (featurequery) + return virQEMUQAPISchemaTraverseHasObjectFeature(featurequery, member); + return 1; + } } return 0; } + /* old-style "values" array doesn't have feature flags so any query is necessarily false */ + if (featurequery) + return 0; + if (!(values = virJSONValueObjectGetArray(cur, "values"))) return -2; @@ -439,7 +456,8 @@ virQEMUQAPISchemaTraverse(const char *baseName, * * The above types can be chained arbitrarily using slashes to construct any * path into the schema tree, booleans must be always the last component as they - * don't refer to a type. + * don't refer to a type. An exception is querying feature of an enum value + * (.../^enumval/$featurename) which is allowed. * * Returns 1 if @query was found in @schema filling @entry if non-NULL, 0 if * @query was not found in @schema and -1 on other errors along with an appropriate -- 2.31.1
QEMU-6.2 is reporting enum values in the new 'members' array which we'll be switching to. Rewrite the logic so that adding the new checker is more straightforward. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemuschema.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index f6347231a8..e75345b582 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -319,7 +319,6 @@ testQEMUSchemaValidateEnum(virJSONValue *obj, { const char *objstr; virJSONValue *values = NULL; - virJSONValue *value; size_t i; if (virJSONValueGetType(obj) != VIR_JSON_TYPE_STRING) { @@ -329,24 +328,24 @@ testQEMUSchemaValidateEnum(virJSONValue *obj, objstr = virJSONValueGetString(obj); - if (!(values = virJSONValueObjectGetArray(root, "values"))) { - virBufferAsprintf(ctxt->debug, "ERROR: missing enum values in schema '%s'", - NULLSTR(virJSONValueObjectGetString(root, "name"))); - return -2; - } - - for (i = 0; i < virJSONValueArraySize(values); i++) { - value = virJSONValueArrayGet(values, i); + if ((values = virJSONValueObjectGetArray(root, "values"))) { + for (i = 0; i < virJSONValueArraySize(values); i++) { + virJSONValue *value = virJSONValueArrayGet(values, i); - if (STREQ_NULLABLE(objstr, virJSONValueGetString(value))) { - virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr)); - return 0; + if (STREQ_NULLABLE(objstr, virJSONValueGetString(value))) { + virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr)); + return 0; + } } + + virBufferAsprintf(ctxt->debug, "ERROR: enum value '%s' is not in schema", + NULLSTR(objstr)); + return -1; } - virBufferAsprintf(ctxt->debug, "ERROR: enum value '%s' is not in schema", - NULLSTR(objstr)); - return -1; + virBufferAsprintf(ctxt->debug, "ERROR: missing enum values in schema '%s'", + NULLSTR(virJSONValueObjectGetString(root, "name"))); + return -2; } -- 2.31.1
Switch to the new more featured way to report enum members which will also allow us to detect use of deprecated members. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemuschema.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index e75345b582..7398c73ccc 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -319,6 +319,7 @@ testQEMUSchemaValidateEnum(virJSONValue *obj, { const char *objstr; virJSONValue *values = NULL; + virJSONValue *members = NULL; size_t i; if (virJSONValueGetType(obj) != VIR_JSON_TYPE_STRING) { @@ -328,6 +329,22 @@ testQEMUSchemaValidateEnum(virJSONValue *obj, objstr = virJSONValueGetString(obj); + /* qemu-6.2 added a "members" array superseding "values" */ + if ((members = virJSONValueObjectGetArray(root, "members"))) { + for (i = 0; i < virJSONValueArraySize(members); i++) { + virJSONValue *member = virJSONValueArrayGet(members, i); + + if (STREQ_NULLABLE(objstr, virJSONValueObjectGetString(member, "name"))) { + virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr)); + return 0; + } + } + + virBufferAsprintf(ctxt->debug, "ERROR: enum value '%s' is not in schema", + NULLSTR(objstr)); + return -1; + } + if ((values = virJSONValueObjectGetArray(root, "values"))) { for (i = 0; i < virJSONValueArraySize(values); i++) { virJSONValue *value = virJSONValueArrayGet(values, i); -- 2.31.1
Move the function to the top of the file so other functions placed towards the top will be able to reuse it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemuschema.c | 82 ++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index 7398c73ccc..38dd0e14bc 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -28,6 +28,47 @@ struct testQEMUSchemaValidateCtxt { }; +static int +testQEMUSchemaValidateDeprecated(virJSONValue *root, + const char *name, + struct testQEMUSchemaValidateCtxt *ctxt) +{ + virJSONValue *features = virJSONValueObjectGetArray(root, "features"); + size_t nfeatures; + size_t i; + + if (!features) + return 0; + + nfeatures = virJSONValueArraySize(features); + + for (i = 0; i < nfeatures; i++) { + virJSONValue *cur = virJSONValueArrayGet(features, i); + const char *curstr; + + if (!cur || + !(curstr = virJSONValueGetString(cur))) { + virBufferAsprintf(ctxt->debug, "ERROR: features of '%s' are malformed", name); + return -2; + } + + if (STREQ(curstr, "deprecated")) { + if (ctxt->allowDeprecated) { + virBufferAsprintf(ctxt->debug, "WARNING: '%s' is deprecated", name); + if (virTestGetVerbose()) + g_fprintf(stderr, "\nWARNING: '%s' is deprecated\n", name); + return 0; + } else { + virBufferAsprintf(ctxt->debug, "ERROR: '%s' is deprecated", name); + return -1; + } + } + } + + return 0; +} + + static int testQEMUSchemaValidateRecurse(virJSONValue *obj, virJSONValue *root, @@ -461,47 +502,6 @@ testQEMUSchemaValidateAlternate(virJSONValue *obj, } -static int -testQEMUSchemaValidateDeprecated(virJSONValue *root, - const char *name, - struct testQEMUSchemaValidateCtxt *ctxt) -{ - virJSONValue *features = virJSONValueObjectGetArray(root, "features"); - size_t nfeatures; - size_t i; - - if (!features) - return 0; - - nfeatures = virJSONValueArraySize(features); - - for (i = 0; i < nfeatures; i++) { - virJSONValue *cur = virJSONValueArrayGet(features, i); - const char *curstr; - - if (!cur || - !(curstr = virJSONValueGetString(cur))) { - virBufferAsprintf(ctxt->debug, "ERROR: features of '%s' are malformed", name); - return -2; - } - - if (STREQ(curstr, "deprecated")) { - if (ctxt->allowDeprecated) { - virBufferAsprintf(ctxt->debug, "WARNING: '%s' is deprecated", name); - if (virTestGetVerbose()) - g_fprintf(stderr, "\nWARNING: '%s' is deprecated\n", name); - return 0; - } else { - virBufferAsprintf(ctxt->debug, "ERROR: '%s' is deprecated", name); - return -1; - } - } - } - - return 0; -} - - static int testQEMUSchemaValidateRecurse(virJSONValue *obj, virJSONValue *root, -- 2.31.1
Starting from QEMU-6.2 enum members can be deprecated. Add support to the validator. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemuschema.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index 38dd0e14bc..b4b5eb1ed6 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -376,6 +376,12 @@ testQEMUSchemaValidateEnum(virJSONValue *obj, virJSONValue *member = virJSONValueArrayGet(members, i); if (STREQ_NULLABLE(objstr, virJSONValueObjectGetString(member, "name"))) { + int rc; + + /* the new 'members' array allows us to check deprecations */ + if ((rc = testQEMUSchemaValidateDeprecated(member, objstr, ctxt)) < 0) + return rc; + virBufferAsprintf(ctxt->debug, "'%s' OK", NULLSTR(objstr)); return 0; } -- 2.31.1
On 9/20/21 3:49 PM, Peter Krempa wrote:
This series is based on Markus' effort to add 'feature' flags to enum values so that e.g. they can be deprecated:
https://listman.redhat.com/archives/libvir-list/2021-September/msg00453.html
This series adapts both the schema query language to support querying for arbitrary feature flags and also the schema validator to reject deprecated enum values.
Libvirt didn't use any deprecated value for now so to see that this really works you can fetch this series including patches which show it's working (tests fail):
git fetch https://gitlab.com/pipo.sk/libvirt.git qemu-deprecate-enum
The above branch also demonstrates that parsing from the new arrays produces identical results as it has updated qemu capabilities.
This series is RFC as it's waiting for the qemu additions first.
Peter Krempa (7): virQEMUQAPISchemaTraverseEnum: Move helper variables into loop virQEMUQAPISchemaTraverseEnum: Use the modern 'members' array virQEMUQAPISchemaTraverseEnum: Allow query of enume type features testQEMUSchemaValidateEnum: Refactor logic to simplify switching to new QMP schema format testQEMUSchemaValidateEnum: Use new 'members' for 'enum' meta type testQEMUSchemaValidateDeprecated: Move to the top testQEMUSchemaValidateEnum: Validate deprecated members
src/qemu/qemu_qapi.c | 46 +++++++++++-- tests/testutilsqemuschema.c | 130 +++++++++++++++++++++--------------- 2 files changed, 117 insertions(+), 59 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Peter Krempa