Extraneous nesting in QAPI schema

Extraneous nesting in QAPI schema

[Markus Armbruster]

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",
     "arguments": {
         "id": "char0",
         "backend": {
             "type": "socket",
             "data": {
                 "addr": {
                     "type": "inet",
                     "data": {
                         "host": "0.0.0.0",
                         "port": "2445"}},
                 "wait": false,
                 "telnet": false,
                 "server": true}}}}

becomes dotted keys

    --chardev id=char0,\
    backend.type=socket,\
    backend.data.addr.type=inet,\
    backend.data.addr.data.host=0.0.0.0,\
    backend.data.addr.data.port=2445,\
    backend.data.wait=off,\
    backend.data.telnet=off,\
    backend.data.server=on

In our actual CLI, it's

    -chardev id=char0,\
    backend=socket,\
    host=0.0.0.0,\
    port=2445,\
    wait=off,\
    telnet=off,\
    server=on

The flattened syntax is provided by custom code.

Custom code doesn't scale.  One of the hurdles for CLI QAPIfication.

Kevin suggested to investigate a more generic flattening solutions.

Of course, flattening is only possible as long as there are no name
clashes.

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.

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.

* 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.

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.

Re: Extraneous nesting in QAPI schema

[Christophe de Dinechin]

> On 16 Dec 2019, at 17:59, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Kevin suggested to investigate a more generic flattening solutions.
> 
> Of course, flattening is only possible as long as there are no name
> clashes.

Or you could reverse the name sequence and allow
the “upper” layers to disambiguate.

backend.data.addr.type -> type.addr.data.backend
other.data.addr.data.type -> type.data.addr.data.other

then “type” would match both, could be disambiguated with
type.other or type.backend, or even type.o or type.b.

That way, you could have some generic way to disambiguate
for the rare cases you need it.

Re: Extraneous nesting in QAPI schema

[Eric Blake]

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