qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio

qapi script: do not allow string discriminator

Since enum based discriminators provide better type-safety and
ensure that future qapi additions do not forget to adjust dependent
unions, forbid using string as discriminator from now on.

Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

authored by

Wenchao Xia and committed by
Luiz Capitulino
5223070c 59ca664e

+53 -18
+1 -4
docs/qapi-code-gen.txt
··· 123 123 124 124 Flat union types avoid the nesting on the wire. They are used whenever a 125 125 specific field of the base type is declared as the discriminator ('type' is 126 - then no longer generated). The discriminator can be a string field or a 127 - predefined enum field. If it is a string field, a hidden enum type will be 128 - generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check 129 - will be done to verify the correctness. It is recommended to use an enum field. 126 + then no longer generated). The discriminator must be of enumeration type. 130 127 The above example can then be modified as follows: 131 128 132 129 { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
+5
scripts/qapi.py
··· 239 239 "type '%s'" 240 240 % (discriminator, base)) 241 241 enum_define = find_enum(discriminator_type) 242 + # Do not allow string discriminator 243 + if not enum_define: 244 + raise QAPIExprError(expr_info, 245 + "Discriminator '%s' must be of enumeration " 246 + "type" % discriminator) 242 247 243 248 # Check every branch 244 249 for (key, value) in members.items():
+2 -1
tests/Makefile
··· 145 145 unclosed-list.json unclosed-object.json unclosed-string.json \ 146 146 duplicate-key.json union-invalid-base.json flat-union-no-base.json \ 147 147 flat-union-invalid-discriminator.json \ 148 - flat-union-invalid-branch-key.json flat-union-reverse-define.json) 148 + flat-union-invalid-branch-key.json flat-union-reverse-define.json \ 149 + flat-union-string-discriminator.json) 149 150 150 151 GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h 151 152
+1
tests/qapi-schema/flat-union-string-discriminator.err
··· 1 + <stdin>:13: Discriminator 'kind' must be of enumeration type
+1
tests/qapi-schema/flat-union-string-discriminator.exit
··· 1 + 1
+17
tests/qapi-schema/flat-union-string-discriminator.json
··· 1 + { 'enum': 'TestEnum', 2 + 'data': [ 'value1', 'value2' ] } 3 + 4 + { 'type': 'TestBase', 5 + 'data': { 'enum1': 'TestEnum', 'kind': 'str' } } 6 + 7 + { 'type': 'TestTypeA', 8 + 'data': { 'string': 'str' } } 9 + 10 + { 'type': 'TestTypeB', 11 + 'data': { 'integer': 'int' } } 12 + 13 + { 'union': 'TestUnion', 14 + 'base': 'TestBase', 15 + 'discriminator': 'kind', 16 + 'data': { 'kind1': 'TestTypeA', 17 + 'kind2': 'TestTypeB' } }
tests/qapi-schema/flat-union-string-discriminator.out

This is a binary file and will not be displayed.

+6 -3
tests/qapi-schema/qapi-schema-test.json
··· 37 37 'base': 'UserDefZero', 38 38 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } } 39 39 40 + { 'type': 'UserDefUnionBase', 41 + 'data': { 'string': 'str', 'enum1': 'EnumOne' } } 42 + 40 43 { 'union': 'UserDefFlatUnion', 41 - 'base': 'UserDefOne', 42 - 'discriminator': 'string', 43 - 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } } 44 + 'base': 'UserDefUnionBase', 45 + 'discriminator': 'enum1', 46 + 'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } } 44 47 # FIXME generated struct UserDefFlatUnion has members for direct base 45 48 # UserDefOne, but lacks members for indirect base UserDefZero 46 49
+3 -2
tests/qapi-schema/qapi-schema-test.out
··· 7 7 OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]), 8 8 OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]), 9 9 OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]), 10 - OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefOne'), ('discriminator', 'string'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]), 10 + OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), 11 + OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]), 11 12 OrderedDict([('union', 'UserDefAnonUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]), 12 13 OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]), 13 14 OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]), ··· 17 18 OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])] 18 19 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']}, 19 20 {'enum_name': 'UserDefUnionKind', 'enum_values': None}, 20 - {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None}, 21 21 {'enum_name': 'UserDefAnonUnionKind', 'enum_values': None}, 22 22 {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}] 23 23 [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]), ··· 27 27 OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]), 28 28 OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]), 29 29 OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]), 30 + OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), 30 31 OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
+4 -1
tests/test-qmp-input-strict.c
··· 146 146 Visitor *v; 147 147 Error *errp = NULL; 148 148 149 - v = validate_test_init(data, "{ 'string': 'a', 'boolean': true }"); 149 + v = validate_test_init(data, 150 + "{ 'enum1': 'value1', " 151 + "'string': 'str', " 152 + "'boolean': true }"); 150 153 /* TODO when generator bug is fixed, add 'integer': 41 */ 151 154 152 155 visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp);
+7 -3
tests/test-qmp-input-visitor.c
··· 310 310 Error *err = NULL; 311 311 UserDefFlatUnion *tmp; 312 312 313 - v = visitor_input_test_init(data, "{ 'string': 'a', 'boolean': true }"); 313 + v = visitor_input_test_init(data, 314 + "{ 'enum1': 'value1', " 315 + "'string': 'str', " 316 + "'boolean': true }"); 314 317 /* TODO when generator bug is fixed, add 'integer': 41 */ 315 318 316 319 visit_type_UserDefFlatUnion(v, &tmp, NULL, &err); 317 320 g_assert(err == NULL); 318 - g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_A); 321 + g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1); 322 + g_assert_cmpstr(tmp->string, ==, "str"); 319 323 /* TODO g_assert_cmpint(tmp->integer, ==, 41); */ 320 - g_assert_cmpint(tmp->a->boolean, ==, true); 324 + g_assert_cmpint(tmp->value1->boolean, ==, true); 321 325 qapi_free_UserDefFlatUnion(tmp); 322 326 } 323 327
+6 -4
tests/test-qmp-output-visitor.c
··· 449 449 Error *err = NULL; 450 450 451 451 UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion)); 452 - tmp->kind = USER_DEF_UNION_KIND_A; 453 - tmp->a = g_malloc0(sizeof(UserDefA)); 452 + tmp->kind = ENUM_ONE_VALUE1; 453 + tmp->string = g_strdup("str"); 454 + tmp->value1 = g_malloc0(sizeof(UserDefA)); 454 455 /* TODO when generator bug is fixed: tmp->integer = 41; */ 455 - tmp->a->boolean = true; 456 + tmp->value1->boolean = true; 456 457 457 458 visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err); 458 459 g_assert(err == NULL); ··· 461 462 g_assert(qobject_type(arg) == QTYPE_QDICT); 462 463 qdict = qobject_to_qdict(arg); 463 464 464 - g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "a"); 465 + g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value1"); 466 + g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "str"); 465 467 /* TODO g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); */ 466 468 g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true); 467 469