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

[Daniel P . Berrangé]

On Wed, May 11, 2022 at 09:22:36AM -0700, Andrea Bolognani wrote:
> On Wed, May 11, 2022 at 04:50:35PM +0100, Daniel P. Berrangé wrote:
> > This would lead to a situation where every struct is duplicated
> > for every version, even though 90% of the time they'll be identical
> > across multiple versions. This is not very ammenable to the desire
> > to be able to dynamically choose per-command which version you
> > want based on which version of QEMU you're connected to.
> >
> > ie
> >
> >      var cmd Command
> >      if qmp.HasVersion(qemu.Version(7, 0, 0)) {
> >         cmd = BlockResizeArguments{
> >            V700: &BlockResizeArguments700{
> >              NodeName: node,
> >              Size: 1 * GiB
> >        }
> >          }
> >      } else {
> >         cmd = BlockResizeArguments{
> >            V520: &BlockResizeArguments520{
> >              Device: dev,
> >              Size: 1 * GiB
> >        }
> >          }
> >      }
> >
> > And of course the HasVersion check is going to be different
> > for each command that matters.
> >
> > Having said that, this perhaps shows the nested structs are
> > overkill. We could have
> >
> >      var cmd Command
> >      if qmp.HasVersion(qemu.Version(7, 0, 0)) {
> >          cmd = &BlockResizeArguments700{
> >              NodeName: node,
> >              Size: 1 * GiB
> >          }
> >      } else {
> >         cmd = &BlockResizeArguments520{
> >              Device: dev,
> >              Size: 1 * GiB
> >          }
> >      }
> 
> Right, making the decision at the import level would require adding a
> level of indirection and make this kind of dynamic logic awkward.
> 
> We shouldn't force users to sprinkle version numbers everywhere
> though, especially since different commands will change at different
> points in time. It should be possible to do something like
> 
>   if !qmp.HasAPI(600) {
>       panic("update QEMU")
>   }
> 
>   cmd := &BlockResizeArguments600{ // really BlockResizeArguments520
>       Device: device,
>       Size:   size,
>   }
> 
> or even
> 
>   if !qmp.HasAPI(qmp.API.Latest) {
>       panic("update QEMU")
>   }
> 
>   cmd := &BlockResizeArguments{
>       NodeName: nodeName,
>       Size:     size,
>   }
> 
> Should be easy enough to achieve with type aliases.

I guess we would have a single package which does

   typedef BlockResizeArguments520 BlockResizeArguments;
   ...for each type...

The interaction with API versioning will be tedious though. For the
versioned structs we'll be forever back compatible, due to the
append-only nature of the versioned struct approach. For the type
aliases, we'll be forever breaking compat as at least 1 struct
alias is likely to need changing every QEMU release.

Might suggest having 2 distinct go modules. A base module which
is versioned as a stable API with semver (forever v1.xxx), and
an add-on module that depends on base module, which is versioned
as an unstable API with semver (forever v0.xxx)


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 :|