Re: Extraneous nesting in QAPI schema

[Eric Blake <eblake@redhat.com>]

On 12/16/19 10:59 AM, Markus Armbruster wrote:
> Extra nesting is merely another set of braces in QMP.  It's bloody
> annoying in QAPIfied CLI using dotted keys syntax.  Example:
> 
> QMP command
> 
>      {"execute": "chardev-add",

<snipped, but concur>


> Let's start with trying to understand the sources of extra nesting.
> 
> The obvious source of nesting is struct members of struct or union type.
> The example above has two: backend and backend.data.addr.
> 
> This kind of nesting can sometimes be avoided by making the member
> (struct) type a base type of the containing type.  Possible when we can
> arrange the base types into a single chain.  In the example above, we'd
> make the implicit argument type of chardev-add explicit, then replace
> member 'backend': 'ChardevBackend' by 'base': 'ChardevBackend'.
> 
> A more general solution would be adding "unboxed" members to the schema
> language.  A member that is normally a JSON object on the wire would
> instead have its JSON object members "spliced in".  Worth the trouble?
> Not sure.

It may also be possible to permit a discriminated union to be the branch 
value of yet another discriminated union: as long as the set of member 
names made visible by the inner union do not collide with any of the 
visible members elsewhere in the outer union, this should be okay, and 
give the same effect of being an unboxed member.

> 
> Special case: &rest arguments.  Example 1: object-add argument @props:
> 
>      {"execute": "object-add",
>       "arguments": {
>           "qom-type": "memory-backend-file",
>           "id": "shmmem-shmem0",
>           "props": {"mem-path": "/dev/shm/my_shmem0",
>                     "size":4194304,
>                     "share":true}}}
> 
> Example 2:
> 
>      {"execute": "device_add",
>       "arguments": {
>           "driver": "virtio-scsi-pci",
>           "bus": "pci.0",
>           "id": "virtio_scsi_pci2",
>           "addr": "0xb"}}
> 
> object-add wraps the properties in an object.  Device_add doesn't, but
> it needs to bypass the generated marshaller with 'gen': false.  We could
> add support for &rest arguments to the schema language.  Worth the
> trouble?  Not sure.
> 
> Another source is "simple" unions.  Both backend and backend.data.addr
> are actually "simple" unions, giving rise to backend.data and
> backend.data.addr.data.
> 
> We wouldn't use "simple" unions today.  With "flat" unions, we'd avoid
> the .data.
> 
> How widespread are "simple" unions today?  Let's have a look.  Five
> occur as command arguments:
> 
> * ChardevBackend
> 
>    Used for command chardev-add and chardev-change argument @backend.
> 
> * SocketAddressLegacy
> 
>    Used for command nbd-server-start argument @addr, and in command
>    chardev-add and chardev-change argument @backend.

I really want to improve at least nbd-server-start to avoid the nesting 
(it was a pain to use that much nesting while working on incremental 
backup).

> 
> * TransactionAction
> 
>    Used for command transaction argument @actions.
> 
> * KeyValue
> 
>    Used for command send-key argument @keys, and in InputEvent (next
>    item)
> 
> * InputEvent
> 
>    Used for command input-send-event argument @events.
> 
> Six commands: chardev-add, chardev-change, nbd-server-start,
> transaction, send-key, input-send-event.  Could be worse.
> 
> Flattening could be done in at least two ways.  One, replace the nested
> commands by flat ones, deprecate.  Two, make the existing commands
> accept both nested and flat, deprecate use of nested.  Two is more
> difficult.

Agree that two is more difficult. For at least nbd-server-start, I'm 
fine with replace-and-deprecate.

> 
> Name clashes could prevent the flattening.  I haven't checked for them.
> 
> Three more "simple" unions appear to occur only in results:
> 
> * ImageInfoSpecific
> 
>    Occurs in value of commands query-named-block-nodes and query-block.
> 
> * MemoryDeviceInfo
> 
>    Occurs in value of command query-memory-devices.
> 
> * TpmTypeOptions
> 
>    Occurs in value of command query-tpm.
> 
> There, the only way to get rid of nesting is replace & deprecate.
> 
> I'd love to eliminate "simple" unions from the schema language.
> Possible because any "simple" union can also be expressed as a flat
> union.

And less special code to maintain.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org