qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: QAPI unions as branches / unifying struct and union types (was: [PAT


From: Het Gala
Subject: Re: QAPI unions as branches / unifying struct and union types (was: [PATCH v2 2/6] migration: Updated QAPI format for 'migrate' qemu monitor command)
Date: Fri, 10 Feb 2023 18:58:32 +0530
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.6.1


On 10/02/23 12:54 pm, Markus Armbruster wrote:
Daniel P. Berrangé <berrange@redhat.com> writes:

[...]

+##
+# @MigrateAddress:
+#
+# The options available for communication transport mechanisms for migration
+#
+# Since 8.0
+##
+{ 'union' : 'MigrateAddress',
+  'base' : { 'transport' : 'MigrateTransport'},
+  'discriminator' : 'transport',
+  'data' : {
+    'socket' : 'MigrateSocketAddr',
+    'exec' : 'MigrateExecAddr',
+    'rdma': 'MigrateRdmaAddr' } }
Ideally this would be

    'data' : {
      'socket' : 'SocketAddress',
      'exec' : 'MigrateCommand',
      'rdma': 'InetSocketAddress' } }

though the first SocketAddress isn't possible unless it is easy to
lift the QAPI limitation.
Context: SocketAddress is a QAPI union, and "the QAPI limitation" is

     scripts/qapi-gen.py: In file included from ../qapi/qapi-schema.json:79:
     ../qapi/migration.json: In union 'MigrateAddress':
     ../qapi/migration.json:1505: branch 'socket' cannot use union type 
'SocketAddress'

Emitted by schema.py like this:

                 if (not isinstance(v.type, QAPISchemaObjectType)
                         or v.type.variants):
                     raise QAPISemError(
                         self.info,
                         "%s cannot use %s"
                         % (v.describe(self.info), v.type.describe()))

This enforces docs/devel/qapi-code-gen.rst's clause

     The BRANCH's value defines the branch's properties, in particular its
     type.  The type must a struct type.  [...]

Next paragraph:

     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.

So, we're splicing in the members of the branch's JSON object.  For that
to even make sense, the branch type needs to map to a JSON object.  This
is fundamental.  It's the first part of the condition in the code
snippet above.

We have two kinds of QAPI types that map to a JSON object: struct and
union.  The second part of the condition restricts to struct.  Unless
I'm missing something (imperfect memory...), this is *not* fundamental,
just a matter of implementing it.  But I'd have to try to be sure.


Instead of simply allowing unions in addition to structs here, I'd like
to go one step further, and fuse the two into "objects".  Let me
explain.

If we abstract from syntax, structs have become almost a special kind of
union.  Unions have a set of common members and sets of variant members,
and a special common member (the tag) selects the set of variant
members.  Structs are unions with zero variants and no tag.

The generator code actually represents both structs and unions as a
common QAPISchemaObjectType already.  QAPI/QMP introspection does the
same: it uses a single meta type 'object' for both.


There is another spot where only structs are allowed: a struct or
union's base type.  That restriction will be awkward to lift, as I made
the mistake of baking the assumption "object type has at most one tag
member" into QAPI/QMP introspection .

Hi Markus, thankyou for explaning in such detail. I tried to understand of what you explained.

So IIUC, you mentioned the QAPI generator treats both structs and unions same, but basically in the schema.py checks is where it tries to distinguish between the two ? and because of the fact that docs/devel/qapi-code-gen.rst states that for a union, it's branches must be 'struct', and that's the reason it gives an error ?

If that's the case, can we improve on our checks and allow union as a part of branch of a union ? or something else ?

or I may have completely misunderstood most of the part 😅. Please let me know


[...]
Regards,
Het Gala



reply via email to

[Prev in Thread] Current Thread [Next in Thread]