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

[Daniel P . Berrangé]

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.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|