On Mon, Oct 16, 2023 at 05:27:03PM +0200, Victor Toso wrote:
> 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.

I guess EmptyCommandReturn is something that you have scrapped in
between revisions, because I see no reference to it in the current
incarnation of the code. Same thing with CommandResult, unless that's
just a typo for CommandReturn?

> Example:
> qapi:
>     | { 'command': 'query-sev', 'returns': 'SevInfo',
>     |   'if': 'TARGET_I386' }
>
> go:
>     | type QuerySevCommandReturn struct {
>     |     MessageId 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.

I'm not convinced this function is particularly useful. I know that
I've suggested something similar for events, but the usage scenarios
are different.

For events, you're going to have some loop listening for them and you
can't know in advance what event you're going to receive at any given
time.

In contrast, a return value will be received as a direct consequence
of running a command, and since the caller knows what command it
called it's fair to assume that it also knows its return type.

> +        if ret_type:
> +            marshal_empty = ""
> +            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"`""",
> +                }
> +            )

This produces

  type QueryAudiodevsCommandReturn struct {
    MessageId string     `json:"id,omitempty"`
    Error     *QAPIError `json:"error,omitempty"`
    Result    []Audiodev `json:"return"`
  }

when the return type is an array. Is that the correct behavior? I
haven't thought too hard about it, but it seems odd so I though I'd
bring it up.

-- 
Andrea Bolognani / Red Hat / Virtualization


Reply via email to