Hi,
On Tue, May 10, 2022 at 11:42:10AM +0100, Daniel P. Berrangé wrote:
> On Sat, Apr 02, 2022 at 12:41:01AM +0200, Victor Toso wrote:
> > This patch handles QAPI event types and generates data structures in
> > Go that handles it.
> >
> > At the moment of this writing, it generates 51 structures (49 events)
> >
> > In Golang, each event is handled as a Go structure and there is no big
> > difference, in the Go generated code, between what is a QAPI event
> > type and what is a QAPI struct.
> >
> > Each QAPI event has the suffix 'Event' in its Golang data structure
> > and contains the fields, mandatory and optional, that can be
> > sent or received.
> >
> > In addition, there are two structures added to handle QAPI
> > specification for event types: 'Event' and 'EventBase'.
> >
> > 'EventBase' contains @Name and @Timestamp fields and then 'Event'
> > extends 'EventBase' with an @Arg field of type 'Any'.
>
> Again, I don't think we should need to use an Any type here.
>
> Rather than
>
> type EventBase struct {
> Name string `json:"event"`
> Timestamp struct {
> Seconds int64 `json:"seconds"`
> Microseconds int64 `json:"microseconds"`
> } `json:"timestamp"`
> }
>
> type Event struct {
> EventBase
> Arg Any `json:"data,omitempty"`
> }
>
> type ShutdownEvent struct {
> Guest bool `json:"guest"`
> Reason ShutdownCause `json:"reason"`
> }
>
>
> I think we should just embed EventBase directly in each specific
> event eg
>
>
> type Event struct {
> Name string `json:"event"`
> Timestamp struct {
> Seconds int64 `json:"seconds"`
> Microseconds int64 `json:"microseconds"`
> } `json:"timestamp"`
> }
>
> type ShutdownEvent struct {
> Event Event
> Guest bool `json:"guest"`
> Reason ShutdownCause `json:"reason"`
> }
>
>
> Or perhaps better still, use an interface
>
> type Event interface {
> GetName() string
> GetTimestamp() string
> }
>
> type Timestamp struct {
> Seconds int64 `json:"seconds"`
> Microseconds int64 `json:"microseconds"`
> }
>
> type ShutdownEvent struct {
> Timestamp Timestamp `json:"timestamp"`
> Guest bool `json:"guest"`
> Reason ShutdownCause `json:"reason"`
> }
>
> func (ev *ShutdownEvent) GetName() string {
> return "SHUTDOWN"
> }
>
> That way you can define public APIs taking 'Event' as a type,
> and impls of the events can be passed directly in/out.
>
> Similar comment for the Command type.
Yeah, if we are removing Any for Unions and Alternates, we can
sure do the same for Events and Commands.
> > The 'Event' type implements the Unmarshaler to decode the QMP JSON
> > Object into the correct Golang (event) struct. The goal here is to
> > facilitate receiving Events.
> >
> > A TODO for this type is to implement Marshaler for 'Event'. It'll
> > containt runtime checks to validate before transforming the struct
> > into a JSON Object.
> >
> > Example:
> > ```go
> > qmpMsg := `{
> > "event" : "MIGRATION",
> > "timestamp":{
> > "seconds": 1432121972,
> > "microseconds": 744001
> > },
> > "data":{
> > "status": "completed"
> > }
> > }`
> >
> > e := Event{}
> > _ = json.Unmarshal([]byte(qmpMsg), &e)
> > // e.Name == "MIGRATION"
> > // e.Arg.(MigrationEvent).Status == MigrationStatusCompleted
> > ```
> >
> > Signed-off-by: Victor Toso <victort...@redhat.com>
> > ---
> > scripts/qapi/golang.py | 92 ++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 84 insertions(+), 8 deletions(-)
> >
> > diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> > index 0a1bf430ba..3bb66d07c7 100644
> > --- a/scripts/qapi/golang.py
> > +++ b/scripts/qapi/golang.py
> > @@ -31,9 +31,10 @@ class QAPISchemaGenGolangVisitor(QAPISchemaVisitor):
> >
> > def __init__(self, prefix: str):
> > super().__init__()
> > - self.target = {name: "" for name in ["alternate", "enum",
> > "helper", "struct", "union"]}
> > + self.target = {name: "" for name in ["alternate", "enum", "event",
> > "helper", "struct", "union"]}
> > self.objects_seen = {}
> > self.schema = None
> > + self.events = {}
> > self._docmap = {}
> > self.golang_package_name = "qapi"
> >
> > @@ -57,6 +58,24 @@ def visit_begin(self, schema):
> > def visit_end(self):
> > self.schema = None
> >
> > + # EventBase and Event are not specified in the QAPI schema,
> > + # so we need to generate it ourselves.
> > + self.target["event"] += '''
> > +type EventBase struct {
> > + Name string `json:"event"`
> > + Timestamp struct {
> > + Seconds int64 `json:"seconds"`
> > + Microseconds int64 `json:"microseconds"`
> > + } `json:"timestamp"`
> > +}
> > +
> > +type Event struct {
> > + EventBase
> > + Arg Any `json:"data,omitempty"`
> > +}
> > +'''
> > + self.target["event"] += generate_marshal_methods('Event',
> > self.events)
> > +
> > self.target["helper"] += '''
> > // Creates a decoder that errors on unknown Fields
> > // Returns true if successfully decoded @from string @into type
> > @@ -279,7 +298,28 @@ def visit_command(self,
> > pass
> >
> > def visit_event(self, name, info, ifcond, features, arg_type, boxed):
> > - pass
> > + assert name == info.defn_name
> > + type_name = qapi_to_go_type_name(name, info.defn_meta)
> > + self.events[name] = type_name
> > +
> > + doc = self._docmap.get(name, None)
> > + self_contained = True if not arg_type or not
> > arg_type.name.startswith("q_obj") else False
> > + content = ""
> > + if self_contained:
> > + doc_struct, _ = qapi_to_golang_struct_docs(doc)
> > + content = generate_struct_type(type_name, "", doc_struct)
> > + else:
> > + assert isinstance(arg_type, QAPISchemaObjectType)
> > + content = qapi_to_golang_struct(name,
> > + doc,
> > + arg_type.info,
> > + arg_type.ifcond,
> > + arg_type.features,
> > + arg_type.base,
> > + arg_type.members,
> > + arg_type.variants)
> > +
> > + self.target["event"] += content
> >
> > def write(self, output_dir: str) -> None:
> > for module_name, content in self.target.items():
> > @@ -351,15 +391,41 @@ def generate_marshal_methods_enum(members:
> > List[QAPISchemaEnumMember]) -> str:
> > }}
> > '''
> >
> > -# Marshal methods for Union types
> > +# Marshal methods for Event and Union types
> > def generate_marshal_methods(type: str,
> > type_dict: Dict[str, str],
> > discriminator: str = "",
> > base: str = "") -> str:
> > - assert base != ""
> > - discriminator = "base." + discriminator
> > -
> > - switch_case_format = '''
> > + type_is_union = False
> > + json_field = ""
> > + struct_field = ""
> > + if type == "Event":
> > + base = type + "Base"
> > + discriminator = "base.Name"
> > + struct_field = "Arg"
> > + json_field = "data"
> > + else:
> > + assert base != ""
> > + discriminator = "base." + discriminator
> > + type_is_union = True
> > +
> > + switch_case_format = ""
> > + if not type_is_union:
> > + switch_case_format = '''
> > + case "{name}":
> > + tmp := struct {{
> > + Value {isptr}{case_type} `json:"{json_field},omitempty"`
> > + }}{{}}
> > + if err := json.Unmarshal(data, &tmp); err != nil {{
> > + return err
> > + }}
> > + if tmp.Value == nil {{
> > + s.{struct_field} = nil
> > + }} else {{
> > + s.{struct_field} = {isptr}tmp.Value
> > + }}'''
> > + else:
> > + switch_case_format = '''
> > case {name}:
> > value := {case_type}{{}}
> > if err := json.Unmarshal(data, &value); err != nil {{
> > @@ -374,12 +440,17 @@ def generate_marshal_methods(type: str,
> > case_type = type_dict[name]
> > isptr = "*" if case_type[0] not in "*[" else ""
> > switch_cases += switch_case_format.format(name = name,
> > + struct_field =
> > struct_field,
> > + json_field = json_field,
> > + isptr = isptr,
> > case_type = case_type)
> > if case_type not in added:
> > if_supported_types += f'''typestr != "{case_type}" &&\n\t\t'''
> > added[case_type] = True
> >
> > - marshalfn = f'''
> > + marshalfn = ""
> > + if type_is_union:
> > + marshalfn = f'''
> > func (s {type}) MarshalJSON() ([]byte, error) {{
> > base, err := json.Marshal(s.{base})
> > if err != nil {{
> > @@ -564,4 +635,9 @@ def qapi_to_go_type_name(name: str, meta: str) -> str:
> > words = [word for word in name.replace("_", "-").split("-")]
> > name = words[0].title() if words[0].islower() or words[0].isupper()
> > else words[0]
> > name += ''.join(word.title() for word in words[1:])
> > +
> > + if meta == "event":
> > + name = name[:-3] if name.endswith("Arg") else name
> > + name += meta.title()
> > +
> > return name
> > --
> > 2.35.1
> >
> >
>
> 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 :|
Cheers,
Victor
signature.asc
Description: PGP signature
