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

2017-01-27 Thread George Dunlap 
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 

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 



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

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

2017-01-23 Thread Ronald Rojas 
Create error type Errorxl for throwing proper xenlight
errors.

Update Ctx functions to throw Errorxl errors.

Signed-off-by: Ronald Rojas 
---
 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 != ""{
+   return s
+   }
+   }
+   return fmt.Sprintf("libxl error: %d", -e)
+
+}
+
 func (Ctx *Context) IsOpen() bool {
return Ctx.ctx != nil
 }
@@ -83,4 +156,4 @@ func (Ctx *Context) CheckOpen() (err error) {
err = fmt.Errorf("Context not opened")
}
return
-}
\ No newline at end of file
+}
-- 
2.7.4


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

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

2017-01-19 Thread George Dunlap 
On 18/01/17 19:56, Ronald Rojas wrote:
> Create error type Errorxl for throwing proper xenlight
> errors.
> 
> Update Ctx functions to throw Errorxl errors.
> 
> Signed-off-by: Ronald Rojas 

Overall, looks good!  Thanks for this.  Just a few minor comments.

> ---
>  tools/golang/xenlight/xenlight.go | 77 
> +--
>  1 file changed, 73 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go 
> b/tools/golang/xenlight/xenlight.go
> index 1f10e51..d58f8b8 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -32,6 +32,77 @@ import (
>  )
>  
>  /*
> + * Errors
> + */
> +type Errorxl int

Is there a reason not to make this just "Error"?  The callers will have
to use the namespace anyway (xenlight.Error).

> +
> +const (
> + ErrorNonspecific  = Errorxl(-C.ERROR_NONSPECIFIC)
> + ErrorVersion  = Errorxl(-C.ERROR_VERSION)
> + ErrorFail = Errorxl(-C.ERROR_FAIL)
> + ErrorNi   = Errorxl(-C.ERROR_NI)
> + ErrorNomem= Errorxl(-C.ERROR_NOMEM)
> + ErrorInval= Errorxl(-C.ERROR_INVAL)
> + ErrorBadfail  = Errorxl(-C.ERROR_BADFAIL)
> + ErrorGuestTimedout= Errorxl(-C.ERROR_GUEST_TIMEDOUT)
> + ErrorTimedout = Errorxl(-C.ERROR_TIMEDOUT)
> + ErrorNoparavirt   = Errorxl(-C.ERROR_NOPARAVIRT)
> + ErrorNotReady = Errorxl(-C.ERROR_NOT_READY)
> + ErrorOseventRegFail   = Errorxl(-C.ERROR_OSEVENT_REG_FAIL)
> + ErrorBufferfull   = Errorxl(-C.ERROR_BUFFERFULL)
> + ErrorUnknownChild = Errorxl(-C.ERROR_UNKNOWN_CHILD)
> + ErrorLockFail = Errorxl(-C.ERROR_LOCK_FAIL)
> + ErrorJsonConfigEmpty  = Errorxl(-C.ERROR_JSON_CONFIG_EMPTY)
> + ErrorDeviceExists = Errorxl(-C.ERROR_DEVICE_EXISTS)
> + ErrorCheckpointDevopsDoesNotMatch = 
> Errorxl(-C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
> + ErrorCheckpointDeviceNotSupported = 
> Errorxl(-C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
> + ErrorVnumaConfigInvalid   = 
> Errorxl(-C.ERROR_VNUMA_CONFIG_INVALID)
> + ErrorDomainNotfound   = Errorxl(-C.ERROR_DOMAIN_NOTFOUND)
> + ErrorAborted  = Errorxl(-C.ERROR_ABORTED)
> + ErrorNotfound = Errorxl(-C.ERROR_NOTFOUND)
> + ErrorDomainDestroyed  = Errorxl(-C.ERROR_DOMAIN_DESTROYED)
> + ErrorFeatureRemoved   = Errorxl(-C.ERROR_FEATURE_REMOVED)
> +)

So here you define the constants as positive integers corresponding to
libxl's negative integers, so that you can use them for array indexes in
the string table below.

But when you return an error from libxl itself, you simply cast it to
Errorxl(), leaving it negative; this means that you can't actually
compare err with the constants here -- you have to negate it, which is a
bit strange.

I think probably negating the C libxl values for the golang constants,
as you have done here, makes sense.  But then you should negate the
values you get back from libxl as well, so that they match.

> +
> +var errors = [...]string{
> + ErrorNonspecific:  "Non-specific error",
> + ErrorVersion:  "Wrong version",
> + ErrorFail: "Failed",
> + ErrorNi:   "Null",

"Not implemented"

> + ErrorNomem:"No memory",
> + ErrorInval:"Invalid",

probably "Invalid argument"

> + ErrorBadfail:  "Bad Fail",
> + ErrorGuestTimedout:"Guest timed out",
> + ErrorTimedout: "Timed out",
> + ErrorNoparavirt:   "No Paravirtualization",
> + ErrorNotReady: "Not ready",
> + ErrorOseventRegFail:   "OS event failed",

"OS event registration failed"

> + ErrorBufferfull:   "Buffer full",
> + ErrorUnknownChild: "Unknown child",
> + ErrorLockFail: "Lock failed",
> + ErrorJsonConfigEmpty:  "JSON config empyt",

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",
> +}
> +
> +func

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

2017-01-19 Thread Dario Faggioli 
On Thu, 2017-01-19 at 16:02 +, George Dunlap wrote:
> On 19/01/17 15:13, Ronald Rojas wrote:
> > It's possible to add the errors as part of the first patch and then
> > add the context functions as the second patch as Go will at least 
> > let you compile the errors on it's own. I can swap the order of the
> > patchs in the next revision.
> 
> Which points out the problem with Dario's suggestion. :-)
> 
> It is indeed normal that you don't fix things from previous patches.
> But the "fix" here is from the very first patch: you can't introduce
> the
> error code before you introduce the directories and the makefile to
> build it.  
>
Sure! But, OOC, is it imperative that the makefile is introduced in the
first patch?

I think that if it were me doing something like this, I'd defer
introducing it, if not at the very end, not before than when there is
something meaningful to make.

A matter of taste, at least up to a certain extent, I know.

> But that of course means that you're not separating out the important
> things from the first patch (the Makefile setup) with the important
> things from the second patch (the Error handling design).
> 
> I'd prefer it be left as it is; 
>
Sure. I'd at least remove the '//FIXME' from patch 1, especially
considering that the changelog already says that error handling will be
reworked.

> but it's Ian and Wei that have the final
> word on that.
> 
Indeed. :-)

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)

signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

2017-01-19 Thread Wei Liu 
On Thu, Jan 19, 2017 at 04:02:46PM +, George Dunlap wrote:
> On 19/01/17 15:13, Ronald Rojas wrote:
> > On Wed, Jan 18, 2017 at 11:16:31PM +0100, Dario Faggioli wrote:
> >> On Wed, 2017-01-18 at 14:56 -0500, Ronald Rojas wrote:
> >>> Create error type Errorxl for throwing proper xenlight
> >>> errors.
> >>>
> >>> Update Ctx functions to throw Errorxl errors.
> >>>
> >>> Signed-off-by: Ronald Rojas 
> >>> ---
> >>>  tools/golang/xenlight/xenlight.go | 77
> >>> +--
> >>>  1 file changed, 73 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/tools/golang/xenlight/xenlight.go
> >>> b/tools/golang/xenlight/xenlight.go
> >>> index 1f10e51..d58f8b8 100644
> >>> --- a/tools/golang/xenlight/xenlight.go
> >>> +++ b/tools/golang/xenlight/xenlight.go
> >>> @@ -32,6 +32,77 @@ import (
> >>>  )
> >>>  
> >>>  /*
> >>> + * Errors
> >>> + */
> >>> +type Errorxl int
> >>> +
> >>> +const (
> >>> + ErrorNonspecific  = Errorxl(-
> >>> C.ERROR_NONSPECIFIC)
> >>> + ErrorVersion  = Errorxl(-
> >>> C.ERROR_VERSION)
> >>> + ErrorFail = Errorxl(-C.ERROR_FAIL)
> >>> + ErrorNi   = Errorxl(-C.ERROR_NI)
> >>> + ErrorNomem= Errorxl(-C.ERROR_NOMEM)
> >>> + ErrorInval= Errorxl(-C.ERROR_INVAL)
> >>> + ErrorBadfail  = Errorxl(-
> >>> C.ERROR_BADFAIL)
> >>> + ErrorGuestTimedout= Errorxl(-
> >>> C.ERROR_GUEST_TIMEDOUT)
> >>> + ErrorTimedout = Errorxl(-
> >>> C.ERROR_TIMEDOUT)
> >>> + ErrorNoparavirt   = Errorxl(-
> >>> C.ERROR_NOPARAVIRT)
> >>> + ErrorNotReady = Errorxl(-
> >>> C.ERROR_NOT_READY)
> >>> + ErrorOseventRegFail   = Errorxl(-
> >>> C.ERROR_OSEVENT_REG_FAIL)
> >>> + ErrorBufferfull   = Errorxl(-
> >>> C.ERROR_BUFFERFULL)
> >>> + ErrorUnknownChild = Errorxl(-
> >>> C.ERROR_UNKNOWN_CHILD)
> >>> + ErrorLockFail = Errorxl(-
> >>> C.ERROR_LOCK_FAIL)
> >>> + ErrorJsonConfigEmpty  = Errorxl(-
> >>> C.ERROR_JSON_CONFIG_EMPTY)
> >>> + ErrorDeviceExists = Errorxl(-
> >>> C.ERROR_DEVICE_EXISTS)
> >>> + ErrorCheckpointDevopsDoesNotMatch = Errorxl(-
> >>> C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
> >>> + ErrorCheckpointDeviceNotSupported = Errorxl(-
> >>> C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
> >>> + ErrorVnumaConfigInvalid   = Errorxl(-
> >>> C.ERROR_VNUMA_CONFIG_INVALID)
> >>> + ErrorDomainNotfound   = Errorxl(-
> >>> C.ERROR_DOMAIN_NOTFOUND)
> >>> + ErrorAborted  = Errorxl(-
> >>> C.ERROR_ABORTED)
> >>> + ErrorNotfound = Errorxl(-
> >>> C.ERROR_NOTFOUND)
> >>> + ErrorDomainDestroyed  = Errorxl(-
> >>> C.ERROR_DOMAIN_DESTROYED)
> >>> + ErrorFeatureRemoved   = Errorxl(-
> >>> C.ERROR_FEATURE_REMOVED)
> >>> +)
> >>> +
> >>> +var errors = [...]string{
> >>> + ErrorNonspecific:  "Non-specific error",
> >>> + ErrorVersion:  "Wrong version",
> >>> + ErrorFail: "Failed",
> >>> + ErrorNi:   "Null",
> >>> + ErrorNomem:"No memory",
> >>> + ErrorInval:"Invalid",
> >>> + ErrorBadfail:  "Bad Fail",
> >>> + ErrorGuestTimedout:"Guest timed out",
> >>> + ErrorTimedout: "Timed out",
> >>> + ErrorNoparavirt:   "No Paravirtualization",
> >>> + ErrorNotReady: "Not ready",
> >>> + ErrorOseventRegFail:   "OS event failed",
> >>> + ErrorBufferfull:   "Buffer full",
> >>> + ErrorUnknownChild: "Unknown child",
> >>> + ErrorLockFail: "Lock failed",
> >>> + ErrorJsonConfigEmpty:  "JSON config empyt",
> >>> + 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",
> >>> +}
> >>> +
> >>> +func (e Errorxl) Error() string {
> >>> + if 0 <= -int(e) && -int(e) < len(errors) {
> >>> + s := errors[-e]
> >>> + if s != "" {
> >>> + return s
> >>> + }
> >>> + }
> >>> + return "errorxl " + strconv.Itoa(int(e))
> >>> +}
> >>> +
> >>> +/*
> >>>   * Types: Builtins
> >>>   */
> >>>  type Context struct {
> >>> @@ -55,8 +126,7 @@ func (Ctx *Context) Open() (err error) {
>

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

2017-01-19 Thread George Dunlap 
On 19/01/17 15:13, Ronald Rojas wrote:
> On Wed, Jan 18, 2017 at 11:16:31PM +0100, Dario Faggioli wrote:
>> On Wed, 2017-01-18 at 14:56 -0500, Ronald Rojas wrote:
>>> Create error type Errorxl for throwing proper xenlight
>>> errors.
>>>
>>> Update Ctx functions to throw Errorxl errors.
>>>
>>> Signed-off-by: Ronald Rojas 
>>> ---
>>>  tools/golang/xenlight/xenlight.go | 77
>>> +--
>>>  1 file changed, 73 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/golang/xenlight/xenlight.go
>>> b/tools/golang/xenlight/xenlight.go
>>> index 1f10e51..d58f8b8 100644
>>> --- a/tools/golang/xenlight/xenlight.go
>>> +++ b/tools/golang/xenlight/xenlight.go
>>> @@ -32,6 +32,77 @@ import (
>>>  )
>>>  
>>>  /*
>>> + * Errors
>>> + */
>>> +type Errorxl int
>>> +
>>> +const (
>>> +   ErrorNonspecific  = Errorxl(-
>>> C.ERROR_NONSPECIFIC)
>>> +   ErrorVersion  = Errorxl(-
>>> C.ERROR_VERSION)
>>> +   ErrorFail = Errorxl(-C.ERROR_FAIL)
>>> +   ErrorNi   = Errorxl(-C.ERROR_NI)
>>> +   ErrorNomem= Errorxl(-C.ERROR_NOMEM)
>>> +   ErrorInval= Errorxl(-C.ERROR_INVAL)
>>> +   ErrorBadfail  = Errorxl(-
>>> C.ERROR_BADFAIL)
>>> +   ErrorGuestTimedout= Errorxl(-
>>> C.ERROR_GUEST_TIMEDOUT)
>>> +   ErrorTimedout = Errorxl(-
>>> C.ERROR_TIMEDOUT)
>>> +   ErrorNoparavirt   = Errorxl(-
>>> C.ERROR_NOPARAVIRT)
>>> +   ErrorNotReady = Errorxl(-
>>> C.ERROR_NOT_READY)
>>> +   ErrorOseventRegFail   = Errorxl(-
>>> C.ERROR_OSEVENT_REG_FAIL)
>>> +   ErrorBufferfull   = Errorxl(-
>>> C.ERROR_BUFFERFULL)
>>> +   ErrorUnknownChild = Errorxl(-
>>> C.ERROR_UNKNOWN_CHILD)
>>> +   ErrorLockFail = Errorxl(-
>>> C.ERROR_LOCK_FAIL)
>>> +   ErrorJsonConfigEmpty  = Errorxl(-
>>> C.ERROR_JSON_CONFIG_EMPTY)
>>> +   ErrorDeviceExists = Errorxl(-
>>> C.ERROR_DEVICE_EXISTS)
>>> +   ErrorCheckpointDevopsDoesNotMatch = Errorxl(-
>>> C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
>>> +   ErrorCheckpointDeviceNotSupported = Errorxl(-
>>> C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
>>> +   ErrorVnumaConfigInvalid   = Errorxl(-
>>> C.ERROR_VNUMA_CONFIG_INVALID)
>>> +   ErrorDomainNotfound   = Errorxl(-
>>> C.ERROR_DOMAIN_NOTFOUND)
>>> +   ErrorAborted  = Errorxl(-
>>> C.ERROR_ABORTED)
>>> +   ErrorNotfound = Errorxl(-
>>> C.ERROR_NOTFOUND)
>>> +   ErrorDomainDestroyed  = Errorxl(-
>>> C.ERROR_DOMAIN_DESTROYED)
>>> +   ErrorFeatureRemoved   = Errorxl(-
>>> C.ERROR_FEATURE_REMOVED)
>>> +)
>>> +
>>> +var errors = [...]string{
>>> +   ErrorNonspecific:  "Non-specific error",
>>> +   ErrorVersion:  "Wrong version",
>>> +   ErrorFail: "Failed",
>>> +   ErrorNi:   "Null",
>>> +   ErrorNomem:"No memory",
>>> +   ErrorInval:"Invalid",
>>> +   ErrorBadfail:  "Bad Fail",
>>> +   ErrorGuestTimedout:"Guest timed out",
>>> +   ErrorTimedout: "Timed out",
>>> +   ErrorNoparavirt:   "No Paravirtualization",
>>> +   ErrorNotReady: "Not ready",
>>> +   ErrorOseventRegFail:   "OS event failed",
>>> +   ErrorBufferfull:   "Buffer full",
>>> +   ErrorUnknownChild: "Unknown child",
>>> +   ErrorLockFail: "Lock failed",
>>> +   ErrorJsonConfigEmpty:  "JSON config empyt",
>>> +   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",
>>> +}
>>> +
>>> +func (e Errorxl) Error() string {
>>> +   if 0 <= -int(e) && -int(e) < len(errors) {
>>> +   s := errors[-e]
>>> +   if s != "" {
>>> +   return s
>>> +   }
>>> +   }
>>> +   return "errorxl " + strconv.Itoa(int(e))
>>> +}
>>> +
>>> +/*
>>>   * Types: Builtins
>>>   */
>>>  type Context struct {
>>> @@ -55,8 +126,7 @@ func (Ctx *Context) Open() (err error) {
>>> ret := C.libxl_ctx_alloc(unsafe.Pointer(),
>>> C.LIBXL_VERSION, 0, nil)
>>>  
>>> if ret != 0 {
>>> -   //FIXME: proper error
>>> -   err = createError("Allocating

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

2017-01-19 Thread Ronald Rojas 
On Wed, Jan 18, 2017 at 11:16:31PM +0100, Dario Faggioli wrote:
> On Wed, 2017-01-18 at 14:56 -0500, Ronald Rojas wrote:
> > Create error type Errorxl for throwing proper xenlight
> > errors.
> > 
> > Update Ctx functions to throw Errorxl errors.
> > 
> > Signed-off-by: Ronald Rojas 
> > ---
> >  tools/golang/xenlight/xenlight.go | 77
> > +--
> >  1 file changed, 73 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/golang/xenlight/xenlight.go
> > b/tools/golang/xenlight/xenlight.go
> > index 1f10e51..d58f8b8 100644
> > --- a/tools/golang/xenlight/xenlight.go
> > +++ b/tools/golang/xenlight/xenlight.go
> > @@ -32,6 +32,77 @@ import (
> >  )
> >  
> >  /*
> > + * Errors
> > + */
> > +type Errorxl int
> > +
> > +const (
> > +   ErrorNonspecific  = Errorxl(-
> > C.ERROR_NONSPECIFIC)
> > +   ErrorVersion  = Errorxl(-
> > C.ERROR_VERSION)
> > +   ErrorFail = Errorxl(-C.ERROR_FAIL)
> > +   ErrorNi   = Errorxl(-C.ERROR_NI)
> > +   ErrorNomem= Errorxl(-C.ERROR_NOMEM)
> > +   ErrorInval= Errorxl(-C.ERROR_INVAL)
> > +   ErrorBadfail  = Errorxl(-
> > C.ERROR_BADFAIL)
> > +   ErrorGuestTimedout= Errorxl(-
> > C.ERROR_GUEST_TIMEDOUT)
> > +   ErrorTimedout = Errorxl(-
> > C.ERROR_TIMEDOUT)
> > +   ErrorNoparavirt   = Errorxl(-
> > C.ERROR_NOPARAVIRT)
> > +   ErrorNotReady = Errorxl(-
> > C.ERROR_NOT_READY)
> > +   ErrorOseventRegFail   = Errorxl(-
> > C.ERROR_OSEVENT_REG_FAIL)
> > +   ErrorBufferfull   = Errorxl(-
> > C.ERROR_BUFFERFULL)
> > +   ErrorUnknownChild = Errorxl(-
> > C.ERROR_UNKNOWN_CHILD)
> > +   ErrorLockFail = Errorxl(-
> > C.ERROR_LOCK_FAIL)
> > +   ErrorJsonConfigEmpty  = Errorxl(-
> > C.ERROR_JSON_CONFIG_EMPTY)
> > +   ErrorDeviceExists = Errorxl(-
> > C.ERROR_DEVICE_EXISTS)
> > +   ErrorCheckpointDevopsDoesNotMatch = Errorxl(-
> > C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
> > +   ErrorCheckpointDeviceNotSupported = Errorxl(-
> > C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
> > +   ErrorVnumaConfigInvalid   = Errorxl(-
> > C.ERROR_VNUMA_CONFIG_INVALID)
> > +   ErrorDomainNotfound   = Errorxl(-
> > C.ERROR_DOMAIN_NOTFOUND)
> > +   ErrorAborted  = Errorxl(-
> > C.ERROR_ABORTED)
> > +   ErrorNotfound = Errorxl(-
> > C.ERROR_NOTFOUND)
> > +   ErrorDomainDestroyed  = Errorxl(-
> > C.ERROR_DOMAIN_DESTROYED)
> > +   ErrorFeatureRemoved   = Errorxl(-
> > C.ERROR_FEATURE_REMOVED)
> > +)
> > +
> > +var errors = [...]string{
> > +   ErrorNonspecific:  "Non-specific error",
> > +   ErrorVersion:  "Wrong version",
> > +   ErrorFail: "Failed",
> > +   ErrorNi:   "Null",
> > +   ErrorNomem:"No memory",
> > +   ErrorInval:"Invalid",
> > +   ErrorBadfail:  "Bad Fail",
> > +   ErrorGuestTimedout:"Guest timed out",
> > +   ErrorTimedout: "Timed out",
> > +   ErrorNoparavirt:   "No Paravirtualization",
> > +   ErrorNotReady: "Not ready",
> > +   ErrorOseventRegFail:   "OS event failed",
> > +   ErrorBufferfull:   "Buffer full",
> > +   ErrorUnknownChild: "Unknown child",
> > +   ErrorLockFail: "Lock failed",
> > +   ErrorJsonConfigEmpty:  "JSON config empyt",
> > +   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",
> > +}
> > +
> > +func (e Errorxl) Error() string {
> > +   if 0 <= -int(e) && -int(e) < len(errors) {
> > +   s := errors[-e]
> > +   if s != "" {
> > +   return s
> > +   }
> > +   }
> > +   return "errorxl " + strconv.Itoa(int(e))
> > +}
> > +
> > +/*
> >   * Types: Builtins
> >   */
> >  type Context struct {
> > @@ -55,8 +126,7 @@ func (Ctx *Context) Open() (err error) {
> >     ret := C.libxl_ctx_alloc(unsafe.Pointer(),
> > C.LIBXL_VERSION, 0, nil)
> >  
> >     if ret != 0 {
> > -   //FIXME: proper error
> > -   err = createError("Allocating libxl context: ", ret)
> > +

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

2017-01-18 Thread Dario Faggioli 
On Wed, 2017-01-18 at 14:56 -0500, Ronald Rojas wrote:
> Create error type Errorxl for throwing proper xenlight
> errors.
> 
> Update Ctx functions to throw Errorxl errors.
> 
> Signed-off-by: Ronald Rojas 
> ---
>  tools/golang/xenlight/xenlight.go | 77
> +--
>  1 file changed, 73 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go
> b/tools/golang/xenlight/xenlight.go
> index 1f10e51..d58f8b8 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -32,6 +32,77 @@ import (
>  )
>  
>  /*
> + * Errors
> + */
> +type Errorxl int
> +
> +const (
> + ErrorNonspecific  = Errorxl(-
> C.ERROR_NONSPECIFIC)
> + ErrorVersion  = Errorxl(-
> C.ERROR_VERSION)
> + ErrorFail = Errorxl(-C.ERROR_FAIL)
> + ErrorNi   = Errorxl(-C.ERROR_NI)
> + ErrorNomem= Errorxl(-C.ERROR_NOMEM)
> + ErrorInval= Errorxl(-C.ERROR_INVAL)
> + ErrorBadfail  = Errorxl(-
> C.ERROR_BADFAIL)
> + ErrorGuestTimedout= Errorxl(-
> C.ERROR_GUEST_TIMEDOUT)
> + ErrorTimedout = Errorxl(-
> C.ERROR_TIMEDOUT)
> + ErrorNoparavirt   = Errorxl(-
> C.ERROR_NOPARAVIRT)
> + ErrorNotReady = Errorxl(-
> C.ERROR_NOT_READY)
> + ErrorOseventRegFail   = Errorxl(-
> C.ERROR_OSEVENT_REG_FAIL)
> + ErrorBufferfull   = Errorxl(-
> C.ERROR_BUFFERFULL)
> + ErrorUnknownChild = Errorxl(-
> C.ERROR_UNKNOWN_CHILD)
> + ErrorLockFail = Errorxl(-
> C.ERROR_LOCK_FAIL)
> + ErrorJsonConfigEmpty  = Errorxl(-
> C.ERROR_JSON_CONFIG_EMPTY)
> + ErrorDeviceExists = Errorxl(-
> C.ERROR_DEVICE_EXISTS)
> + ErrorCheckpointDevopsDoesNotMatch = Errorxl(-
> C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
> + ErrorCheckpointDeviceNotSupported = Errorxl(-
> C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
> + ErrorVnumaConfigInvalid   = Errorxl(-
> C.ERROR_VNUMA_CONFIG_INVALID)
> + ErrorDomainNotfound   = Errorxl(-
> C.ERROR_DOMAIN_NOTFOUND)
> + ErrorAborted  = Errorxl(-
> C.ERROR_ABORTED)
> + ErrorNotfound = Errorxl(-
> C.ERROR_NOTFOUND)
> + ErrorDomainDestroyed  = Errorxl(-
> C.ERROR_DOMAIN_DESTROYED)
> + ErrorFeatureRemoved   = Errorxl(-
> C.ERROR_FEATURE_REMOVED)
> +)
> +
> +var errors = [...]string{
> + ErrorNonspecific:  "Non-specific error",
> + ErrorVersion:  "Wrong version",
> + ErrorFail: "Failed",
> + ErrorNi:   "Null",
> + ErrorNomem:"No memory",
> + ErrorInval:"Invalid",
> + ErrorBadfail:  "Bad Fail",
> + ErrorGuestTimedout:"Guest timed out",
> + ErrorTimedout: "Timed out",
> + ErrorNoparavirt:   "No Paravirtualization",
> + ErrorNotReady: "Not ready",
> + ErrorOseventRegFail:   "OS event failed",
> + ErrorBufferfull:   "Buffer full",
> + ErrorUnknownChild: "Unknown child",
> + ErrorLockFail: "Lock failed",
> + ErrorJsonConfigEmpty:  "JSON config empyt",
> + 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",
> +}
> +
> +func (e Errorxl) Error() string {
> + if 0 <= -int(e) && -int(e) < len(errors) {
> + s := errors[-e]
> + if s != "" {
> + return s
> + }
> + }
> + return "errorxl " + strconv.Itoa(int(e))
> +}
> +
> +/*
>   * Types: Builtins
>   */
>  type Context struct {
> @@ -55,8 +126,7 @@ func (Ctx *Context) Open() (err error) {
>   ret := C.libxl_ctx_alloc(unsafe.Pointer(),
> C.LIBXL_VERSION, 0, nil)
>  
>   if ret != 0 {
> - //FIXME: proper error
> - err = createError("Allocating libxl context: ", ret)
> + err = Errorxl(ret)
>
In general, it's not good practise to do something in patch n, and then
undo/redo it in patch n+1 (or in general n+m), within the same series.

Reason is, it makes things

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

2017-01-18 Thread Ronald Rojas 
Create error type Errorxl for throwing proper xenlight
errors.

Update Ctx functions to throw Errorxl errors.

Signed-off-by: Ronald Rojas 
---
 tools/golang/xenlight/xenlight.go | 77 +--
 1 file changed, 73 insertions(+), 4 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 1f10e51..d58f8b8 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -32,6 +32,77 @@ import (
 )
 
 /*
+ * Errors
+ */
+type Errorxl int
+
+const (
+   ErrorNonspecific  = Errorxl(-C.ERROR_NONSPECIFIC)
+   ErrorVersion  = Errorxl(-C.ERROR_VERSION)
+   ErrorFail = Errorxl(-C.ERROR_FAIL)
+   ErrorNi   = Errorxl(-C.ERROR_NI)
+   ErrorNomem= Errorxl(-C.ERROR_NOMEM)
+   ErrorInval= Errorxl(-C.ERROR_INVAL)
+   ErrorBadfail  = Errorxl(-C.ERROR_BADFAIL)
+   ErrorGuestTimedout= Errorxl(-C.ERROR_GUEST_TIMEDOUT)
+   ErrorTimedout = Errorxl(-C.ERROR_TIMEDOUT)
+   ErrorNoparavirt   = Errorxl(-C.ERROR_NOPARAVIRT)
+   ErrorNotReady = Errorxl(-C.ERROR_NOT_READY)
+   ErrorOseventRegFail   = Errorxl(-C.ERROR_OSEVENT_REG_FAIL)
+   ErrorBufferfull   = Errorxl(-C.ERROR_BUFFERFULL)
+   ErrorUnknownChild = Errorxl(-C.ERROR_UNKNOWN_CHILD)
+   ErrorLockFail = Errorxl(-C.ERROR_LOCK_FAIL)
+   ErrorJsonConfigEmpty  = Errorxl(-C.ERROR_JSON_CONFIG_EMPTY)
+   ErrorDeviceExists = Errorxl(-C.ERROR_DEVICE_EXISTS)
+   ErrorCheckpointDevopsDoesNotMatch = 
Errorxl(-C.ERROR_CHECKPOINT_DEVOPS_DOES_NOT_MATCH)
+   ErrorCheckpointDeviceNotSupported = 
Errorxl(-C.ERROR_CHECKPOINT_DEVICE_NOT_SUPPORTED)
+   ErrorVnumaConfigInvalid   = 
Errorxl(-C.ERROR_VNUMA_CONFIG_INVALID)
+   ErrorDomainNotfound   = Errorxl(-C.ERROR_DOMAIN_NOTFOUND)
+   ErrorAborted  = Errorxl(-C.ERROR_ABORTED)
+   ErrorNotfound = Errorxl(-C.ERROR_NOTFOUND)
+   ErrorDomainDestroyed  = Errorxl(-C.ERROR_DOMAIN_DESTROYED)
+   ErrorFeatureRemoved   = Errorxl(-C.ERROR_FEATURE_REMOVED)
+)
+
+var errors = [...]string{
+   ErrorNonspecific:  "Non-specific error",
+   ErrorVersion:  "Wrong version",
+   ErrorFail: "Failed",
+   ErrorNi:   "Null",
+   ErrorNomem:"No memory",
+   ErrorInval:"Invalid",
+   ErrorBadfail:  "Bad Fail",
+   ErrorGuestTimedout:"Guest timed out",
+   ErrorTimedout: "Timed out",
+   ErrorNoparavirt:   "No Paravirtualization",
+   ErrorNotReady: "Not ready",
+   ErrorOseventRegFail:   "OS event failed",
+   ErrorBufferfull:   "Buffer full",
+   ErrorUnknownChild: "Unknown child",
+   ErrorLockFail: "Lock failed",
+   ErrorJsonConfigEmpty:  "JSON config empyt",
+   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",
+}
+
+func (e Errorxl) Error() string {
+   if 0 <= -int(e) && -int(e) < len(errors) {
+   s := errors[-e]
+   if s != "" {
+   return s
+   }
+   }
+   return "errorxl " + strconv.Itoa(int(e))
+}
+
+/*
  * Types: Builtins
  */
 type Context struct {
@@ -55,8 +126,7 @@ func (Ctx *Context) Open() (err error) {
ret := C.libxl_ctx_alloc(unsafe.Pointer(), C.LIBXL_VERSION, 0, 
nil)
 
if ret != 0 {
-   //FIXME: proper error
-   err = createError("Allocating libxl context: ", ret)
+   err = Errorxl(ret)
}
return
 }
@@ -66,8 +136,7 @@ func (Ctx *Context) Close() (err error) {
Ctx.ctx = nil
 
if ret != 0 {
-   //FIXME: proper error
-   err = createError("Freeing libxl context: ", ret)
+   err = Errorxl(ret)
}
return
 }
-- 
2.7.4


___