Hi,
On Thu, Sep 28, 2023 at 04:03:12PM +0100, Daniel P. Berrangé wrote:
> On Wed, Sep 27, 2023 at 01:25:43PM +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.
> >
> > 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 <victort...@redhat.com>
> > ---
> > scripts/qapi/golang.py | 72 ++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 70 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> > index 52a9124641..48ca0deab0 100644
> > --- a/scripts/qapi/golang.py
> > +++ b/scripts/qapi/golang.py
> > @@ -40,6 +40,15 @@
> > '''
> >
> > TEMPLATE_HELPER = '''
> > +type QapiError struct {
>
> QAPIError as the name for this
Agreed. I'll fix it.
> > + Class string `json:"class"`
> > + Description string `json:"desc"`
> > +}
>
> > +
> > +func (err *QapiError) Error() string {
> > + return fmt.Sprintf("%s: %s", err.Class, err.Description)
> > +}
>
> My gut feeling is that this should be just
>
> return err.Description
>
> on the basis that long ago we pretty much decided that the
> 'Class' field was broadly a waste of time except for a
> couple of niche use cases. The error description is always
> self contained and sufficient to diagnose problems, without
> knowing the Class.
>
> Keep the Class field in the struct though, as it could be
> useful to check in certain cases
I'll trust you on this. I'll change it.
Cheers,
Victor
signature.asc
Description: PGP signature
