Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, May 10, 2022 at 01:34:03PM +0100, Daniel P. Berrangé wrote:
>> On Tue, May 10, 2022 at 02:02:56PM +0200, Markus Armbruster wrote:
>> > > For a minimum viable use case, this doesn't feel all that difficult, as
>> > > conceptually instead of deleting the field from QAPI, we just need to
>> > > annotate it to say when it was deleted from the QEMU side.  The QAPI
>> > > generator for internal QEMU usage, can omit any fields annotated as
>> > > deleted in QAPI schema. The QAPI generator for external app usage,
>> > > can (optionally) be told to include deleted fields ranging back to
>> > > a given version number. So apps can chooses what degree of compat
>> > > they wish to retain.
>> > 
>> > Consider this evolution of command block_resize
>> 
>> To help us understand, I'll illustrate some possible interfaces
>> in both Go and Python, since that covers dynamic and static
>> languages
>> 
>> > * Initially, it has a mandatory argument @device[*].
>> 
>> Python definition:
>> 
>>    def block_resize(device, size)
>> 
>> Caller:
>> 
>>   block_resize('dev0', 1*GiB)
>> 
>> 
>> Golang definition
>> 
>>    type BlockResizeCommand struct {
>>        Device string
>>        Size int
>>    }
>> 
>> Caller
>> 
>>    cmd := &BlockResizeCommand{
>>        Device: "dev0",
>>        Size: 1 * GiB,
>>    }
>> 
>> > * An alternative way to specify the command's object emerges: new
>> >   argument @node-name.  Both old @device and new @node-name become
>> >   optional, and exactly one of them must be specified.  This is commit
>> >   3b1dbd11a6 "qmp: Allow block_resize to manipulate bs graph nodes."
>> 
>> Python definition. Tricky, as non-optional params must be before
>> optional params, but size is naturally the last arg. One option
>> is to pointlessly mark 'size' as optional
>> 
>>    def block_resize(device=None, node_name=None, size=None)
>> 
>> Caller
>> 
>>     block_resize(device="dev0", size=1*GiB)
>>     block_resize(node_name="devnode0", size=1*GiB)
>> 
>> 
>> In golang definition
>> 
>>    type BlockResizeArguments struct {
>>        Device string
>>        NodeName string
>>        Size int
>>    }
>> 
>> Caller choice of
>> 
>>    cmd := &BlockResizeCommand{
>>        Device: "dev0",
>>        Size: 1 * GiB,
>>    }
>> 
>>    cmd := &BlockResizeCommand{
>>        NodeName: "devnode0",
>>        Size: 1 * GiB,
>>    }
>> 
>> 
>> Neither case can easily prevent passing Device and NodeName
>> at same time.
>> 
>> > * At some future date, the old way gets deprecated: argument @device
>> >   acquires feature @deprecated.
>> 
>> Ok, no change needed to the APIs in either case. Possibly have
>> code emit a warning if a deprecated field is set.
>> 
>> > * Still later, the old way gets removed: @device is deleted, and
>> >   @node-name becomes mandatory.
>> 
>> Again no change needed to APIs, but QEMU will throw back an
>> error if the wrong one is used. 
>> 
>> > What is the proper version-spanning interface?
>> > 
>> > I figure it's both arguments optional, must specify the right one for
>> > the version of QEMU actually in use.  This spans versions, but it fails
>> > to abstract from them.
>> 
>> Yep, I think that's inevitable in this scenario. THe plus side
>> is that apps that want to span versions can do so. The downside
>> is that apps that don't want smarts to span version, may loose
>> compile time warnings about use of the now deleted field.
>
> Having said that, a different way to approach the problem is to expose
> the versioning directly in the generated code.
>
> Consider a QAPI with versioning info about the fields
>
>   { 'command': 'block_resize',
>     'since': '5.0.0',
>     'data': { 'device': ['type': 'str', 'until': '5.2.0' ],
>               '*device': ['type': 'str', 'since': '5.2.0', 'until': '7.0.0' ],
>               '*node-name': ['type': 'str', 'since': '5.2.0', 'until: '7.0.0' ],
>               'node-name': ['type': 'str', 'since': '7.0.0' ],
>               'size': 'int' } }
>
> Meaning
>
>   * Introduced in 5.0.0, with 'device' mandatory
>   * In 5.2.0, 'device' becomes optional, with optional 'node-name' as alternative
>   * In 7.0.0, 'device' is deleted, and 'node-name' becomes mandatory
>
> Now consider the Go structs
>
> In 5.0.0 we can generate:
>
>    type BlockResizeArguments struct {
>        V500 *BlockResizeArguments500
>    }
>
>    type BlockResizeArgumentsV1 struct {
>         Device string
>         Size int
>     }
>
> app can use
>
>     dev := "dev0"
>     cmd := BlockResizeArguments{
>        V500: &BlockResizeArguments500{
>           Device: dev,
> 	  Size: 1 * GiB
>        }
>     }
>
>
> In 5.2.0 we can now generate
>
>    type BlockResizeArguments struct {
>        V500 *BlockResizeArgumentsV500
>        V520 *BlockResizeArgumentsV520
>    }
>
>    type BlockResizeArgumentsV500 struct {
>         Device string
>         Size int
>     }
>
>    type BlockResizeArgumentsV520 struct {
>         Device *string
> 	NodeName *string
>         Size int
>     }
>
>
> App can use the same as before, or switch to one of
>
>     dev := "dev0"
>     cmd := BlockResizeArguments{
>        V520: &BlockResizeArguments520{
>           Device: &dev,
> 	  Size: 1 * GiB
>        }
>     }
>
> or
>
>     node := "nodedev0"
>     cmd := BlockResizeArguments{
>        V520: &BlockResizeArguments520{
>           NodeName: &node,
> 	  Size: 1 * GiB
>        }
>     }
>
>
>
> In 7.0.0 we can now generate
>
>
>    type BlockResizeArguments struct {
>        V500 *BlockResizeArgumentsV500
>        V520 *BlockResizeArgumentsV520
>        V700 *BlockResizeArgumentsV700
>    }
>
>    type BlockResizeArgumentsV500 struct {
>         Device string
>         Size int
>    }
>
>    type BlockResizeArgumentsV520 struct {
>         Device *string
> 	NodeName *string
>         Size int
>    }
>
>    type BlockResizeArgumentsV700 struct {
> 	NodeName string
>         Size int
>    }
>
>
>
> App can use the same as before, or switch to
>
>     node := "nodedev0"
>     cmd := BlockResizeArguments{
>        V700: &BlockResizeArguments700{
>           NodeName: node,
> 	  Size: 1 * GiB
>        }
>     }
>
>
> This kind of per-command/type versioning is not uncommon when defining API
> protocols/interfaces.

1. At every release, put a copy of the QAPI schema in the freezer.

2. For every copy, generate Go types with a suitable name suffix.
   Collect all the name stems.

3. For each name stem, define a Go struct that contains all the suffixed
   Go types with that stem.

Look Ma, no cluttering the QAPI schema with a full history!  Also no
complicating the schema language to provide the means for that.