[RFC PATCH v2 7/8] qapi: golang: Add CommandResult type to Go

Victor Toso posted 8 patches 3 years, 7 months ago
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>
[RFC PATCH v2 7/8] qapi: golang: Add CommandResult type to Go
Posted by Victor Toso 3 years, 7 months ago
This patch adds a struct type in Go that will handle return values for
QAPI's command types.

The return value of a Command is, encouraged to be, QAPI's complex
types or an Array of those.

Every Command has a underlying CommandResult. The EmptyCommandReturn
is for those that don't expect any data e.g: `{ "return": {} }`.

All CommandReturn types implement the CommandResult interface.

Example:
qapi:
  | { 'command': 'query-sev', 'returns': 'SevInfo',
  |   'if': 'TARGET_I386' }

go:
  | type QuerySevCommandReturn struct {
  |         CommandId string     `json:"id,omitempty"`
  |         Result    *SevInfo   `json:"return"`
  |         Error     *QapiError `json:"error,omitempty"`
  | }

usage:
  | // One can use QuerySevCommandReturn directly or
  | // command's interface GetReturnType() instead.
  |
  | input := `{ "return": { "enabled": true, "api-major" : 0,` +
  |                        `"api-minor" : 0, "build-id" : 0,` +
  |                        `"policy" : 0, "state" : "running",` +
  |                        `"handle" : 1 } } `
  | ret := QuerySevCommandReturn{}
  | err := json.Unmarshal([]byte(input), &ret)
  | if ret.Error != nil {
  |     // Handle command failure {"error": { ...}}
  | } else if ret.Result != nil {
  |     // ret.Result.Enable == true
  | }

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 scripts/qapi/golang.py | 73 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
index 123179cced..ab91cf124f 100644
--- a/scripts/qapi/golang.py
+++ b/scripts/qapi/golang.py
@@ -89,7 +89,8 @@
 }}
 '''
 
-# Only variable is @unm_cases to handle all command's names and associated types.
+# Only variable is @unm_cases to handle
+# all command's names and associated types.
 TEMPLATE_COMMAND = '''
 type Command interface {{
     GetId()         string
@@ -145,10 +146,49 @@
 }}
 '''
 
+TEMPLATE_COMMAND_RETURN = '''
+type CommandReturn interface {
+    GetId()          string
+    GetCommandName() string
+    GetError()       error
+}
+
+type EmptyCommandReturn struct {
+    CommandId string          `json:"id,omitempty"`
+    Error     *QapiError      `json:"error,omitempty"`
+    Name      string          `json:"-"`
+}
+
+func (r EmptyCommandReturn) MarshalJSON() ([]byte, error) {
+    return []byte(`{"return":{}}`), nil
+}
+
+func (r *EmptyCommandReturn) GetId() string {
+    return r.CommandId
+}
+
+func (r *EmptyCommandReturn) GetCommandName() string {
+    return r.Name
+}
+
+func (r *EmptyCommandReturn) GetError() error {
+    return r.Error
+}
+'''
+
 TEMPLATE_HELPER = '''
 // Alias for go version lower than 1.18
 type Any = interface{}
 
+type QapiError struct {
+    Class       string `json:"class"`
+    Description string `json:"desc"`
+}
+
+func (err *QapiError) Error() string {
+    return fmt.Sprintf("%s: %s", err.Class, err.Description)
+}
+
 // Creates a decoder that errors on unknown Fields
 // Returns true if successfully decoded @from string @into type
 // Returns false without error is failed with "unknown field"
@@ -176,6 +216,7 @@ def __init__(self, prefix: str):
         self.schema = None
         self.events = {}
         self.commands = {}
+        self.command_results = {}
         self.golang_package_name = "qapi"
 
     def visit_begin(self, schema):
@@ -224,6 +265,7 @@ def visit_end(self):
 '''
         self.target["command"] += TEMPLATE_COMMAND.format(unm_cases=unm_cases)
 
+        self.target["command"] += TEMPLATE_COMMAND_RETURN
 
     def visit_object_type(self: QAPISchemaGenGolangVisitor,
                           name: str,
@@ -390,6 +432,31 @@ def visit_command(self,
         self.commands[name] = type_name
         command_ret = ""
         init_ret_type_name = f'''EmptyCommandReturn {{ Name: "{name}" }}'''
+        if ret_type:
+            cmd_ret_name = qapi_to_go_type_name(name, "command return")
+            ret_type_name = qapi_schema_type_to_go_type(ret_type.name)
+            init_ret_type_name = f'''{cmd_ret_name}{{}}'''
+            isptr = "*" if ret_type_name[0] not in "*[" else ""
+            self.command_results[name] = ret_type_name
+            command_ret = f'''
+type {cmd_ret_name} struct {{
+    CommandId  string                `json:"id,omitempty"`
+    Result    {isptr}{ret_type_name} `json:"return"`
+    Error     *QapiError             `json:"error,omitempty"`
+}}
+
+func (r *{cmd_ret_name}) GetCommandName() string {{
+    return "{name}"
+}}
+
+func (r *{cmd_ret_name}) GetId() string {{
+    return r.CommandId
+}}
+
+func (r *{cmd_ret_name}) GetError() error {{
+    return r.Error
+}}
+'''
 
         self_contained = True
         if arg_type and arg_type.name.startswith("q_obj"):
@@ -423,7 +490,7 @@ def visit_command(self,
     return &{init_ret_type_name}
 }}
 '''
-        self.target["command"] += content + methods
+        self.target["command"] += content + methods + command_ret
 
     def visit_event(self, name, info, ifcond, features, arg_type, boxed):
         assert name == info.defn_name
@@ -686,7 +753,7 @@ def qapi_to_go_type_name(name: str, meta: str) -> str:
 
     name += ''.join(word.title() for word in words[1:])
 
-    if meta in ["event", "command"]:
+    if meta in ["event", "command", "command return"]:
         name = name[:-3] if name.endswith("Arg") else name
         name += meta.title().replace(" ", "")
 
-- 
2.36.1
Re: [RFC PATCH v2 7/8] qapi: golang: Add CommandResult type to Go
Posted by Andrea Bolognani 3 years, 7 months ago
On Fri, Jun 17, 2022 at 02:19:31PM +0200, Victor Toso wrote:
> +type EmptyCommandReturn struct {
> +    CommandId string          `json:"id,omitempty"`
> +    Error     *QapiError      `json:"error,omitempty"`
> +    Name      string          `json:"-"`
> +}

Do we need a specific type for this? Can't we just generate an
appropriately-named type for each of the commands that don't return
any data? It's not like we would have to write that code manually :)

> +func (r *EmptyCommandReturn) GetCommandName() string {
> +    return r.Name
> +}

Just like Event.GetName() and Command.GetName(), I'm not convinced we
should have this.


Of course, all the comments about how marshalling and unmarshalling
are handled made for events also apply here.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [RFC PATCH v2 7/8] qapi: golang: Add CommandResult type to Go
Posted by Daniel P. Berrangé 3 years, 7 months ago
On Tue, Jul 05, 2022 at 08:46:21AM -0700, Andrea Bolognani wrote:
> On Fri, Jun 17, 2022 at 02:19:31PM +0200, Victor Toso wrote:
> > +type EmptyCommandReturn struct {
> > +    CommandId string          `json:"id,omitempty"`
> > +    Error     *QapiError      `json:"error,omitempty"`
> > +    Name      string          `json:"-"`
> > +}
> 
> Do we need a specific type for this? Can't we just generate an
> appropriately-named type for each of the commands that don't return
> any data? It's not like we would have to write that code manually :)

Yes, I think having an explicit named return struct even for commands
not /currently/ returning data is good, and anticipates future changes
that might add extra return data fields to the QAPI schema.

> 
> > +func (r *EmptyCommandReturn) GetCommandName() string {
> > +    return r.Name
> > +}
> 
> Just like Event.GetName() and Command.GetName(), I'm not convinced we
> should have this.
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [RFC PATCH v2 7/8] qapi: golang: Add CommandResult type to Go
Posted by Victor Toso 3 years, 5 months ago
Hi,

On Tue, Jul 05, 2022 at 05:49:22PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 05, 2022 at 08:46:21AM -0700, Andrea Bolognani wrote:
> > On Fri, Jun 17, 2022 at 02:19:31PM +0200, Victor Toso wrote:
> > > +type EmptyCommandReturn struct {
> > > +    CommandId string          `json:"id,omitempty"`
> > > +    Error     *QapiError      `json:"error,omitempty"`
> > > +    Name      string          `json:"-"`
> > > +}
> >
> > Do we need a specific type for this? Can't we just generate an
> > appropriately-named type for each of the commands that don't return
> > any data? It's not like we would have to write that code manually :)
>
> Yes, I think having an explicit named return struct even for commands
> not /currently/ returning data is good, and anticipates future changes
> that might add extra return data fields to the QAPI schema.

Sure.

Cheers,
Victor