[PATCH 0/2] qemu: Add a last-resort warning if object-add/device_add are QAPIfied
See patch 2/2. Peter Krempa (2): testutilsqemuschema: Add template checker for schema entries qemumonitorjsontest: Add a last-resort warning if object-add/device_add are QAPIfied tests/qemumonitorjsontest.c | 53 +++++++++++++++++ tests/testutilsqemuschema.c | 115 ++++++++++++++++++++++++++++++++++++ tests/testutilsqemuschema.h | 5 ++ 3 files changed, 173 insertions(+) -- 2.26.2
We'll need to match that a certain part of the qemu schema hasn't grown new properties unexpectedly. Add a helper which matches an 'object' QMP schema entry against a template and reports errors if expected types don't match or new entries are added. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutilsqemuschema.c | 115 ++++++++++++++++++++++++++++++++++++ tests/testutilsqemuschema.h | 5 ++ 2 files changed, 120 insertions(+) diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c index d9bc00903d..21a58cb4c6 100644 --- a/tests/testutilsqemuschema.c +++ b/tests/testutilsqemuschema.c @@ -607,6 +607,121 @@ testQEMUSchemaValidateCommand(const char *command, } +/** + * testQEMUSchemaEntryMatchTemplate: + * + * @schemaentry: a JSON object representing a 'object' node in the QAPI schema + * ...: a NULL terminated list of strings representing the template of properties + * which the QMP object needs to have. + * + * The strings have following format: + * + * "type:name" + * "?type:name" + * + * "type" corresponds to the 'type' property of the member to check (str, bool, any ...) + * "name" corresponds to the name of the member to check + * + * If the query string starts with an '?' and member 'name' may be missing. + * + * This function matches that @schemaentry has all expected members and the + * members have expected types. @schemaentry also must not have any unknown + * members. + */ +int +testQEMUSchemaEntryMatchTemplate(virJSONValuePtr schemaentry, + ...) +{ + g_autoptr(virJSONValue) members = NULL; + va_list ap; + const char *next; + int ret = -1; + + if (STRNEQ_NULLABLE(virJSONValueObjectGetString(schemaentry, "meta-type"), "object")) { + VIR_TEST_VERBOSE("schemaentry is not an object"); + return -1; + } + + if (!(members = virJSONValueCopy(virJSONValueObjectGetArray(schemaentry, "members")))) { + VIR_TEST_VERBOSE("failed to copy 'members'"); + return -1; + } + + va_start(ap, schemaentry); + + /* pass 1 */ + + while ((next = va_arg(ap, const char *))) { + char modifier = *next; + g_autofree char *type = NULL; + char *name; + size_t i; + bool found = false; + bool optional = false; + + if (!g_ascii_isalpha(modifier)) + next++; + + if (modifier == '?') + optional = true; + + type = g_strdup(next); + + if ((name = strchr(type, ':'))) { + *(name++) = '\0'; + } else { + VIR_TEST_VERBOSE("malformed template string '%s'", next); + goto cleanup; + } + + for (i = 0; i < virJSONValueArraySize(members); i++) { + virJSONValuePtr member = virJSONValueArrayGet(members, i); + const char *membername = virJSONValueObjectGetString(member, "name"); + const char *membertype = virJSONValueObjectGetString(member, "type"); + + if (STRNEQ_NULLABLE(name, membername)) + continue; + + if (STRNEQ_NULLABLE(membertype, type)) { + VIR_TEST_VERBOSE("member '%s' is of unexpected type '%s' (expected '%s')", + NULLSTR(membername), NULLSTR(membertype), type); + goto cleanup; + } + + found = true; + break; + } + + if (found) { + virJSONValueFree(virJSONValueArraySteal(members, i)); + } else { + if (!optional) { + VIR_TEST_VERBOSE("mandatory member '%s' not found", name); + goto cleanup; + } + } + } + + /* pass 2 - check any unexpected members */ + if (virJSONValueArraySize(members) > 0) { + size_t i; + + for (i = 0; i < virJSONValueArraySize(members); i++) { + VIR_TEST_VERBOSE("unexpected member '%s'", + NULLSTR(virJSONValueObjectGetString(virJSONValueArrayGet(members, i), "name"))); + } + + goto cleanup; + } + + ret = 0; + + cleanup: + va_end(ap); + return ret; +} + + static virJSONValuePtr testQEMUSchemaLoadReplies(const char *filename) { diff --git a/tests/testutilsqemuschema.h b/tests/testutilsqemuschema.h index c90a6b626d..1649ad78b5 100644 --- a/tests/testutilsqemuschema.h +++ b/tests/testutilsqemuschema.h @@ -37,6 +37,11 @@ testQEMUSchemaValidateCommand(const char *command, bool allowRemoved, virBufferPtr debug); +int +testQEMUSchemaEntryMatchTemplate(virJSONValuePtr schemaentry, + ...); + + virJSONValuePtr testQEMUSchemaGetLatest(const char* arch); -- 2.26.2
When netdev-add was qapified it took us by surprise and we had to scramble to fix the internals to format conformant monitor arguments. Add a last-resort early warning system if this happens to object-add or device_add. Hopefully qemu developers notify us sooner than this. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 53 +++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 77e70c1dc4..bc25958e70 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2888,6 +2888,55 @@ testQAPISchemaValidate(const void *opaque) } +/** + * testQAPISchemaObjectDeviceAdd: + * + * Purpose of this test is to add a last-resort notification that 'object-add' + * and 'device_add' are not covered by the QMP schema by surprise. Ideally QEMU + * developers will notify us before they switch so we have time to adapt our + * generators first. This didn't work out when netdev-add was converted. + * + * We validate that the QMP schema describes only the expected types and nothing + * else assuming that no new field will be added until final conversion. + */ +static int +testQAPISchemaObjectDeviceAdd(const void *opaque) +{ + virHashTablePtr schema = (virHashTablePtr) opaque; + virJSONValuePtr entry; + + if (virQEMUQAPISchemaPathGet("device_add/arg-type", schema, &entry) < 0) { + fprintf(stderr, "schema for 'device_add' not found\n"); + return -1; + } + + if (testQEMUSchemaEntryMatchTemplate(entry, + "str:driver", + "str:bus", + "str:id", + NULL) < 0) { + VIR_TEST_VERBOSE("device_add has unexpected members in schema"); + return -1; + } + + if (virQEMUQAPISchemaPathGet("object-add/arg-type", schema, &entry) < 0) { + fprintf(stderr, "schema for 'objectadd' not found\n"); + return -1; + } + + if (testQEMUSchemaEntryMatchTemplate(entry, + "str:qom-type", + "str:id", + "any:props", + NULL) < 0) { + VIR_TEST_VERBOSE("object-add has unexpected members in schema"); + return -1; + } + + return 0; +} + + static void testQueryJobsPrintJob(virBufferPtr buf, qemuMonitorJobInfoPtr job) @@ -3380,6 +3429,10 @@ mymain(void) #undef DO_TEST_QAPI_VALIDATE + if (virTestRun("validate that object-add and device_add don't have schema", + testQAPISchemaObjectDeviceAdd, qapiData.schema) < 0) + ret = -1; + #define DO_TEST_QUERY_JOBS(name) \ do { \ struct testQueryJobsData data = { name, driver.xmlopt}; \ -- 2.26.2
On 8/6/20 2:50 PM, Peter Krempa wrote:
See patch 2/2.
Peter Krempa (2): testutilsqemuschema: Add template checker for schema entries qemumonitorjsontest: Add a last-resort warning if object-add/device_add are QAPIfied
tests/qemumonitorjsontest.c | 53 +++++++++++++++++ tests/testutilsqemuschema.c | 115 ++++++++++++++++++++++++++++++++++++ tests/testutilsqemuschema.h | 5 ++ 3 files changed, 173 insertions(+)
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
participants (2)
-
Daniel Henrique Barboza -
Peter Krempa