Re: [PATCH v1 9/9] docs: add notes on Golang code generator

[Victor Toso]

Hi,

Sigh. I'm sorry about this one. I've added the doc after
reorganizing the patch series. I've been overly focused on python
script and the generated part and less so with checks in qemu
itself.

On Thu, Sep 28, 2023 at 02:22:06PM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 27, 2023 at 01:25:44PM +0200, Victor Toso wrote:
> > The goal of this patch is converge discussions into a documentation,
> > to make it easy and explicit design decisions, known issues and what
> > else might help a person interested in how the Go module is generated.
> > 
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > ---
> >  docs/devel/qapi-golang-code-gen.rst | 341 ++++++++++++++++++++++++++++
> 
> docs/devel/index.rst needs editting to reference this new doc to
> prevent

I've added it in docs/devel/index-build.rst, together with
qapi-code-gen.

>   Warning, treated as error:
>   
> /var/home/berrange/src/virt/qemu/docs/devel/qapi-golang-code-gen.rst:document 
> isn't included in any toctree
>   ninja: build stopped: subcommand failed.
> 
> 
> >  1 file changed, 341 insertions(+)
> >  create mode 100644 docs/devel/qapi-golang-code-gen.rst
> > 
> > diff --git a/docs/devel/qapi-golang-code-gen.rst 
> > b/docs/devel/qapi-golang-code-gen.rst
> > new file mode 100644
> > index 0000000000..2a91f8fc60
> > --- /dev/null
> > +++ b/docs/devel/qapi-golang-code-gen.rst
> > @@ -0,0 +1,341 @@
> > +==========================
> > +QAPI Golang code generator
> > +==========================
> > +
> > +..
> > +   Copyright (C) 2023 Red Hat, Inc.
> > +
> > +   This work is licensed under the terms of the GNU GPL, version 2 or
> > +   later.  See the COPYING file in the top-level directory.
> > +
> > +
> > +Introduction
> > +============
> > +
> > +This document provides information of how the generated Go code maps
> > +with the QAPI specification, clarifying design decisions when needed.
> > +
> > +
> > +Scope of the generated Go code
> > +==============================
> > +
> > +The scope is limited to data structures that can interpret and be used
> > +to generate valid QMP messages. These data structures are generated
> > +from a QAPI schema and should be able to handle QMP messages from the
> > +same schema.
> > +
> > +The generated Go code is a Go module with data structs that uses Go
> > +standard library `encoding/json`, implementing its field tags and
> > +Marshal interface whenever needed.
> 
> 
> Needs to use `` instead of `
> 
>   Warning, treated as error:
>   
> /var/home/berrange/src/virt/qemu/docs/devel/qapi-golang-code-gen.rst:27:'any' 
> reference target not found: encoding/json
>   ninja: build stopped: subcommand failed.
> 
> 
> Repeated several other placs.

Fixed them all.

> > +
> > +
> > +QAPI types to Go structs
> > +========================
> > +
> > +Enum
> > +----
> > +
> > +Enums are mapped as strings in Go, using a specified string type per
> > +Enum to help with type safety in the Go application.
> > +
> > +.. code-block:: JSON
> > +    { 'enum': 'HostMemPolicy',
> > +      'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
> 
> Needs a blank line after every 'code-block:: JSON' or build fails
> with:
> 
> Warning, treated as error:
> /var/home/berrange/src/virt/qemu/docs/devel/qapi-golang-code-gen.rst:41:Error 
> in "code-block" directive:
> maximum 1 argument(s) allowed, 12 supplied.
> 
> .. code-block:: JSON
>     { 'enum': 'HostMemPolicy',
>       'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
> ninja: build stopped: subcommand failed.
> 
> 
> If fixing that then it still isn't happy for reasons I can't
> immediately figure out.

Not sure either. I can reproduce. While looking around, saw Peter
had this issue in May too

    https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg04398.html

So I'm following him, changing this from code block to a literal
block instead.

After this, build works as expected.

Cheers,
Victor
 
> Warning, treated as error:
> /var/home/berrange/src/virt/qemu/docs/devel/qapi-golang-code-gen.rst:41:Could 
> not lex literal_block as "JSON". Highlighting skipped.
> ninja: build stopped: subcommand failed.
> 
> > +
> > +.. code-block:: go
> > +    type HostMemPolicy string
> > +
> > +    const (
> > +        HostMemPolicyDefault    HostMemPolicy = "default"
> > +        HostMemPolicyPreferred  HostMemPolicy = "preferred"
> > +        HostMemPolicyBind       HostMemPolicy = "bind"
> > +        HostMemPolicyInterleave HostMemPolicy = "interleave"
> > +    )
> > +
> > +
> > +Struct
> > +------
> > +
> > +The mapping between a QAPI struct in Go struct is very straightforward.
> > + - Each member of the QAPI struct has its own field in a Go struct.
> > + - Optional members are pointers type with 'omitempty' field tag set
> > +
> > +One important design decision was to _not_ embed base struct, copying
> > +the base members to the original struct. This reduces the complexity
> > +for the Go application.
> > +
> > +.. code-block:: JSON
> > +    { 'struct': 'BlockExportOptionsNbdBase',
> > +      'data': { '*name': 'str', '*description': 'str' } }
> > +
> > +    { 'struct': 'BlockExportOptionsNbd',
> > +      'base': 'BlockExportOptionsNbdBase',
> > +      'data': { '*bitmaps': ['BlockDirtyBitmapOrStr'],
> > +                '*allocation-depth': 'bool' } }
> > +
> > +.. code-block:: go
> > +    type BlockExportOptionsNbd struct {
> > +        Name        *string `json:"name,omitempty"`
> > +        Description *string `json:"description,omitempty"`
> > +
> > +        Bitmaps         []BlockDirtyBitmapOrStr `json:"bitmaps,omitempty"`
> > +        AllocationDepth *bool                   
> > `json:"allocation-depth,omitempty"`
> > +    }
> > +
> > +
> > +Union
> > +-----
> > +
> > +Unions in QAPI are binded to a Enum type which provides all possible
> > +branches of the union. The most important caveat here is that the Union
> > +does not need to implement all possible branches for the Enum.
> > +Receiving a enum value of a unimplemented branch is valid. For this
> > +reason, we keep a discriminator field in each Union Go struct and also
> > +implement the Marshal interface.
> > +
> > +As each Union Go struct type has both the discriminator field and
> > +optional fields, it is important to note that when converting Go struct
> > +to JSON, we only consider the discriminator field if no optional field
> > +member was set. In practice, the user should use the optional fields if
> > +the QAPI Union type has defined them, otherwise the user can set the
> > +discriminator field for the unbranched enum value.
> > +
> > +.. code-block:: JSON
> > +    { 'union': 'ImageInfoSpecificQCow2Encryption',
> > +      'base': 'ImageInfoSpecificQCow2EncryptionBase',
> > +      'discriminator': 'format',
> > +      'data': { 'luks': 'QCryptoBlockInfoLUKS' } }
> > +
> > +.. code-block:: go
> > +    type ImageInfoSpecificQCow2Encryption struct {
> > +        Format BlockdevQcow2EncryptionFormat `json:"format"`
> > +
> > +        // Variants fields
> > +        Luks *QCryptoBlockInfoLUKS `json:"-"`
> > +    }
> > +
> > +    func (s ImageInfoSpecificQCow2Encryption) MarshalJSON() ([]byte, 
> > error) {
> > +        // Normal logic goes here
> > +        // ...
> > +
> > +        // Check for valid values without field members
> > +        if len(bytes) == 0 && err == nil &&
> > +            (s.Format == BlockdevQcow2EncryptionFormatAes) {
> > +            type Alias ImageInfoSpecificQCow2Encryption
> > +            bytes, err = json.Marshal(Alias(s))
> > +        }
> > +        // ...
> > +    }
> > +
> > +
> > +    func (s *ImageInfoSpecificQCow2Encryption) UnmarshalJSON(data []byte) 
> > error {
> > +        // Normal logic goes here
> > +        // ...
> > +
> > +        switch tmp.Format {
> > +        case BlockdevQcow2EncryptionFormatLuks:
> > +            // ...
> > +        default:
> > +            // Check for valid values without field members
> > +            if tmp.Format != BlockdevQcow2EncryptionFormatAes {
> > +                return fmt.Errorf(...)
> > +            }
> > +        }
> > +        return nil
> > +    }
> > +
> > +
> > +Alternate
> > +---------
> > +
> > +Like Unions, alternates can have a few branches. Unlike Unions, they
> > +don't have a discriminator field and each branch should be a different
> > +class of Type entirely (e.g: You can't have two branches of type int in
> > +one Alternate).
> > +
> > +While the marshalling is similar to Unions, the unmarshalling uses a
> > +try-and-error approach, trying to fit the data payload in one of the
> > +Alternate fields.
> > +
> > +The biggest caveat is handling Alternates that can take JSON Null as
> > +value. The issue lies on `encoding/json` library limitation where
> > +unmarshalling JSON Null data to a Go struct which has the 'omitempty'
> > +field that, it bypass the Marshal interface. The same happens when
> > +marshalling, if the field tag 'omitempty' is used, a nil pointer would
> > +never be translated to null JSON value.
> > +
> > +The problem being, we use pointer to type plus `omitempty` field to
> > +express a QAPI optional member.
> > +
> > +In order to handle JSON Null, the generator needs to do the following:
> > +  - Read the QAPI schema prior to generate any code and cache
> > +    all alternate types that can take JSON Null
> > +  - For all Go structs that should be considered optional and they type
> > +    are one of those alternates, do not set `omitempty` and implement
> > +    Marshal interface for this Go struct, to properly handle JSON Null
> > +  - In the Alternate, uses a boolean 'IsNull' to express a JSON Null
> > +    and implement the AbsentAlternate interface, to help sturcts know
> > +    if a given Alternate type should be considered Absent (not set) or
> > +    any other possible Value, including JSON Null.
> > +
> > +.. code-block:: JSON
> > +    { 'alternate': 'BlockdevRefOrNull',
> > +      'data': { 'definition': 'BlockdevOptions',
> > +                'reference': 'str',
> > +                'null': 'null' } }
> > +
> > +.. code-block:: go
> > +    type BlockdevRefOrNull struct {
> > +        Definition *BlockdevOptions
> > +        Reference  *string
> > +        IsNull     bool
> > +    }
> > +
> > +    func (s *BlockdevRefOrNull) ToAnyOrAbsent() (any, bool) {
> > +        if s != nil {
> > +            if s.IsNull {
> > +                return nil, false
> > +            } else if s.Definition != nil {
> > +                return *s.Definition, false
> > +            } else if s.Reference != nil {
> > +                return *s.Reference, false
> > +            }
> > +        }
> > +
> > +        return nil, true
> > +    }
> > +
> > +    func (s BlockdevRefOrNull) MarshalJSON() ([]byte, error) {
> > +        if s.IsNull {
> > +            return []byte("null"), nil
> > +        } else if s.Definition != nil {
> > +            return json.Marshal(s.Definition)
> > +        } else if s.Reference != nil {
> > +            return json.Marshal(s.Reference)
> > +        }
> > +        return []byte("{}"), nil
> > +    }
> > +
> > +    func (s *BlockdevRefOrNull) UnmarshalJSON(data []byte) error {
> > +        // Check for json-null first
> > +        if string(data) == "null" {
> > +            s.IsNull = true
> > +            return nil
> > +        }
> > +        // Check for BlockdevOptions
> > +        {
> > +            s.Definition = new(BlockdevOptions)
> > +            if err := StrictDecode(s.Definition, data); err == nil {
> > +                return nil
> > +            }
> > +            s.Definition = nil
> > +        }
> > +        // Check for string
> > +        {
> > +            s.Reference = new(string)
> > +            if err := StrictDecode(s.Reference, data); err == nil {
> > +                return nil
> > +            }
> > +            s.Reference = nil
> > +        }
> > +
> > +        return fmt.Errorf("Can't convert to BlockdevRefOrNull: %s", 
> > string(data))
> > +    }
> > +
> > +
> > +Event
> > +-----
> > +
> > +All events are mapped to its own struct with the additional
> > +MessageTimestamp field, for the over-the-wire 'timestamp' value.
> > +
> > +Marshaling and Unmarshaling happens over the Event interface, so users
> > +should use the MarshalEvent() and UnmarshalEvent() methods.
> > +
> > +.. code-block:: JSON
> > +    { 'event': 'SHUTDOWN',
> > +      'data': { 'guest': 'bool',
> > +                'reason': 'ShutdownCause' } }
> > +
> > +.. code-block:: go
> > +    type Event interface {
> > +        GetName() string
> > +        GetTimestamp() Timestamp
> > +    }
> > +
> > +    type ShutdownEvent struct {
> > +        MessageTimestamp Timestamp     `json:"-"`
> > +        Guest            bool          `json:"guest"`
> > +        Reason           ShutdownCause `json:"reason"`
> > +    }
> > +
> > +    func (s *ShutdownEvent) GetName() string {
> > +        return "SHUTDOWN"
> > +    }
> > +
> > +    func (s *ShutdownEvent) GetTimestamp() Timestamp {
> > +        return s.MessageTimestamp
> > +    }
> > +
> > +
> > +Command
> > +-------
> > +
> > +All commands are mapped to its own struct with the additional MessageId
> > +field for the optional 'id'. If the command has a boxed data struct,
> > +the option struct will be embed in the command struct.
> > +
> > +As commands do require a return value, every command has its own return
> > +type. The Command interface has a GetReturnType() method that returns a
> > +CommandReturn interface, to help Go application handling the data.
> > +
> > +Marshaling and Unmarshaling happens over the Command interface, so
> > +users should use the MarshalCommand() and UnmarshalCommand() methods.
> > +
> > +.. code-block:: JSON
> > +   { 'command': 'set_password',
> > +     'boxed': true,
> > +     'data': 'SetPasswordOptions' }
> > +
> > +.. code-block:: go
> > +    type Command interface {
> > +        GetId() string
> > +        GetName() string
> > +        GetReturnType() CommandReturn
> > +    }
> > +
> > +    // SetPasswordOptions is embed
> > +    type SetPasswordCommand struct {
> > +        SetPasswordOptions
> > +        MessageId string `json:"-"`
> > +    }
> > +
> > +    // This is an union
> > +    type SetPasswordOptions struct {
> > +        Protocol  DisplayProtocol    `json:"protocol"`
> > +        Password  string             `json:"password"`
> > +        Connected *SetPasswordAction `json:"connected,omitempty"`
> > +
> > +        // Variants fields
> > +        Vnc *SetPasswordOptionsVnc `json:"-"`
> > +    }
> > +
> > +Now an example of a command without boxed type.
> > +
> > +.. code-block:: JSON
> > +    { 'command': 'set_link',
> > +      'data': {'name': 'str', 'up': 'bool'} }
> > +
> > +.. code-block:: go
> > +    type SetLinkCommand struct {
> > +        MessageId string `json:"-"`
> > +        Name      string `json:"name"`
> > +        Up        bool   `json:"up"`
> > +    }
> > +
> > +Known issues
> > +============
> > +
> > +- Type names might not follow proper Go convention. Andrea suggested an
> > +  annotation to the QAPI schema that could solve it.
> > +  https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg00127.html
> > -- 
> > 2.41.0
> > 
> 
> 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 :|
>

signature.asc
Description: PGP signature