* [PATCH v3 1/8] qapi: golang: Generate enum type
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-14 8:52 ` Markus Armbruster
2025-01-17 10:10 ` Daniel P. Berrangé
2025-01-10 10:49 ` [PATCH v3 2/8] qapi: golang: Generate alternate types Victor Toso
` (8 subsequent siblings)
9 siblings, 2 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
This patch handles QAPI enum types and generates its equivalent in Go.
We sort the output based on enum's type name.
Enums are being handled as strings in Golang.
1. For each QAPI enum, we will define a string type in Go to be the
assigned type of this specific enum.
2. Naming: CamelCase will be used in any identifier that we want to
export, which is everything.
Example:
qapi:
| ##
| # @DisplayProtocol:
| #
| # Display protocols which support changing password options.
| #
| # Since: 7.0
| ##
| { 'enum': 'DisplayProtocol',
| 'data': [ 'vnc', 'spice' ] }
go:
| // Display protocols which support changing password options.
| //
| // Since: 7.0
| type DisplayProtocol string
|
| const (
| DisplayProtocolVnc DisplayProtocol = "vnc"
| DisplayProtocolSpice DisplayProtocol = "spice"
| )
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
scripts/qapi/golang.py | 266 +++++++++++++++++++++++++++++++++++++++++
scripts/qapi/main.py | 3 +
2 files changed, 269 insertions(+)
create mode 100644 scripts/qapi/golang.py
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
new file mode 100644
index 0000000000..1e04c99f1c
--- /dev/null
+++ b/scripts/qapi/golang.py
@@ -0,0 +1,266 @@
+"""
+Golang QAPI generator
+"""
+
+# Copyright (c) 2025 Red Hat Inc.
+#
+# Authors:
+# Victor Toso <victortoso@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+
+# Just for type hint on self
+from __future__ import annotations
+
+import os, textwrap
+from typing import List, Optional
+
+from .schema import (
+ QAPISchema,
+ QAPISchemaBranches,
+ QAPISchemaEnumMember,
+ QAPISchemaFeature,
+ QAPISchemaIfCond,
+ QAPISchemaObjectType,
+ QAPISchemaObjectTypeMember,
+ QAPISchemaType,
+ QAPISchemaVariants,
+ QAPISchemaVisitor,
+)
+from .source import QAPISourceInfo
+
+
+TEMPLATE_ENUM = """
+{maindoc}
+type {name} string
+
+const (
+{fields}
+)
+"""
+
+
+# Takes the documentation object of a specific type and returns
+# that type's documentation and its member's docs.
+def qapi_to_golang_struct_docs(doc: QAPIDoc) -> (str, Dict[str, str]):
+ if doc is None:
+ return "", {}
+
+ cmt = "// "
+ fmt = textwrap.TextWrapper(
+ width=70, initial_indent=cmt, subsequent_indent=cmt
+ )
+ main = fmt.fill(doc.body.text)
+
+ for section in doc.sections:
+ # TODO is not a relevant section to Go applications
+ if section.tag in ["TODO"]:
+ continue
+
+ if main != "":
+ # Give empty line as space for the tag.
+ main += "\n//\n"
+
+ tag = "" if section.tag is None else f"{section.tag}: "
+ text = section.text.replace(" ", " ")
+ main += fmt.fill(f"{tag}{text}")
+
+ fields = {}
+ for key, value in doc.args.items():
+ if len(value.text) > 0:
+ fields[key] = " ".join(value.text.replace("\n", " ").split())
+
+ return main, fields
+
+
+def gen_golang(schema: QAPISchema, output_dir: str, prefix: str) -> None:
+ vis = QAPISchemaGenGolangVisitor(prefix)
+ schema.visit(vis)
+ vis.write(output_dir)
+
+
+def qapi_to_field_name_enum(name: str) -> str:
+ return name.title().replace("-", "")
+
+
+def fetch_indent_blocks_over_enum_with_docs(
+ name: str, members: List[QAPISchemaEnumMember], docfields: Dict[str, str]
+) -> Tuple[int]:
+ maxname = 0
+ blocks: List[int] = [0]
+ for member in members:
+ # For simplicity, every time we have doc, we add a new indent block
+ hasdoc = member.name is not None and member.name in docfields
+
+ enum_name = f"{name}{qapi_to_field_name_enum(member.name)}"
+ maxname = (
+ max(maxname, len(enum_name)) if not hasdoc else len(enum_name)
+ )
+
+ if hasdoc:
+ blocks.append(maxname)
+ else:
+ blocks[-1] = maxname
+
+ return blocks
+
+
+def generate_content_from_dict(data: dict[str, str]) -> str:
+ content = ""
+
+ for name in sorted(data):
+ content += data[name]
+
+ return content.replace("\n\n\n", "\n\n")
+
+
+class QAPISchemaGenGolangVisitor(QAPISchemaVisitor):
+ # pylint: disable=too-many-arguments
+ def __init__(self, _: str):
+ super().__init__()
+ types = ("enum",)
+ self.target = dict.fromkeys(types, "")
+ self.schema: QAPISchema
+ self.golang_package_name = "qapi"
+ self.enums: dict[str, str] = {}
+ self.docmap = {}
+
+ def visit_begin(self, schema: QAPISchema) -> None:
+ self.schema = schema
+
+ # iterate once in schema.docs to map doc objects to its name
+ for doc in schema.docs:
+ if doc.symbol is None:
+ continue
+ self.docmap[doc.symbol] = doc
+
+ # Every Go file needs to reference its package name
+ for target in self.target:
+ self.target[target] = f"package {self.golang_package_name}"
+
+ def visit_end(self) -> None:
+ del self.schema
+ self.target["enum"] += generate_content_from_dict(self.enums)
+
+ def visit_object_type(
+ self,
+ name: str,
+ info: Optional[QAPISourceInfo],
+ ifcond: QAPISchemaIfCond,
+ features: List[QAPISchemaFeature],
+ base: Optional[QAPISchemaObjectType],
+ members: List[QAPISchemaObjectTypeMember],
+ branches: Optional[QAPISchemaBranches],
+ ) -> None:
+ pass
+
+ def visit_alternate_type(
+ self,
+ name: str,
+ info: Optional[QAPISourceInfo],
+ ifcond: QAPISchemaIfCond,
+ features: List[QAPISchemaFeature],
+ variants: QAPISchemaVariants,
+ ) -> None:
+ pass
+
+ def visit_enum_type(
+ self,
+ name: str,
+ info: Optional[QAPISourceInfo],
+ ifcond: QAPISchemaIfCond,
+ features: List[QAPISchemaFeature],
+ members: List[QAPISchemaEnumMember],
+ prefix: Optional[str],
+ ) -> None:
+ assert name not in self.enums
+ doc = self.docmap.get(name, None)
+ maindoc, docfields = qapi_to_golang_struct_docs(doc)
+
+ # The logic below is to generate QAPI enums as blocks of Go consts
+ # each with its own type for type safety inside Go applications.
+ #
+ # Block of const() blocks are vertically indented so we have to
+ # first iterate over all names to calculate space between
+ # $var_name and $var_type. This is achieved by helper function
+ # @fetch_indent_blocks_over_enum_with_docs()
+ #
+ # A new indentation block is defined by empty line or a comment.
+
+ indent_block = iter(
+ fetch_indent_blocks_over_enum_with_docs(name, members, docfields)
+ )
+ maxname = next(indent_block)
+ fields = ""
+ for index, member in enumerate(members):
+ # For simplicity, every time we have doc, we go to next indent block
+ hasdoc = member.name is not None and member.name in docfields
+
+ if hasdoc:
+ maxname = next(indent_block)
+
+ enum_name = f"{name}{qapi_to_field_name_enum(member.name)}"
+ name2type = " " * (maxname - len(enum_name) + 1)
+
+ if hasdoc:
+ docstr = (
+ textwrap.TextWrapper(width=80)
+ .fill(docfields[member.name])
+ .replace("\n", "\n\t// ")
+ )
+ fields += f"""\t// {docstr}\n"""
+
+ fields += f"""\t{enum_name}{name2type}{name} = "{member.name}"\n"""
+
+ self.enums[name] = TEMPLATE_ENUM.format(
+ maindoc=maindoc, name=name, fields=fields[:-1]
+ )
+
+ def visit_array_type(
+ self,
+ name: str,
+ info: Optional[QAPISourceInfo],
+ ifcond: QAPISchemaIfCond,
+ element_type: QAPISchemaType,
+ ) -> None:
+ pass
+
+ def visit_command(
+ self,
+ name: str,
+ info: Optional[QAPISourceInfo],
+ ifcond: QAPISchemaIfCond,
+ features: List[QAPISchemaFeature],
+ arg_type: Optional[QAPISchemaObjectType],
+ ret_type: Optional[QAPISchemaType],
+ gen: bool,
+ success_response: bool,
+ boxed: bool,
+ allow_oob: bool,
+ allow_preconfig: bool,
+ coroutine: bool,
+ ) -> None:
+ pass
+
+ def visit_event(
+ self,
+ name: str,
+ info: Optional[QAPISourceInfo],
+ ifcond: QAPISchemaIfCond,
+ features: List[QAPISchemaFeature],
+ arg_type: Optional[QAPISchemaObjectType],
+ boxed: bool,
+ ) -> None:
+ pass
+
+ def write(self, output_dir: str) -> None:
+ for module_name, content in self.target.items():
+ go_module = module_name + "s.go"
+ go_dir = "go"
+ pathname = os.path.join(output_dir, go_dir, go_module)
+ odir = os.path.dirname(pathname)
+ os.makedirs(odir, exist_ok=True)
+
+ with open(pathname, "w", encoding="utf8") as outfile:
+ outfile.write(content)
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 316736b6a2..f1f813b466 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -15,6 +15,7 @@
from .common import must_match
from .error import QAPIError
from .events import gen_events
+from .golang import gen_golang
from .introspect import gen_introspect
from .schema import QAPISchema
from .types import gen_types
@@ -54,6 +55,8 @@ def generate(schema_file: str,
gen_events(schema, output_dir, prefix)
gen_introspect(schema, output_dir, prefix, unmask)
+ gen_golang(schema, output_dir, prefix)
+
def main() -> int:
"""
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 1/8] qapi: golang: Generate enum type
2025-01-10 10:49 ` [PATCH v3 1/8] qapi: golang: Generate enum type Victor Toso
@ 2025-01-14 8:52 ` Markus Armbruster
2025-01-14 9:38 ` Victor Toso
2025-01-14 10:36 ` Daniel P. Berrangé
2025-01-17 10:10 ` Daniel P. Berrangé
1 sibling, 2 replies; 23+ messages in thread
From: Markus Armbruster @ 2025-01-14 8:52 UTC (permalink / raw)
To: Victor Toso
Cc: qemu-devel, John Snow, Daniel P . Berrangé, Andrea Bolognani
Victor Toso <victortoso@redhat.com> writes:
> This patch handles QAPI enum types and generates its equivalent in Go.
> We sort the output based on enum's type name.
Any particular reason for sorting?
The existing backends generate output it source order, on the (bold?)
assumption that developers care to pick an order that makes sense.
> Enums are being handled as strings in Golang.
>
> 1. For each QAPI enum, we will define a string type in Go to be the
> assigned type of this specific enum.
>
> 2. Naming: CamelCase will be used in any identifier that we want to
> export, which is everything.
>
> Example:
>
> qapi:
> | ##
> | # @DisplayProtocol:
> | #
> | # Display protocols which support changing password options.
> | #
> | # Since: 7.0
> | ##
> | { 'enum': 'DisplayProtocol',
> | 'data': [ 'vnc', 'spice' ] }
>
> go:
> | // Display protocols which support changing password options.
> | //
> | // Since: 7.0
> | type DisplayProtocol string
> |
> | const (
> | DisplayProtocolVnc DisplayProtocol = "vnc"
> | DisplayProtocolSpice DisplayProtocol = "spice"
> | )
>
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
> scripts/qapi/golang.py | 266 +++++++++++++++++++++++++++++++++++++++++
> scripts/qapi/main.py | 3 +
> 2 files changed, 269 insertions(+)
> create mode 100644 scripts/qapi/golang.py
>
> diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> new file mode 100644
> index 0000000000..1e04c99f1c
> --- /dev/null
> +++ b/scripts/qapi/golang.py
[...]
> +class QAPISchemaGenGolangVisitor(QAPISchemaVisitor):
[...]
> + def write(self, output_dir: str) -> None:
> + for module_name, content in self.target.items():
> + go_module = module_name + "s.go"
> + go_dir = "go"
> + pathname = os.path.join(output_dir, go_dir, go_module)
> + odir = os.path.dirname(pathname)
> + os.makedirs(odir, exist_ok=True)
> +
> + with open(pathname, "w", encoding="utf8") as outfile:
> + outfile.write(content)
Your write() serves the same purpose as QAPIGen.write(). The latter
touches output files only when their contents actually changes.
Have you considered use of QAPIGen?
The backends generating C use QAPISchemaMonolithicCVisitor or
QAPISchemaModularCVisitor, which use QAPIGenC, QAPIGenH and
QAPIGenTrace, all specializations of QAPIGen.
> diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
> index 316736b6a2..f1f813b466 100644
> --- a/scripts/qapi/main.py
> +++ b/scripts/qapi/main.py
> @@ -15,6 +15,7 @@
> from .common import must_match
> from .error import QAPIError
> from .events import gen_events
> +from .golang import gen_golang
> from .introspect import gen_introspect
> from .schema import QAPISchema
> from .types import gen_types
> @@ -54,6 +55,8 @@ def generate(schema_file: str,
> gen_events(schema, output_dir, prefix)
> gen_introspect(schema, output_dir, prefix, unmask)
>
> + gen_golang(schema, output_dir, prefix)
> +
>
> def main() -> int:
> """
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 1/8] qapi: golang: Generate enum type
2025-01-14 8:52 ` Markus Armbruster
@ 2025-01-14 9:38 ` Victor Toso
2025-01-14 10:36 ` Daniel P. Berrangé
1 sibling, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-14 9:38 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, John Snow, Daniel P . Berrangé, Andrea Bolognani
[-- Attachment #1: Type: text/plain, Size: 3961 bytes --]
Hi,
On Tue, Jan 14, 2025 at 09:52:23AM +0100, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
>
> > This patch handles QAPI enum types and generates its equivalent in Go.
> > We sort the output based on enum's type name.
>
> Any particular reason for sorting?
It was a request from Daniel that I've accepted.
https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg07042.html
We did the same thing in the code generator in libvirt-go-module
and the result helps with navigating the code and also with the
diff itself when we regenerate.
> The existing backends generate output it source order, on the (bold?)
> assumption that developers care to pick an order that makes sense.
>
> > Enums are being handled as strings in Golang.
> >
> > 1. For each QAPI enum, we will define a string type in Go to be the
> > assigned type of this specific enum.
> >
> > 2. Naming: CamelCase will be used in any identifier that we want to
> > export, which is everything.
> >
> > Example:
> >
> > qapi:
> > | ##
> > | # @DisplayProtocol:
> > | #
> > | # Display protocols which support changing password options.
> > | #
> > | # Since: 7.0
> > | ##
> > | { 'enum': 'DisplayProtocol',
> > | 'data': [ 'vnc', 'spice' ] }
> >
> > go:
> > | // Display protocols which support changing password options.
> > | //
> > | // Since: 7.0
> > | type DisplayProtocol string
> > |
> > | const (
> > | DisplayProtocolVnc DisplayProtocol = "vnc"
> > | DisplayProtocolSpice DisplayProtocol = "spice"
> > | )
> >
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > ---
> > scripts/qapi/golang.py | 266 +++++++++++++++++++++++++++++++++++++++++
> > scripts/qapi/main.py | 3 +
> > 2 files changed, 269 insertions(+)
> > create mode 100644 scripts/qapi/golang.py
> >
> > diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> > new file mode 100644
> > index 0000000000..1e04c99f1c
> > --- /dev/null
> > +++ b/scripts/qapi/golang.py
>
> [...]
>
> > +class QAPISchemaGenGolangVisitor(QAPISchemaVisitor):
>
> [...]
>
> > + def write(self, output_dir: str) -> None:
> > + for module_name, content in self.target.items():
> > + go_module = module_name + "s.go"
> > + go_dir = "go"
> > + pathname = os.path.join(output_dir, go_dir, go_module)
> > + odir = os.path.dirname(pathname)
> > + os.makedirs(odir, exist_ok=True)
> > +
> > + with open(pathname, "w", encoding="utf8") as outfile:
> > + outfile.write(content)
>
> Your write() serves the same purpose as QAPIGen.write(). The latter
> touches output files only when their contents actually changes.
>
> Have you considered use of QAPIGen?
I have not, didn't know that actually.
I have to investigate if it'd be beneficial. Considering this is
once per release execution, shouldn't be a big problem either
way.
> The backends generating C use QAPISchemaMonolithicCVisitor or
> QAPISchemaModularCVisitor, which use QAPIGenC, QAPIGenH and
> QAPIGenTrace, all specializations of QAPIGen.
>
> > diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
> > index 316736b6a2..f1f813b466 100644
> > --- a/scripts/qapi/main.py
> > +++ b/scripts/qapi/main.py
> > @@ -15,6 +15,7 @@
> > from .common import must_match
> > from .error import QAPIError
> > from .events import gen_events
> > +from .golang import gen_golang
> > from .introspect import gen_introspect
> > from .schema import QAPISchema
> > from .types import gen_types
> > @@ -54,6 +55,8 @@ def generate(schema_file: str,
> > gen_events(schema, output_dir, prefix)
> > gen_introspect(schema, output_dir, prefix, unmask)
> >
> > + gen_golang(schema, output_dir, prefix)
> > +
> >
> > def main() -> int:
> > """
Thanks for the review,
Victor
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 1/8] qapi: golang: Generate enum type
2025-01-14 8:52 ` Markus Armbruster
2025-01-14 9:38 ` Victor Toso
@ 2025-01-14 10:36 ` Daniel P. Berrangé
1 sibling, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2025-01-14 10:36 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Victor Toso, qemu-devel, John Snow, Andrea Bolognani
On Tue, Jan 14, 2025 at 09:52:23AM +0100, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
>
> > This patch handles QAPI enum types and generates its equivalent in Go.
> > We sort the output based on enum's type name.
>
> Any particular reason for sorting?
>
> The existing backends generate output it source order, on the (bold?)
> assumption that developers care to pick an order that makes sense.
Even if that assumption is valid (and I question whether it really is),
what makes sense to a QEMU maintainer is not the same as what makes
sense to a consumer of QEMU.
Our approach to file splitting is fairly arbitrary
First, we've got a split across sub-systems, except where we don't.
Sometimes one subsystem splits into many files. eg block.json vs
block-core.json vs block-export.json
Sometimes we just pull something into a standalone file even when it
is not really a subsystem just common code. eg sockets.json
Sometimes we have a split because its convenient for QEMU linux-user
vs system compilation & ELF linker needs. eg misc.json vs misc-target.json
At best we can say that the split of files is intentionally done for the
convenience of the QEMU maintainers. This is irrevelant (and harmful)
for anyone who isn't a QEMU maintainer, which covers any consumer of the
Go code.
IMHO it is also a bug that this file split leaks out into our existing
QAPI docs.
Then when adding to files, sometimes people append to the end of a
file, sometimes it is injected in the middle. I can't say that is an
intentional/concious ordering decision.
The overall result is that wearing my hat of a consumer of QEMU, when
looking at the QAPI schema there is no discernable ordering.
Given the source ordering is not useful, there are two choices
* Sort to have declarations before use as primary key, and then
alphabetical as a secondary sort key
* Sort alphabetically as the primary key
Fortunately the Go compiler has no requirement for forward declarations.
Sorting declarations before use also doesn't really have much effect on
most declarations, so the first option ends up 90% alphabetical sorted
anyway. Might as well just go the whole way and do pure alphabetical
sorting.
Comparing the enums code in "source order":
https://gitlab.com/victortoso/qapi-go/-/blob/qapi-golang-v1-by-tags/pkg/qapi/enums.go
vs alphabetical order:
https://gitlab.com/victortoso/qapi-go/-/blob/qapi-golang-v3-by-tags/pkg/qapi/enums.go
the improvement is massive IMHO.
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 :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/8] qapi: golang: Generate enum type
2025-01-10 10:49 ` [PATCH v3 1/8] qapi: golang: Generate enum type Victor Toso
2025-01-14 8:52 ` Markus Armbruster
@ 2025-01-17 10:10 ` Daniel P. Berrangé
2025-01-17 10:22 ` Victor Toso
1 sibling, 1 reply; 23+ messages in thread
From: Daniel P. Berrangé @ 2025-01-17 10:10 UTC (permalink / raw)
To: Victor Toso; +Cc: qemu-devel, Markus Armbruster, John Snow, Andrea Bolognani
On Fri, Jan 10, 2025 at 11:49:39AM +0100, Victor Toso wrote:
> This patch handles QAPI enum types and generates its equivalent in Go.
> We sort the output based on enum's type name.
>
> Enums are being handled as strings in Golang.
>
> 1. For each QAPI enum, we will define a string type in Go to be the
> assigned type of this specific enum.
>
> 2. Naming: CamelCase will be used in any identifier that we want to
> export, which is everything.
>
> Example:
>
> qapi:
> | ##
> | # @DisplayProtocol:
> | #
> | # Display protocols which support changing password options.
> | #
> | # Since: 7.0
> | ##
> | { 'enum': 'DisplayProtocol',
> | 'data': [ 'vnc', 'spice' ] }
>
> go:
> | // Display protocols which support changing password options.
> | //
> | // Since: 7.0
> | type DisplayProtocol string
> |
> | const (
> | DisplayProtocolVnc DisplayProtocol = "vnc"
> | DisplayProtocolSpice DisplayProtocol = "spice"
> | )
>
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
> scripts/qapi/golang.py | 266 +++++++++++++++++++++++++++++++++++++++++
> scripts/qapi/main.py | 3 +
> 2 files changed, 269 insertions(+)
> create mode 100644 scripts/qapi/golang.py
>
> diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> new file mode 100644
> index 0000000000..1e04c99f1c
> --- /dev/null
> +++ b/scripts/qapi/golang.py
> @@ -0,0 +1,266 @@
> +"""
> +Golang QAPI generator
> +"""
> +
> +# Copyright (c) 2025 Red Hat Inc.
> +#
> +# Authors:
> +# Victor Toso <victortoso@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
Did you mean to say GPL-2.0-only ? Generally we'd expect
new code to be GPL-2.0-or-later.
Also we'd now desire "SPDX-License-Identifier: GPL-2.0-or-later"
for any newly added files, which reminds me to finish my patches
to checkpatch.pl to enforce SPDX usage.
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 :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 1/8] qapi: golang: Generate enum type
2025-01-17 10:10 ` Daniel P. Berrangé
@ 2025-01-17 10:22 ` Victor Toso
0 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-17 10:22 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Markus Armbruster, John Snow, Andrea Bolognani
[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]
Hi,
On Fri, Jan 17, 2025 at 10:10:51AM +0000, Daniel P. Berrangé wrote:
> On Fri, Jan 10, 2025 at 11:49:39AM +0100, Victor Toso wrote:
> > This patch handles QAPI enum types and generates its equivalent in Go.
> > We sort the output based on enum's type name.
> >
> > Enums are being handled as strings in Golang.
> >
> > 1. For each QAPI enum, we will define a string type in Go to be the
> > assigned type of this specific enum.
> >
> > 2. Naming: CamelCase will be used in any identifier that we want to
> > export, which is everything.
> >
> > Example:
> >
> > qapi:
> > | ##
> > | # @DisplayProtocol:
> > | #
> > | # Display protocols which support changing password options.
> > | #
> > | # Since: 7.0
> > | ##
> > | { 'enum': 'DisplayProtocol',
> > | 'data': [ 'vnc', 'spice' ] }
> >
> > go:
> > | // Display protocols which support changing password options.
> > | //
> > | // Since: 7.0
> > | type DisplayProtocol string
> > |
> > | const (
> > | DisplayProtocolVnc DisplayProtocol = "vnc"
> > | DisplayProtocolSpice DisplayProtocol = "spice"
> > | )
> >
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > ---
> > scripts/qapi/golang.py | 266 +++++++++++++++++++++++++++++++++++++++++
> > scripts/qapi/main.py | 3 +
> > 2 files changed, 269 insertions(+)
> > create mode 100644 scripts/qapi/golang.py
> >
> > diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> > new file mode 100644
> > index 0000000000..1e04c99f1c
> > --- /dev/null
> > +++ b/scripts/qapi/golang.py
> > @@ -0,0 +1,266 @@
> > +"""
> > +Golang QAPI generator
> > +"""
> > +
> > +# Copyright (c) 2025 Red Hat Inc.
> > +#
> > +# Authors:
> > +# Victor Toso <victortoso@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.
> > +# See the COPYING file in the top-level directory.
>
> Did you mean to say GPL-2.0-only ? Generally we'd expect
> new code to be GPL-2.0-or-later.
Right. I've copied from another scripts/qapi/*.py
I'll fix it.
> Also we'd now desire "SPDX-License-Identifier: GPL-2.0-or-later"
> for any newly added files,
I'll add it too.
> which reminds me to finish my patches to checkpatch.pl to
> enforce SPDX usage.
Cheers,
Victor
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 2/8] qapi: golang: Generate alternate types
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
2025-01-10 10:49 ` [PATCH v3 1/8] qapi: golang: Generate enum type Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-10 10:49 ` [PATCH v3 3/8] qapi: golang: Generate struct types Victor Toso
` (7 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
This patch handles QAPI alternate types and generates data structures
in Go that handles it.
Alternate types are similar to Union but without a discriminator that
can be used to identify the underlying value on the wire.
1. Over the wire, we need to infer underlying value by its type
2. Pointer to types are mapped as optional. Absent value can be a
valid value.
3. We use Go's standard 'encoding/json' library with its Marshal
and Unmarshal interfaces.
4. As an exceptional but valid case, there are types that accept
JSON NULL as value. Due to limitations with Go's standard library
(point 3) combined with Absent being a possibility (point 2), we
translante NULL values to a boolean field called 'IsNull'. See the
second example and docs/devel/qapi-golang-code-gen.rst under
Alternate section.
* First example:
qapi:
| ##
| # @BlockdevRef:
| #
| # Reference to a block device.
| #
| # @definition: defines a new block device inline
| #
| # @reference: references the ID of an existing block device
| #
| # Since: 2.9
| ##
| { 'alternate': 'BlockdevRef',
| 'data': { 'definition': 'BlockdevOptions',
| 'reference': 'str' } }
go:
| // Reference to a block device.
| //
| // Since: 2.9
| type BlockdevRef struct {
| // defines a new block device inline
| Definition *BlockdevOptions
| // references the ID of an existing block device
| Reference *string
| }
|
| func (s BlockdevRef) MarshalJSON() ([]byte, error) {
| ...
| }
|
| func (s *BlockdevRef) UnmarshalJSON(data []byte) error {
| ...
| }
usage:
| input := `{"driver":"qcow2","data-file":"/some/place/my-image"}`
| k := BlockdevRef{}
| err := json.Unmarshal([]byte(input), &k)
| if err != nil {
| panic(err)
| }
| // *k.Definition.Qcow2.DataFile.Reference == "/some/place/my-image"
* Second example:
qapi:
| { 'alternate': 'StrOrNull',
| 'data': { 's': 'str',
| 'n': 'null' } }
| // This is a string value or the explicit lack of a string (null
| // pointer in C). Intended for cases when 'optional absent' already
| // has a different meaning.
| //
| // Since: 2.10
| type StrOrNull struct {
| // the string value
| S *string
| // no string value
| IsNull bool
| }
|
| // Helper function to get its underlying Go value or absent of value
| func (s *StrOrNull) ToAnyOrAbsent() (any, bool) {
| ...
| }
|
| func (s StrOrNull) MarshalJSON() ([]byte, error) {
| ...
| }
|
| func (s *StrOrNull) UnmarshalJSON(data []byte) error {
| ...
| }
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
scripts/qapi/golang.py | 346 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 343 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index 1e04c99f1c..805e54427c 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -14,10 +14,11 @@
from __future__ import annotations
import os, textwrap
-from typing import List, Optional
+from typing import List, Optional, Tuple
from .schema import (
QAPISchema,
+ QAPISchemaAlternateType,
QAPISchemaBranches,
QAPISchemaEnumMember,
QAPISchemaFeature,
@@ -30,6 +31,7 @@
)
from .source import QAPISourceInfo
+FOUR_SPACES = " "
TEMPLATE_ENUM = """
{maindoc}
@@ -40,6 +42,72 @@
)
"""
+TEMPLATE_HELPER = """
+// Creates a decoder that errors on unknown Fields
+// Returns nil if successfully decoded @from payload to @into type
+// Returns error if failed to decode @from payload to @into type
+func StrictDecode(into interface{}, from []byte) error {
+ dec := json.NewDecoder(strings.NewReader(string(from)))
+ dec.DisallowUnknownFields()
+
+ if err := dec.Decode(into); err != nil {
+ return err
+ }
+ return nil
+}
+"""
+
+TEMPLATE_ALTERNATE_CHECK_INVALID_JSON_NULL = """
+ // Check for json-null first
+ if string(data) == "null" {{
+ return errors.New(`null not supported for {name}`)
+ }}"""
+
+TEMPLATE_ALTERNATE_NULLABLE_CHECK = """
+ }} else if s.{var_name} != nil {{
+ return *s.{var_name}, false"""
+
+TEMPLATE_ALTERNATE_MARSHAL_CHECK = """
+ if s.{var_name} != nil {{
+ return json.Marshal(s.{var_name})
+ }} else """
+
+TEMPLATE_ALTERNATE_UNMARSHAL_CHECK = """
+ // Check for {var_type}
+ {{
+ s.{var_name} = new({var_type})
+ if err := StrictDecode(s.{var_name}, data); err == nil {{
+ return nil
+ }}
+ s.{var_name} = nil
+ }}
+
+"""
+
+TEMPLATE_ALTERNATE_NULLABLE_MARSHAL_CHECK = """
+ if s.IsNull {
+ return []byte("null"), nil
+ } else """
+
+TEMPLATE_ALTERNATE_NULLABLE_UNMARSHAL_CHECK = """
+ // Check for json-null first
+ if string(data) == "null" {
+ s.IsNull = true
+ return nil
+ }"""
+
+TEMPLATE_ALTERNATE_METHODS = """
+func (s {name}) MarshalJSON() ([]byte, error) {{
+{marshal_check_fields}
+ return {marshal_return_default}
+}}
+
+func (s *{name}) UnmarshalJSON(data []byte) error {{
+{unmarshal_check_fields}
+ return fmt.Errorf("Can't convert to {name}: %s", string(data))
+}}
+"""
+
# Takes the documentation object of a specific type and returns
# that type's documentation and its member's docs.
@@ -80,10 +148,88 @@ def gen_golang(schema: QAPISchema, output_dir: str, prefix: str) -> None:
vis.write(output_dir)
+def qapi_to_field_name(name: str) -> str:
+ return name.title().replace("_", "").replace("-", "")
+
+
def qapi_to_field_name_enum(name: str) -> str:
return name.title().replace("-", "")
+def qapi_schema_type_to_go_type(qapitype: str) -> str:
+ schema_types_to_go = {
+ "str": "string",
+ "null": "nil",
+ "bool": "bool",
+ "number": "float64",
+ "size": "uint64",
+ "int": "int64",
+ "int8": "int8",
+ "int16": "int16",
+ "int32": "int32",
+ "int64": "int64",
+ "uint8": "uint8",
+ "uint16": "uint16",
+ "uint32": "uint32",
+ "uint64": "uint64",
+ "any": "any",
+ "QType": "QType",
+ }
+
+ prefix = ""
+ if qapitype.endswith("List"):
+ prefix = "[]"
+ qapitype = qapitype[:-4]
+
+ qapitype = schema_types_to_go.get(qapitype, qapitype)
+ return prefix + qapitype
+
+
+# Helper for Alternate generation
+def qapi_field_to_alternate_go_field(
+ member_name: str, type_name: str
+) -> Tuple[str, str, str]:
+ # Nothing to generate on null types. We update some
+ # variables to handle json-null on marshalling methods.
+ if type_name == "null":
+ return "IsNull", "bool", ""
+
+ # On Alternates, fields are optional represented in Go as pointer
+ return (
+ qapi_to_field_name(member_name),
+ qapi_schema_type_to_go_type(type_name),
+ "*",
+ )
+
+
+def fetch_indent_blocks_over_args(
+ args: List[dict[str:str]],
+) -> Tuple[int, int]:
+ maxname, maxtype = 0, 0
+ blocks: tuple(int, int) = []
+ for arg in args:
+ if "comment" in arg or "doc" in arg:
+ blocks.append((maxname, maxtype))
+ maxname, maxtype = 0, 0
+
+ if "comment" in arg:
+ # They are single blocks
+ continue
+
+ if "type" not in arg:
+ # Embed type are on top of the struct and the following
+ # fields do not consider it for formatting
+ blocks.append((maxname, maxtype))
+ maxname, maxtype = 0, 0
+ continue
+
+ maxname = max(maxname, len(arg.get("name", "")))
+ maxtype = max(maxtype, len(arg.get("type", "")))
+
+ blocks.append((maxname, maxtype))
+ return blocks
+
+
def fetch_indent_blocks_over_enum_with_docs(
name: str, members: List[QAPISchemaEnumMember], docfields: Dict[str, str]
) -> Tuple[int]:
@@ -106,6 +252,137 @@ def fetch_indent_blocks_over_enum_with_docs(
return blocks
+# Helper function for boxed or self contained structures.
+def generate_struct_type(
+ type_name,
+ type_doc: str = "",
+ args: List[dict[str:str]] = None,
+ indent: int = 0,
+) -> str:
+ base_indent = FOUR_SPACES * indent
+
+ with_type = ""
+ if type_name != "":
+ with_type = f"\n{base_indent}type {type_name}"
+
+ if type_doc != "":
+ # Append line jump only if type_doc exists
+ type_doc = f"\n{type_doc}"
+
+ if args is None:
+ # No args, early return
+ return f"""{type_doc}{with_type} struct{{}}"""
+
+ # The logic below is to generate fields of the struct.
+ # We have to be mindful of the different indentation possibilities between
+ # $var_name $var_type $var_tag that are vertically indented with gofmt.
+ #
+ # So, we first have to iterate over all args and find all indent blocks
+ # by calculating the spaces between (1) member and type and between (2)
+ # the type and tag. (1) and (2) is the tuple present in List returned
+ # by the helper function fetch_indent_blocks_over_args.
+ inner_indent = base_indent + FOUR_SPACES
+ doc_indent = inner_indent + "// "
+ fmt = textwrap.TextWrapper(
+ width=70, initial_indent=doc_indent, subsequent_indent=doc_indent
+ )
+
+ indent_block = iter(fetch_indent_blocks_over_args(args))
+ maxname, maxtype = next(indent_block)
+ members = " {\n"
+ for index, arg in enumerate(args):
+ if "comment" in arg:
+ maxname, maxtype = next(indent_block)
+ members += f""" // {arg["comment"]}\n"""
+ # comments are single blocks, so we can skip to next arg
+ continue
+
+ name2type = ""
+ if "doc" in arg:
+ maxname, maxtype = next(indent_block)
+ members += fmt.fill(arg["doc"])
+ members += "\n"
+
+ name = arg["name"]
+ if "type" in arg:
+ namelen = len(name)
+ name2type = " " * max(1, (maxname - namelen + 1))
+
+ type2tag = ""
+ if "tag" in arg:
+ typelen = len(arg["type"])
+ type2tag = " " * max(1, (maxtype - typelen + 1))
+
+ gotype = arg.get("type", "")
+ tag = arg.get("tag", "")
+ members += (
+ f"""{inner_indent}{name}{name2type}{gotype}{type2tag}{tag}\n"""
+ )
+
+ members += f"{base_indent}}}\n"
+ return f"""{type_doc}{with_type} struct{members}"""
+
+
+def generate_template_alternate(
+ self: QAPISchemaGenGolangVisitor,
+ name: str,
+ variants: Optional[QAPISchemaVariants],
+) -> str:
+ args: List[dict[str:str]] = []
+ nullable = name in self.accept_null_types
+ if nullable:
+ # Examples in QEMU QAPI schema: StrOrNull and BlockdevRefOrNull
+ marshal_return_default = """[]byte("{}"), nil"""
+ marshal_check_fields = TEMPLATE_ALTERNATE_NULLABLE_MARSHAL_CHECK[1:]
+ unmarshal_check_fields = TEMPLATE_ALTERNATE_NULLABLE_UNMARSHAL_CHECK
+ else:
+ marshal_return_default = f'nil, errors.New("{name} has empty fields")'
+ marshal_check_fields = ""
+ unmarshal_check_fields = (
+ TEMPLATE_ALTERNATE_CHECK_INVALID_JSON_NULL.format(name=name)
+ )
+
+ doc = self.docmap.get(name, None)
+ content, docfields = qapi_to_golang_struct_docs(doc)
+ if variants:
+ for var in variants.variants:
+ var_name, var_type, isptr = qapi_field_to_alternate_go_field(
+ var.name, var.type.name
+ )
+ args.append(
+ {
+ "name": f"{var_name}",
+ "type": f"{isptr}{var_type}",
+ "doc": docfields.get(var.name, ""),
+ }
+ )
+ # Null is special, handled first
+ if var.type.name == "null":
+ assert nullable
+ continue
+
+ skip_indent = 1 + len(FOUR_SPACES)
+ if marshal_check_fields == "":
+ skip_indent = 1
+ marshal_check_fields += TEMPLATE_ALTERNATE_MARSHAL_CHECK[
+ skip_indent:
+ ].format(var_name=var_name)
+ unmarshal_check_fields += TEMPLATE_ALTERNATE_UNMARSHAL_CHECK[
+ :-1
+ ].format(var_name=var_name, var_type=var_type)
+
+ content += string_to_code(generate_struct_type(name, args=args))
+ content += string_to_code(
+ TEMPLATE_ALTERNATE_METHODS.format(
+ name=name,
+ marshal_check_fields=marshal_check_fields[:-6],
+ marshal_return_default=marshal_return_default,
+ unmarshal_check_fields=unmarshal_check_fields[1:],
+ )
+ )
+ return "\n" + content
+
+
def generate_content_from_dict(data: dict[str, str]) -> str:
content = ""
@@ -115,20 +392,56 @@ def generate_content_from_dict(data: dict[str, str]) -> str:
return content.replace("\n\n\n", "\n\n")
+def string_to_code(text: str) -> str:
+ DOUBLE_BACKTICK = "``"
+ result = ""
+ for line in text.splitlines():
+ # replace left four spaces with tabs
+ limit = len(line) - len(line.lstrip())
+ result += line[:limit].replace(FOUR_SPACES, "\t")
+
+ # work with the rest of the line
+ if line[limit : limit + 2] == "//":
+ # gofmt tool does not like comments with backticks.
+ result += line[limit:].replace(DOUBLE_BACKTICK, '"')
+ else:
+ result += line[limit:]
+ result += "\n"
+
+ return result
+
+
class QAPISchemaGenGolangVisitor(QAPISchemaVisitor):
# pylint: disable=too-many-arguments
def __init__(self, _: str):
super().__init__()
- types = ("enum",)
+ types = (
+ "alternate",
+ "enum",
+ "helper",
+ )
self.target = dict.fromkeys(types, "")
self.schema: QAPISchema
self.golang_package_name = "qapi"
self.enums: dict[str, str] = {}
+ self.alternates: dict[str, str] = {}
+ self.accept_null_types = []
self.docmap = {}
def visit_begin(self, schema: QAPISchema) -> None:
self.schema = schema
+ # We need to be aware of any types that accept JSON NULL
+ for name, entity in self.schema._entity_dict.items():
+ if not isinstance(entity, QAPISchemaAlternateType):
+ # Assume that only Alternate types accept JSON NULL
+ continue
+
+ for var in entity.alternatives.variants:
+ if var.type.name == "null":
+ self.accept_null_types.append(name)
+ break
+
# iterate once in schema.docs to map doc objects to its name
for doc in schema.docs:
if doc.symbol is None:
@@ -136,12 +449,36 @@ def visit_begin(self, schema: QAPISchema) -> None:
self.docmap[doc.symbol] = doc
# Every Go file needs to reference its package name
+ # and most have some imports too.
for target in self.target:
self.target[target] = f"package {self.golang_package_name}"
+ imports = "\n"
+ if target == "helper":
+ imports += """
+import (
+ "encoding/json"
+ "fmt"
+ "strings"
+)
+"""
+ else:
+ imports += """
+import (
+ "encoding/json"
+ "errors"
+ "fmt"
+)
+"""
+ if target != "enum":
+ self.target[target] += string_to_code(imports)
+
+ self.target["helper"] += string_to_code(TEMPLATE_HELPER)
+
def visit_end(self) -> None:
del self.schema
self.target["enum"] += generate_content_from_dict(self.enums)
+ self.target["alternate"] += generate_content_from_dict(self.alternates)
def visit_object_type(
self,
@@ -163,7 +500,10 @@ def visit_alternate_type(
features: List[QAPISchemaFeature],
variants: QAPISchemaVariants,
) -> None:
- pass
+ assert name not in self.alternates
+ self.alternates[name] = generate_template_alternate(
+ self, name, variants
+ )
def visit_enum_type(
self,
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 3/8] qapi: golang: Generate struct types
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
2025-01-10 10:49 ` [PATCH v3 1/8] qapi: golang: Generate enum type Victor Toso
2025-01-10 10:49 ` [PATCH v3 2/8] qapi: golang: Generate alternate types Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-10 10:49 ` [PATCH v3 4/8] qapi: golang: structs: Address nullable members Victor Toso
` (6 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
This patch handles QAPI struct types and generates the equivalent
types in Go. The following patch adds extra logic when a member of the
struct has a Type that can take JSON Null value (e.g: StrOrNull in
QEMU)
The highlights of this implementation are:
1. Generating a 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.
Note that embedding a @base type is recursive, that is, if the
@base type has a @base, all of those fields will be copied over.
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. Exception only for types that can
take JSON Null value.
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.
Example:
qapi:
| ##
| # @BlockdevCreateOptionsFile:
| #
| # Driver specific image creation options for file.
| #
| # @filename: Filename for the new image file
| #
| # @size: Size of the virtual disk in bytes
| #
| # @preallocation: Preallocation mode for the new image (default: off;
| # allowed values: off, falloc (if CONFIG_POSIX_FALLOCATE), full
| # (if CONFIG_POSIX))
| #
| # @nocow: Turn off copy-on-write (valid only on btrfs; default: off)
| #
| # @extent-size-hint: Extent size hint to add to the image file; 0 for
| # not adding an extent size hint (default: 1 MB, since 5.1)
| #
| # Since: 2.12
| ##
| { 'struct': 'BlockdevCreateOptionsFile',
| 'data': { 'filename': 'str',
| 'size': 'size',
| '*preallocation': 'PreallocMode',
| '*nocow': 'bool',
| '*extent-size-hint': 'size'} }
go:
| // Driver specific image creation options for file.
| //
| // Since: 2.12
| type BlockdevCreateOptionsFile struct {
| // Filename for the new image file
| Filename string `json:"filename"`
| // Size of the virtual disk in bytes
| Size uint64 `json:"size"`
| // Preallocation mode for the new image (default: off; allowed
| // values: off, falloc (if CONFIG_POSIX_FALLOCATE), full (if
| // CONFIG_POSIX))
| Preallocation *PreallocMode `json:"preallocation,omitempty"`
| // Turn off copy-on-write (valid only on btrfs; default: off)
| Nocow *bool `json:"nocow,omitempty"`
| // Extent size hint to add to the image file; 0 for not adding an
| // extent size hint (default: 1 MB, since 5.1)
| ExtentSizeHint *uint64 `json:"extent-size-hint,omitempty"`
| }
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
scripts/qapi/golang.py | 199 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 197 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index 805e54427c..df40bd89f2 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -148,6 +148,14 @@ def gen_golang(schema: QAPISchema, output_dir: str, prefix: str) -> None:
vis.write(output_dir)
+def qapi_name_is_base(name: str) -> bool:
+ return qapi_name_is_object(name) and name.endswith("-base")
+
+
+def qapi_name_is_object(name: str) -> bool:
+ return name.startswith("q_obj_")
+
+
def qapi_to_field_name(name: str) -> str:
return name.title().replace("_", "").replace("-", "")
@@ -156,6 +164,27 @@ def qapi_to_field_name_enum(name: str) -> str:
return name.title().replace("-", "")
+def qapi_to_go_type_name(name: str) -> str:
+ # We want to keep CamelCase for Golang types. We want to avoid removing
+ # already set CameCase names while fixing uppercase ones, eg:
+ # 1) q_obj_SocketAddress_base -> SocketAddressBase
+ # 2) q_obj_WATCHDOG-arg -> WatchdogArg
+
+ if qapi_name_is_object(name):
+ # Remove q_obj_ prefix
+ name = name[6:]
+
+ # Handle CamelCase
+ words = list(name.replace("_", "-").split("-"))
+ name = words[0]
+ if name.islower() or name.isupper():
+ name = name.title()
+
+ name += "".join(word.title() for word in words[1:])
+
+ return name
+
+
def qapi_schema_type_to_go_type(qapitype: str) -> str:
schema_types_to_go = {
"str": "string",
@@ -323,6 +352,131 @@ def generate_struct_type(
return f"""{type_doc}{with_type} struct{members}"""
+def get_struct_field(
+ self: QAPISchemaGenGolangVisitor,
+ qapi_name: str,
+ qapi_type_name: str,
+ field_doc: str,
+ is_optional: bool,
+ is_variant: bool,
+) -> dict[str:str]:
+ field = qapi_to_field_name(qapi_name)
+ member_type = qapi_schema_type_to_go_type(qapi_type_name)
+
+ optional = ""
+ if is_optional:
+ if member_type not in self.accept_null_types:
+ optional = ",omitempty"
+
+ # Use pointer to type when field is optional
+ isptr = "*" if is_optional and member_type[0] not in "*[" else ""
+
+ fieldtag = (
+ '`json:"-"`' if is_variant else f'`json:"{qapi_name}{optional}"`'
+ )
+ arg = {
+ "name": f"{field}",
+ "type": f"{isptr}{member_type}",
+ "tag": f"{fieldtag}",
+ }
+ if field_doc != "":
+ arg["doc"] = field_doc
+
+ return arg
+
+
+def recursive_base(
+ self: QAPISchemaGenGolangVisitor,
+ base: Optional[QAPISchemaObjectType],
+) -> List[dict[str:str]]:
+ fields: List[dict[str:str]] = []
+
+ if not base:
+ return fields
+
+ if base.base is not None:
+ embed_base = self.schema.lookup_entity(base.base.name)
+ fields = recursive_base(self, embed_base)
+
+ doc = self.docmap.get(base.name, None)
+ _, docfields = qapi_to_golang_struct_docs(doc)
+
+ for member in base.local_members:
+ field_doc = docfields.get(member.name, "")
+ field = get_struct_field(
+ self,
+ member.name,
+ member.type.name,
+ field_doc,
+ member.optional,
+ False,
+ )
+ fields.append(field)
+
+ return fields
+
+
+# Helper function that is used for most of QAPI types
+def qapi_to_golang_struct(
+ self: QAPISchemaGenGolangVisitor,
+ name: str,
+ info: Optional[QAPISourceInfo],
+ __: QAPISchemaIfCond,
+ ___: List[QAPISchemaFeature],
+ base: Optional[QAPISchemaObjectType],
+ members: List[QAPISchemaObjectTypeMember],
+ variants: Optional[QAPISchemaVariants],
+ indent: int = 0,
+ doc_enabled: bool = True,
+) -> str:
+ fields = recursive_base(self, base)
+
+ doc = self.docmap.get(name, None)
+ type_doc, docfields = qapi_to_golang_struct_docs(doc)
+ if not doc_enabled:
+ type_doc = ""
+
+ if members:
+ for member in members:
+ field_doc = docfields.get(member.name, "") if doc_enabled else ""
+ field = get_struct_field(
+ self,
+ member.name,
+ member.type.name,
+ field_doc,
+ member.optional,
+ False,
+ )
+ fields.append(field)
+
+ exists = {}
+ if variants:
+ fields.append({"comment": "Variants fields"})
+ for variant in variants.variants:
+ if variant.type.is_implicit():
+ continue
+
+ exists[variant.name] = True
+ field_doc = docfields.get(variant.name, "") if doc_enabled else ""
+ field = get_struct_field(
+ self,
+ variant.name,
+ variant.type.name,
+ field_doc,
+ True,
+ True,
+ )
+ fields.append(field)
+
+ type_name = qapi_to_go_type_name(name)
+ content = string_to_code(
+ generate_struct_type(
+ type_name, type_doc=type_doc, args=fields, indent=indent
+ )
+ )
+ return content
+
+
def generate_template_alternate(
self: QAPISchemaGenGolangVisitor,
name: str,
@@ -419,12 +573,14 @@ def __init__(self, _: str):
"alternate",
"enum",
"helper",
+ "struct",
)
self.target = dict.fromkeys(types, "")
self.schema: QAPISchema
self.golang_package_name = "qapi"
self.enums: dict[str, str] = {}
self.alternates: dict[str, str] = {}
+ self.structs: dict[str, str] = {}
self.accept_null_types = []
self.docmap = {}
@@ -454,7 +610,11 @@ def visit_begin(self, schema: QAPISchema) -> None:
self.target[target] = f"package {self.golang_package_name}"
imports = "\n"
- if target == "helper":
+ if target == "struct":
+ imports += """
+import "encoding/json"
+"""
+ elif target == "helper":
imports += """
import (
"encoding/json"
@@ -479,6 +639,7 @@ def visit_end(self) -> None:
del self.schema
self.target["enum"] += generate_content_from_dict(self.enums)
self.target["alternate"] += generate_content_from_dict(self.alternates)
+ self.target["struct"] += generate_content_from_dict(self.structs)
def visit_object_type(
self,
@@ -490,7 +651,41 @@ def visit_object_type(
members: List[QAPISchemaObjectTypeMember],
branches: Optional[QAPISchemaBranches],
) -> None:
- pass
+ # Do not handle anything besides struct.
+ if (
+ name == self.schema.the_empty_object_type.name
+ or not isinstance(name, str)
+ or info.defn_meta not in ["struct"]
+ ):
+ return
+
+ # Base structs are embed
+ if qapi_name_is_base(name):
+ return
+
+ # visit all inner objects as well, they are not going to be
+ # called by python's generator.
+ if branches:
+ for branch in branches.variants:
+ assert isinstance(branch.type, QAPISchemaObjectType)
+ self.visit_object_type(
+ self,
+ branch.type.name,
+ branch.type.info,
+ branch.type.ifcond,
+ branch.type.base,
+ branch.type.local_members,
+ branch.type.branches,
+ )
+
+ # Save generated Go code to be written later
+ if info.defn_meta == "struct":
+ assert name not in self.structs
+ self.structs[name] = string_to_code(
+ qapi_to_golang_struct(
+ self, name, info, ifcond, features, base, members, branches
+ )
+ )
def visit_alternate_type(
self,
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 4/8] qapi: golang: structs: Address nullable members
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
` (2 preceding siblings ...)
2025-01-10 10:49 ` [PATCH v3 3/8] qapi: golang: Generate struct types Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-10 10:49 ` [PATCH v3 5/8] qapi: golang: Generate union type Victor Toso
` (5 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
Explaining why this is needed needs some context, so taking the
example of StrOrNull alternate type and considering a simplified
struct that has two fields:
qapi:
| { 'struct': 'MigrationExample',
| 'data': { '*label': 'StrOrNull',
| 'target': 'StrOrNull' } }
We have an optional member 'label' which can have three JSON values:
1. A string: { "target": "a.host.com", "label": "happy" }
2. A null : { "target": "a.host.com", "label": null }
3. Absent : { "target": null}
The member 'target' is not optional, hence it can't be absent.
A Go struct that contains an optional type that can be JSON Null like
'label' in the example above, will need extra care when Marshaling and
Unmarshaling from JSON.
This patch handles this very specific case:
- It implements the Marshaler interface for these structs to properly
handle these values.
- It adds the interface AbsentAlternate() and implement it for any
Alternate that can be JSON Null. See its uses in map_and_set()
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
scripts/qapi/golang.py | 300 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 289 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index df40bd89f2..ada89f0ce8 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -55,6 +55,28 @@
}
return nil
}
+
+// This helper is used to move struct's fields into a map.
+// This function is useful to merge JSON objects.
+func unwrapToMap(m map[string]any, data any) error {
+ if bytes, err := json.Marshal(&data); err != nil {
+ return fmt.Errorf("unwrapToMap: %s", err)
+ } else if err := json.Unmarshal(bytes, &m); err != nil {
+ return fmt.Errorf("unwrapToMap: %s, data=%s", err, string(bytes))
+ }
+ return nil
+}
+"""
+
+TEMPLATE_ALTERNATE = """
+// Only implemented on Alternate types that can take JSON NULL as value.
+//
+// This is a helper for the marshalling code. It should return true only when
+// the Alternate is empty (no members are set), otherwise it returns false and
+// the member set to be Marshalled.
+type AbsentAlternate interface {
+ ToAnyOrAbsent() (any, bool)
+}
"""
TEMPLATE_ALTERNATE_CHECK_INVALID_JSON_NULL = """
@@ -96,6 +118,19 @@
return nil
}"""
+TEMPLATE_ALTERNATE_NULLABLE = """
+func (s *{name}) ToAnyOrAbsent() (any, bool) {{
+ if s != nil {{
+ if s.IsNull {{
+ return nil, false
+{absent_check_fields}
+ }}
+ }}
+
+ return nil, true
+}}
+"""
+
TEMPLATE_ALTERNATE_METHODS = """
func (s {name}) MarshalJSON() ([]byte, error) {{
{marshal_check_fields}
@@ -109,6 +144,26 @@
"""
+TEMPLATE_STRUCT_WITH_NULLABLE_MARSHAL = """
+func (s {type_name}) MarshalJSON() ([]byte, error) {{
+ m := make(map[string]any)
+{map_members}{map_special}
+ return json.Marshal(&m)
+}}
+
+func (s *{type_name}) UnmarshalJSON(data []byte) error {{
+ tmp := {struct}{{}}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {{
+ return err
+ }}
+
+{set_members}{set_special}
+ return nil
+}}
+"""
+
+
# Takes the documentation object of a specific type and returns
# that type's documentation and its member's docs.
def qapi_to_golang_struct_docs(doc: QAPIDoc) -> (str, Dict[str, str]):
@@ -357,20 +412,30 @@ def get_struct_field(
qapi_name: str,
qapi_type_name: str,
field_doc: str,
+ within_nullable_struct: bool,
is_optional: bool,
is_variant: bool,
-) -> dict[str:str]:
+) -> Tuple[dict[str:str], bool]:
field = qapi_to_field_name(qapi_name)
member_type = qapi_schema_type_to_go_type(qapi_type_name)
+ is_nullable = False
optional = ""
if is_optional:
- if member_type not in self.accept_null_types:
+ if member_type in self.accept_null_types:
+ is_nullable = True
+ else:
optional = ",omitempty"
# Use pointer to type when field is optional
isptr = "*" if is_optional and member_type[0] not in "*[" else ""
+ if within_nullable_struct:
+ # Within a struct which has a field of type that can hold JSON NULL,
+ # we have to _not_ use a pointer, otherwise the Marshal methods are
+ # not called.
+ isptr = "" if member_type in self.accept_null_types else isptr
+
fieldtag = (
'`json:"-"`' if is_variant else f'`json:"{qapi_name}{optional}"`'
)
@@ -382,38 +447,228 @@ def get_struct_field(
if field_doc != "":
arg["doc"] = field_doc
- return arg
+ return arg, is_nullable
+
+
+# This helper is used whithin a struct that has members that accept JSON NULL.
+def map_and_set(
+ is_nullable: bool, field: str, field_is_optional: bool, name: str
+) -> Tuple[str, str]:
+ mapstr = ""
+ setstr = ""
+ if is_nullable:
+ mapstr = f"""
+ if val, absent := s.{field}.ToAnyOrAbsent(); !absent {{
+ m["{name}"] = val
+ }}
+"""
+ setstr += f"""
+ if _, absent := (&tmp.{field}).ToAnyOrAbsent(); !absent {{
+ s.{field} = &tmp.{field}
+ }}
+"""
+ elif field_is_optional:
+ mapstr = f"""
+ if s.{field} != nil {{
+ m["{name}"] = s.{field}
+ }}
+"""
+ setstr = f""" s.{field} = tmp.{field}\n"""
+ else:
+ mapstr = f""" m["{name}"] = s.{field}\n"""
+ setstr = f""" s.{field} = tmp.{field}\n"""
+
+ return mapstr, setstr
+
+
+def recursive_base_nullable(
+ self: QAPISchemaGenGolangVisitor, base: Optional[QAPISchemaObjectType]
+) -> Tuple[List[dict[str:str]], str, str, str, str]:
+ fields: List[dict[str:str]] = []
+ map_members = ""
+ set_members = ""
+ map_special = ""
+ set_special = ""
+
+ if not base:
+ return fields, map_members, set_members, map_special, set_special
+
+ doc = self.docmap.get(base.name, None)
+ _, docfields = qapi_to_golang_struct_docs(doc)
+
+ if base.base is not None:
+ embed_base = self.schema.lookup_entity(base.base.name)
+ (
+ fields,
+ map_members,
+ set_members,
+ map_special,
+ set_special,
+ ) = recursive_base_nullable(self, embed_base)
+
+ for member in base.local_members:
+ field_doc = docfields.get(member.name, "")
+ field, _ = get_struct_field(
+ self,
+ member.name,
+ member.type.name,
+ field_doc,
+ True,
+ member.optional,
+ False,
+ )
+ fields.append(field)
+
+ member_type = qapi_schema_type_to_go_type(member.type.name)
+ nullable = member_type in self.accept_null_types
+ field_name = qapi_to_field_name(member.name)
+ tomap, toset = map_and_set(
+ nullable, field_name, member.optional, member.name
+ )
+ if nullable:
+ map_special += tomap
+ set_special += toset
+ else:
+ map_members += tomap
+ set_members += toset
+
+ return fields, map_members, set_members, map_special, set_special
+
+
+# Helper function. This is executed when the QAPI schema has members
+# that could accept JSON NULL (e.g: StrOrNull in QEMU"s QAPI schema).
+# This struct will need to be extended with Marshal/Unmarshal methods to
+# properly handle such atypical members.
+#
+# Only the Marshallaing methods are generated but we do need to iterate over
+# all the members to properly set/check them in those methods.
+def struct_with_nullable_generate_marshal(
+ self: QAPISchemaGenGolangVisitor,
+ name: str,
+ base: Optional[QAPISchemaObjectType],
+ members: List[QAPISchemaObjectTypeMember],
+ variants: Optional[QAPISchemaVariants],
+) -> str:
+ (
+ fields,
+ map_members,
+ set_members,
+ map_special,
+ set_special,
+ ) = recursive_base_nullable(self, base)
+
+ doc = self.docmap.get(name, None)
+ _, docfields = qapi_to_golang_struct_docs(doc)
+
+ if members:
+ for member in members:
+ field_doc = docfields.get(member.name, "")
+ field, _ = get_struct_field(
+ self,
+ member.name,
+ member.type.name,
+ field_doc,
+ True,
+ member.optional,
+ False,
+ )
+ fields.append(field)
+
+ member_type = qapi_schema_type_to_go_type(member.type.name)
+ nullable = member_type in self.accept_null_types
+ tomap, toset = map_and_set(
+ nullable,
+ qapi_to_field_name(member.name),
+ member.optional,
+ member.name,
+ )
+ if nullable:
+ map_special += tomap
+ set_special += toset
+ else:
+ map_members += tomap
+ set_members += toset
+
+ if variants:
+ for variant in variants.variants:
+ if variant.type.is_implicit():
+ continue
+
+ field, _ = get_struct_field(
+ self,
+ variant.name,
+ variant.type.name,
+ True,
+ variant.optional,
+ True,
+ )
+ fields.append(field)
+
+ member_type = qapi_schema_type_to_go_type(variant.type.name)
+ nullable = member_type in self.accept_null_types
+ tomap, toset = map_and_set(
+ nullable,
+ qapi_to_field_name(variant.name),
+ variant.optional,
+ variant.name,
+ )
+ if nullable:
+ map_special += tomap
+ set_special += toset
+ else:
+ map_members += tomap
+ set_members += toset
+
+ type_name = qapi_to_go_type_name(name)
+ struct = generate_struct_type("", args=fields, indent=1)
+ return string_to_code(
+ TEMPLATE_STRUCT_WITH_NULLABLE_MARSHAL.format(
+ struct=struct[1:-1],
+ type_name=type_name,
+ map_members=map_members,
+ map_special=map_special,
+ set_members=set_members,
+ set_special=set_special,
+ )
+ )
def recursive_base(
self: QAPISchemaGenGolangVisitor,
base: Optional[QAPISchemaObjectType],
-) -> List[dict[str:str]]:
+ discriminator: Optional[str] = None,
+) -> Tuple[List[dict[str:str]], bool]:
fields: List[dict[str:str]] = []
+ with_nullable = False
if not base:
- return fields
+ return fields, with_nullable
if base.base is not None:
embed_base = self.schema.lookup_entity(base.base.name)
- fields = recursive_base(self, embed_base)
+ fields, with_nullable = recursive_base(self, embed_base, discriminator)
doc = self.docmap.get(base.name, None)
_, docfields = qapi_to_golang_struct_docs(doc)
for member in base.local_members:
+ if discriminator and member.name == discriminator:
+ continue
+
field_doc = docfields.get(member.name, "")
- field = get_struct_field(
+ field, nullable = get_struct_field(
self,
member.name,
member.type.name,
field_doc,
+ False,
member.optional,
False,
)
fields.append(field)
+ with_nullable = True if nullable else with_nullable
- return fields
+ return fields, with_nullable
# Helper function that is used for most of QAPI types
@@ -429,7 +684,8 @@ def qapi_to_golang_struct(
indent: int = 0,
doc_enabled: bool = True,
) -> str:
- fields = recursive_base(self, base)
+ discriminator = None if not variants else variants.tag_member.name
+ fields, with_nullable = recursive_base(self, base, discriminator)
doc = self.docmap.get(name, None)
type_doc, docfields = qapi_to_golang_struct_docs(doc)
@@ -439,15 +695,17 @@ def qapi_to_golang_struct(
if members:
for member in members:
field_doc = docfields.get(member.name, "") if doc_enabled else ""
- field = get_struct_field(
+ field, nullable = get_struct_field(
self,
member.name,
member.type.name,
field_doc,
+ False,
member.optional,
False,
)
fields.append(field)
+ with_nullable = True if nullable else with_nullable
exists = {}
if variants:
@@ -458,15 +716,17 @@ def qapi_to_golang_struct(
exists[variant.name] = True
field_doc = docfields.get(variant.name, "") if doc_enabled else ""
- field = get_struct_field(
+ field, nullable = get_struct_field(
self,
variant.name,
variant.type.name,
field_doc,
+ False,
True,
True,
)
fields.append(field)
+ with_nullable = True if nullable else with_nullable
type_name = qapi_to_go_type_name(name)
content = string_to_code(
@@ -474,6 +734,10 @@ def qapi_to_golang_struct(
type_name, type_doc=type_doc, args=fields, indent=indent
)
)
+ if with_nullable:
+ content += struct_with_nullable_generate_marshal(
+ self, name, base, members, variants
+ )
return content
@@ -482,6 +746,7 @@ def generate_template_alternate(
name: str,
variants: Optional[QAPISchemaVariants],
) -> str:
+ absent_check_fields = ""
args: List[dict[str:str]] = []
nullable = name in self.accept_null_types
if nullable:
@@ -515,6 +780,12 @@ def generate_template_alternate(
assert nullable
continue
+ if nullable:
+ absent_check_fields += string_to_code(
+ TEMPLATE_ALTERNATE_NULLABLE_CHECK[1:].format(
+ var_name=var_name
+ )
+ )
skip_indent = 1 + len(FOUR_SPACES)
if marshal_check_fields == "":
skip_indent = 1
@@ -526,6 +797,12 @@ def generate_template_alternate(
].format(var_name=var_name, var_type=var_type)
content += string_to_code(generate_struct_type(name, args=args))
+ if nullable:
+ content += string_to_code(
+ TEMPLATE_ALTERNATE_NULLABLE.format(
+ name=name, absent_check_fields=absent_check_fields[:-1]
+ )
+ )
content += string_to_code(
TEMPLATE_ALTERNATE_METHODS.format(
name=name,
@@ -634,6 +911,7 @@ def visit_begin(self, schema: QAPISchema) -> None:
self.target[target] += string_to_code(imports)
self.target["helper"] += string_to_code(TEMPLATE_HELPER)
+ self.target["alternate"] += string_to_code(TEMPLATE_ALTERNATE)
def visit_end(self) -> None:
del self.schema
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 5/8] qapi: golang: Generate union type
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
` (3 preceding siblings ...)
2025-01-10 10:49 ` [PATCH v3 4/8] qapi: golang: structs: Address nullable members Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-10 10:49 ` [PATCH v3 6/8] qapi: golang: Generate event type Victor Toso
` (4 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
This patch handles QAPI union types and generates the equivalent data
structures and methods in Go to handle it.
The QAPI union type has two types of fields: The @base and the
@Variants members. The @base fields can be considered common members
for the union while only one field maximum is set for the @Variants.
In the QAPI specification, it defines a @discriminator field, which is
an Enum type. The purpose of the @discriminator is to identify which
@variant type is being used.
For the @discriminator's enum that are not handled by the QAPI Union,
we add in the Go struct a separate block as "Unbranched enum fields".
The rationale for this extra block is to allow the user to pass that
enum value under the discriminator, without extra payload.
The union types implement the Marshaler and Unmarshaler interfaces to
seamless decode from JSON objects to Golang structs and vice versa.
qapi:
| ##
| # @SetPasswordOptions:
| #
| # Options for set_password.
| #
| # @protocol:
| # - 'vnc' to modify the VNC server password
| # - 'spice' to modify the Spice server password
| #
| # @password: the new password
| #
| # @connected: How to handle existing clients when changing the
| # password. If nothing is specified, defaults to 'keep'. For
| # VNC, only 'keep' is currently implemented.
| #
| # Since: 7.0
| ##
| { 'union': 'SetPasswordOptions',
| 'base': { 'protocol': 'DisplayProtocol',
| 'password': 'str',
| '*connected': 'SetPasswordAction' },
| 'discriminator': 'protocol',
| 'data': { 'vnc': 'SetPasswordOptionsVnc' } }
go:
| // Options for set_password.
| //
| // Since: 7.0
| type SetPasswordOptions struct {
| // the new password
| Password string `json:"password"`
| // How to handle existing clients when changing the password. If
| // nothing is specified, defaults to 'keep'. For VNC, only 'keep'
| // is currently implemented.
| Connected *SetPasswordAction `json:"connected,omitempty"`
| // Variants fields
| Vnc *SetPasswordOptionsVnc `json:"-"`
| // Unbranched enum fields
| Spice bool `json:"-"`
| }
|
| func (s SetPasswordOptions) MarshalJSON() ([]byte, error) {
| ...
| }
|
| func (s *SetPasswordOptions) UnmarshalJSON(data []byte) error {
| ...
| }
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
scripts/qapi/golang.py | 208 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 205 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index ada89f0ce8..330891ede9 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -164,6 +164,81 @@
"""
+TEMPLATE_UNION_CHECK_VARIANT_FIELD = """
+ if s.{field} != nil && err == nil {{
+ if len(bytes) != 0 {{
+ err = errors.New(`multiple variant fields set`)
+ }} else if err = unwrapToMap(m, s.{field}); err == nil {{
+ m["{discriminator}"] = {go_enum_value}
+ bytes, err = json.Marshal(m)
+ }}
+ }}
+"""
+
+TEMPLATE_UNION_CHECK_UNBRANCHED_FIELD = """
+ if s.{field} && err == nil {{
+ if len(bytes) != 0 {{
+ err = errors.New(`multiple variant fields set`)
+ }} else {{
+ m["{discriminator}"] = {go_enum_value}
+ bytes, err = json.Marshal(m)
+ }}
+ }}
+"""
+
+TEMPLATE_UNION_DRIVER_VARIANT_CASE = """
+ case {go_enum_value}:
+ s.{field} = new({member_type})
+ if err := json.Unmarshal(data, s.{field}); err != nil {{
+ s.{field} = nil
+ return err
+ }}"""
+
+TEMPLATE_UNION_DRIVER_UNBRANCHED_CASE = """
+ case {go_enum_value}:
+ s.{field} = true
+"""
+
+TEMPLATE_UNION_METHODS = """
+func (s {type_name}) MarshalJSON() ([]byte, error) {{
+ var bytes []byte
+ var err error
+ m := make(map[string]any)
+ {{
+ type Alias {type_name}
+ v := Alias(s)
+ unwrapToMap(m, &v)
+ }}
+{check_fields}
+ if err != nil {{
+ return nil, fmt.Errorf("marshal {type_name} due:'%s' struct='%+v'", err, s)
+ }} else if len(bytes) == 0 {{
+ return nil, fmt.Errorf("marshal {type_name} unsupported, struct='%+v'", s)
+ }}
+ return bytes, nil
+}}
+
+func (s *{type_name}) UnmarshalJSON(data []byte) error {{
+{base_type_def}
+ tmp := struct {{
+ {base_type_name}
+ }}{{}}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {{
+ return err
+ }}
+{base_type_assign_unmarshal}
+ switch tmp.{discriminator} {{
+{driver_cases}
+ default:
+ return fmt.Errorf("unmarshal {type_name} received unrecognized value '%s'",
+ tmp.{discriminator})
+ }}
+ return nil
+}}
+"""
+
+
# Takes the documentation object of a specific type and returns
# that type's documentation and its member's docs.
def qapi_to_golang_struct_docs(doc: QAPIDoc) -> (str, Dict[str, str]):
@@ -211,6 +286,12 @@ def qapi_name_is_object(name: str) -> bool:
return name.startswith("q_obj_")
+def qapi_base_name_to_parent(name: str) -> str:
+ if qapi_name_is_base(name):
+ name = name[6:-5]
+ return name
+
+
def qapi_to_field_name(name: str) -> str:
return name.title().replace("_", "").replace("-", "")
@@ -648,7 +729,7 @@ def recursive_base(
embed_base = self.schema.lookup_entity(base.base.name)
fields, with_nullable = recursive_base(self, embed_base, discriminator)
- doc = self.docmap.get(base.name, None)
+ doc = self.docmap.get(qapi_base_name_to_parent(base.name), None)
_, docfields = qapi_to_golang_struct_docs(doc)
for member in base.local_members:
@@ -728,6 +809,24 @@ def qapi_to_golang_struct(
fields.append(field)
with_nullable = True if nullable else with_nullable
+ if info.defn_meta == "union" and variants:
+ enum_name = variants.tag_member.type.name
+ enum_obj = self.schema.lookup_entity(enum_name)
+ if len(exists) != len(enum_obj.members):
+ fields.append({"comment": "Unbranched enum fields"})
+ for member in enum_obj.members:
+ if member.name in exists:
+ continue
+
+ field_doc = (
+ docfields.get(member.name, "") if doc_enabled else ""
+ )
+ field, nullable = get_struct_field(
+ self, member.name, "bool", field_doc, False, False, True
+ )
+ fields.append(field)
+ with_nullable = True if nullable else with_nullable
+
type_name = qapi_to_go_type_name(name)
content = string_to_code(
generate_struct_type(
@@ -741,6 +840,98 @@ def qapi_to_golang_struct(
return content
+def qapi_to_golang_methods_union(
+ self: QAPISchemaGenGolangVisitor,
+ name: str,
+ base: Optional[QAPISchemaObjectType],
+ variants: Optional[QAPISchemaVariants],
+) -> str:
+ type_name = qapi_to_go_type_name(name)
+
+ assert base
+ base_type_assign_unmarshal = ""
+ base_type_name = qapi_to_go_type_name(base.name)
+ base_type_def = qapi_to_golang_struct(
+ self,
+ base.name,
+ base.info,
+ base.ifcond,
+ base.features,
+ base.base,
+ base.members,
+ base.branches,
+ indent=1,
+ doc_enabled=False,
+ )
+
+ discriminator = qapi_to_field_name(variants.tag_member.name)
+ for member in base.local_members:
+ field = qapi_to_field_name(member.name)
+ if field == discriminator:
+ continue
+ base_type_assign_unmarshal += f"""
+ s.{field} = tmp.{field}"""
+
+ driver_cases = ""
+ check_fields = ""
+ exists = {}
+ enum_name = variants.tag_member.type.name
+ if variants:
+ for var in variants.variants:
+ if var.type.is_implicit():
+ continue
+
+ field = qapi_to_field_name(var.name)
+ enum_value = qapi_to_field_name_enum(var.name)
+ member_type = qapi_schema_type_to_go_type(var.type.name)
+ go_enum_value = f"""{enum_name}{enum_value}"""
+ exists[go_enum_value] = True
+
+ check_fields += TEMPLATE_UNION_CHECK_VARIANT_FIELD.format(
+ field=field,
+ discriminator=variants.tag_member.name,
+ go_enum_value=go_enum_value,
+ )
+ driver_cases += TEMPLATE_UNION_DRIVER_VARIANT_CASE.format(
+ go_enum_value=go_enum_value,
+ field=field,
+ member_type=member_type,
+ )
+
+ enum_obj = self.schema.lookup_entity(enum_name)
+ if len(exists) != len(enum_obj.members):
+ for member in enum_obj.members:
+ value = qapi_to_field_name_enum(member.name)
+ go_enum_value = f"""{enum_name}{value}"""
+
+ if go_enum_value in exists:
+ continue
+
+ field = qapi_to_field_name(member.name)
+
+ check_fields += TEMPLATE_UNION_CHECK_UNBRANCHED_FIELD.format(
+ field=field,
+ discriminator=variants.tag_member.name,
+ go_enum_value=go_enum_value,
+ )
+ driver_cases += TEMPLATE_UNION_DRIVER_UNBRANCHED_CASE.format(
+ go_enum_value=go_enum_value,
+ field=field,
+ )
+
+ return string_to_code(
+ TEMPLATE_UNION_METHODS.format(
+ type_name=type_name,
+ check_fields=check_fields[1:],
+ base_type_def=base_type_def[1:],
+ base_type_name=base_type_name,
+ base_type_assign_unmarshal=base_type_assign_unmarshal,
+ discriminator=discriminator,
+ driver_cases=driver_cases[1:],
+ )
+ )
+
+
def generate_template_alternate(
self: QAPISchemaGenGolangVisitor,
name: str,
@@ -851,6 +1042,7 @@ def __init__(self, _: str):
"enum",
"helper",
"struct",
+ "union",
)
self.target = dict.fromkeys(types, "")
self.schema: QAPISchema
@@ -858,6 +1050,7 @@ def __init__(self, _: str):
self.enums: dict[str, str] = {}
self.alternates: dict[str, str] = {}
self.structs: dict[str, str] = {}
+ self.unions: dict[str, str] = {}
self.accept_null_types = []
self.docmap = {}
@@ -918,6 +1111,7 @@ def visit_end(self) -> None:
self.target["enum"] += generate_content_from_dict(self.enums)
self.target["alternate"] += generate_content_from_dict(self.alternates)
self.target["struct"] += generate_content_from_dict(self.structs)
+ self.target["union"] += generate_content_from_dict(self.unions)
def visit_object_type(
self,
@@ -929,11 +1123,11 @@ def visit_object_type(
members: List[QAPISchemaObjectTypeMember],
branches: Optional[QAPISchemaBranches],
) -> None:
- # Do not handle anything besides struct.
+ # Do not handle anything besides struct and unions.
if (
name == self.schema.the_empty_object_type.name
or not isinstance(name, str)
- or info.defn_meta not in ["struct"]
+ or info.defn_meta not in ["struct", "union"]
):
return
@@ -964,6 +1158,14 @@ def visit_object_type(
self, name, info, ifcond, features, base, members, branches
)
)
+ else:
+ assert name not in self.unions
+ self.unions[name] = qapi_to_golang_struct(
+ self, name, info, ifcond, features, base, members, branches
+ )
+ self.unions[name] += qapi_to_golang_methods_union(
+ self, name, base, branches
+ )
def visit_alternate_type(
self,
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 6/8] qapi: golang: Generate event type
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
` (4 preceding siblings ...)
2025-01-10 10:49 ` [PATCH v3 5/8] qapi: golang: Generate union type Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-10 10:49 ` [PATCH v3 7/8] qapi: golang: Generate command type Victor Toso
` (3 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
This patch handles QAPI event types and generates data structures in
Go that handles it.
1. Naming: Every event type has an Event suffix.
2. Timestamp: Every event has a MessageTimestamp field with a
reference to the Timestamp struct (not included in the QAPI spec
but defined in docs/interop/qmp-spec.rst)
3. Every event implements the Event interface.
Example:
qapi:
| ##
| # @MEMORY_DEVICE_SIZE_CHANGE:
| #
| # Emitted when the size of a memory device changes. Only emitted for
| # memory devices that can actually change the size (e.g., virtio-mem
| # due to guest action).
| #
| # @id: device's ID
| #
| # @size: the new size of memory that the device provides
| #
| # @qom-path: path to the device object in the QOM tree (since 6.2)
| #
| # .. note:: This event is rate-limited.
| #
| # Since: 5.1
| #
| # .. qmp-example::
| #
| # <- { "event": "MEMORY_DEVICE_SIZE_CHANGE",
| # "data": { "id": "vm0", "size": 1073741824,
| # "qom-path": "/machine/unattached/device[2]" },
| # "timestamp": { "seconds": 1588168529, "microseconds": 201316 } }
| ##
| { 'event': 'MEMORY_DEVICE_SIZE_CHANGE',
| 'data': { '*id': 'str', 'size': 'size', 'qom-path' : 'str'} }
go:
| // Emitted when the size of a memory device changes. Only emitted for
| // memory devices that can actually change the size (e.g., virtio-mem
| // due to guest action).
| //
| // .. note:: This event is rate-limited.
| //
| // Since: 5.1
| //
| // .. qmp-example:: <- { "event": "MEMORY_DEVICE_SIZE_CHANGE",
| // "data": { "id": "vm0", "size": 1073741824, "qom-path":
| // "/machine/unattached/device[2]" }, "timestamp": { "seconds":
| // 1588168529, "microseconds": 201316 } }
| type MemoryDeviceSizeChangeEvent struct {
| MessageTimestamp Timestamp `json:"-"`
| // device's ID
| Id *string `json:"id,omitempty"`
| // the new size of memory that the device provides
| Size uint64 `json:"size"`
| // path to the device object in the QOM tree (since 6.2)
| QomPath string `json:"qom-path"`
| }
|
| func (s MemoryDeviceSizeChangeEvent) MarshalJSON() ([]byte, error) {
| ...
| }
|
| func (s *MemoryDeviceSizeChangeEvent) UnmarshalJSON(data []byte) error {
| ...
| }
usage:
| input := `{"event":"MEMORY_DEVICE_SIZE_CHANGE",` +
| `"timestamp":{"seconds":1588168529,"microseconds":201316},` +
| `"data":{"id":"vm0","size":1073741824,"qom-path":"/machine/unattached/device[2]"}}`
|
| // Straight forward if you know the event type
| {
| mdsc := MemoryDeviceSizeChangeEvent{}
| err := json.Unmarshal([]byte(input), &mdsc)
| if err != nil {
| panic(err)
| }
| // mdsc.QomPath == "/machine/unattached/device[2]"
| }
|
| // Generic way, using Event interface and helper function
| if event, err := GetEventType(input); err != nil {
| // handle bad data or unknown event
| }
|
| if err := json.Unmarshal(input, event); err != nil {
| // handle bad data or unknown event fields
| }
|
| if mdsc, ok := event.(*MemoryDeviceSizeChangeEvent); ok {
| // mdsc.QomPath == "/machine/unattached/device[2]"
| }
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
scripts/qapi/golang.py | 143 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 139 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index 330891ede9..6a8f5cf230 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -238,6 +238,73 @@
}}
"""
+TEMPLATE_EVENT = """
+type Timestamp struct {{
+ Seconds int64 `json:"seconds"`
+ Microseconds int64 `json:"microseconds"`
+}}
+
+type Event interface {{
+ json.Marshaler
+ json.Unmarshaler
+}}
+
+func marshalEvent(obj interface{{}}, name string, ts Timestamp) ([]byte, error) {{
+ m := make(map[string]any)
+ m["event"] = name
+ m["timestamp"] = ts
+ if bytes, err := json.Marshal(obj); err != nil {{
+ return []byte{{}}, err
+ }} else if len(bytes) > 2 {{
+ m["data"] = obj
+ }}
+ return json.Marshal(m)
+}}
+
+func GetEventType(data []byte) (Event, error) {{
+ tmp := struct {{
+ Name string `json:"event"`
+ }}{{}}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {{
+ return nil, fmt.Errorf("Failed to unmarshal: %s", string(data))
+ }}
+
+ switch tmp.Name {{{cases}
+ default:
+ return nil, fmt.Errorf("Event %s not match to any type", tmp.Name)
+ }}
+}}
+"""
+
+TEMPLATE_EVENT_METHODS = """
+func (s {type_name}) MarshalJSON() ([]byte, error) {{
+ type Alias {type_name}
+ return marshalEvent(Alias(s), "{name}", s.MessageTimestamp)
+}}
+
+func (s *{type_name}) UnmarshalJSON(data []byte) error {{
+ type Alias {type_name}
+ tmp := struct {{
+ Name string `json:"event"`
+ Time Timestamp `json:"timestamp"`
+ Data Alias `json:"data"`
+ }}{{}}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {{
+ return fmt.Errorf("Failed to unmarshal: %s", string(data))
+ }}
+
+ if !strings.EqualFold(tmp.Name, "{name}") {{
+ return fmt.Errorf("Event type does not match with %s", tmp.Name)
+ }}
+
+ *s = {type_name}(tmp.Data)
+ s.MessageTimestamp = tmp.Time
+ return nil
+}}
+"""
+
# Takes the documentation object of a specific type and returns
# that type's documentation and its member's docs.
@@ -300,7 +367,7 @@ def qapi_to_field_name_enum(name: str) -> str:
return name.title().replace("-", "")
-def qapi_to_go_type_name(name: str) -> str:
+def qapi_to_go_type_name(name: str, meta: Optional[str] = None) -> str:
# We want to keep CamelCase for Golang types. We want to avoid removing
# already set CameCase names while fixing uppercase ones, eg:
# 1) q_obj_SocketAddress_base -> SocketAddressBase
@@ -318,6 +385,12 @@ def qapi_to_go_type_name(name: str) -> str:
name += "".join(word.title() for word in words[1:])
+ # Handle specific meta suffix
+ types = ["event"]
+ if meta in types:
+ name = name[:-3] if name.endswith("Arg") else name
+ name += meta.title().replace(" ", "")
+
return name
@@ -773,6 +846,16 @@ def qapi_to_golang_struct(
if not doc_enabled:
type_doc = ""
+ if info.defn_meta == "event":
+ fields.insert(
+ 0,
+ {
+ "name": "MessageTimestamp",
+ "type": "Timestamp",
+ "tag": """`json:"-"`""",
+ },
+ )
+
if members:
for member in members:
field_doc = docfields.get(member.name, "") if doc_enabled else ""
@@ -827,7 +910,8 @@ def qapi_to_golang_struct(
fields.append(field)
with_nullable = True if nullable else with_nullable
- type_name = qapi_to_go_type_name(name)
+ type_name = qapi_to_go_type_name(name, info.defn_meta)
+
content = string_to_code(
generate_struct_type(
type_name, type_doc=type_doc, args=fields, indent=indent
@@ -1005,6 +1089,21 @@ def generate_template_alternate(
return "\n" + content
+def generate_template_event(events: dict[str, Tuple[str, str]]) -> str:
+ content = ""
+ cases = ""
+ for name in sorted(events):
+ type_name, gocode = events[name]
+ content += gocode
+ cases += f"""
+ case "{name}":
+ return &{type_name}{{}}, nil
+"""
+
+ content += string_to_code(TEMPLATE_EVENT.format(cases=cases))
+ return content
+
+
def generate_content_from_dict(data: dict[str, str]) -> str:
content = ""
@@ -1040,12 +1139,14 @@ def __init__(self, _: str):
types = (
"alternate",
"enum",
+ "event",
"helper",
"struct",
"union",
)
self.target = dict.fromkeys(types, "")
self.schema: QAPISchema
+ self.events: dict[str, Tuple[str, str]] = {}
self.golang_package_name = "qapi"
self.enums: dict[str, str] = {}
self.alternates: dict[str, str] = {}
@@ -1084,7 +1185,7 @@ def visit_begin(self, schema: QAPISchema) -> None:
imports += """
import "encoding/json"
"""
- elif target == "helper":
+ elif target == "helper" or target == "event":
imports += """
import (
"encoding/json"
@@ -1112,6 +1213,7 @@ def visit_end(self) -> None:
self.target["alternate"] += generate_content_from_dict(self.alternates)
self.target["struct"] += generate_content_from_dict(self.structs)
self.target["union"] += generate_content_from_dict(self.unions)
+ self.target["event"] += generate_template_event(self.events)
def visit_object_type(
self,
@@ -1267,7 +1369,40 @@ def visit_event(
arg_type: Optional[QAPISchemaObjectType],
boxed: bool,
) -> None:
- pass
+ assert name == info.defn_name
+ assert name not in self.events
+ type_name = qapi_to_go_type_name(name, info.defn_meta)
+
+ if isinstance(arg_type, QAPISchemaObjectType):
+ content = string_to_code(
+ qapi_to_golang_struct(
+ self,
+ name,
+ info,
+ arg_type.ifcond,
+ arg_type.features,
+ arg_type.base,
+ arg_type.members,
+ arg_type.branches,
+ )
+ )
+ else:
+ args: List[dict[str:str]] = []
+ args.append(
+ {
+ "name": "MessageTimestamp",
+ "type": "Timestamp",
+ "tag": """`json:"-"`""",
+ }
+ )
+ content = string_to_code(
+ generate_struct_type(type_name, args=args)
+ )
+
+ content += string_to_code(
+ TEMPLATE_EVENT_METHODS.format(name=name, type_name=type_name)
+ )
+ self.events[name] = (type_name, content)
def write(self, output_dir: str) -> None:
for module_name, content in self.target.items():
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 7/8] qapi: golang: Generate command type
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
` (5 preceding siblings ...)
2025-01-10 10:49 ` [PATCH v3 6/8] qapi: golang: Generate event type Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-10 10:49 ` [PATCH v3 8/8] docs: add notes on Golang code generator Victor Toso
` (2 subsequent siblings)
9 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
This patch handles QAPI command types and generates data structures in
Go that handles it.
This patch also generates the Command's return type. Each command has a
specific type for its expected return value.
1. Command:
i. Naming: Every command type has a Command suffix.
ii. Id: Every command has a MessageId field of string type.
iii. Every command implements the Command interface.
iv. The Command interface includes GetReturnType() which returns the
expected return type for that Command
2. CommandReturn:
i. Naming: Every command return type has a CommandReturn suffix
ii. Id: Every command return has a MessageId field of string type.
iii. Every command return implements the CommandReturn interface.
* Example:
qapi:
| ##
| # @set_password:
| #
| # Set the password of a remote display server.
| #
| # Errors:
| # - If Spice is not enabled, DeviceNotFound
| #
| # Since: 0.14
| #
| # .. qmp-example::
| #
| # -> { "execute": "set_password", "arguments": { "protocol": "vnc",
| # "password": "secret" } }
| # <- { "return": {} }
| ##
| { 'command': 'set_password', 'boxed': true, 'data': 'SetPasswordOptions' }
go:
| // Set the password of a remote display server.
| //
| // Errors: - If Spice is not enabled, DeviceNotFound
| //
| // Since: 0.14
| //
| // .. qmp-example:: -> { "execute": "set_password", "arguments": {
| // "protocol": "vnc", "password": "secret" }
| // } <- { "return": {} }
| type SetPasswordCommand struct {
| SetPasswordOptions
| MessageId string `json:"-"`
| }
|
| type SetPasswordCommandReturn struct {
| MessageId string `json:"id,omitempty"`
| Error *QAPIError `json:"error,omitempty"`
| }
usage:
| input := `{"execute":"set_password",` +
| `"arguments":{"protocol":"vnc",` +
| `"password":"secret"}}`
|
| // Straight forward if you know the event type
| {
| c := SetPasswordCommand{}
| err := json.Unmarshal([]byte(input), &c)
| if err != nil {
| panic(err)
| }
| // c.Password == "secret"
| }
|
| // Generic way, using Command interface and helper function
| if cmd, err := GetCommandType(input); err != nil {
| // handle bad data or unknown event
| }
|
| if err := json.Unmarshal(input, cmd); err != nil {
| // handle bad data or unknown event fields
| }
|
| if c, ok := cmd.(*SetPasswordCommand); ok {
| // c.Password == "secret"
| }
Signed-off-by: Victor Toso <victortoso@redhat.com>
---
scripts/qapi/golang.py | 233 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 231 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index 6a8f5cf230..085cdd89f6 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -43,6 +43,15 @@
"""
TEMPLATE_HELPER = """
+type QAPIError struct {
+ Class string `json:"class"`
+ Description string `json:"desc"`
+}
+
+func (err *QAPIError) Error() string {
+ return err.Description
+}
+
// Creates a decoder that errors on unknown Fields
// Returns nil if successfully decoded @from payload to @into type
// Returns error if failed to decode @from payload to @into type
@@ -305,6 +314,111 @@
}}
"""
+TEMPLATE_COMMAND_METHODS = """
+func (s {type_name}) MarshalJSON() ([]byte, error) {{
+ type Alias {type_name}
+ return marshalCommand(Alias(s), "{name}", s.MessageId)
+}}
+
+func (s *{type_name}) UnmarshalJSON(data []byte) error {{
+ type Alias {type_name}
+ tmp := struct {{
+ MessageId string `json:"id,omitempty"`
+ Name string `json:"execute"`
+ Args Alias `json:"arguments"`
+ }}{{}}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {{
+ return fmt.Errorf("Failed to unmarshal: %s", string(data))
+ }}
+
+ if !strings.EqualFold(tmp.Name, "{name}") {{
+ return fmt.Errorf("Command type does not match with %s", tmp.Name)
+ }}
+
+ *s = {type_name}(tmp.Args)
+ s.MessageId = tmp.MessageId
+ return nil
+}}
+
+func (s *{type_name}) GetReturnType() CommandReturn {{
+ return &{cmd_ret_type_name}{{}}
+}}
+"""
+
+TEMPLATE_COMMAND = """
+type Command interface {{
+ json.Marshaler
+ json.Unmarshaler
+ GetReturnType() CommandReturn
+}}
+
+func marshalCommand(obj interface{{}}, name, id string) ([]byte, error) {{
+ m := make(map[string]any)
+ m["execute"] = name
+ if len(id) > 0 {{
+ m["id"] = id
+ }}
+ if bytes, err := json.Marshal(obj); err != nil {{
+ return []byte{{}}, err
+ }} else if len(bytes) > 2 {{
+ m["arguments"] = obj
+ }}
+ return json.Marshal(m)
+}}
+
+func GetCommandType(data []byte) (Command, error) {{
+ tmp := struct {{
+ Name string `json:"execute"`
+ }}{{}}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {{
+ return nil, fmt.Errorf("Failed to decode command: %s", string(data))
+ }}
+
+ switch tmp.Name {{{cases}
+ }}
+ return nil, errors.New("Failed to recognize command")
+}}
+"""
+
+TEMPLATE_COMMAND_RETURN = """
+type CommandReturn interface {
+ json.Marshaler
+}
+
+func marshalCommandReturn(result, qerror any, id string) ([]byte, error) {
+ m := make(map[string]any)
+ if len(id) > 0 {
+ m["id"] = id
+ }
+ if qerror != nil && qerror.(*QAPIError) != nil {
+ m["error"] = qerror
+ } else if result != nil {
+ m["return"] = result
+ } else {
+ m["return"] = struct{}{}
+ }
+ return json.Marshal(m)
+}
+"""
+
+TEMPLATE_COMMAND_RETURN_METHODS = """
+func (r {cmd_ret_type_name}) MarshalJSON() ([]byte, error) {{
+ return marshalCommandReturn({cmd_ret_field}, r.Error, r.MessageId)
+}}
+"""
+
+TEMPLATE_COMMAND_RETURN_MARSHAL_EMPTY = """
+func (r {cmd_ret_ype_name}) MarshalJSON() ([]byte, error) {{
+ if r.Error != nil {{
+ type Alias {cmd_ret_type_name}
+ return json.Marshal(Alias(r))
+ }}
+ return []byte(`{{"return":{{}}}}`), nil
+}}
+"""
+
# Takes the documentation object of a specific type and returns
# that type's documentation and its member's docs.
@@ -386,7 +500,7 @@ def qapi_to_go_type_name(name: str, meta: Optional[str] = None) -> str:
name += "".join(word.title() for word in words[1:])
# Handle specific meta suffix
- types = ["event"]
+ types = ["event", "command", "command return"]
if meta in types:
name = name[:-3] if name.endswith("Arg") else name
name += meta.title().replace(" ", "")
@@ -855,6 +969,10 @@ def qapi_to_golang_struct(
"tag": """`json:"-"`""",
},
)
+ elif info.defn_meta == "command":
+ fields.insert(
+ 0, {"name": "MessageId", "type": "string", "tag": """`json:"-"`"""}
+ )
if members:
for member in members:
@@ -1089,6 +1207,21 @@ def generate_template_alternate(
return "\n" + content
+def generate_template_command(commands: dict[str, Tuple[str, str]]) -> str:
+ cases = ""
+ content = ""
+ for name in sorted(commands):
+ type_name, gocode = commands[name]
+ content += gocode
+ cases += f"""
+ case "{name}":
+ return &{type_name}{{}}, nil
+"""
+ content += string_to_code(TEMPLATE_COMMAND.format(cases=cases))
+ content += string_to_code(TEMPLATE_COMMAND_RETURN)
+ return content
+
+
def generate_template_event(events: dict[str, Tuple[str, str]]) -> str:
content = ""
cases = ""
@@ -1138,6 +1271,7 @@ def __init__(self, _: str):
super().__init__()
types = (
"alternate",
+ "command",
"enum",
"event",
"helper",
@@ -1147,6 +1281,7 @@ def __init__(self, _: str):
self.target = dict.fromkeys(types, "")
self.schema: QAPISchema
self.events: dict[str, Tuple[str, str]] = {}
+ self.commands: dict[str, Tuple[str, str]] = {}
self.golang_package_name = "qapi"
self.enums: dict[str, str] = {}
self.alternates: dict[str, str] = {}
@@ -1192,6 +1327,15 @@ def visit_begin(self, schema: QAPISchema) -> None:
"fmt"
"strings"
)
+"""
+ elif target == "command":
+ imports += """
+import (
+ "encoding/json"
+ "errors"
+ "fmt"
+ "strings"
+)
"""
else:
imports += """
@@ -1214,6 +1358,7 @@ def visit_end(self) -> None:
self.target["struct"] += generate_content_from_dict(self.structs)
self.target["union"] += generate_content_from_dict(self.unions)
self.target["event"] += generate_template_event(self.events)
+ self.target["command"] += generate_template_command(self.commands)
def visit_object_type(
self,
@@ -1358,7 +1503,91 @@ def visit_command(
allow_preconfig: bool,
coroutine: bool,
) -> None:
- pass
+ assert name == info.defn_name
+ assert name not in self.commands
+
+ type_name = qapi_to_go_type_name(name, info.defn_meta)
+
+ doc = self.docmap.get(name, None)
+ type_doc, _ = qapi_to_golang_struct_docs(doc)
+
+ cmd_ret_type_name = qapi_to_go_type_name(name, "command return")
+ cmd_ret_field = "nil"
+ retargs: List[dict[str:str]] = [
+ {
+ "name": "MessageId",
+ "type": "string",
+ "tag": """`json:"id,omitempty"`""",
+ },
+ {
+ "name": "Error",
+ "type": "*QAPIError",
+ "tag": """`json:"error,omitempty"`""",
+ },
+ ]
+ if ret_type:
+ cmd_ret_field = "r.Result"
+ ret_type_name = qapi_schema_type_to_go_type(ret_type.name)
+ isptr = "*" if ret_type_name[0] not in "*[" else ""
+ retargs.append(
+ {
+ "name": "Result",
+ "type": f"{isptr}{ret_type_name}",
+ "tag": """`json:"return"`""",
+ }
+ )
+
+ content = ""
+ if boxed or not arg_type or not qapi_name_is_object(arg_type.name):
+ args: List[dict[str:str]] = []
+ if arg_type:
+ args.append(
+ {
+ "name": f"{arg_type.name}",
+ }
+ )
+ args.append(
+ {
+ "name": "MessageId",
+ "type": "string",
+ "tag": """`json:"-"`""",
+ }
+ )
+ content += string_to_code(
+ generate_struct_type(type_name, type_doc=type_doc, args=args)
+ )
+ else:
+ assert isinstance(arg_type, QAPISchemaObjectType)
+ content += string_to_code(
+ qapi_to_golang_struct(
+ self,
+ name,
+ arg_type.info,
+ arg_type.ifcond,
+ arg_type.features,
+ arg_type.base,
+ arg_type.members,
+ arg_type.branches,
+ )
+ )
+
+ content += string_to_code(
+ TEMPLATE_COMMAND_METHODS.format(
+ name=name,
+ type_name=type_name,
+ cmd_ret_type_name=cmd_ret_type_name,
+ )
+ )
+ content += string_to_code(
+ generate_struct_type(cmd_ret_type_name, args=retargs)
+ )
+ content += string_to_code(
+ TEMPLATE_COMMAND_RETURN_METHODS.format(
+ cmd_ret_type_name=cmd_ret_type_name,
+ cmd_ret_field=cmd_ret_field,
+ )
+ )
+ self.commands[name] = (type_name, content)
def visit_event(
self,
--
2.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v3 8/8] docs: add notes on Golang code generator
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
` (6 preceding siblings ...)
2025-01-10 10:49 ` [PATCH v3 7/8] qapi: golang: Generate command type Victor Toso
@ 2025-01-10 10:49 ` Victor Toso
2025-01-13 12:52 ` [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Markus Armbruster
2025-01-16 21:59 ` Daniel P. Berrangé
9 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-10 10:49 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, John Snow, Daniel P . Berrangé,
Andrea Bolognani
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/index-build.rst | 1 +
docs/devel/qapi-golang-code-gen.rst | 548 ++++++++++++++++++++++++++++
2 files changed, 549 insertions(+)
create mode 100644 docs/devel/qapi-golang-code-gen.rst
diff --git a/docs/devel/index-build.rst b/docs/devel/index-build.rst
index 0023953be3..68f8936cc7 100644
--- a/docs/devel/index-build.rst
+++ b/docs/devel/index-build.rst
@@ -11,4 +11,5 @@ some of the basics if you are adding new files and targets to the build.
kconfig
docs
qapi-code-gen
+ qapi-golang-code-gen
control-flow-integrity
diff --git a/docs/devel/qapi-golang-code-gen.rst b/docs/devel/qapi-golang-code-gen.rst
new file mode 100644
index 0000000000..05b7bb4e8d
--- /dev/null
+++ b/docs/devel/qapi-golang-code-gen.rst
@@ -0,0 +1,548 @@
+==========================
+QAPI Golang code generator
+==========================
+
+..
+ Copyright (C) 2025 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.
+
+
+QAPI Documentation
+==================
+
+The documentation included in QAPI schema such as type and type's
+fields information, comments, examples and more, they are converted
+and embed in the Go generated source code. Metadata information that
+might not be relevant to developers are excluded (e.g: TODOs)
+
+
+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.
+
+::
+
+ { 'enum': 'HostMemPolicy',
+ 'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
+
+.. code-block:: go
+
+ // Host memory policy types
+ //
+ // Since: 2.1
+ type HostMemPolicy string
+
+ const (
+ // restore default policy, remove any nondefault policy
+ HostMemPolicyDefault HostMemPolicy = "default"
+ // set the preferred host nodes for allocation
+ HostMemPolicyPreferred HostMemPolicy = "preferred"
+ // a strict policy that restricts memory allocation to the host nodes specified
+ HostMemPolicyBind HostMemPolicy = "bind"
+ // memory allocations are interleaved across the set of host nodes specified
+ 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.
+
+::
+
+ { 'struct': 'BlockExportOptionsNbdBase',
+ 'data': { '*name': 'str', '*description': 'str' } }
+
+ { 'struct': 'BlockExportOptionsNbd',
+ 'base': 'BlockExportOptionsNbdBase',
+ 'data': { '*bitmaps': ['BlockDirtyBitmapOrStr'],
+ '*allocation-depth': 'bool' } }
+
+.. code-block:: go
+
+ // An NBD block export (distinct options used in the NBD branch of
+ // block-export-add).
+ //
+ // Since: 5.2
+ type BlockExportOptionsNbd struct {
+ // Export name. If unspecified, the @device parameter is used as
+ // the export name. (Since 2.12)
+ Name *string `json:"name,omitempty"`
+ // Free-form description of the export, up to 4096 bytes. (Since
+ // 5.0)
+ Description *string `json:"description,omitempty"`
+ // Also export each of the named dirty bitmaps reachable from
+ // @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT
+ // with the metadata context name "qemu:dirty-bitmap:BITMAP" to
+ // inspect each bitmap. Since 7.1 bitmap may be specified by
+ // node/name pair.
+ Bitmaps []BlockDirtyBitmapOrStr `json:"bitmaps,omitempty"`
+ // Also export the allocation depth map for @device, so the NBD
+ // client can use NBD_OPT_SET_META_CONTEXT with the metadata
+ // context name "qemu:allocation-depth" to inspect allocation
+ // details. (since 5.2)
+ AllocationDepth *bool `json:"allocation-depth,omitempty"`
+ }
+
+
+Union
+-----
+
+Unions in QAPI are bounded 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 have a complex type implemented for all possible
+branches of the Enum. Receiving a enum value of a empty branch is valid.
+
+The generated Go struct will then define a field for each
+Enum value. The type for Enum values of empty branch is bool. Only one
+field can be set at time.
+
+::
+
+ { 'union': 'ImageInfoSpecificQCow2Encryption',
+ 'base': 'ImageInfoSpecificQCow2EncryptionBase',
+ 'discriminator': 'format',
+ 'data': { 'luks': 'QCryptoBlockInfoLUKS' } }
+
+ { 'struct': 'ImageInfoSpecificQCow2EncryptionBase',
+ 'data': { 'format': 'BlockdevQcow2EncryptionFormat'}}
+
+ { 'enum': 'BlockdevQcow2EncryptionFormat',
+ 'data': [ 'aes', 'luks' ] }
+
+.. code-block:: go
+
+ type ImageInfoSpecificQCow2Encryption struct {
+ // Variants fields
+ Luks *QCryptoBlockInfoLUKS `json:"-"`
+ // Empty branched enum fields
+ Aes bool `json:"-"`
+ }
+
+ func (s ImageInfoSpecificQCow2Encryption) MarshalJSON() ([]byte, error) {
+ // ...
+ // Logic for branched Enum
+ if s.Luks != nil && err == nil {
+ if len(bytes) != 0 {
+ err = errors.New(`multiple variant fields set`)
+ } else if err = unwrapToMap(m, s.Luks); err == nil {
+ m["format"] = BlockdevQcow2EncryptionFormatLuks
+ bytes, err = json.Marshal(m)
+ }
+ }
+
+ // Logic for unbranched Enum
+ if s.Aes && err == nil {
+ if len(bytes) != 0 {
+ err = errors.New(`multiple variant fields set`)
+ } else {
+ m["format"] = BlockdevQcow2EncryptionFormatAes
+ bytes, err = json.Marshal(m)
+ }
+ }
+
+ // ...
+ // Handle errors
+ }
+
+
+ func (s *ImageInfoSpecificQCow2Encryption) UnmarshalJSON(data []byte) error {
+ // ...
+
+ switch tmp.Format {
+ case BlockdevQcow2EncryptionFormatLuks:
+ s.Luks = new(QCryptoBlockInfoLUKS)
+ if err := json.Unmarshal(data, s.Luks); err != nil {
+ s.Luks = nil
+ return err
+ }
+ case BlockdevQcow2EncryptionFormatAes:
+ s.Aes = true
+
+ default:
+ return fmt.Errorf("error: unmarshal: ImageInfoSpecificQCow2Encryption: received unrecognized value: '%s'",
+ tmp.Format)
+ }
+ return nil
+ }
+
+
+Alternate
+---------
+
+Like Unions, alternates can have 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 as it will 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 here is that we do
+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 structs know
+ if a given Alternate type should be considered Absent (not set) or
+ any other possible Value, including JSON Null.
+
+::
+
+ { 'alternate': 'BlockdevRefOrNull',
+ 'data': { 'definition': 'BlockdevOptions',
+ 'reference': 'str',
+ 'null': 'null' } }
+
+.. code-block:: go
+
+ // Reference to a block device.
+ //
+ // Since: 2.9
+ type BlockdevRefOrNull struct {
+ // defines a new block device inline
+ Definition *BlockdevOptions
+ // references the ID of an existing block device. An empty string
+ // means that no block device should be referenced. Deprecated;
+ // use null instead.
+ Reference *string
+ // No block device should be referenced (since 2.10)
+ 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
+-----
+
+Each event is mapped to its own struct with the additional
+MessageTimestamp field, for the over-the-wire 'timestamp' value.
+
+The Event interface includes json.Marshaler and json.Unmarshaler which
+requires every Event to implement Marshal and Unmarshal functions.
+
+There is an helper function called GetEventType() that can return an
+Event based on the json message.
+
+::
+
+ { 'event': 'SHUTDOWN',
+ 'data': { 'guest': 'bool',
+ 'reason': 'ShutdownCause' } }
+
+.. code-block:: go
+
+ type Event interface {
+ json.Marshaler
+ json.Unmarshaler
+ }
+
+ // Emitted when the virtual machine has shut down, indicating that
+ // qemu is about to exit.
+ //
+ // .. note:: If the command-line option "-no-shutdown" has been
+ // specified, qemu will not exit, and a STOP event will eventually
+ // follow the SHUTDOWN event.
+ //
+ // Since: 0.12
+ //
+ // .. qmp-example:: <- { "event": "SHUTDOWN", "data": {
+ // "guest": true, "reason": "guest-shutdown" }, "timestamp": {
+ // "seconds": 1267040730, "microseconds": 682951 } }
+ type ShutdownEvent struct {
+ MessageTimestamp Timestamp `json:"-"`
+ // If true, the shutdown was triggered by a guest request (such as
+ // a guest-initiated ACPI shutdown request or other hardware-
+ // specific action) rather than a host request (such as sending
+ // qemu a SIGINT). (since 2.10)
+ Guest bool `json:"guest"`
+ // The @ShutdownCause which resulted in the SHUTDOWN. (since 4.0)
+ Reason ShutdownCause `json:"reason"`
+ }
+
+ func (s ShutdownEvent) MarshalJSON() ([]byte, error) {
+ type Alias ShutdownEvent
+ return marshalEvent(Alias(s), "SHUTDOWN", s.MessageTimestamp)
+ }
+
+ func (s *ShutdownEvent) UnmarshalJSON(data []byte) error {
+ type Alias ShutdownEvent
+ tmp := struct {
+ Name string `json:"event"`
+ Time Timestamp `json:"timestamp"`
+ Data Alias `json:"data"`
+ }{}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {
+ return fmt.Errorf("Failed to unmarshal: %s", string(data))
+ }
+
+ if !strings.EqualFold(tmp.Name, "SHUTDOWN") {
+ return fmt.Errorf("Event type does not match with %s", tmp.Name)
+ }
+
+ *s = ShutdownEvent(tmp.Data)
+ s.MessageTimestamp = tmp.Time
+ return nil
+ }
+
+
+Command
+-------
+
+Each commands is 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; it
+also includes json.Marshaler and json.Unmarshaler, requiring every
+Command to implement Marshal and Unmarshal methods.
+
+Marshaling and Unmarshaling happens over the Command interface, so
+users should use the MarshalCommand() and UnmarshalCommand() methods.
+
+There is a helper function called GetCommandType() that returns the
+Command interface (pointer to the kkj
+
+::
+
+ { 'command': 'set_password',
+ 'boxed': true,
+ 'data': 'SetPasswordOptions' }
+
+ { 'union': 'SetPasswordOptions',
+ 'base': { 'protocol': 'DisplayProtocol',
+ 'password': 'str',
+ '*connected': 'SetPasswordAction' },
+ 'discriminator': 'protocol',
+ 'data': { 'vnc': 'SetPasswordOptionsVnc' } }
+
+.. code-block:: go
+
+ type Command interface {
+ json.Marshaler
+ json.Unmarshaler
+ GetReturnType() CommandReturn
+ }
+
+ type CommandReturn interface {
+ json.Marshaler
+ }
+
+ // Set the password of a remote display server.
+ // Errors: - If Spice is not enabled, DeviceNotFound
+ //
+ // Since: 0.14
+ //
+ // .. qmp-example:: -> { "execute": "set_password", "arguments": {
+ // "protocol": "vnc", "password": "secret" }
+ // } <- { "return": {} }
+ type SetPasswordCommand struct {
+ SetPasswordOptions
+ MessageId string `json:"-"`
+ }
+
+ func (s SetPasswordCommand) MarshalJSON() ([]byte, error) {
+ type Alias SetPasswordCommand
+ return marshalCommand(Alias(s), "set_password", s.MessageId)
+ }
+
+ func (s *SetPasswordCommand) UnmarshalJSON(data []byte) error {
+ type Alias SetPasswordCommand
+ tmp := struct {
+ MessageId string `json:"id,omitempty"`
+ Name string `json:"execute"`
+ Args Alias `json:"arguments"`
+ }{}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {
+ return fmt.Errorf("Failed to unmarshal: %s", string(data))
+ }
+
+ if !strings.EqualFold(tmp.Name, "set_password") {
+ return fmt.Errorf("Command type does not match with %s", tmp.Name)
+ }
+
+ *s = SetPasswordCommand(tmp.Args)
+ s.MessageId = tmp.MessageId
+ return nil
+ }
+
+ func (s *SetPasswordCommand) GetReturnType() CommandReturn {
+ return &SetPasswordCommandReturn{}
+ }
+
+ type SetPasswordCommandReturn struct {
+ MessageId string `json:"id,omitempty"`
+ Error *QAPIError `json:"error,omitempty"`
+ }
+
+ func (r SetPasswordCommandReturn) MarshalJSON() ([]byte, error) {
+ return marshalCommandReturn(nil, r.Error, r.MessageId)
+ }
+
+Now an example of a command without boxed type.
+
+::
+
+ { 'command': 'set_link',
+ 'data': {'name': 'str', 'up': 'bool'} }
+
+.. code-block:: go
+
+ // Sets the link status of a virtual network adapter.
+ //
+ // Errors: - If @name is not a valid network device, DeviceNotFound
+ //
+ // Since: 0.14
+ //
+ // .. note:: Not all network adapters support setting link status.
+ // This command will succeed even if the network adapter does not
+ // support link status notification. .. qmp-example:: -> {
+ // "execute": "set_link", "arguments": { "name": "e1000.0", "up":
+ // false } } <- { "return": {} }
+ type SetLinkCommand struct {
+ MessageId string `json:"-"`
+ // the device name of the virtual network adapter
+ Name string `json:"name"`
+ // true to set the link status to be up
+ Up bool `json:"up"`
+ }
+
+ func (s SetLinkCommand) MarshalJSON() ([]byte, error) {
+ type Alias SetLinkCommand
+ return marshalCommand(Alias(s), "set_link", s.MessageId)
+ }
+
+ func (s *SetLinkCommand) UnmarshalJSON(data []byte) error {
+ type Alias SetLinkCommand
+ tmp := struct {
+ MessageId string `json:"id,omitempty"`
+ Name string `json:"execute"`
+ Args Alias `json:"arguments"`
+ }{}
+
+ if err := json.Unmarshal(data, &tmp); err != nil {
+ return fmt.Errorf("Failed to unmarshal: %s", string(data))
+ }
+
+ if !strings.EqualFold(tmp.Name, "set_link") {
+ return fmt.Errorf("Command type does not match with %s", tmp.Name)
+ }
+
+ *s = SetLinkCommand(tmp.Args)
+ s.MessageId = tmp.MessageId
+ return nil
+ }
+
+ func (s *SetLinkCommand) GetReturnType() CommandReturn {
+ return &SetLinkCommandReturn{}
+ }
+
+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.47.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
` (7 preceding siblings ...)
2025-01-10 10:49 ` [PATCH v3 8/8] docs: add notes on Golang code generator Victor Toso
@ 2025-01-13 12:52 ` Markus Armbruster
2025-01-14 9:44 ` Victor Toso
2025-01-17 10:32 ` Daniel P. Berrangé
2025-01-16 21:59 ` Daniel P. Berrangé
9 siblings, 2 replies; 23+ messages in thread
From: Markus Armbruster @ 2025-01-13 12:52 UTC (permalink / raw)
To: Victor Toso
Cc: qemu-devel, John Snow, Daniel P . Berrangé, Andrea Bolognani
Victor Toso <victortoso@redhat.com> writes:
> This patch series intent is to introduce a generator that produces a Go
> module for Go applications to interact over QMP with QEMU.
>
> The initial Goal is to have a Go module that works as intended and can
> be improved upon. I'd consider initial releases to be alpha while we
> work with utilities tools and libraries on top of this.
>
> The generated code should reside in a separated Git repository, similar
> to python-qemu-qmp.
>
> Applications should be able to consume this under qemu.org
> namespace (e.g: import "qemu.org/go/qemu"), see Daniel's suggestion:
> https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg07024.html
>
> This is the third iteration:
> v2: https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg04785.html
>
> I've pushed this series in my gitlab fork:
> https://gitlab.com/victortoso/qapi-go/
>
> The fork contains some tests, including tests that were generated from
> QAPI's own examples from another generator created for testing, if you
> are interested in it:
> https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04946.html
>
> I've generated the qapi-go module over each commit of this series, see:
> https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-patch
>
> I've also generated the qapi-go module over QEMU tags: v9.1.0, v9.2.0:
> https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-tags
>
> --
>
> Sorry that its been awhile between v2 and v3, I had to prioritize other
> things. I hope to get this back on track in 2025.
>
> Cheers,
> Victor
>
> * Changes:
>
> On generated go:
> - the output should be formatted as gofmt/goimports tools (Daniel)
>
> - Included QAPI's documentation too (Daniel), see:
> https://lists.gnu.org/archive/html/qemu-devel/2024-11/msg01621.html
>
> - Commands and Events should Marshal directly (Andrea)
>
> On python script:
> - rebased: now uses QAPISchemaBranches, QAPISchemaAlternatives
>
> - use textwrap as much as possible (Andrea)
>
> - lots of changes to make the output like gofmt does
>
> Victor Toso (8):
> qapi: golang: Generate enum type
> qapi: golang: Generate alternate types
> qapi: golang: Generate struct types
> qapi: golang: structs: Address nullable members
> qapi: golang: Generate union type
> qapi: golang: Generate event type
> qapi: golang: Generate command type
> docs: add notes on Golang code generator
>
> docs/devel/index-build.rst | 1 +
> docs/devel/qapi-golang-code-gen.rst | 548 +++++++++
> scripts/qapi/golang.py | 1645 +++++++++++++++++++++++++++
> scripts/qapi/main.py | 3 +
> 4 files changed, 2197 insertions(+)
> create mode 100644 docs/devel/qapi-golang-code-gen.rst
> create mode 100644 scripts/qapi/golang.py
This is series adds a backend that slots in cleanly, i.e. without any
changes to the core. That makes it as low-risk to merge as it gets.
I'd like an Acked-by for the generated Go from someone familiar the kind
of software that could use it.
The Go backend is a single golang.py, which generates:
* Types:
alternates.go enums.go structs.go unions.go
* Commands:
commands.go
* Events:
events.go
It doesn't generate visitors or introspection code.
Correct?
The existing C backend generates code for
* Types (types.py):
qapi-builtin-types.[ch]
qapi-types.[ch] qapi-types-*.[ch]
* Visitors (visit.py):
qapi-builtin-visit.h
qapi-visit.[ch] qapi-visit-*.[ch]
* Commands (commands.py):
qapi-init-commands.h
qapi-commands.[ch] qapi-commands-*.[ch]
* Events (events.py):
qapi-emit-events.h
qapi-events.[ch] qapi-events-*.[ch]
* Introspection (introspect.py):
qapi-introspect.h
The -* files are all one pair of files per module (the things pulled in
with include directives), if any. We do this to avoid "touch the QAPI
schema, recompile the world."
The generated Go is monolithic. No "recompile the world" problem with
Go?
golang.py is somewhat big. Whether splitting it up along the lines of
the C backend would improve things I can't say. No need to worry about
that now.
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-01-13 12:52 ` [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Markus Armbruster
@ 2025-01-14 9:44 ` Victor Toso
2025-01-17 10:32 ` Daniel P. Berrangé
1 sibling, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-14 9:44 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, John Snow, Daniel P . Berrangé, Andrea Bolognani
[-- Attachment #1: Type: text/plain, Size: 5336 bytes --]
Hi,
On Mon, Jan 13, 2025 at 01:52:25PM +0100, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
>
> > This patch series intent is to introduce a generator that produces a Go
> > module for Go applications to interact over QMP with QEMU.
> >
> > The initial Goal is to have a Go module that works as intended and can
> > be improved upon. I'd consider initial releases to be alpha while we
> > work with utilities tools and libraries on top of this.
> >
> > The generated code should reside in a separated Git repository, similar
> > to python-qemu-qmp.
> >
> > Applications should be able to consume this under qemu.org
> > namespace (e.g: import "qemu.org/go/qemu"), see Daniel's suggestion:
> > https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg07024.html
> >
> > This is the third iteration:
> > v2: https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg04785.html
> >
> > I've pushed this series in my gitlab fork:
> > https://gitlab.com/victortoso/qapi-go/
> >
> > The fork contains some tests, including tests that were generated from
> > QAPI's own examples from another generator created for testing, if you
> > are interested in it:
> > https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04946.html
> >
> > I've generated the qapi-go module over each commit of this series, see:
> > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-patch
> >
> > I've also generated the qapi-go module over QEMU tags: v9.1.0, v9.2.0:
> > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-tags
> >
> > --
> >
> > Sorry that its been awhile between v2 and v3, I had to prioritize other
> > things. I hope to get this back on track in 2025.
> >
> > Cheers,
> > Victor
> >
> > * Changes:
> >
> > On generated go:
> > - the output should be formatted as gofmt/goimports tools (Daniel)
> >
> > - Included QAPI's documentation too (Daniel), see:
> > https://lists.gnu.org/archive/html/qemu-devel/2024-11/msg01621.html
> >
> > - Commands and Events should Marshal directly (Andrea)
> >
> > On python script:
> > - rebased: now uses QAPISchemaBranches, QAPISchemaAlternatives
> >
> > - use textwrap as much as possible (Andrea)
> >
> > - lots of changes to make the output like gofmt does
> >
> > Victor Toso (8):
> > qapi: golang: Generate enum type
> > qapi: golang: Generate alternate types
> > qapi: golang: Generate struct types
> > qapi: golang: structs: Address nullable members
> > qapi: golang: Generate union type
> > qapi: golang: Generate event type
> > qapi: golang: Generate command type
> > docs: add notes on Golang code generator
> >
> > docs/devel/index-build.rst | 1 +
> > docs/devel/qapi-golang-code-gen.rst | 548 +++++++++
> > scripts/qapi/golang.py | 1645 +++++++++++++++++++++++++++
> > scripts/qapi/main.py | 3 +
> > 4 files changed, 2197 insertions(+)
> > create mode 100644 docs/devel/qapi-golang-code-gen.rst
> > create mode 100644 scripts/qapi/golang.py
>
> This is series adds a backend that slots in cleanly, i.e. without any
> changes to the core. That makes it as low-risk to merge as it gets.
>
> I'd like an Acked-by for the generated Go from someone familiar the kind
> of software that could use it.
>
> The Go backend is a single golang.py, which generates:
>
> * Types:
> alternates.go enums.go structs.go unions.go
>
> * Commands:
> commands.go
>
> * Events:
> events.go
>
> It doesn't generate visitors or introspection code.
>
> Correct?
Correct. I've actually thought about following what we did with
libvirt-go-module which appends _generated to the files that are
generated, meaning that we would also have files that are without
_generated in the name, with custoam/manual code related to that
file.
Either way, that does not change the Go module (does not break
API/ABI) so we can customize it if needed at a later time.
> The existing C backend generates code for
>
> * Types (types.py):
> qapi-builtin-types.[ch]
> qapi-types.[ch] qapi-types-*.[ch]
>
> * Visitors (visit.py):
>
> qapi-builtin-visit.h
> qapi-visit.[ch] qapi-visit-*.[ch]
>
> * Commands (commands.py):
>
> qapi-init-commands.h
> qapi-commands.[ch] qapi-commands-*.[ch]
>
> * Events (events.py):
>
> qapi-emit-events.h
> qapi-events.[ch] qapi-events-*.[ch]
>
> * Introspection (introspect.py):
>
> qapi-introspect.h
>
> The -* files are all one pair of files per module (the things pulled in
> with include directives), if any. We do this to avoid "touch the QAPI
> schema, recompile the world."
>
> The generated Go is monolithic. No "recompile the world" problem with
> Go?
From my experience, Go compiler only builds when the .go file
changes. If the QAPI changes and the generator outputs different
.go files, it'll recompile at the project using it.
> golang.py is somewhat big. Whether splitting it up along the lines of
> the C backend would improve things I can't say. No need to worry about
> that now.
Yes, I'm not super experienced with python. I'm all ears to any
suggestions on how to organize the source better. I'm happy to do
it as a follow-up.
Cheers,
Victor
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-01-13 12:52 ` [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Markus Armbruster
2025-01-14 9:44 ` Victor Toso
@ 2025-01-17 10:32 ` Daniel P. Berrangé
1 sibling, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2025-01-17 10:32 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Victor Toso, qemu-devel, John Snow, Andrea Bolognani
On Mon, Jan 13, 2025 at 01:52:25PM +0100, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
>
> > This patch series intent is to introduce a generator that produces a Go
> > module for Go applications to interact over QMP with QEMU.
> >
> > The initial Goal is to have a Go module that works as intended and can
> > be improved upon. I'd consider initial releases to be alpha while we
> > work with utilities tools and libraries on top of this.
> >
> > The generated code should reside in a separated Git repository, similar
> > to python-qemu-qmp.
> >
> > Applications should be able to consume this under qemu.org
> > namespace (e.g: import "qemu.org/go/qemu"), see Daniel's suggestion:
> > https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg07024.html
> >
> > This is the third iteration:
> > v2: https://lists.gnu.org/archive/html/qemu-devel/2023-10/msg04785.html
> >
> > I've pushed this series in my gitlab fork:
> > https://gitlab.com/victortoso/qapi-go/
> >
> > The fork contains some tests, including tests that were generated from
> > QAPI's own examples from another generator created for testing, if you
> > are interested in it:
> > https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04946.html
> >
> > I've generated the qapi-go module over each commit of this series, see:
> > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-patch
> >
> > I've also generated the qapi-go module over QEMU tags: v9.1.0, v9.2.0:
> > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-tags
> >
> > --
> >
> > Sorry that its been awhile between v2 and v3, I had to prioritize other
> > things. I hope to get this back on track in 2025.
> >
> > Cheers,
> > Victor
> >
> > * Changes:
> >
> > On generated go:
> > - the output should be formatted as gofmt/goimports tools (Daniel)
> >
> > - Included QAPI's documentation too (Daniel), see:
> > https://lists.gnu.org/archive/html/qemu-devel/2024-11/msg01621.html
> >
> > - Commands and Events should Marshal directly (Andrea)
> >
> > On python script:
> > - rebased: now uses QAPISchemaBranches, QAPISchemaAlternatives
> >
> > - use textwrap as much as possible (Andrea)
> >
> > - lots of changes to make the output like gofmt does
> >
> > Victor Toso (8):
> > qapi: golang: Generate enum type
> > qapi: golang: Generate alternate types
> > qapi: golang: Generate struct types
> > qapi: golang: structs: Address nullable members
> > qapi: golang: Generate union type
> > qapi: golang: Generate event type
> > qapi: golang: Generate command type
> > docs: add notes on Golang code generator
> >
> > docs/devel/index-build.rst | 1 +
> > docs/devel/qapi-golang-code-gen.rst | 548 +++++++++
> > scripts/qapi/golang.py | 1645 +++++++++++++++++++++++++++
> > scripts/qapi/main.py | 3 +
> > 4 files changed, 2197 insertions(+)
> > create mode 100644 docs/devel/qapi-golang-code-gen.rst
> > create mode 100644 scripts/qapi/golang.py
>
> This is series adds a backend that slots in cleanly, i.e. without any
> changes to the core. That makes it as low-risk to merge as it gets.
>
> I'd like an Acked-by for the generated Go from someone familiar the kind
> of software that could use it.
In my other (huge) reply to this thread I tried to provide that
analysis by imagining how I would want to consume a QEMU API in
Go, creating an example application and trying to see how well
it fits with this design.
> The -* files are all one pair of files per module (the things pulled in
> with include directives), if any. We do this to avoid "touch the QAPI
> schema, recompile the world."
>
> The generated Go is monolithic. No "recompile the world" problem with
> Go?
IIUC, the intent is that we dn't generate the go code as part of the
regular QEMU build. IOW, most of the time it would be generate once
against a release tag. If someone was actively working on QAPI schema
on git master, at the same time as developing a Go application, they'll
have a bit more of a repeated compile penalty. I'm not convinced that's
a big enough common case to worry about modularizing though.
> golang.py is somewhat big. Whether splitting it up along the lines of
> the C backend would improve things I can't say. No need to worry about
> that now.
With the monolothic go code, generated once per release tag, Go is
going to be reusing cached previously compiled objects most of the
time. I think that's likely to be good enough.
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 :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-01-10 10:49 [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Victor Toso
` (8 preceding siblings ...)
2025-01-13 12:52 ` [PATCH v3 0/8] qapi-go: add generator for Golang interfaces Markus Armbruster
@ 2025-01-16 21:59 ` Daniel P. Berrangé
2025-01-17 10:44 ` Markus Armbruster
` (2 more replies)
9 siblings, 3 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2025-01-16 21:59 UTC (permalink / raw)
To: Victor Toso; +Cc: qemu-devel, Markus Armbruster, John Snow, Andrea Bolognani
On Fri, Jan 10, 2025 at 11:49:38AM +0100, Victor Toso wrote:
> I've pushed this series in my gitlab fork:
> https://gitlab.com/victortoso/qapi-go/
>
> The fork contains some tests, including tests that were generated from
> QAPI's own examples from another generator created for testing, if you
> are interested in it:
> https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04946.html
>
> I've generated the qapi-go module over each commit of this series, see:
> https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-patch
>
> I've also generated the qapi-go module over QEMU tags: v9.1.0, v9.2.0:
> https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-tags
Apologies in advance for what will be a long mail.....
I can't recall all the discussions we had on previous versions now, but
I've been having a look at the generated code again, and have thoughts
around the design of the Event / Command types.
I've been wondering how to actually use the generated code to implement
a client and a server API. We previously said such APIs are out of scope
for this series, but at the same time I think they are pretty important
to consider as a way to evaluate the design approach.
Sending messages is easy as you always know what you're sending. The
big design issue is around receiving data off the wire, and the
associated unmarshalling. QMP has several different message types
(command, return, event, error, QMP greeting). When we've read a JSON
JSON doc off the wire, we (usually) have no idea what message type we
are going to unmarshal. Even if we know the message type, we then
might not know the sub-type.
IOW we have a major chicken & egg problem with unmarshalling.
Taking BalloonChangeEvent:
type BalloonChangeEvent struct {
MessageTimestamp Timestamp `json:"-"`
// the logical size of the VM in bytes Formula used:
// logical_vm_size = vm_ram_size - balloon_size
Actual int64 `json:"actual"`
}
which has an UnmarshallJSON method that consumes this whole doc:
{ "event": "BALLOON_CHANGE",
"data": { "actual": 944766976 },
"timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
Before we can instantiate a BalloonChangeEvent struct and call its
UnmarshallJSON method, we have to already know we've received an
event and also know the event type is for BalloonChangeEvent.
This means we must have already partially unmarshalled it before
we can unmarshall into BalloonChangeEvent.
IMHO the idea of having UnmarshallJSON methods on the "Event" interface
(and "Command" interface) that consumes a complete message is thus
flawed.
Instead, I believe we need to have a two phase approach to unmarshalling.
For the 1st phase we need a struct which can unmarshall *any* type of
QMP message, but only 1 level deep. Essentially something like this:
type Message struct {
QMP *json.RawMessage `json:"QMP,omitempty"`
Execute string `json:"execute,omitempty"`
ExecOOB string `json:"exec-oob,omitempty"`
Event string `json:"event,omitempty"`
Error *json.RawMessage `json:"error,omitempty"`
Return *json.RawMessage `json:"return,omitempty"`
ID string `json:"id,omitempty"`
Timestamp *Timestamp `json:"timestamp,omitempty"`
Data *json.RawMessage `json:"data,omitempty"`
Arguments *json.RawMessage `json:"arguments,omitempty"`
}
Once that Message is unmarshalled, we can examine the 'QMP', 'Execute'
'ExecOOB', 'Event', 'Error' and 'Return' fields, to understand what type
of message we've received.
Then it is possible to identify the right Event or Command or Return
class to use to perform the second level of unmarshalling using the
json.RawMessage data the 1st level of unmarshalling captured.
IOW, if 'Event' is 'BALLOON_CHANGE_EVENT', we know we can use a
BalloonChangeEvent struct to Unmarshall. This struct, however, would
look different to what you've proposed, it would only need to contain
the 'data' fields for the event
type BalloonChangeEvent struct {
// the logical size of the VM in bytes Formula used:
// logical_vm_size = vm_ram_size - balloon_size
Actual int64 `json:"actual"`
}
This would also mean there is no need to provide any UnmarshallJSON
or MarshallJSON methods at all on the BalloonChangeEvent, as the json
marshall code can work exclusively from the field annotations.
The same thoughts above apply to the command / return structs. I don't
think any of them need to have UnmarshallJSON/MarshallJSON methods.
<<<< Pause here if you've read enough for now. >>>>
As a way to validate these thoughts, I spent a day to mock up a demo
of a QAPI client and server implementation.
First I created some manually written structs for the core QMP types
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/protocol.go
Then I've got (what would be) generated code creating types for the
specific QAPI schema. I've just illustrated this with query-machines
and qmp_capabilities commands, and STOP, CONT, BALLOON_CHANGE events:
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_types.go
Essentially this is all the types you generated, but without any of
the (un)marshalling method impls which IMHO are all redundant. I put
everything in one file just for convenience, your per-file split makes
more sense in a real impl.
IMHO that's all that's needed to cover the scope of what is done in
this series, but to illustrate a real world usage model, I've then
gone on to implement both clients and servers.
At the very lowest level, both clients & servers need a way to send
and recv "Message" instances, so I created a "Channel" object with
SendMessage/RecvMessage methods:
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/channel.go
This is the most basic low level client one could have for QMP
From that we can derive a object for QMP Clients, giving slightly higher
level APIs to send commands, receive replies & events, avoiding the need
to touch the "Message" object directly:
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/client.go
And derive a similar object for QMP severs, giving APIs to dispatch commands,
and send replies, events & errors:
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/server.go
So far this is all static written code common to any QAPI schema impl.
The APIs can be used with any schema specific structs, however, the
latter might be defined.
Being low level, none of these APIs are strongly typed, which is not nice
as an application API in Go, so need a way to introduce better type safety.
In a real application I don't think developers should really be touching
the structs directly for commands/responses/events. Instead I believe
they need to be given formal APIs:
Thus I have (what would be) auto-generated interfaces for covering all
the commands and events:
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_commands.go
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_events.go
these interfaces are applicable to both clients and servers, just needing
different private implementations.
On the client side the implementations of these interfaces:
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_sync.go
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_async.go
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_events.go
And on the server side the implementations of these interfaces
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_server.go
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_server_events.go
Finally we can consider what an implementation of a client looks like
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/sync-client/main.go
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/async-client/main.go
And what an implementation of a server looks like
https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/server/main.go
The key observation at the end is that the client/server impls are all
using strongly typed APIs and don't have to concern themselves with JSON
at all.
The two demo clients should work with an real QEMU or the demo server.
The demo server won't work with qmp-shell, because I blindly assumed
each JSON message had a terminating newlinue for ease of impl and this
isn't compatible with qmp-shell. Solvable but I couldn't be bothered
for this demoware.
So what does this all mean wrt your series ? Not that much, probably
just a little code deletion and some small tweaks.
First, it illustrates that the approach taken for the Command / Event
interfaces & the corresponding UnmarshallJSON/MarshallJSON methods is
redundant code. Aside from that, I think everything else in your series
around generating the basic types is essentially good.
Second, it shows a way to build on top of your series, to define a higher
level API to make interacting with QMP much easier for app developers,
eliminating all exposure of JSON & marshalling, in favour of formal APIs.
This doesn't have to be done now, it can be a phase two, as long as we
have an approximate idea of what the phase two API will look like, so
we can validate the phase one design against these future plans.
NB What I've not especially considered in any of this is the impact of
differing QEMU versions & their schema changes. The easy way out is to
just re-run the generator for each version, putting them in a separate
Go package. So you can do 'import' of "qapi/qemu/abi920" for 9.2.0
schema APIs, or "qapi/qemu/910" for 9.1.0 schema APIs. The app dev would
have to choose which version (or versions) they consume & implement
against. Splitting versions across the whole package, avoids having to
consider versioning within parameters of a single command/event.
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 :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-01-16 21:59 ` Daniel P. Berrangé
@ 2025-01-17 10:44 ` Markus Armbruster
2025-01-29 19:53 ` Victor Toso
2025-02-11 10:25 ` Victor Toso
2 siblings, 0 replies; 23+ messages in thread
From: Markus Armbruster @ 2025-01-17 10:44 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Victor Toso, qemu-devel, John Snow, Andrea Bolognani
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Fri, Jan 10, 2025 at 11:49:38AM +0100, Victor Toso wrote:
>> I've pushed this series in my gitlab fork:
>> https://gitlab.com/victortoso/qapi-go/
>>
>> The fork contains some tests, including tests that were generated from
>> QAPI's own examples from another generator created for testing, if you
>> are interested in it:
>> https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04946.html
>>
>> I've generated the qapi-go module over each commit of this series, see:
>> https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-patch
>>
>> I've also generated the qapi-go module over QEMU tags: v9.1.0, v9.2.0:
>> https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-tags
>
> Apologies in advance for what will be a long mail.....
I decline the opportunity for the pot to call the kettle black ;)
[...]
> NB What I've not especially considered in any of this is the impact of
> differing QEMU versions & their schema changes. The easy way out is to
> just re-run the generator for each version, putting them in a separate
> Go package. So you can do 'import' of "qapi/qemu/abi920" for 9.2.0
> schema APIs, or "qapi/qemu/910" for 9.1.0 schema APIs. The app dev would
> have to choose which version (or versions) they consume & implement
> against. Splitting versions across the whole package, avoids having to
> consider versioning within parameters of a single command/event.
I endorse this approach.
First, a practical point. We want to generate bindings from the schema.
The generator we have works on the current version of the schema. To
generate something spanning versions, we would have to make it work on
several versions, wouldn't we? I don't think we can afford such
ambition.
Next, some supporting thoughts on interfaces and why bindings spanning
versions feels like way too much trouble to me.
QMP was designed for compatible schema evolution: we can evolve the
schema without breaking clients of the wire interface as long as they
stick to certain rules, such as "ignore unknown members in client
input".
Occasionally, the schema needs to change in ways that can affect such
clients. We promise to give clients notice and time to adjust: we
formally deprecate the old interface, but keep it working for a grace
period, namely the release it was deprecated and one further release.
So, if your QMP client targets a specific version of the wire interface,
and avoids deprecated stuff, it'll be fine for at least two more
releases. After that, you better upgrade the client.
This is a moderately flexible coupling between QMP client and server.
When you stretch it beyond the limit, things for the most part continue
to work. Only the things we changed incompatibly can break.
If you need more flexible coupling, you can use QMP introspection to
target a *range* of wire interfaces. The price is additional complexity
in the client.
Downstreams complicate this story a bit, but not in interesting ways, I
believe.
The existing C bindings are for the current schema version only. That's
fine, because the only customer (QEMU) only cares for the current
version.
It would also be fine for external customers as long as they only need
the moderately flexible coupling described above.
Bindings spanning schema versions are significantly harder.
Do our rules for compatible schema evolution transfer from the
JSON-based wire interface to the bindings? Not necessarily. May
require acrobatics that make the bindings ugly.
Example: adding an argument to an event is compatible evolution.
Adding it to the C function to send the event is not compatible: it
breaks the build, and all callers need to be adjusted to fix it. Yes,
this may not be an issue with certain other languages, but my point
stands: not necessarily.
Example: adding a member to an object type is compatible evolution.
Adding it to the type in the ABI is not compatible: mixing ABI before
and after won't link if you're lucky, and run amok at run time if you're
not.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-01-16 21:59 ` Daniel P. Berrangé
2025-01-17 10:44 ` Markus Armbruster
@ 2025-01-29 19:53 ` Victor Toso
2025-02-11 10:25 ` Victor Toso
2 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-01-29 19:53 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Markus Armbruster, John Snow, Andrea Bolognani
[-- Attachment #1: Type: text/plain, Size: 11839 bytes --]
Hi,
On Thu, Jan 16, 2025 at 09:59:52PM +0000, Daniel P. Berrangé wrote:
> On Fri, Jan 10, 2025 at 11:49:38AM +0100, Victor Toso wrote:
> > I've pushed this series in my gitlab fork:
> > https://gitlab.com/victortoso/qapi-go/
> >
> > The fork contains some tests, including tests that were generated from
> > QAPI's own examples from another generator created for testing, if you
> > are interested in it:
> > https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg04946.html
> >
> > I've generated the qapi-go module over each commit of this series, see:
> > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-patch
> >
> > I've also generated the qapi-go module over QEMU tags: v9.1.0, v9.2.0:
> > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v3-by-tags
>
> Apologies in advance for what will be a long mail.....
>
> I can't recall all the discussions we had on previous versions now, but
> I've been having a look at the generated code again, and have thoughts
> around the design of the Event / Command types.
>
>
> I've been wondering how to actually use the generated code to implement
> a client and a server API. We previously said such APIs are out of scope
> for this series, but at the same time I think they are pretty important
> to consider as a way to evaluate the design approach.
>
> Sending messages is easy as you always know what you're sending. The
> big design issue is around receiving data off the wire, and the
> associated unmarshalling. QMP has several different message types
> (command, return, event, error, QMP greeting). When we've read a JSON
> JSON doc off the wire, we (usually) have no idea what message type we
> are going to unmarshal. Even if we know the message type, we then
> might not know the sub-type.
>
> IOW we have a major chicken & egg problem with unmarshalling.
>
> Taking BalloonChangeEvent:
>
> type BalloonChangeEvent struct {
> MessageTimestamp Timestamp `json:"-"`
> // the logical size of the VM in bytes Formula used:
> // logical_vm_size = vm_ram_size - balloon_size
> Actual int64 `json:"actual"`
> }
>
> which has an UnmarshallJSON method that consumes this whole doc:
>
> { "event": "BALLOON_CHANGE",
> "data": { "actual": 944766976 },
> "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
>
> Before we can instantiate a BalloonChangeEvent struct and call its
> UnmarshallJSON method, we have to already know we've received an
> event and also know the event type is for BalloonChangeEvent.
> This means we must have already partially unmarshalled it before
> we can unmarshall into BalloonChangeEvent.
>
> IMHO the idea of having UnmarshallJSON methods on the "Event" interface
> (and "Command" interface) that consumes a complete message is thus
> flawed.
>
>
> Instead, I believe we need to have a two phase approach to unmarshalling.
>
> For the 1st phase we need a struct which can unmarshall *any* type of
> QMP message, but only 1 level deep. Essentially something like this:
>
> type Message struct {
> QMP *json.RawMessage `json:"QMP,omitempty"`
> Execute string `json:"execute,omitempty"`
> ExecOOB string `json:"exec-oob,omitempty"`
> Event string `json:"event,omitempty"`
> Error *json.RawMessage `json:"error,omitempty"`
> Return *json.RawMessage `json:"return,omitempty"`
> ID string `json:"id,omitempty"`
> Timestamp *Timestamp `json:"timestamp,omitempty"`
> Data *json.RawMessage `json:"data,omitempty"`
> Arguments *json.RawMessage `json:"arguments,omitempty"`
> }
/* Note to self, should have probably tested 'exec-oob' */
> Once that Message is unmarshalled, we can examine the 'QMP', 'Execute'
> 'ExecOOB', 'Event', 'Error' and 'Return' fields, to understand what type
> of message we've received.
>
> Then it is possible to identify the right Event or Command or Return
> class to use to perform the second level of unmarshalling using the
> json.RawMessage data the 1st level of unmarshalling captured.
>
>
> IOW, if 'Event' is 'BALLOON_CHANGE_EVENT', we know we can use a
> BalloonChangeEvent struct to Unmarshall. This struct, however, would
> look different to what you've proposed, it would only need to contain
> the 'data' fields for the event
>
> type BalloonChangeEvent struct {
> // the logical size of the VM in bytes Formula used:
> // logical_vm_size = vm_ram_size - balloon_size
> Actual int64 `json:"actual"`
> }
>
> This would also mean there is no need to provide any UnmarshallJSON
> or MarshallJSON methods at all on the BalloonChangeEvent, as the json
> marshall code can work exclusively from the field annotations.
>
> The same thoughts above apply to the command / return structs. I don't
> think any of them need to have UnmarshallJSON/MarshallJSON methods.
>
>
>
> <<<< Pause here if you've read enough for now. >>>>
>
>
>
> As a way to validate these thoughts, I spent a day to mock up a demo
> of a QAPI client and server implementation.
>
> First I created some manually written structs for the core QMP types
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/protocol.go
>
> Then I've got (what would be) generated code creating types for the
> specific QAPI schema. I've just illustrated this with query-machines
> and qmp_capabilities commands, and STOP, CONT, BALLOON_CHANGE events:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_types.go
>
> Essentially this is all the types you generated, but without any of
> the (un)marshalling method impls which IMHO are all redundant. I put
> everything in one file just for convenience, your per-file split makes
> more sense in a real impl.
>
>
> IMHO that's all that's needed to cover the scope of what is done in
> this series, but to illustrate a real world usage model, I've then
> gone on to implement both clients and servers.
>
> At the very lowest level, both clients & servers need a way to send
> and recv "Message" instances, so I created a "Channel" object with
> SendMessage/RecvMessage methods:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/channel.go
>
> This is the most basic low level client one could have for QMP
>
>
>
> From that we can derive a object for QMP Clients, giving slightly higher
> level APIs to send commands, receive replies & events, avoiding the need
> to touch the "Message" object directly:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/client.go
>
> And derive a similar object for QMP severs, giving APIs to dispatch commands,
> and send replies, events & errors:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/server.go
>
> So far this is all static written code common to any QAPI schema impl.
> The APIs can be used with any schema specific structs, however, the
> latter might be defined.
>
>
> Being low level, none of these APIs are strongly typed, which is not nice
> as an application API in Go, so need a way to introduce better type safety.
>
>
> In a real application I don't think developers should really be touching
> the structs directly for commands/responses/events. Instead I believe
> they need to be given formal APIs:
>
> Thus I have (what would be) auto-generated interfaces for covering all
> the commands and events:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_commands.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_events.go
>
> these interfaces are applicable to both clients and servers, just needing
> different private implementations.
>
> On the client side the implementations of these interfaces:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_sync.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_async.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_events.go
>
> And on the server side the implementations of these interfaces
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_server.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_server_events.go
>
>
> Finally we can consider what an implementation of a client looks like
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/sync-client/main.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/async-client/main.go
>
> And what an implementation of a server looks like
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/server/main.go
>
> The key observation at the end is that the client/server impls are all
> using strongly typed APIs and don't have to concern themselves with JSON
> at all.
>
> The two demo clients should work with an real QEMU or the demo server.
> The demo server won't work with qmp-shell, because I blindly assumed
> each JSON message had a terminating newlinue for ease of impl and this
> isn't compatible with qmp-shell. Solvable but I couldn't be bothered
> for this demoware.
>
>
> So what does this all mean wrt your series ? Not that much, probably
> just a little code deletion and some small tweaks.
>
>
> First, it illustrates that the approach taken for the Command / Event
> interfaces & the corresponding UnmarshallJSON/MarshallJSON methods is
> redundant code. Aside from that, I think everything else in your series
> around generating the basic types is essentially good.
Alright. The addition of Marshallers was done in the last
interaction. Before, I had the following for command and similar
to event:
func MarshalCommand(c Command) ([]byte, error)
func UnmarshalCommand(data []byte) (Command, error)
Each of this series iterations had a more or less a different
idea of how the consumer of this generated code would use it, but
without a proper PoC on top, like you've done here and thanks for
that.
I'm happy with the suggestions and I'll incorporate them.
> Second, it shows a way to build on top of your series, to define a higher
> level API to make interacting with QMP much easier for app developers,
> eliminating all exposure of JSON & marshalling, in favour of formal APIs.
> This doesn't have to be done now, it can be a phase two, as long as we
> have an approximate idea of what the phase two API will look like, so
> we can validate the phase one design against these future plans.
Just want to put emphasis that the module that we will maintain
would still be considered alpha/unstable until we get the library
in a state we agree is good.
> NB What I've not especially considered in any of this is the impact of
> differing QEMU versions & their schema changes. The easy way out is to
> just re-run the generator for each version, putting them in a separate
> Go package. So you can do 'import' of "qapi/qemu/abi920" for 9.2.0
> schema APIs, or "qapi/qemu/910" for 9.1.0 schema APIs.
Yeah, plus what Markus said. Choosing qapi/qemu/900 should stay
working as long as QMP would guarantee (end of grace period of a
breaking change).
I'd consider sane Applications to bump the go module when they
bump the minimum required QEMU version for their supported use
case, to be able to handle new events and changes.
> The app dev would have to choose which version (or versions)
> they consume & implement against. Splitting versions across the
> whole package, avoids having to consider versioning within
> parameters of a single command/event.
Thanks again,
Victor
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-01-16 21:59 ` Daniel P. Berrangé
2025-01-17 10:44 ` Markus Armbruster
2025-01-29 19:53 ` Victor Toso
@ 2025-02-11 10:25 ` Victor Toso
2025-02-11 11:10 ` Daniel P. Berrangé
2 siblings, 1 reply; 23+ messages in thread
From: Victor Toso @ 2025-02-11 10:25 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Markus Armbruster, John Snow, Andrea Bolognani
[-- Attachment #1: Type: text/plain, Size: 9131 bytes --]
Hi,
On Thu, Jan 16, 2025 at 09:59:52PM +0000, Daniel P. Berrangé wrote:
> <<<< Pause here if you've read enough for now. >>>>
>
>
>
> As a way to validate these thoughts, I spent a day to mock up a demo
> of a QAPI client and server implementation.
>
> First I created some manually written structs for the core QMP types
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/protocol.go
>
> Then I've got (what would be) generated code creating types for the
> specific QAPI schema. I've just illustrated this with query-machines
> and qmp_capabilities commands, and STOP, CONT, BALLOON_CHANGE events:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_types.go
>
> Essentially this is all the types you generated, but without any of
> the (un)marshalling method impls which IMHO are all redundant. I put
> everything in one file just for convenience, your per-file split makes
> more sense in a real impl.
>
>
> IMHO that's all that's needed to cover the scope of what is done in
> this series, but to illustrate a real world usage model, I've then
> gone on to implement both clients and servers.
>
> At the very lowest level, both clients & servers need a way to send
> and recv "Message" instances, so I created a "Channel" object with
> SendMessage/RecvMessage methods:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/channel.go
>
> This is the most basic low level client one could have for QMP
>
>
>
> From that we can derive a object for QMP Clients, giving slightly higher
> level APIs to send commands, receive replies & events, avoiding the need
> to touch the "Message" object directly:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/client.go
>
> And derive a similar object for QMP severs, giving APIs to dispatch commands,
> and send replies, events & errors:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/server.go
>
> So far this is all static written code common to any QAPI schema impl.
> The APIs can be used with any schema specific structs, however, the
> latter might be defined.
>
>
> Being low level, none of these APIs are strongly typed, which is not nice
> as an application API in Go, so need a way to introduce better type safety.
>
>
> In a real application I don't think developers should really be touching
> the structs directly for commands/responses/events. Instead I believe
> they need to be given formal APIs:
>
> Thus I have (what would be) auto-generated interfaces for covering all
> the commands and events:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_commands.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_events.go
One thing that bothers me a little is using event's fields
directly instead of event type in the interface itself:
type Events interface {
StopEvent(time.Time) error
ContEvent(time.Time) error
BalloonChangeEvent(int64, time.Time) error
}
In the gen_server_events.go:
func (cmds *serverEvents) BalloonChangeEvent(actual int64, when time.Time) error {
ev := &BalloonChangeEvent{
Actual: actual,
}
return cmds.QMP.SendEvent("BALLOON_CHANGE", ev, when)
}
I'm actually in favor of moving the Type itself to the method:
type Events interface {
StopEvent(time.Time) error
ContEvent(time.Time) error
BalloonChangeEvent(BalloonChangeEvent, time.Time) error
}
func (cmds *serverEvents) BalloonChangeEvent(ev BalloonChangeEvent, when time.Time) error {
return cmds.QMP.SendEvent("BALLOON_CHANGE", &ev, when)
}
For the caller it might be an extra step but I see a few
benefits:
1. Documentation: I'm generating the QAPI documentation of
the events in its types. IDE's can jump-to-definition and
see the documentation there. Obviously we could move the
documentation here or duplicate it but the doc in the Type
looks the right place imho.
2. Changes in the Event fields over time would not impact the
Breaking the Methods API. While some fields my be
added/removed/changed, I think this would help break less
the application when bumping the go module version.
I'm not sure about empty types like StopEvent. I'm generating it
with its doc but as it goes, it is bound to not be used, being
generated just as source of documentation. Still, if it so
happens that it does change in the future, we would need to
extend the Method with its field or its type.
// Emitted when the virtual machine is stopped
//
// Since: 0.12
//
// .. qmp-example:: <- { "event": "STOP", "timestamp": {
// "seconds": 1267041730, "microseconds": 281295 } }
type StopEvent struct{}
That also applies for commands..
// Resume guest VM execution.
//
// Since: 0.14
//
// .. note:: This command will succeed if the guest is currently
// running. It will also succeed if the guest is in the "inmigrate"
// state; in this case, the effect of the command is to make sure
// the guest starts once migration finishes, removing the effect of
// the "-S" command line option if it was passed. If the VM was
// previously suspended, and not been reset or woken, this command
// will transition back to the "suspended" state. (Since 9.0) ..
// qmp-example:: -> { "execute": "cont" } <- { "return": {} }
type ContCommand struct {}
I honestly did the requested changes a little while ago but I was
thinking about this and tinkering a little with the demo.
Cheers,
Victor
> these interfaces are applicable to both clients and servers, just needing
> different private implementations.
>
> On the client side the implementations of these interfaces:
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_sync.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_async.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_client_events.go
>
> And on the server side the implementations of these interfaces
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_server.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_server_events.go
>
>
> Finally we can consider what an implementation of a client looks like
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/sync-client/main.go
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/async-client/main.go
>
> And what an implementation of a server looks like
>
> https://gitlab.com/berrange/qapi-go-demo/-/blob/main/cmd/server/main.go
>
> The key observation at the end is that the client/server impls are all
> using strongly typed APIs and don't have to concern themselves with JSON
> at all.
>
> The two demo clients should work with an real QEMU or the demo server.
> The demo server won't work with qmp-shell, because I blindly assumed
> each JSON message had a terminating newlinue for ease of impl and this
> isn't compatible with qmp-shell. Solvable but I couldn't be bothered
> for this demoware.
>
>
> So what does this all mean wrt your series ? Not that much, probably
> just a little code deletion and some small tweaks.
>
>
> First, it illustrates that the approach taken for the Command / Event
> interfaces & the corresponding UnmarshallJSON/MarshallJSON methods is
> redundant code. Aside from that, I think everything else in your series
> around generating the basic types is essentially good.
>
> Second, it shows a way to build on top of your series, to define a higher
> level API to make interacting with QMP much easier for app developers,
> eliminating all exposure of JSON & marshalling, in favour of formal APIs.
> This doesn't have to be done now, it can be a phase two, as long as we
> have an approximate idea of what the phase two API will look like, so
> we can validate the phase one design against these future plans.
>
>
> NB What I've not especially considered in any of this is the impact of
> differing QEMU versions & their schema changes. The easy way out is to
> just re-run the generator for each version, putting them in a separate
> Go package. So you can do 'import' of "qapi/qemu/abi920" for 9.2.0
> schema APIs, or "qapi/qemu/910" for 9.1.0 schema APIs. The app dev would
> have to choose which version (or versions) they consume & implement
> against. Splitting versions across the whole package, avoids having to
> consider versioning within parameters of a single command/event.
>
> 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 :|
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-02-11 10:25 ` Victor Toso
@ 2025-02-11 11:10 ` Daniel P. Berrangé
2025-02-11 12:08 ` Victor Toso
0 siblings, 1 reply; 23+ messages in thread
From: Daniel P. Berrangé @ 2025-02-11 11:10 UTC (permalink / raw)
To: Victor Toso; +Cc: qemu-devel, Markus Armbruster, John Snow, Andrea Bolognani
On Tue, Feb 11, 2025 at 11:25:05AM +0100, Victor Toso wrote:
> Hi,
>
> On Thu, Jan 16, 2025 at 09:59:52PM +0000, Daniel P. Berrangé wrote:
> > <<<< Pause here if you've read enough for now. >>>>
> >
> >
> >
> > As a way to validate these thoughts, I spent a day to mock up a demo
> > of a QAPI client and server implementation.
> >
> > First I created some manually written structs for the core QMP types
> >
> > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/protocol.go
> >
> > Then I've got (what would be) generated code creating types for the
> > specific QAPI schema. I've just illustrated this with query-machines
> > and qmp_capabilities commands, and STOP, CONT, BALLOON_CHANGE events:
> >
> > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_types.go
> >
> > Essentially this is all the types you generated, but without any of
> > the (un)marshalling method impls which IMHO are all redundant. I put
> > everything in one file just for convenience, your per-file split makes
> > more sense in a real impl.
> >
> >
> > IMHO that's all that's needed to cover the scope of what is done in
> > this series, but to illustrate a real world usage model, I've then
> > gone on to implement both clients and servers.
> >
> > At the very lowest level, both clients & servers need a way to send
> > and recv "Message" instances, so I created a "Channel" object with
> > SendMessage/RecvMessage methods:
> >
> > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/channel.go
> >
> > This is the most basic low level client one could have for QMP
> >
> >
> >
> > From that we can derive a object for QMP Clients, giving slightly higher
> > level APIs to send commands, receive replies & events, avoiding the need
> > to touch the "Message" object directly:
> >
> > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/client.go
> >
> > And derive a similar object for QMP severs, giving APIs to dispatch commands,
> > and send replies, events & errors:
> >
> > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/server.go
> >
> > So far this is all static written code common to any QAPI schema impl.
> > The APIs can be used with any schema specific structs, however, the
> > latter might be defined.
> >
> >
> > Being low level, none of these APIs are strongly typed, which is not nice
> > as an application API in Go, so need a way to introduce better type safety.
> >
> >
> > In a real application I don't think developers should really be touching
> > the structs directly for commands/responses/events. Instead I believe
> > they need to be given formal APIs:
> >
> > Thus I have (what would be) auto-generated interfaces for covering all
> > the commands and events:
> >
> > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_commands.go
> > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_events.go
>
> One thing that bothers me a little is using event's fields
> directly instead of event type in the interface itself:
>
> type Events interface {
> StopEvent(time.Time) error
> ContEvent(time.Time) error
> BalloonChangeEvent(int64, time.Time) error
> }
>
> In the gen_server_events.go:
>
> func (cmds *serverEvents) BalloonChangeEvent(actual int64, when time.Time) error {
> ev := &BalloonChangeEvent{
> Actual: actual,
> }
> return cmds.QMP.SendEvent("BALLOON_CHANGE", ev, when)
> }
>
> I'm actually in favor of moving the Type itself to the method:
>
> type Events interface {
> StopEvent(time.Time) error
> ContEvent(time.Time) error
> BalloonChangeEvent(BalloonChangeEvent, time.Time) error
> }
>
> func (cmds *serverEvents) BalloonChangeEvent(ev BalloonChangeEvent, when time.Time) error {
> return cmds.QMP.SendEvent("BALLOON_CHANGE", &ev, when)
> }
I choose to expand all the types inline in events & commands
because I felt like this gives a more convenient API design
for applications, without the extra indirectino of packing
and unpacking parameters
.
> For the caller it might be an extra step but I see a few
> benefits:
>
> 1. Documentation: I'm generating the QAPI documentation of
> the events in its types. IDE's can jump-to-definition and
> see the documentation there. Obviously we could move the
> documentation here or duplicate it but the doc in the Type
> looks the right place imho.
Yep, it would complicate docs to have to unpack it against the
method. I was coming at this from the POV of an application
developer though, intentionally ignoring what is the most
convenient for the code generator. The latter is fixed cost
while writing the generator, while the API design is an ongoing
cost time, so makes more sense to optimize for the latter.
> 2. Changes in the Event fields over time would not impact the
> Breaking the Methods API. While some fields my be
> added/removed/changed, I think this would help break less
> the application when bumping the go module version.
Yep, hiding everything behind a struct can reduce breakage.
The tricky (impossible) question is how beneficial it is in
the real world usage ?
As guidance, taking protobuf as a commonly used package though,
if I look at Kubevirt's protobuf generated APIs:
https://github.com/kubevirt/kubevirt/blob/main/pkg/handler-launcher-com/cmd/v1/cmd.pb.go#L1237
I can see they're using structs for parameters and return
values. So that widely used prior art, suggests we go the
way you outline and use structs.
> I'm not sure about empty types like StopEvent. I'm generating it
> with its doc but as it goes, it is bound to not be used, being
> generated just as source of documentation. Still, if it so
> happens that it does change in the future, we would need to
> extend the Method with its field or its type.
>
> // Emitted when the virtual machine is stopped
> //
> // Since: 0.12
> //
> // .. qmp-example:: <- { "event": "STOP", "timestamp": {
> // "seconds": 1267041730, "microseconds": 281295 } }
> type StopEvent struct{}
Yes, if the goal of using structs is to reduce breakage when
fields are added/removed, then IMHO the logical conclusion
is that empty structs must be generated.
> That also applies for commands..
>
> // Resume guest VM execution.
> //
> // Since: 0.14
> //
> // .. note:: This command will succeed if the guest is currently
> // running. It will also succeed if the guest is in the "inmigrate"
> // state; in this case, the effect of the command is to make sure
> // the guest starts once migration finishes, removing the effect of
> // the "-S" command line option if it was passed. If the VM was
> // previously suspended, and not been reset or woken, this command
> // will transition back to the "suspended" state. (Since 9.0) ..
> // qmp-example:: -> { "execute": "cont" } <- { "return": {} }
> type ContCommand struct {}
>
>
> I honestly did the requested changes a little while ago but I was
> thinking about this and tinkering a little with the demo.
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 :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v3 0/8] qapi-go: add generator for Golang interfaces
2025-02-11 11:10 ` Daniel P. Berrangé
@ 2025-02-11 12:08 ` Victor Toso
0 siblings, 0 replies; 23+ messages in thread
From: Victor Toso @ 2025-02-11 12:08 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Markus Armbruster, John Snow, Andrea Bolognani
[-- Attachment #1: Type: text/plain, Size: 13082 bytes --]
Hi,
On Tue, Feb 11, 2025 at 11:10:37AM +0000, Daniel P. Berrangé wrote:
> On Tue, Feb 11, 2025 at 11:25:05AM +0100, Victor Toso wrote:
> > Hi,
> >
> > On Thu, Jan 16, 2025 at 09:59:52PM +0000, Daniel P. Berrangé wrote:
> > > <<<< Pause here if you've read enough for now. >>>>
> > >
> > >
> > >
> > > As a way to validate these thoughts, I spent a day to mock up a demo
> > > of a QAPI client and server implementation.
> > >
> > > First I created some manually written structs for the core QMP types
> > >
> > > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/protocol.go
> > >
> > > Then I've got (what would be) generated code creating types for the
> > > specific QAPI schema. I've just illustrated this with query-machines
> > > and qmp_capabilities commands, and STOP, CONT, BALLOON_CHANGE events:
> > >
> > > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_types.go
> > >
> > > Essentially this is all the types you generated, but without any of
> > > the (un)marshalling method impls which IMHO are all redundant. I put
> > > everything in one file just for convenience, your per-file split makes
> > > more sense in a real impl.
> > >
> > >
> > > IMHO that's all that's needed to cover the scope of what is done in
> > > this series, but to illustrate a real world usage model, I've then
> > > gone on to implement both clients and servers.
> > >
> > > At the very lowest level, both clients & servers need a way to send
> > > and recv "Message" instances, so I created a "Channel" object with
> > > SendMessage/RecvMessage methods:
> > >
> > > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/channel.go
> > >
> > > This is the most basic low level client one could have for QMP
> > >
> > >
> > >
> > > From that we can derive a object for QMP Clients, giving slightly higher
> > > level APIs to send commands, receive replies & events, avoiding the need
> > > to touch the "Message" object directly:
> > >
> > > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/client.go
> > >
> > > And derive a similar object for QMP severs, giving APIs to dispatch commands,
> > > and send replies, events & errors:
> > >
> > > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/server.go
> > >
> > > So far this is all static written code common to any QAPI schema impl.
> > > The APIs can be used with any schema specific structs, however, the
> > > latter might be defined.
> > >
> > >
> > > Being low level, none of these APIs are strongly typed, which is not nice
> > > as an application API in Go, so need a way to introduce better type safety.
> > >
> > >
> > > In a real application I don't think developers should really be touching
> > > the structs directly for commands/responses/events. Instead I believe
> > > they need to be given formal APIs:
> > >
> > > Thus I have (what would be) auto-generated interfaces for covering all
> > > the commands and events:
> > >
> > > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_commands.go
> > > https://gitlab.com/berrange/qapi-go-demo/-/blob/main/pkg/qapi/gen_events.go
> >
> > One thing that bothers me a little is using event's fields
> > directly instead of event type in the interface itself:
> >
> > type Events interface {
> > StopEvent(time.Time) error
> > ContEvent(time.Time) error
> > BalloonChangeEvent(int64, time.Time) error
> > }
> >
> > In the gen_server_events.go:
> >
> > func (cmds *serverEvents) BalloonChangeEvent(actual int64, when time.Time) error {
> > ev := &BalloonChangeEvent{
> > Actual: actual,
> > }
> > return cmds.QMP.SendEvent("BALLOON_CHANGE", ev, when)
> > }
> >
> > I'm actually in favor of moving the Type itself to the method:
> >
> > type Events interface {
> > StopEvent(time.Time) error
> > ContEvent(time.Time) error
> > BalloonChangeEvent(BalloonChangeEvent, time.Time) error
> > }
> >
> > func (cmds *serverEvents) BalloonChangeEvent(ev BalloonChangeEvent, when time.Time) error {
> > return cmds.QMP.SendEvent("BALLOON_CHANGE", &ev, when)
> > }
>
> I choose to expand all the types inline in events & commands
> because I felt like this gives a more convenient API design
> for applications, without the extra indirectino of packing
> and unpacking parameters
Sure, just a choice to be made.
> > For the caller it might be an extra step but I see a few
> > benefits:
> >
> > 1. Documentation: I'm generating the QAPI documentation of
> > the events in its types. IDE's can jump-to-definition and
> > see the documentation there. Obviously we could move the
> > documentation here or duplicate it but the doc in the Type
> > looks the right place imho.
>
> Yep, it would complicate docs to have to unpack it against the
> method. I was coming at this from the POV of an application
> developer though, intentionally ignoring what is the most
> convenient for the code generator. The latter is fixed cost
> while writing the generator, while the API design is an ongoing
> cost time, so makes more sense to optimize for the latter.
Right. I don't mind doing the work to inline it, but I'm not sure
it is best. Inline commands/events that don't have fields are
straight forward and look nice but what when they have several
fields instead?
Wrt to mandatory and optional fields, I think it is
straightforward when looking at the struct definition that might
be confusing to the methods args.
As example, block-stream command has 12 fields, only 1 mandatory.
// .. qmp-example:: -> { "execute": "block-stream",
// "arguments": { "device": "virtio0", "base":
// "/tmp/master.qcow2" } } <- { "return": {} }
type BlockStreamCommand struct {
// identifier for the newly-created block job. If omitted, the
// device name will be used. (Since 2.7)
JobId *string `json:"job-id,omitempty"`
// the device or node name of the top image
Device string `json:"device"`
// the common backing file name. It cannot be set if @base-node or
// @bottom is also set.
Base *string `json:"base,omitempty"`
// the node name of the backing file. It cannot be set if @base or
// @bottom is also set. (Since 2.8)
BaseNode *string `json:"base-node,omitempty"`
// The backing file string to write into the top image. This
// filename is not validated. If a pathname string is such that it
// cannot be resolved by QEMU, that means that subsequent QMP or
// HMP commands must use node-names for the image in question, as
// filename lookup methods will fail. If not specified, QEMU will
// automatically determine the backing file string to use, or
// error out if there is no obvious choice. Care should be taken
// when specifying the string, to specify a valid filename or
// protocol. (Since 2.1)
BackingFile *string `json:"backing-file,omitempty"`
// If true, replace any protocol mentioned in the 'backing file
// format' with 'raw', rather than storing the protocol name as
// the backing format. Can be used even when no image header will
// be updated (default false; since 9.0).
BackingMaskProtocol *bool `json:"backing-mask-protocol,omitempty"`
// the last node in the chain that should be streamed into top. It
// cannot be set if @base or @base-node is also set. It cannot be
// filter node. (Since 6.0)
Bottom *string `json:"bottom,omitempty"`
// the maximum speed, in bytes per second
Speed *int64 `json:"speed,omitempty"`
// the action to take on an error (default report). 'stop' and
// 'enospc' can only be used if the block device supports io-
// status (see BlockInfo). (Since 1.3)
OnError *BlockdevOnError `json:"on-error,omitempty"`
// the node name that should be assigned to the filter driver that
// the stream job inserts into the graph above @device. If this
// option is not given, a node name is autogenerated. (Since: 6.0)
FilterNodeName *string `json:"filter-node-name,omitempty"`
// When false, this job will wait in a PENDING state after it has
// finished its work, waiting for @block-job-finalize before
// making any block graph changes. When true, this job will
// automatically perform its abort or commit actions. Defaults to
// true. (Since 3.1)
AutoFinalize *bool `json:"auto-finalize,omitempty"`
// When false, this job will wait in a CONCLUDED state after it
// has completely ceased all work, and awaits @block-job-dismiss.
// When true, this job will automatically disappear from the query
// list without user intervention. Defaults to true. (Since 3.1)
AutoDismiss *bool `json:"auto-dismiss,omitempty"`
}
Might be easier in that case to
cmds.BlockStream(BlockStreamCommand{device:"virtio"})
instead of defining all optional args as nil.
> > 2. Changes in the Event fields over time would not impact the
> > Breaking the Methods API. While some fields my be
> > added/removed/changed, I think this would help break less
> > the application when bumping the go module version.
>
> Yep, hiding everything behind a struct can reduce breakage.
>
> The tricky (impossible) question is how beneficial it is in
> the real world usage ?
To me it was just an extra benefit, not a vital one. We are bound
to break when user bumps it. In the real world, as far as I
recall from discussing this with Markus (feel free to correct me)
i. Removing fields is somewhat rare. I remember he put out
an example but I could not find it.
ii. Adding fields is not so rare although they are usually
optional.
With inlining approach we would break application in both cases
100% of the time. It'd be less so by using a type.
> As guidance, taking protobuf as a commonly used package though,
> if I look at Kubevirt's protobuf generated APIs:
>
> https://github.com/kubevirt/kubevirt/blob/main/pkg/handler-launcher-com/cmd/v1/cmd.pb.go#L1237
>
> I can see they're using structs for parameters and return
> values. So that widely used prior art, suggests we go the
> way you outline and use structs.
Oh, but if I'm not mistaken, if you do change the type you are
required to bump the version meaning that you'll need to
generated a new set of types with the new version. That enforces
client/side to implement the new type if they want to talk over
new version (enforced by capabilites negotiation). Not a great
dev experience but it surely works and performance is good too...
but I digress.
> > I'm not sure about empty types like StopEvent. I'm generating it
> > with its doc but as it goes, it is bound to not be used, being
> > generated just as source of documentation. Still, if it so
> > happens that it does change in the future, we would need to
> > extend the Method with its field or its type.
> >
> > // Emitted when the virtual machine is stopped
> > //
> > // Since: 0.12
> > //
> > // .. qmp-example:: <- { "event": "STOP", "timestamp": {
> > // "seconds": 1267041730, "microseconds": 281295 } }
> > type StopEvent struct{}
>
> Yes, if the goal of using structs is to reduce breakage when
> fields are added/removed, then IMHO the logical conclusion
> is that empty structs must be generated.
imho, just a extra benefit. Breaking is somewhat fine if it
happens only when we bump the version and is not something that
happens so often that it becomes a bother.
> > That also applies for commands..
> >
> > // Resume guest VM execution.
> > //
> > // Since: 0.14
> > //
> > // .. note:: This command will succeed if the guest is currently
> > // running. It will also succeed if the guest is in the "inmigrate"
> > // state; in this case, the effect of the command is to make sure
> > // the guest starts once migration finishes, removing the effect of
> > // the "-S" command line option if it was passed. If the VM was
> > // previously suspended, and not been reset or woken, this command
> > // will transition back to the "suspended" state. (Since 9.0) ..
> > // qmp-example:: -> { "execute": "cont" } <- { "return": {} }
> > type ContCommand struct {}
> >
> >
> > I honestly did the requested changes a little while ago but I was
> > thinking about this and tinkering a little with the demo.
>
> With regards,
> Daniel
Thanks for the quick replies,
Victor
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread