Re: [Xen-devel] [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 23/01/17 16:43, Ronald Rojas wrote:
> Create error type Errorxl for throwing proper xenlight
> errors.
> 
> Update Ctx functions to throw Errorxl errors.
> 
> Signed-off-by: Ronald Rojas <ronladred@xxxxxxxxx>

Everything here looks, good with a few nitpicks...

> ---
>  tools/golang/xenlight/xenlight.go | 75 
> ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go 
> b/tools/golang/xenlight/xenlight.go
> index f82e14e..eee2254 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -38,10 +38,72 @@ import (
>       "unsafe"
>  )
>  
> +/*
> + * Errors
> + */
> +
> +type Error int
> +
> +const (
> +     ErrorNonspecific                  = Error(-C.ERROR_NONSPECIFIC)
> +     ErrorVersion                      = Error(-C.ERROR_VERSION)
> +     ErrorFail                         = Error(-C.ERROR_FAIL)
> +     ErrorNi                           = Error(-C.ERROR_NI)
> +     ErrorNomem                        = Error(-C.ERROR_NOMEM)
> +     ErrorInval                        = Error(-C.ERROR_INVAL)
> +     ErrorBadfail                      = Error(-C.ERROR_BADFAIL)
> +     ErrorGuestTimedout                = Error(-C.ERROR_GUEST_TIMEDOUT)
> +     ErrorTimedout                     = Error(-C.ERROR_TIMEDOUT)
> +     ErrorNoparavirt                   = Error(-C.ERROR_NOPARAVIRT)
> +     ErrorNotReady                     = Error(-C.ERROR_NOT_READY)
> +     ErrorOseventRegFail               = Error(-C.ERROR_OSEVENT_REG_FAIL)
> +     ErrorBufferfull                   = Error(-C.ERROR_BUFFERFULL)
> +     ErrorUnknownChild                 = Error(-C.ERROR_UNKNOWN_CHILD)
> +     ErrorLockFail                     = Error(-C.ERROR_LOCK_FAIL)
> +     ErrorJsonConfigEmpty              = Error(-C.ERROR_JSON_CONFIG_EMPTY)
> +     ErrorDeviceExists                 = Error(-C.ERROR_DEVICE_EXISTS)
> +     ErrorCheckpointDevopsDoesNotMatch = 
> Error(-C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
> +     ErrorCheckpointDeviceNotSupported = 
> Error(-C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
> +     ErrorVnumaConfigInvalid           = Error(-C.ERROR_VNUMA_CONFIG_INVALID)
> +     ErrorDomainNotfound               = Error(-C.ERROR_DOMAIN_NOTFOUND)
> +     ErrorAborted                      = Error(-C.ERROR_ABORTED)
> +     ErrorNotfound                     = Error(-C.ERROR_NOTFOUND)
> +     ErrorDomainDestroyed              = Error(-C.ERROR_DOMAIN_DESTROYED)
> +     ErrorFeatureRemoved               = Error(-C.ERROR_FEATURE_REMOVED)
> +)
> +
> +var errors = [...]string{
> +     ErrorNonspecific: "Non-specific error",
> +     ErrorVersion: "Wrong version",
> +     ErrorFail: "Failed",
> +     ErrorNi: "Not Implemented",
> +     ErrorNomem: "No memory",
> +     ErrorInval: "Invalid argument",
> +     ErrorBadfail: "Bad Fail",
> +     ErrorGuestTimedout: "Guest timed out",
> +     ErrorTimedout: "Timed out",
> +     ErrorNoparavirt: "No Paravirtualization",
> +     ErrorNotReady: "Not ready",
> +     ErrorOseventRegFail: "OS event registration failed",
> +     ErrorBufferfull: "Buffer full",
> +     ErrorUnknownChild: "Unknown child",
> +     ErrorLockFail: "Lock failed",
> +     ErrorJsonConfigEmpty: "JSON config empty",
> +     ErrorDeviceExists: "Device exists",
> +     ErrorCheckpointDevopsDoesNotMatch: "Checkpoint devops does not match",
> +     ErrorCheckpointDeviceNotSupported: "Checkpoint device not supported",
> +     ErrorVnumaConfigInvalid: "VNUMA config invalid",
> +     ErrorDomainNotfound: "Domain not found",
> +     ErrorAborted: "Aborted",
> +     ErrorNotfound: "Not found",
> +     ErrorDomainDestroyed: "Domain destroyed",
> +     ErrorFeatureRemoved: "Feature removed",
> +}
>  
>  /*
>   * Types: Builtins
>   */
> +
>  type Context struct {
>       ctx *C.libxl_ctx
>  }
> @@ -51,6 +113,17 @@ type Context struct {
>   */
>  var Ctx Context
>  
> +func (e Error) Error() string{
> +     if 0< int(e) && int(e) < len(errors){
> +             s:= errors[e]
> +             if s != ""{

We tend to be picky about style; so a spaces:
 - Between 0 and <
 - between len(errors) and {
 - between "" and {

I could fix these up on check-in, but since you'll be re-sending this
anyway, I'll let you do it.

And of course you'll have to change the fmt.Errorf() to Error(-ret); but
with those changes:

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel