Re: [PATCH 07/10] qemu-replies-tool: Add validation of known fields in 'query-qmp-schema'

On Tue, Jan 16, 2024 at 05:12:41PM +0100, Peter Krempa wrote: > +def validate_qmp_schema_check_keys(entry, mandatory, optional): > + keys = set(entry.keys()) > + > + for k, t in mandatory: > + try: > + keys.remove(k) > + except KeyError: > + raise qmpSchemaException("missing mandatory key '%s' in schema '%s'" % (k, entry)) > + > + if not isinstance(entry[k], t): > + raise qmpSchemaException("key '%s' is not of the expected type '%s' in schema '%s'" % (k, t, entry)) > + > + for k, t in optional: > + if k in keys: > + keys.discard(k) > + > + if t is not None: > + if not isinstance(entry[k], t): > + raise qmpSchemaException("key '%s' is not of the expected type '%s' in schema '%s'" % (k, t, entry)) This doesn't cover the case where the value for the key is supposed to be JSON null. You can either change this to something like if ((t is not None and not isinstance(entry[k], t)) or (t is None and entry[k] is not None)): raise qmpSchemaException(...) or, more simply, drop the check on t being None... > +def validate_qmp_schema(schemalist): > + for entry in schemalist: > + if not isinstance(entry, dict): > + raise qmpSchemaException("schema entry '%s' is not a JSON Object (dict)" % (entry)) > + > + match entry.get('meta-type', None): > + case 'object': > + for m in entry.get('members', []): > + validate_qmp_schema_check_keys(m, > + mandatory=[('name', str), > + ('type', str)], > + optional=[('default', None), ... and change the second member of the tuple to type(None) here. > def process_one(filename, args): > try: > conv = qemu_replies_load(filename) > > modify_replies(conv) > > + for (cmd, rep) in conv: > + if cmd['execute'] == 'query-qmp-schema': > + validate_qmp_schema(rep['return']) Should we check whether query-qmp-schema and its output are in fact present in the replies file? Or can we just assume that they're going to be there?