[PATCH v1 0/9] qapi-go: add generator for Golang interface

Victor Toso posted 9 patches 7 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230927112544.85011-1-victortoso@redhat.com
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>
There is a newer version of this series
docs/devel/qapi-golang-code-gen.rst |  341 +++++++++
scripts/qapi/golang.py              | 1047 +++++++++++++++++++++++++++
scripts/qapi/main.py                |    2 +
3 files changed, 1390 insertions(+)
create mode 100644 docs/devel/qapi-golang-code-gen.rst
create mode 100644 scripts/qapi/golang.py
[PATCH v1 0/9] qapi-go: add generator for Golang interface
Posted by Victor Toso 7 months, 1 week ago
Hi, long time no see!

This patch series intent is to introduce a generator that produces a Go
module for Go applications to interact over QMP with QEMU.

This idea was discussed before, as RFC:
 (RFC v1) https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
 (RFC v2) https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html

The work got stuck due to changes needed around types that can take JSON
Null as value, but that's now fixed.

I've pushed this series in my gitlab fork:
    https://gitlab.com/victortoso/qemu/-/tree/qapi-golang-v1

I've also generated the qapi-go module over QEMU tags: v7.0.0, v7.1.0,
v7.2.6, v8.0.0 and v8.1.1, see the commits history here:
    https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v1-by-tags

I've also generated the qapi-go module over each commit of this series,
see the commits history here (using previous refered qapi-golang-v1)
    https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v1-by-patch


 * Why this?

My main goal is to allow Go applications that interact with QEMU to have
a native way of doing so.

Ideally, we can merge a new QAPI command, update qapi-go module to allow
Go applications to consume the new command in no time (e.g: if
development of said applications are using latest QEMU)


 * Expectations

From previous discussions, there are things that are still missing. One
simple example is Andrea's annotation suggestion to fix type names. My
proposal is to have a qapi-go module in a formal non-stable version till
some of those tasks get addressed or we declare it a non-problem.

I've created a docs/devel/qapi-golang-code-gen.rst to add information
from the discussions we might have in this series. Suggestions always
welcome.

P.S: Sorry about my broken python :)

Cheers,
Victor

Victor Toso (9):
  qapi: golang: Generate qapi's enum types in Go
  qapi: golang: Generate qapi's alternate types in Go
  qapi: golang: Generate qapi's struct types in Go
  qapi: golang: structs: Address 'null' members
  qapi: golang: Generate qapi's union types in Go
  qapi: golang: Generate qapi's event types in Go
  qapi: golang: Generate qapi's command types in Go
  qapi: golang: Add CommandResult type to Go
  docs: add notes on Golang code generator

 docs/devel/qapi-golang-code-gen.rst |  341 +++++++++
 scripts/qapi/golang.py              | 1047 +++++++++++++++++++++++++++
 scripts/qapi/main.py                |    2 +
 3 files changed, 1390 insertions(+)
 create mode 100644 docs/devel/qapi-golang-code-gen.rst
 create mode 100644 scripts/qapi/golang.py

-- 
2.41.0
Re: [PATCH v1 0/9] qapi-go: add generator for Golang interface
Posted by Victor Toso 7 months, 1 week ago
On Wed, Sep 27, 2023 at 01:25:35PM +0200, Victor Toso wrote:
> Hi, long time no see!
> 
> This patch series intent is to introduce a generator that produces a Go
> module for Go applications to interact over QMP with QEMU.
> 
> This idea was discussed before, as RFC:
>  (RFC v1) https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
>  (RFC v2) https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html

Bad copy-paste, the correct one:
    https://lists.gnu.org/archive/html/qemu-devel/2022-06/msg03105.html

> 
> The work got stuck due to changes needed around types that can take JSON
> Null as value, but that's now fixed.
> 
> I've pushed this series in my gitlab fork:
>     https://gitlab.com/victortoso/qemu/-/tree/qapi-golang-v1
> 
> I've also generated the qapi-go module over QEMU tags: v7.0.0, v7.1.0,
> v7.2.6, v8.0.0 and v8.1.1, see the commits history here:
>     https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v1-by-tags
> 
> I've also generated the qapi-go module over each commit of this series,
> see the commits history here (using previous refered qapi-golang-v1)
>     https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v1-by-patch
> 
> 
>  * Why this?
> 
> My main goal is to allow Go applications that interact with QEMU to have
> a native way of doing so.
> 
> Ideally, we can merge a new QAPI command, update qapi-go module to allow
> Go applications to consume the new command in no time (e.g: if
> development of said applications are using latest QEMU)
> 
> 
>  * Expectations
> 
> From previous discussions, there are things that are still missing. One
> simple example is Andrea's annotation suggestion to fix type names. My
> proposal is to have a qapi-go module in a formal non-stable version till
> some of those tasks get addressed or we declare it a non-problem.
> 
> I've created a docs/devel/qapi-golang-code-gen.rst to add information
> from the discussions we might have in this series. Suggestions always
> welcome.
> 
> P.S: Sorry about my broken python :)
> 
> Cheers,
> Victor
> 
> Victor Toso (9):
>   qapi: golang: Generate qapi's enum types in Go
>   qapi: golang: Generate qapi's alternate types in Go
>   qapi: golang: Generate qapi's struct types in Go
>   qapi: golang: structs: Address 'null' members
>   qapi: golang: Generate qapi's union types in Go
>   qapi: golang: Generate qapi's event types in Go
>   qapi: golang: Generate qapi's command types in Go
>   qapi: golang: Add CommandResult type to Go
>   docs: add notes on Golang code generator
> 
>  docs/devel/qapi-golang-code-gen.rst |  341 +++++++++
>  scripts/qapi/golang.py              | 1047 +++++++++++++++++++++++++++
>  scripts/qapi/main.py                |    2 +
>  3 files changed, 1390 insertions(+)
>  create mode 100644 docs/devel/qapi-golang-code-gen.rst
>  create mode 100644 scripts/qapi/golang.py
> 
> -- 
> 2.41.0
> 
> 
Re: [PATCH v1 0/9] qapi-go: add generator for Golang interface
Posted by Daniel P. Berrangé 7 months, 1 week ago
On Wed, Sep 27, 2023 at 01:25:35PM +0200, Victor Toso wrote:
> Hi, long time no see!
> 
> This patch series intent is to introduce a generator that produces a Go
> module for Go applications to interact over QMP with QEMU.

snip
 
> Victor Toso (9):
>   qapi: golang: Generate qapi's enum types in Go
>   qapi: golang: Generate qapi's alternate types in Go
>   qapi: golang: Generate qapi's struct types in Go
>   qapi: golang: structs: Address 'null' members
>   qapi: golang: Generate qapi's union types in Go
>   qapi: golang: Generate qapi's event types in Go
>   qapi: golang: Generate qapi's command types in Go
>   qapi: golang: Add CommandResult type to Go
>   docs: add notes on Golang code generator
> 
>  docs/devel/qapi-golang-code-gen.rst |  341 +++++++++
>  scripts/qapi/golang.py              | 1047 +++++++++++++++++++++++++++
>  scripts/qapi/main.py                |    2 +
>  3 files changed, 1390 insertions(+)
>  create mode 100644 docs/devel/qapi-golang-code-gen.rst
>  create mode 100644 scripts/qapi/golang.py

So the formatting of the code is kinda all over the place eg

func (s *StrOrNull) ToAnyOrAbsent() (any, bool) {
    if s != nil {
        if s.IsNull {
            return nil, false
    } else if s.S != nil {
        return *s.S, false
        }
    }

    return nil, true
}


ought to be

func (s *StrOrNull) ToAnyOrAbsent() (any, bool) {
        if s != nil {
                if s.IsNull {
                        return nil, false
                } else if s.S != nil {
                        return *s.S, false
                }
        }

        return nil, true
}

I'd say we should add a 'make check-go' target, wired into 'make check'
that runs 'go fmt' on the generated code to validate that we generated
correct code. Then fix the generator to actually emit the reqired
format.


Having said that, this brings up the question of how we expect apps to
consume the Go code. Generally an app would expect to just add the
module to their go.mod file, and have the toolchain download it on
the fly during build.

If we assume a go namespace under qemu.org, we'll want one or more
modules. Either we have one module, containing APIs for all of QEMU,
QGA, and QSD, or we have separate go modules for each. I'd probably
tend towards the latter, since it means we can version them separately
which might be useful if we're willing to break API in one of them,
but not the others.

IOW, integrating this directly into qemu.git as a build time output
is not desirable in this conext though, as 'go build' can't consume
that.

IOW, it would push towards

   go-qemu.git
   go-qga.git
   go-qsd.git

and at time of each QEMU release, we would need to invoke the code
generator, and store its output in the respective git modules.

This would also need the generator application to be a standalone
tool, separate from the C QAPI generator.

Finally Go apps would want to do

   import (
       qemu.org/go/qemu
       qemu.org/go/qga
       qemu.org/go/gsd
   )

and would need us to create the "https://qemu.org/go/qemu" page
containing dummy HTML content with 

    <meta name="go-import" content="qemu.org/go/qemu git https://gitlab.com/qemu-project/go-qemu.git@/>

and likewise for the other modules.

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 :|
Re: [PATCH v1 0/9] qapi-go: add generator for Golang interface
Posted by Victor Toso 7 months ago
Hi,

On Thu, Sep 28, 2023 at 02:40:27PM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 27, 2023 at 01:25:35PM +0200, Victor Toso wrote:
> > Hi, long time no see!
> > 
> > This patch series intent is to introduce a generator that produces a Go
> > module for Go applications to interact over QMP with QEMU.
> 
> snip
>  
> > Victor Toso (9):
> >   qapi: golang: Generate qapi's enum types in Go
> >   qapi: golang: Generate qapi's alternate types in Go
> >   qapi: golang: Generate qapi's struct types in Go
> >   qapi: golang: structs: Address 'null' members
> >   qapi: golang: Generate qapi's union types in Go
> >   qapi: golang: Generate qapi's event types in Go
> >   qapi: golang: Generate qapi's command types in Go
> >   qapi: golang: Add CommandResult type to Go
> >   docs: add notes on Golang code generator
> > 
> >  docs/devel/qapi-golang-code-gen.rst |  341 +++++++++
> >  scripts/qapi/golang.py              | 1047 +++++++++++++++++++++++++++
> >  scripts/qapi/main.py                |    2 +
> >  3 files changed, 1390 insertions(+)
> >  create mode 100644 docs/devel/qapi-golang-code-gen.rst
> >  create mode 100644 scripts/qapi/golang.py
> 
> So the formatting of the code is kinda all over the place eg
> 
> func (s *StrOrNull) ToAnyOrAbsent() (any, bool) {
>     if s != nil {
>         if s.IsNull {
>             return nil, false
>     } else if s.S != nil {
>         return *s.S, false
>         }
>     }
> 
>     return nil, true
> }
> 
> 
> ought to be
> 
> func (s *StrOrNull) ToAnyOrAbsent() (any, bool) {
>         if s != nil {
>                 if s.IsNull {
>                         return nil, false
>                 } else if s.S != nil {
>                         return *s.S, false
>                 }
>         }
> 
>         return nil, true
> }
> 
> I'd say we should add a 'make check-go' target, wired into 'make check'
> that runs 'go fmt' on the generated code to validate that we generated
> correct code. Then fix the generator to actually emit the reqired
> format.

As mentioned in another thread, my main concern with this is
requiring go binaries in the make script. Might be fine if the
scope is only when a release is done, but shouldn't be a default
requirement.

> Having said that, this brings up the question of how we expect apps to
> consume the Go code. Generally an app would expect to just add the
> module to their go.mod file, and have the toolchain download it on
> the fly during build.
> 
> If we assume a go namespace under qemu.org, we'll want one or more
> modules. Either we have one module, containing APIs for all of QEMU,
> QGA, and QSD, or we have separate go modules for each. I'd probably
> tend towards the latter, since it means we can version them separately
> which might be useful if we're willing to break API in one of them,
> but not the others.
> 
> IOW, integrating this directly into qemu.git as a build time output
> is not desirable in this conext though, as 'go build' can't consume
> that.
> 
> IOW, it would push towards
> 
>    go-qemu.git
>    go-qga.git
>    go-qsd.git
> 
> and at time of each QEMU release, we would need to invoke the code
> generator, and store its output in the respective git modules.

In which point, I think it is fair to run the gofmt and goimports.
Still, if you think it isn't a problem to add such make check-go
target with tooling specific to go code in them, I'll add that to
next iteration.

> This would also need the generator application to be a
> standalone tool, separate from the C QAPI generator.

It is. I mean, both run together now but that can be improved.

> Finally Go apps would want to do
> 
>    import (
>        qemu.org/go/qemu
>        qemu.org/go/qga
>        qemu.org/go/gsd
>    )
> 
> and would need us to create the "https://qemu.org/go/qemu" page
> containing dummy HTML content with 
> 
>     <meta name="go-import" content="qemu.org/go/qemu git https://gitlab.com/qemu-project/go-qemu.git@/>

Neat. I didn't know this. Yes, we want that, but with different
name for the git [0]. Perhaps just another folder:

    https://gitlab.com/qemu-project/go/qemu.git
    https://gitlab.com/qemu-project/go/qga.git
    https://gitlab.com/qemu-project/go/gsd.git

> and likewise for the other modules.

[0] https://github.com/digitalocean/go-qemu

Thanks again for the reviews!

Cheers,
Victor
Re: [PATCH v1 0/9] qapi-go: add generator for Golang interface
Posted by Daniel P. Berrangé 7 months, 1 week ago
On Thu, Sep 28, 2023 at 02:40:27PM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 27, 2023 at 01:25:35PM +0200, Victor Toso wrote:
> > Hi, long time no see!
> > 
> > This patch series intent is to introduce a generator that produces a Go
> > module for Go applications to interact over QMP with QEMU.
> 
> snip
>  
> > Victor Toso (9):
> >   qapi: golang: Generate qapi's enum types in Go
> >   qapi: golang: Generate qapi's alternate types in Go
> >   qapi: golang: Generate qapi's struct types in Go
> >   qapi: golang: structs: Address 'null' members
> >   qapi: golang: Generate qapi's union types in Go
> >   qapi: golang: Generate qapi's event types in Go
> >   qapi: golang: Generate qapi's command types in Go
> >   qapi: golang: Add CommandResult type to Go
> >   docs: add notes on Golang code generator
> > 
> >  docs/devel/qapi-golang-code-gen.rst |  341 +++++++++
> >  scripts/qapi/golang.py              | 1047 +++++++++++++++++++++++++++
> >  scripts/qapi/main.py                |    2 +
> >  3 files changed, 1390 insertions(+)
> >  create mode 100644 docs/devel/qapi-golang-code-gen.rst
> >  create mode 100644 scripts/qapi/golang.py
> 
> So the formatting of the code is kinda all over the place eg
> 
> func (s *StrOrNull) ToAnyOrAbsent() (any, bool) {
>     if s != nil {
>         if s.IsNull {
>             return nil, false
>     } else if s.S != nil {
>         return *s.S, false
>         }
>     }
> 
>     return nil, true
> }
> 
> 
> ought to be
> 
> func (s *StrOrNull) ToAnyOrAbsent() (any, bool) {
>         if s != nil {
>                 if s.IsNull {
>                         return nil, false
>                 } else if s.S != nil {
>                         return *s.S, false
>                 }
>         }
> 
>         return nil, true
> }
> 
> I'd say we should add a 'make check-go' target, wired into 'make check'
> that runs 'go fmt' on the generated code to validate that we generated
> correct code. Then fix the generator to actually emit the reqired
> format.
> 
> 
> Having said that, this brings up the question of how we expect apps to
> consume the Go code. Generally an app would expect to just add the
> module to their go.mod file, and have the toolchain download it on
> the fly during build.
> 
> If we assume a go namespace under qemu.org, we'll want one or more
> modules. Either we have one module, containing APIs for all of QEMU,
> QGA, and QSD, or we have separate go modules for each. I'd probably
> tend towards the latter, since it means we can version them separately
> which might be useful if we're willing to break API in one of them,
> but not the others.
> 
> IOW, integrating this directly into qemu.git as a build time output
> is not desirable in this conext though, as 'go build' can't consume
> that.
> 
> IOW, it would push towards
> 
>    go-qemu.git
>    go-qga.git
>    go-qsd.git
> 
> and at time of each QEMU release, we would need to invoke the code
> generator, and store its output in the respective git modules.
> 
> This would also need the generator application to be a standalone
> tool, separate from the C QAPI generator.

Oh, and we need to assume that all CONFIG conditionals in the QAPI
files are true, as we want the Go API to be feature complete such
that it can be used with any QEMU build, regardless of which CONFIG
conditions are turned on/off. We also don't want applications to
suddenly fail to compile because some API was stopped being generated
by a disabled CONFIG condition - it needs to be a runtime error
that apps can catch and handle as they desire.

> 
> Finally Go apps would want to do
> 
>    import (
>        qemu.org/go/qemu
>        qemu.org/go/qga
>        qemu.org/go/gsd
>    )
> 
> and would need us to create the "https://qemu.org/go/qemu" page
> containing dummy HTML content with 
> 
>     <meta name="go-import" content="qemu.org/go/qemu git https://gitlab.com/qemu-project/go-qemu.git@/>
> 
> and likewise for the other modules.
> 
> 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 :|
> 
> 

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


Re: [PATCH v1 0/9] qapi-go: add generator for Golang interface
Posted by Victor Toso 7 months ago
Hi,

On Thu, Sep 28, 2023 at 02:54:10PM +0100, Daniel P. Berrangé wrote:
> On Thu, Sep 28, 2023 at 02:40:27PM +0100, Daniel P. Berrangé wrote:
> > On Wed, Sep 27, 2023 at 01:25:35PM +0200, Victor Toso wrote:
> > IOW, it would push towards
> > 
> >    go-qemu.git
> >    go-qga.git
> >    go-qsd.git
> > 
> > and at time of each QEMU release, we would need to invoke the code
> > generator, and store its output in the respective git modules.
> > 
> > This would also need the generator application to be a standalone
> > tool, separate from the C QAPI generator.
> 
> Oh, and we need to assume that all CONFIG conditionals in the QAPI
> files are true, as we want the Go API to be feature complete such
> that it can be used with any QEMU build, regardless of which CONFIG
> conditions are turned on/off. We also don't want applications to
> suddenly fail to compile because some API was stopped being generated
> by a disabled CONFIG condition - it needs to be a runtime error
> that apps can catch and handle as they desire.

I haven't tested if what is provided to scripts/qapi/golang.py
relies on enabled CONFIG only, I think not. But yes, the
generated module should have it all.

Cheers,
Victor