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

qapi: Forbid base without discriminator in unions

None of the existing QMP or QGA interfaces uses a union with a
base type but no discriminator; it is easier to avoid this in the
generator to save room for other future extensions more likely to
be useful. An earlier commit added a union-base-no-discriminator
test to ensure that we eventually give a decent error message;
likewise, removing UserDefUnion outright is okay, because we moved
all the tests we wish to keep into the tests of the simple union
UserDefNativeListUnion in the previous commit. Now is the time to
actually forbid simple union with base, and remove the last
vestiges from the testsuite.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

authored by

Eric Blake and committed by
Markus Armbruster
a8d4a2e4 805017b7

+21 -91
+3 -4
scripts/qapi-types.py
··· 242 242 ''') 243 243 244 244 if base: 245 - base_fields = find_struct(base)['data'] 246 - if discriminator: 247 - base_fields = base_fields.copy() 248 - del base_fields[discriminator] 245 + assert discriminator 246 + base_fields = find_struct(base)['data'].copy() 247 + del base_fields[discriminator] 249 248 ret += generate_struct_fields(base_fields) 250 249 else: 251 250 assert not discriminator
+5 -6
scripts/qapi-visit.py
··· 310 310 ret = "" 311 311 disc_type = enum_define['enum_name'] 312 312 else: 313 - # There will always be a discriminator in the C switch code, by default it 314 - # is an enum type generated silently as "'%sKind' % (name)" 313 + # There will always be a discriminator in the C switch code, by default 314 + # it is an enum type generated silently as "'%sKind' % (name)" 315 315 ret = generate_visit_enum('%sKind' % name, members.keys()) 316 316 disc_type = '%sKind' % (name) 317 317 318 318 if base: 319 - base_fields = find_struct(base)['data'] 320 - if discriminator: 321 - base_fields = base_fields.copy() 322 - del base_fields[discriminator] 319 + assert discriminator 320 + base_fields = find_struct(base)['data'].copy() 321 + del base_fields[discriminator] 323 322 ret += generate_visit_struct_fields(name, "", "", base_fields) 324 323 325 324 if discriminator:
+10 -10
scripts/qapi.py
··· 259 259 discriminator = expr.get('discriminator') 260 260 members = expr['data'] 261 261 262 - # If the object has a member 'base', its value must name a complex type. 263 - if base: 262 + # If the object has a member 'base', its value must name a complex type, 263 + # and there must be a discriminator. 264 + if base is not None: 265 + if discriminator is None: 266 + raise QAPIExprError(expr_info, 267 + "Union '%s' requires a discriminator to go " 268 + "along with base" %name) 264 269 base_fields = find_base_fields(base) 265 270 if not base_fields: 266 271 raise QAPIExprError(expr_info, 267 272 "Base '%s' is not a valid type" 268 273 % base) 269 274 270 - # If the union object has no member 'discriminator', it's an 271 - # ordinary union. 272 - if not discriminator: 273 - enum_define = None 274 - 275 - # Else if the value of member 'discriminator' is {}, it's an 276 - # anonymous union. 277 - elif discriminator == {}: 275 + # If the union object has no member 'discriminator', it's a 276 + # simple union. If 'discriminator' is {}, it is an anonymous union. 277 + if not discriminator or discriminator == {}: 278 278 enum_define = None 279 279 280 280 # Else, it's a flat union.
-4
tests/qapi-schema/qapi-schema-test.json
··· 36 36 { 'type': 'UserDefC', 37 37 'data': { 'string1': 'str', 'string2': 'str' } } 38 38 39 - { 'union': 'UserDefUnion', 40 - 'base': 'UserDefZero', 41 - 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } } 42 - 43 39 { 'type': 'UserDefUnionBase', 44 40 'data': { 'string': 'str', 'enum1': 'EnumOne' } } 45 41
-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([('type', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]), 10 - OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]), 11 10 OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), 12 11 OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]), 13 12 OrderedDict([('union', 'UserDefFlatUnion2'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefC'), ('value2', 'UserDefB'), ('value3', 'UserDefA')]))]), ··· 24 23 OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'), ('*b', 'UserDefOne'), ('c', 'str')]))]), 25 24 OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))])] 26 25 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']}, 27 - {'enum_name': 'UserDefUnionKind', 'enum_values': None}, 28 26 {'enum_name': 'UserDefAnonUnionKind', 'enum_values': None}, 29 27 {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}] 30 28 [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
+1
tests/qapi-schema/union-base-no-discriminator.err
··· 1 + tests/qapi-schema/union-base-no-discriminator.json:11: Union 'TestUnion' requires a discriminator to go along with base
+1 -1
tests/qapi-schema/union-base-no-discriminator.exit
··· 1 - 0 1 + 1
+1 -1
tests/qapi-schema/union-base-no-discriminator.json
··· 1 - # FIXME: we should reject simple unions with a base 1 + # we reject simple unions with a base (or flat unions without discriminator) 2 2 { 'type': 'TestTypeA', 3 3 'data': { 'string': 'str' } } 4 4
-8
tests/qapi-schema/union-base-no-discriminator.out
··· 1 - [OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]), 2 - OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))]), 3 - OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))]), 4 - OrderedDict([('union', 'TestUnion'), ('base', 'Base'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))])] 5 - [{'enum_name': 'TestUnionKind', 'enum_values': None}] 6 - [OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]), 7 - OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))]), 8 - OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))])]
-19
tests/test-qmp-input-visitor.c
··· 293 293 qapi_free_UserDefOneList(head); 294 294 } 295 295 296 - static void test_visitor_in_union(TestInputVisitorData *data, 297 - const void *unused) 298 - { 299 - Visitor *v; 300 - Error *err = NULL; 301 - UserDefUnion *tmp; 302 - 303 - v = visitor_input_test_init(data, "{ 'type': 'b', 'integer': 41, 'data' : { 'integer': 42 } }"); 304 - 305 - visit_type_UserDefUnion(v, &tmp, NULL, &err); 306 - g_assert(err == NULL); 307 - g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_B); 308 - g_assert_cmpint(tmp->integer, ==, 41); 309 - g_assert_cmpint(tmp->b->integer, ==, 42); 310 - qapi_free_UserDefUnion(tmp); 311 - } 312 - 313 296 static void test_visitor_in_union_flat(TestInputVisitorData *data, 314 297 const void *unused) 315 298 { ··· 679 662 &in_visitor_data, test_visitor_in_struct_nested); 680 663 input_visitor_test_add("/visitor/input/list", 681 664 &in_visitor_data, test_visitor_in_list); 682 - input_visitor_test_add("/visitor/input/union", 683 - &in_visitor_data, test_visitor_in_union); 684 665 input_visitor_test_add("/visitor/input/union-flat", 685 666 &in_visitor_data, test_visitor_in_union_flat); 686 667 input_visitor_test_add("/visitor/input/union-anon",
-36
tests/test-qmp-output-visitor.c
··· 422 422 qapi_free_UserDefNestedList(head); 423 423 } 424 424 425 - static void test_visitor_out_union(TestOutputVisitorData *data, 426 - const void *unused) 427 - { 428 - QObject *arg, *qvalue; 429 - QDict *qdict, *value; 430 - 431 - Error *err = NULL; 432 - 433 - UserDefUnion *tmp = g_malloc0(sizeof(UserDefUnion)); 434 - tmp->kind = USER_DEF_UNION_KIND_A; 435 - tmp->integer = 41; 436 - tmp->a = g_malloc0(sizeof(UserDefA)); 437 - tmp->a->boolean = true; 438 - 439 - visit_type_UserDefUnion(data->ov, &tmp, NULL, &err); 440 - g_assert(err == NULL); 441 - arg = qmp_output_get_qobject(data->qov); 442 - 443 - g_assert(qobject_type(arg) == QTYPE_QDICT); 444 - qdict = qobject_to_qdict(arg); 445 - 446 - g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "a"); 447 - g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); 448 - 449 - qvalue = qdict_get(qdict, "data"); 450 - g_assert(data != NULL); 451 - g_assert(qobject_type(qvalue) == QTYPE_QDICT); 452 - value = qobject_to_qdict(qvalue); 453 - g_assert_cmpint(qdict_get_bool(value, "boolean"), ==, true); 454 - 455 - qapi_free_UserDefUnion(tmp); 456 - QDECREF(qdict); 457 - } 458 - 459 425 static void test_visitor_out_union_flat(TestOutputVisitorData *data, 460 426 const void *unused) 461 427 { ··· 862 828 &out_visitor_data, test_visitor_out_list); 863 829 output_visitor_test_add("/visitor/output/list-qapi-free", 864 830 &out_visitor_data, test_visitor_out_list_qapi_free); 865 - output_visitor_test_add("/visitor/output/union", 866 - &out_visitor_data, test_visitor_out_union); 867 831 output_visitor_test_add("/visitor/output/union-flat", 868 832 &out_visitor_data, test_visitor_out_union_flat); 869 833 output_visitor_test_add("/visitor/output/union-anon",