Hi,

This is the second iteration of RFC v1:
  https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html


# What this is about?

To generate a simple Golang interface that could communicate with QEMU
over QMP. The Go code that is generated is meant to be used as the bare
bones to exchange QMP messages.

The goal is to have this as a Go module in QEMU gitlab namespace,
similar to what have been done to pyhon-qemu-qmp
  https://gitlab.com/qemu-project/python-qemu-qmp


# Issues raised in RFC v1

  The leading '*' for issues I addressed in this iteration

* 1) Documentation was removed to avoid License issues, by Daniel
     Thread: https://lists.nongnu.org/archive/html/qemu-devel/2022-05/msg01889.html

     It is important for the generated Go module to be compatible with
     Licenses used by projects that would be using this. Copying the
     documentation of the QAPI spec might conflict with GPLv2+.

     I have not proposed another license in this iteration, but I'm
     planning to go with MIT No Attribution, aka MIT-0 [0]. Does it make
     sense to bind the generated code's license to MIT-0 already at
     generator level?

     [0] https://github.com/aws/mit-0/blob/master/MIT-0

  2) Inconsistent generated Names, by Andrea + Markus
     Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg05026.html
     Example 1:
         |    qapi    |    Go     |  Expected |
         | @logappend | Logappend | LogAppend |
 
     Example 2:
         (acronyms) VncInfo and DisplayReloadOptionsVNC
 
     This was not addressed in RFC v2 mainly because it seems to need
     more metadata support from the QAPI spec to handle specific
     scenarios. The solution seems either an extra metadata proposal by
     Andrea [1] or reviving Kevin's work [2]
     
     [1] https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg00127.html
     [2] https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg04703.html

* 3) Better type safety, by Andrea + Daniel
     Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01906.html
 
     Most of the 'Any' type (interface {}) has been removed. The only
     place it still exists is for fields that uses QAPI's any type, like
     with command qom-set or the struct type ObjectPropertyInfo.

* 4) QAPI enums mapped to String instead of Int type, by Daniel.
     Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01904.html

     I'm still over the fence about using string here, mostly by the
     same issue reported here:
     https://gitlab.com/libvirt/libvirt-go-module/-/merge_requests/30#note_975517740

* 5) Events and Commands as interface, by Daniel
     Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01914.html

     So, instead of having a Command/Event struct with a Any type for
     the Arguments (which could be set with SetPasswordCommand struct
     type for example); now we have a Command interface which all
     previous structs that behaved as Arguments implement.

     I've included Marshal{Command Event} and Unmarshal{Command Event}
     helper functions that operate on top of each interface.

* 6) Removing Any from Unions, by Daniel
     Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01912.html

     I basically followed the above suggestion to all other types that
     used Any. Specifically to unions were the removal of the
     'discriminator' field, as proposed also in the above link.

* 7) Flat structs by removing embed types. Discussion with Andrea
     Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01590.html 

     No one required it but I decided to give it a try. Major issue that
     I see with this approach is to have generated a few 'Base' structs
     that are now useless. Overall, less nested structs seems better to
     me. Opnions?

     Example:
      | /* This is now useless, should be removed? */
      | type InetSocketAddressBase struct {
      |     Host string `json:"host"`
      |     Port string `json:"port"`
      | }
      |
      | type InetSocketAddress struct {
      |     // Base fields
      |     Host string `json:"host"`
      |     Port string `json:"port"`
      |
      |
      |     Numeric   *bool   `json:"numeric,omitempty"`
      |     To        *uint16 `json:"to,omitempty"`
      |     Ipv4      *bool   `json:"ipv4,omitempty"`
      |     Ipv6      *bool   `json:"ipv6,omitempty"`
      |     KeepAlive *bool   `json:"keep-alive,omitempty"`
      |     Mptcp     *bool   `json:"mptcp,omitempty"`
      | }

  8) Supporting multiple versions
     Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg02147.html

     I'm keen to working on the proposed solution above as it seems a
     good compromise to make code that can be compatible with multiple
     versions of qmp/qemu.

     But the basis needs to be defined first, so this is for the future.

* 9) Handling { "error": { ... } }
     This was missing in the RFC v1. I've added a QapiError type that is
     included in all CommandReturn types, following Andrea's suggestion:

     https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg02199.html


# victortoso/qapi-go and victortoso/qemu

I'm currently hosting the generated code from this RFCv2 at:
    https://gitlab.com/victortoso/qapi-go/-/tree/main/pkg/qapi

This series can also be found at:
    https://gitlab.com/victortoso/qemu/-/commits/qapi-golang


Thanks for taking a look,
Victor

Victor Toso (8):
  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: 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
  qapi: golang: document skip function visit_array_types

 scripts/qapi/golang.py | 765 +++++++++++++++++++++++++++++++++++++++++
 scripts/qapi/main.py   |   2 +
 2 files changed, 767 insertions(+)
 create mode 100644 scripts/qapi/golang.py

-- 
2.36.1

Victor Toso <victortoso@redhat.com> writes:

> Hi,
>
> This is the second iteration of RFC v1:
>   https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
>
>
> # What this is about?
>
> To generate a simple Golang interface that could communicate with QEMU
> over QMP. The Go code that is generated is meant to be used as the bare
> bones to exchange QMP messages.
>
> The goal is to have this as a Go module in QEMU gitlab namespace,
> similar to what have been done to pyhon-qemu-qmp
>   https://gitlab.com/qemu-project/python-qemu-qmp

Aspects of review:

(1) Impact on common code, if any

    I care, because any messes made there are likely to affect me down
    the road.

(2) The generated Go code

    Is it (close to) what we want long term?  If not, is it good enough
    short term, and how could we make necessary improvements?

    I'd prefer to leave this to folks who actually know their Go.

(3) General Python sanity

    We need eyes, but not necessarily mine.  Any takers?

[...]

>  scripts/qapi/golang.py | 765 +++++++++++++++++++++++++++++++++++++++++
>  scripts/qapi/main.py   |   2 +
>  2 files changed, 767 insertions(+)
>  create mode 100644 scripts/qapi/golang.py

This adds a new generator and calls it from generate(), i.e. review
aspect (1) is empty.  "Empty" is a quick & easy way to get my ACK!

No tests?

No documentation?

Hi Markus,

On Mon, Jun 27, 2022 at 09:15:53AM +0200, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
> 
> > Hi,
> >
> > This is the second iteration of RFC v1:
> >   https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
> >
> >
> > # What this is about?
> >
> > To generate a simple Golang interface that could communicate with QEMU
> > over QMP. The Go code that is generated is meant to be used as the bare
> > bones to exchange QMP messages.
> >
> > The goal is to have this as a Go module in QEMU gitlab namespace,
> > similar to what have been done to pyhon-qemu-qmp
> >   https://gitlab.com/qemu-project/python-qemu-qmp
> 
> Aspects of review:
> 
> (1) Impact on common code, if any
> 
>     I care, because any messes made there are likely to affect me down
>     the road.

For the first version of the Go generated interface, my goal is
to have something that works and can be considered alpha to other
Go projects.

With this first version, I don't want to bring huge changes to
the python library or to the QAPI spec and its usage in QEMU.
Unless someone finds that a necessity.

So I hope (1) is simple :)

> (2) The generated Go code
> 
>     Is it (close to) what we want long term?  If not, is it good enough
>     short term, and how could we make necessary improvements?
> 
>     I'd prefer to leave this to folks who actually know their Go.
> (3) General Python sanity
> 
>     We need eyes, but not necessarily mine.  Any takers?
> 
> [...]
> 
> >  scripts/qapi/golang.py | 765 +++++++++++++++++++++++++++++++++++++++++
> >  scripts/qapi/main.py   |   2 +
> >  2 files changed, 767 insertions(+)
> >  create mode 100644 scripts/qapi/golang.py
> 
> This adds a new generator and calls it from generate(), i.e.
> review aspect (1) is empty.  "Empty" is a quick & easy way to
> get my ACK!
> 
> No tests?

I've added tests but on the qapi-go module, those are the files
with _test.go prefix on them. Example for commands:

    https://gitlab.com/victortoso/qapi-go/-/blob/main/pkg/qapi/commands_test.go

Should the generator itself have tests or offloading that to the
qapi-go seems reasonable?

> No documentation?

Yes, this iteration removed the documentation of the generated
types. I'm a bit sad about that. I want to consume the
documentation in the QAPI files to provide the latest info from
types/fields but we can't 'copy' it, the only solution is 'refer'
to it with hyperlink, which I haven't done yet.

Cheers,
Victor

Victor Toso <victortoso@redhat.com> writes:

> Hi Markus,
>
> On Mon, Jun 27, 2022 at 09:15:53AM +0200, Markus Armbruster wrote:
>> Victor Toso <victortoso@redhat.com> writes:
>> 
>> > Hi,
>> >
>> > This is the second iteration of RFC v1:
>> >   https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
>> >
>> >
>> > # What this is about?
>> >
>> > To generate a simple Golang interface that could communicate with QEMU
>> > over QMP. The Go code that is generated is meant to be used as the bare
>> > bones to exchange QMP messages.
>> >
>> > The goal is to have this as a Go module in QEMU gitlab namespace,
>> > similar to what have been done to pyhon-qemu-qmp
>> >   https://gitlab.com/qemu-project/python-qemu-qmp
>> 
>> Aspects of review:
>> 
>> (1) Impact on common code, if any
>> 
>>     I care, because any messes made there are likely to affect me down
>>     the road.
>
> For the first version of the Go generated interface, my goal is
> to have something that works and can be considered alpha to other
> Go projects.
>
> With this first version, I don't want to bring huge changes to
> the python library or to the QAPI spec and its usage in QEMU.
> Unless someone finds that a necessity.
>
> So I hope (1) is simple :)
>
>> (2) The generated Go code
>> 
>>     Is it (close to) what we want long term?  If not, is it good enough
>>     short term, and how could we make necessary improvements?
>> 
>>     I'd prefer to leave this to folks who actually know their Go.
>> (3) General Python sanity
>> 
>>     We need eyes, but not necessarily mine.  Any takers?
>> 
>> [...]
>> 
>> >  scripts/qapi/golang.py | 765 +++++++++++++++++++++++++++++++++++++++++
>> >  scripts/qapi/main.py   |   2 +
>> >  2 files changed, 767 insertions(+)
>> >  create mode 100644 scripts/qapi/golang.py
>> 
>> This adds a new generator and calls it from generate(), i.e.
>> review aspect (1) is empty.  "Empty" is a quick & easy way to
>> get my ACK!
>> 
>> No tests?
>
> I've added tests but on the qapi-go module, those are the files
> with _test.go prefix on them. Example for commands:
>
>     https://gitlab.com/victortoso/qapi-go/-/blob/main/pkg/qapi/commands_test.go
>
> Should the generator itself have tests or offloading that to the
> qapi-go seems reasonable?

Offloading may be reasonable, but how am I to run the tests then?
Documentation should tell me.

We have roughly three kinds of tests so far:

1. Front end tests in tests/qapi-schema

2. Unit tests in tests/unit/

   To find them:

        $ git-grep '#include ".*qapi-.*\.h"' tests/unit/

3. Many tests talking QMP in tests/qtest/

Since you leave the front end alone, you don't need the first kind.

The other two kinds are less clear.

>> No documentation?
>
> Yes, this iteration removed the documentation of the generated
> types. I'm a bit sad about that. I want to consume the
> documentation in the QAPI files to provide the latest info from
> types/fields but we can't 'copy' it, the only solution is 'refer'
> to it with hyperlink, which I haven't done yet.

Two kinds of documentation: generated documentation for the generated Go
code, and documentation about the generator.  I was thinking of the
latter.  Specifically, docs/devel/qapi-code-gen.rst section "Code
generation".  Opinions on its usefulness differ.  I like it.

Hi,

On Mon, Jun 27, 2022 at 05:29:26PM +0200, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
>
> > Hi Markus,
> >
> > On Mon, Jun 27, 2022 at 09:15:53AM +0200, Markus Armbruster wrote:
> >> Victor Toso <victortoso@redhat.com> writes:
> >> 
> >> > Hi,
> >> >
> >> > This is the second iteration of RFC v1:
> >> >   https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg00226.html
> >> >
> >> >
> >> > # What this is about?
> >> >
> >> > To generate a simple Golang interface that could communicate with QEMU
> >> > over QMP. The Go code that is generated is meant to be used as the bare
> >> > bones to exchange QMP messages.
> >> >
> >> > The goal is to have this as a Go module in QEMU gitlab namespace,
> >> > similar to what have been done to pyhon-qemu-qmp
> >> >   https://gitlab.com/qemu-project/python-qemu-qmp
> >>
> >> Aspects of review:
> >> 
> >> (1) Impact on common code, if any
> >> 
> >>     I care, because any messes made there are likely to affect me down
> >>     the road.
> >
> > For the first version of the Go generated interface, my goal is
> > to have something that works and can be considered alpha to other
> > Go projects.
> >
> > With this first version, I don't want to bring huge changes to
> > the python library or to the QAPI spec and its usage in QEMU.
> > Unless someone finds that a necessity.
> >
> > So I hope (1) is simple :)
> >
> >> (2) The generated Go code
> >> 
> >>     Is it (close to) what we want long term?  If not, is it good enough
> >>     short term, and how could we make necessary improvements?
> >> 
> >>     I'd prefer to leave this to folks who actually know their Go.
> >> (3) General Python sanity
> >> 
> >>     We need eyes, but not necessarily mine.  Any takers?
> >> 
> >> [...]
> >> 
> >> >  scripts/qapi/golang.py | 765 +++++++++++++++++++++++++++++++++++++++++
> >> >  scripts/qapi/main.py   |   2 +
> >> >  2 files changed, 767 insertions(+)
> >> >  create mode 100644 scripts/qapi/golang.py
> >> 
> >> This adds a new generator and calls it from generate(), i.e.
> >> review aspect (1) is empty.  "Empty" is a quick & easy way to
> >> get my ACK!
> >> 
> >> No tests?
> >
> > I've added tests but on the qapi-go module, those are the files
> > with _test.go prefix on them. Example for commands:
> >
> >     https://gitlab.com/victortoso/qapi-go/-/blob/main/pkg/qapi/commands_test.go
> >
> > Should the generator itself have tests or offloading that to the
> > qapi-go seems reasonable?
>
> Offloading may be reasonable, but how am I to run the tests then?
> Documentation should tell me.
>
> We have roughly three kinds of tests so far:
>
> 1. Front end tests in tests/qapi-schema
>
> 2. Unit tests in tests/unit/
>
>    To find them:
>
>         $ git-grep '#include ".*qapi-.*\.h"' tests/unit/
>
> 3. Many tests talking QMP in tests/qtest/

I'm thinking on the tests in QEMU side. Perhaps adding something
with Avocado that generates the qapi-go and communicates with a
running QEMU with that generated Go module?

One thing that I try to keep in mind is to not add Go
dependencies in QEMU and this Golang work is not internal to QEMU
itself.

> Since you leave the front end alone, you don't need the first
> kind.
>
> The other two kinds are less clear.

I'm open for suggestions. I thought that, if we have a qapi-go Go
module in Gitlab's qemu-project namespace, we could leverage most
of the tests to the consumer of the actual generated code but I
agree that it is necessary to have something in qemu too.

> >> No documentation?
> >
> > Yes, this iteration removed the documentation of the generated
> > types. I'm a bit sad about that. I want to consume the
> > documentation in the QAPI files to provide the latest info from
> > types/fields but we can't 'copy' it, the only solution is 'refer'
> > to it with hyperlink, which I haven't done yet.
>
> Two kinds of documentation: generated documentation for the generated Go
> code, and documentation about the generator.  I was thinking of the
> latter.  Specifically, docs/devel/qapi-code-gen.rst section "Code
> generation".  Opinions on its usefulness differ.  I like it.

Me too. I'll add documentation for the next iteration, thanks for
pointing it out.

Cheers,
Victor

I've commented in detail to the single patches, just a couple of
additional points.


On Fri, Jun 17, 2022 at 02:19:24PM +0200, Victor Toso wrote:
> * 7) Flat structs by removing embed types. Discussion with Andrea
>      Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01590.html
>
>      No one required it but I decided to give it a try. Major issue that
>      I see with this approach is to have generated a few 'Base' structs
>      that are now useless. Overall, less nested structs seems better to
>      me. Opnions?
>
>      Example:
>       | /* This is now useless, should be removed? */
>       | type InetSocketAddressBase struct {
>       |     Host string `json:"host"`
>       |     Port string `json:"port"`
>       | }

Can we somehow keep track, in the generator, of types that are only
used as building blocks for other types, and prevent them from
showing up in the generated code?


Finally, looking at the repository containing the generated code I
see that the generated type are sorted by kind, e.g. all unions are
in a file, all events in another one and so on. I believe the
structure should match more closely that of the QAPI schema, so e.g.
block-related types should all go in one file, net-related types in
another one and so on.


Looking forward to the next iteration :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Hi,

On Tue, Jul 05, 2022 at 08:46:34AM -0700, Andrea Bolognani wrote:
> I've commented in detail to the single patches, just a couple of
> additional points.
>
> On Fri, Jun 17, 2022 at 02:19:24PM +0200, Victor Toso wrote:
> > * 7) Flat structs by removing embed types. Discussion with Andrea
> >      Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01590.html
> >
> >      No one required it but I decided to give it a try. Major issue that
> >      I see with this approach is to have generated a few 'Base' structs
> >      that are now useless. Overall, less nested structs seems better to
> >      me. Opnions?
> >
> >      Example:
> >       | /* This is now useless, should be removed? */
> >       | type InetSocketAddressBase struct {
> >       |     Host string `json:"host"`
> >       |     Port string `json:"port"`
> >       | }
>
> Can we somehow keep track, in the generator, of types that are
> only used as building blocks for other types, and prevent them
> from showing up in the generated code?

I'm not 100% sure it is good to remove them from generated code
because technically it is a valid qapi type. If all @base types
are embed types and they don't show in other way or form, sure we
can remove them from generated code... I'm not sure if it is
possible to guarantee this.

But yes, if possible, I'd like to remove what seems useless type
definitions.

> Finally, looking at the repository containing the generated
> code I see that the generated type are sorted by kind, e.g. all
> unions are in a file, all events in another one and so on. I
> believe the structure should match more closely that of the
> QAPI schema, so e.g.  block-related types should all go in one
> file, net-related types in another one and so on.

That's something I don't mind adding but some hardcoded mapping
is needed. If you look into git history of qapi/ folder, .json
files can come and go, types be moved around, etc. So, we need to
proper map types in a way that the generated code would be kept
stable even if qapi files would have been rearranged. What I
proposed was only the simplest solution.

Also, the generator takes a qapi-schema.json as input. We are
more focused in qemu/qapi/qapi-schema.json generated coded but
would not hurt to think we could even use it for qemu-guest-agent
from qemu/qga/qapi-schema.json -- this to say that the hardcoded
mapping needs to take into account non qemu qapi schemas too.

> Looking forward to the next iteration :)

Me too, thanks again!

Cheers,
Victor

Victor Toso <victortoso@redhat.com> writes:

> Hi,
>
> On Tue, Jul 05, 2022 at 08:46:34AM -0700, Andrea Bolognani wrote:
>> I've commented in detail to the single patches, just a couple of
>> additional points.
>>
>> On Fri, Jun 17, 2022 at 02:19:24PM +0200, Victor Toso wrote:
>> > * 7) Flat structs by removing embed types. Discussion with Andrea
>> >      Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01590.html
>> >
>> >      No one required it but I decided to give it a try. Major issue that
>> >      I see with this approach is to have generated a few 'Base' structs
>> >      that are now useless. Overall, less nested structs seems better to
>> >      me. Opnions?
>> >
>> >      Example:
>> >       | /* This is now useless, should be removed? */
>> >       | type InetSocketAddressBase struct {
>> >       |     Host string `json:"host"`
>> >       |     Port string `json:"port"`
>> >       | }
>>
>> Can we somehow keep track, in the generator, of types that are
>> only used as building blocks for other types, and prevent them
>> from showing up in the generated code?
>
> I'm not 100% sure it is good to remove them from generated code
> because technically it is a valid qapi type. If all @base types
> are embed types and they don't show in other way or form, sure we
> can remove them from generated code... I'm not sure if it is
> possible to guarantee this.
>
> But yes, if possible, I'd like to remove what seems useless type
> definitions.

The existing C generators have to generate all the types, because the
generated code is for QEMU's own use, where we need all the types.

The existing introspection generator generates only the types visible in
QAPI/QMP introspection.

The former generate for internal use (where we want all the types), and
the latter for external use (where only the types visible in the
external interface are actually useful).

>> Finally, looking at the repository containing the generated
>> code I see that the generated type are sorted by kind, e.g. all
>> unions are in a file, all events in another one and so on. I
>> believe the structure should match more closely that of the
>> QAPI schema, so e.g.  block-related types should all go in one
>> file, net-related types in another one and so on.
>
> That's something I don't mind adding but some hardcoded mapping
> is needed. If you look into git history of qapi/ folder, .json
> files can come and go, types be moved around, etc. So, we need to
> proper map types in a way that the generated code would be kept
> stable even if qapi files would have been rearranged. What I
> proposed was only the simplest solution.
>
> Also, the generator takes a qapi-schema.json as input. We are
> more focused in qemu/qapi/qapi-schema.json generated coded but
> would not hurt to think we could even use it for qemu-guest-agent
> from qemu/qga/qapi-schema.json -- this to say that the hardcoded
> mapping needs to take into account non qemu qapi schemas too.

In the beginning, the QAPI schema was monolithic.  qga/qapi-schema.json
still is.

When keeping everything in a single qapi-schema.json became unwieldy, we
split it into "modules" tied together with a simple include directive.
Generated code remained monolithic.

When monolithic generated code became too annoying (touch schema,
recompile everything), we made it match the module structure: code for
FOO.json goes into *-FOO.c and *-FOO.h, where the *-FOO.h #include the
generated headers for the .json modules FOO.json includes.

Schema code motion hasn't been much of a problem.  Moving from FOO.json
to one of the modules it includes is transparent.  Non-transparent moves
are relatively rare as long as the split into modules actually makes
sense.

>> Looking forward to the next iteration :)
>
> Me too, thanks again!
>
> Cheers,
> Victor

Hi,

On Mon, Aug 29, 2022 at 01:53:51PM +0200, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
>
> > Hi,
> >
> > On Tue, Jul 05, 2022 at 08:46:34AM -0700, Andrea Bolognani wrote:
> >> I've commented in detail to the single patches, just a couple of
> >> additional points.
> >>
> >> On Fri, Jun 17, 2022 at 02:19:24PM +0200, Victor Toso wrote:
> >> > * 7) Flat structs by removing embed types. Discussion with Andrea
> >> >      Thread: https://lists.gnu.org/archive/html/qemu-devel/2022-05/msg01590.html
> >> >
> >> >      No one required it but I decided to give it a try. Major issue that
> >> >      I see with this approach is to have generated a few 'Base' structs
> >> >      that are now useless. Overall, less nested structs seems better to
> >> >      me. Opnions?
> >> >
> >> >      Example:
> >> >       | /* This is now useless, should be removed? */
> >> >       | type InetSocketAddressBase struct {
> >> >       |     Host string `json:"host"`
> >> >       |     Port string `json:"port"`
> >> >       | }
> >>
> >> Can we somehow keep track, in the generator, of types that are
> >> only used as building blocks for other types, and prevent them
> >> from showing up in the generated code?
> >
> > I'm not 100% sure it is good to remove them from generated code
> > because technically it is a valid qapi type. If all @base types
> > are embed types and they don't show in other way or form, sure we
> > can remove them from generated code... I'm not sure if it is
> > possible to guarantee this.
> >
> > But yes, if possible, I'd like to remove what seems useless type
> > definitions.
>
> The existing C generators have to generate all the types, because the
> generated code is for QEMU's own use, where we need all the types.
>
> The existing introspection generator generates only the types visible in
> QAPI/QMP introspection.
>
> The former generate for internal use (where we want all the types), and
> the latter for external use (where only the types visible in the
> external interface are actually useful).

My doubt are on types that might be okay to be hidden because
they are embed in other types, like InetSocketAddressBase.

Note that what I mean with the struct being embed is that the
actual fields of InetSocketAddressBase being added to the type
which uses it, like InetSocketAddress.

  | type InetSocketAddressBase struct {
  |     Host string `json:"host"`
  |     Port string `json:"port"`
  | }
  |
  | type InetSocketAddress struct {
  |     // Base fields
  |     Host string `json:"host"`
  |     Port string `json:"port"`
  |
  |     Numeric   *bool   `json:"numeric,omitempty"`
  |     To        *uint16 `json:"to,omitempty"`
  |     Ipv4      *bool   `json:"ipv4,omitempty"`
  |     Ipv6      *bool   `json:"ipv6,omitempty"`
  |     KeepAlive *bool   `json:"keep-alive,omitempty"`
  |     Mptcp     *bool   `json:"mptcp,omitempty"`
  | }

Andrea's suggestions is to have the generator to track if a given
type is always embed in which case we can skip generating it in
the Go module.

I think that could work indeed. In the hypothetical case that
hiddens structs like InetSocketAddressBase becomes a parameter on
command in the future, the generator would know and start
generating this type from that point onwards.

> >> Finally, looking at the repository containing the generated
> >> code I see that the generated type are sorted by kind, e.g. all
> >> unions are in a file, all events in another one and so on. I
> >> believe the structure should match more closely that of the
> >> QAPI schema, so e.g.  block-related types should all go in one
> >> file, net-related types in another one and so on.
> >
> > That's something I don't mind adding but some hardcoded mapping
> > is needed. If you look into git history of qapi/ folder, .json
> > files can come and go, types be moved around, etc. So, we need to
> > proper map types in a way that the generated code would be kept
> > stable even if qapi files would have been rearranged. What I
> > proposed was only the simplest solution.
> >
> > Also, the generator takes a qapi-schema.json as input. We are
> > more focused in qemu/qapi/qapi-schema.json generated coded but
> > would not hurt to think we could even use it for qemu-guest-agent
> > from qemu/qga/qapi-schema.json -- this to say that the hardcoded
> > mapping needs to take into account non qemu qapi schemas too.
>
> In the beginning, the QAPI schema was monolithic.
> qga/qapi-schema.json still is.
>
> When keeping everything in a single qapi-schema.json became
> unwieldy, we split it into "modules" tied together with a
> simple include directive.  Generated code remained monolithic.

> When monolithic generated code became too annoying (touch
> schema, recompile everything), we made it match the module
> structure: code for FOO.json goes into *-FOO.c and *-FOO.h,
> where the *-FOO.h #include the generated headers for the .json
> modules FOO.json includes.
>
> Schema code motion hasn't been much of a problem.  Moving from
> FOO.json to one of the modules it includes is transparent.
> Non-transparent moves are relatively rare as long as the split
> into modules actually makes sense.

To be honest, splitting it into different files based on their
module names should not be a problem if we keep them in a single
Go module as do intend to for this generated go code.

So I'll go ahead and split it.

Cheers,
Victor