Re: [RFC 01/15] scripts/qapi: support type-based unions
On 28.03.24 12:15, Markus Armbruster wrote: Vladimir Sementsov-Ogievskiy writes: Look at block-job-change command: we have to specify both 'id' to chose the job to operate on and 'type' for QAPI union be parsed. But for user this looks redundant: when we specify 'id', QEMU should be able to get corresponding job's type. This commit brings such a possibility: just specify some Enum type as 'discriminator', and define a function somewhere with prototype bool YourUnionType_mapper(YourUnionType *, EnumType *out, Error **errp) Further commits will use this functionality to upgrade block-job-change interface and introduce some new interfaces. Signed-off-by: Vladimir Sementsov-Ogievskiy --- scripts/qapi/introspect.py | 5 +++- scripts/qapi/schema.py | 50 +++--- scripts/qapi/types.py | 3 ++- scripts/qapi/visit.py | 43 ++-- 4 files changed, 73 insertions(+), 28 deletions(-) I believe you need to update docs/devel/qapi-code-gen.rst. Current text: Union types --- Syntax:: UNION = { 'union': STRING, 'base': ( MEMBERS | STRING ), 'discriminator': STRING, 'data': BRANCHES, '*if': COND, '*features': FEATURES } BRANCHES = { BRANCH, ... } BRANCH = STRING : TYPE-REF | STRING : { 'type': TYPE-REF, '*if': COND } Member 'union' names the union type. The 'base' member defines the common members. If it is a MEMBERS_ object, it defines common members just like a struct type's 'data' member defines struct type members. If it is a STRING, it names a struct type whose members are the common members. Member 'discriminator' must name a non-optional enum-typed member of the base struct. That member's value selects a branch by its name. If no such branch exists, an empty branch is assumed. If I understand your commit message correctly, this paragraph is no longer true. Right. Like this: Member 'discriminator' must name either a non-optional enum-typed member, or an enum type name. (and more description follow, about user defined function and so on). Do you think that mixing member name and type name here is OK? Or should I instead add another field 'discriminator-type', so that exactly one of 'discriminator' and 'discriminator-type' should be in union definition? Each BRANCH of the 'data' object defines a branch of the union. A union must have at least one branch. The BRANCH's STRING name is the branch name. It must be a value of the discriminator enum type. The BRANCH's value defines the branch's properties, in particular its type. The type must a struct type. The form TYPE-REF_ is shorthand for :code:`{ 'type': TYPE-REF }`. In the Client JSON Protocol, a union is represented by an object with the common members (from the base type) and the selected branch's members. The two sets of member names must be disjoint. Example:: { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] } { 'union': 'BlockdevOptions', 'base': { 'driver': 'BlockdevDriver', '*read-only': 'bool' }, 'discriminator': 'driver', 'data': { 'file': 'BlockdevOptionsFile', 'qcow2': 'BlockdevOptionsQcow2' } } Resulting in these JSON objects:: { "driver": "file", "read-only": true, "filename": "/some/place/my-image" } { "driver": "qcow2", "read-only": false, "backing": "/some/place/my-image", "lazy-refcounts": true } The order of branches need not match the order of the enum values. The branches need not cover all possible enum values. In the resulting generated C data types, a union is represented as a struct with the base members in QAPI schema order, and then a union of structures for each branch of the struct. The optional 'if' member specifies a conditional. See `Configuring the schema`_ below for more on this. The optional 'features' member specifies features. See Features_ below for more on this. -- Best regards, Vladimir
Re: [RFC 01/15] scripts/qapi: support type-based unions
On 28.03.24 12:40, Markus Armbruster wrote: Subject: all unions are type-based. Perhaps "support implicit union tags on the wire"? Yes, sounds good. Do you need this schema language feature for folding block jobs into the jobs abstraction, or is it just for making the wire protocol nicer in places? It's not necessary, we can proceed with job-* API, specifying both type and id. But I think, as we are not in a hurry, better to make new job-* API more effective from the beginning. -- Best regards, Vladimir
Re: [RFC 01/15] scripts/qapi: support type-based unions
Subject: all unions are type-based. Perhaps "support implicit union tags on the wire"? Do you need this schema language feature for folding block jobs into the jobs abstraction, or is it just for making the wire protocol nicer in places?
Re: [RFC 01/15] scripts/qapi: support type-based unions
Vladimir Sementsov-Ogievskiy writes: > Look at block-job-change command: we have to specify both 'id' to chose > the job to operate on and 'type' for QAPI union be parsed. But for user > this looks redundant: when we specify 'id', QEMU should be able to get > corresponding job's type. > > This commit brings such a possibility: just specify some Enum type as > 'discriminator', and define a function somewhere with prototype > > bool YourUnionType_mapper(YourUnionType *, EnumType *out, Error **errp) > > Further commits will use this functionality to upgrade block-job-change > interface and introduce some new interfaces. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > scripts/qapi/introspect.py | 5 +++- > scripts/qapi/schema.py | 50 +++--- > scripts/qapi/types.py | 3 ++- > scripts/qapi/visit.py | 43 ++-- > 4 files changed, 73 insertions(+), 28 deletions(-) I believe you need to update docs/devel/qapi-code-gen.rst. Current text: Union types --- Syntax:: UNION = { 'union': STRING, 'base': ( MEMBERS | STRING ), 'discriminator': STRING, 'data': BRANCHES, '*if': COND, '*features': FEATURES } BRANCHES = { BRANCH, ... } BRANCH = STRING : TYPE-REF | STRING : { 'type': TYPE-REF, '*if': COND } Member 'union' names the union type. The 'base' member defines the common members. If it is a MEMBERS_ object, it defines common members just like a struct type's 'data' member defines struct type members. If it is a STRING, it names a struct type whose members are the common members. Member 'discriminator' must name a non-optional enum-typed member of the base struct. That member's value selects a branch by its name. If no such branch exists, an empty branch is assumed. If I understand your commit message correctly, this paragraph is no longer true. Each BRANCH of the 'data' object defines a branch of the union. A union must have at least one branch. The BRANCH's STRING name is the branch name. It must be a value of the discriminator enum type. The BRANCH's value defines the branch's properties, in particular its type. The type must a struct type. The form TYPE-REF_ is shorthand for :code:`{ 'type': TYPE-REF }`. In the Client JSON Protocol, a union is represented by an object with the common members (from the base type) and the selected branch's members. The two sets of member names must be disjoint. Example:: { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] } { 'union': 'BlockdevOptions', 'base': { 'driver': 'BlockdevDriver', '*read-only': 'bool' }, 'discriminator': 'driver', 'data': { 'file': 'BlockdevOptionsFile', 'qcow2': 'BlockdevOptionsQcow2' } } Resulting in these JSON objects:: { "driver": "file", "read-only": true, "filename": "/some/place/my-image" } { "driver": "qcow2", "read-only": false, "backing": "/some/place/my-image", "lazy-refcounts": true } The order of branches need not match the order of the enum values. The branches need not cover all possible enum values. In the resulting generated C data types, a union is represented as a struct with the base members in QAPI schema order, and then a union of structures for each branch of the struct. The optional 'if' member specifies a conditional. See `Configuring the schema`_ below for more on this. The optional 'features' member specifies features. See Features_ below for more on this.