Hi,
On Fri, Jun 17, 2022 at 03:41:10PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jun 17, 2022 at 02:19:27PM +0200, Victor Toso wrote:
> > This patch handles QAPI struct types and generates the equivalent
> > types in Go.
> >
> > At the time of this writing, it generates 388 structures.
> >
> > The highlights of this implementation are:
> >
> > 1. Generating an Go struct that requires a @base type, the @base type
> > fields are copied over to the Go struct. The advantage of this
> > approach is to not have embed structs in any of the QAPI types.
> > The downside are some generated Types that are likely useless now,
> > like InetSocketAddressBase from InetSocketAddress.
> >
> > 2. About the Go struct's fields:
> >
> > i) They can be either by Value or Reference.
> >
> > ii) Every field that is marked as optional in the QAPI specification
> > are translated to Reference fields in its Go structure. This design
> > decision is the most straightforward way to check if a given field
> > was set or not.
> >
> > iii) Mandatory fields are always by Value with the exception of QAPI
> > arrays, which are handled by Reference (to a block of memory) by Go.
> >
> > iv) All the fields are named with Uppercase due Golang's export
> > convention.
> >
> > v) In order to avoid any kind of issues when encoding ordecoding, to
> > or from JSON, we mark all fields with its @name and, when it is
> > optional, member, with @omitempty
> >
> > Example:
> >
> > qapi:
> > | { 'struct': 'BlockdevCreateOptionsFile',
> > | 'data': { 'filename': 'str',
> > | 'size': 'size',
> > | '*preallocation': 'PreallocMode',
> > | '*nocow': 'bool',
> > | '*extent-size-hint': 'size'} }
> >
> > go:
> > | type BlockdevCreateOptionsFile struct {
> > | Filename string `json:"filename"`
> > | Size uint64 `json:"size"`
> > | Preallocation *PreallocMode `json:"preallocation,omitempty"`
> > | Nocow *bool `json:"nocow,omitempty"`
> > | ExtentSizeHint *uint64 `json:"extent-size-hint,omitempty"`
> > | }
>
> One thing to bear in mind here
>
> At the QAPI level, changing a field from mandatory to optional has
> been considered a backwards compatible change by QEMU maintainers,
> because any existing caller can happily continue passing the
> optional field with no downside.
>
> With this Go design, changing a field from mandatory to optional
> will be an API breakage, because the developer will need to change
> from passing a literal value, to a pointer to the value, when
> initializing the struct.
>
> IOW, this Go impl provides weaker compat guarantees than even
> QAPI does, and QAPI compat guarantees were already weaker than
> I would like as an app developer.
I think the current draft should be considered an interface that
can work with the QEMU version this was generated from. That is
the first thing we should get right.
> If we want to make ourselves future proof, we would have to
> make all struct fields optional from the start, even if they
> are mandatory at QAPI level. This would make the code less
> self-documenting though, so that's not very appealing either.
> If we want to avoid this, we would need the same approach I
> suggested wrt support multiple versions of the API
> concurrently. Namely have versioned structs, so every time
> there's a field change of any kind, we introduce a new struct
> version.
That's more or less what I had in mind. I mentioned it in the
item 8 of the cover-letter. I just did not want to address it at
before deciding what the structs should look like first, for the
version we are generating from.
Just to clarify, so far I plan to follow the suggestion:
https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg02147.html
Of course, If there are other ideas, we can discuss it too.
Cheers,
Victor
signature.asc
Description: PGP signature
