Re: [RFC PATCH v1 2/8] qapi: golang: Generate qapi's alternate types in Go

[Daniel P . Berrangé]

On Sat, Apr 02, 2022 at 12:40:58AM +0200, Victor Toso wrote:
> This patch handles QAPI alternate types and generates data structures
> in Go that handles it.
> 
> At this moment, there are 5 alternates in qemu/qapi, they are:
>  * BlockDirtyBitmapMergeSource
>  * Qcow2OverlapChecks
>  * BlockdevRef
>  * BlockdevRefOrNull
>  * StrOrNull
> 
> Alternate types are similar to Union but without a discriminator that
> can be used to identify the underlying value on the wire. It is needed
> to infer it. That can't be easily mapped in Go.

I don't buy that. Given this example:

  type BlockdevRef struct {
        // Options are:
        // * definition (BlockdevOptions): defines a new block device inline
        // * reference (string): references the ID of an existing block device
        Value Any
  }

What is the problem with having this Go struct:

  type BlockdevRef struct {
        Definition *BlockdevOptions
        Reference *string
  }

when deserializing from JSON, we know exactly which one of these two
fields to populate. The programmer consuming this can look at which
field is non-nil.

When serializing to JSON, we serialize which ever field is non-nil.

If both fields are non-nil that's a programmer bug. Either ignore it
and only serialize the first non-nil field, or raise an error.

> 
> For each Alternate type, we will be using a Any type to hold the
> value. 'Any' is an alias type for interface{} (similar to void* in C).
> 
> Similarly to the Enum types (see previous commit), we will implement
> Marshaler and Unmarshaler interfaces for the Alternate types and in
> those MarshalJSON() and UnmarshalJSON() methods is where we are going
> to put the logic to read/set alternate's value.
> 
> Note that on UnmarshalJSON(), a helper function called StrictDecode()
> will be used. This function is the main logic to infer if a given JSON
> object fits in a given Go struct. Because we only have 5 alternate
> types, it is not hard to validate the unmarshaling logic but we might
> need to improve it in the future if Alternate with branches that have
> similar fields appear.
> 
> Examples:
>  * BlockdevRef
> ```go
>     // Data to set in BlockdevOptions
>     qcow2 := BlockdevOptionsQcow2{}
>     // BlockdevRef using a string
>     qcow2.File = BlockdevRef{Value: "/some/place/my-image"}
>     opt := BlockdevOptions{}
>     opt.Driver = BlockdevDriverQcow2
>     opt.Value = qcow2
> 
>     b, _ := json.Marshal(data.s)
>     // string(b) == `{"driver":"qcow2","file":"/some/place/my-image"}`
> ```
> 
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  scripts/qapi/golang.py | 157 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 155 insertions(+), 2 deletions(-)

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