xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 1/8] golang/xenlight: Create stub package
@ 2017-01-18 19:56 Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
                   ` (11 more replies)
  0 siblings, 12 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Create a basic Makefile to build and install libxenlight Golang
bindings. Also add a stub package which only opens libxl context.

Include a global xenlight.Ctx variable which can be used as the
default context by the entire program if desired.

For now, return simple errors. Proper error handling will be
added in next patch.

Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
 tools/Makefile                    | 15 +++++++-
 tools/golang/xenlight/Makefile    | 29 ++++++++++++++
 tools/golang/xenlight/xenlight.go | 80 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 tools/golang/xenlight/Makefile
 create mode 100644 tools/golang/xenlight/xenlight.go

diff --git a/tools/Makefile b/tools/Makefile
index 77e0723..fd49e7f 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -6,12 +6,13 @@ SUBDIRS-y += include
 SUBDIRS-y += libs
 SUBDIRS-y += libxc
 SUBDIRS-y += flask
-SUBDIRS-y += fuzz
 SUBDIRS-y += xenstore
 SUBDIRS-y += misc
 SUBDIRS-y += examples
 SUBDIRS-y += hotplug
 SUBDIRS-y += xentrace
+#Uncomment line to build Golang libxl
+#SUBDIRS-y += golang/xenlight
 SUBDIRS-$(CONFIG_XCUTILS) += xcutils
 SUBDIRS-$(CONFIG_X86) += firmware
 SUBDIRS-y += console
@@ -322,6 +323,18 @@ subdir-install-debugger/kdd: .phony
 subdir-all-debugger/kdd: .phony
 	$(MAKE) -C debugger/kdd all
 
+subdir-clean-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight clean
+
+subdir-distclean-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight distclean
+
+subdir-install-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight install
+
+subdir-all-golang/xenlight: .phony
+	$(MAKE) -C golang/xenlight all
+
 subdir-distclean-firmware: .phony
 	$(MAKE) -C firmware distclean
 
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
new file mode 100644
index 0000000..a45336b
--- /dev/null
+++ b/tools/golang/xenlight/Makefile
@@ -0,0 +1,29 @@
+XEN_ROOT=$(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+BINARY = xenlightgo
+GO = go
+
+.PHONY: all
+all: build
+
+.PHONY: build
+build: xenlight
+
+.PHONY: install
+install: build
+	! [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
+
+.PHONY: clean
+clean:
+	$(RM) $(BINARY)
+
+.PHONY: distclean
+distclean: clean
+
+
+xenlight: xenlight.go
+	$(GO) build -o $(BINARY) xenlight.go
+
+
+-include $(DEPS)
diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
new file mode 100644
index 0000000..1f10e51
--- /dev/null
+++ b/tools/golang/xenlight/xenlight.go
@@ -0,0 +1,80 @@
+/*
+ * Copyright (C) 2016 George W. Dunlap, Citrix Systems UK Ltd
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 of the
+ * License only.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+package xenlight
+
+/*
+#cgo LDFLAGS: -lxenlight -lyajl
+#include <stdlib.h>
+#include <libxl.h>
+*/
+import "C"
+
+import (
+	"fmt"
+	"time"
+	"unsafe"
+)
+
+/*
+ * Types: Builtins
+ */
+type Context struct {
+	ctx *C.libxl_ctx
+}
+
+/*
+ * Context
+ */
+var Ctx Context
+
+func (Ctx *Context) IsOpen() bool {
+	return Ctx.ctx != nil
+}
+
+func (Ctx *Context) Open() (err error) {
+	if Ctx.ctx != nil {
+		return
+	}
+
+	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, nil)
+
+	if ret != 0 {
+		//FIXME: proper error
+		err = createError("Allocating libxl context: ", ret)
+	}
+	return
+}
+
+func (Ctx *Context) Close() (err error) {
+	ret := C.libxl_ctx_free(unsafe.Pointer(Ctx.ctx))
+	Ctx.ctx = nil
+
+	if ret != 0 {
+		//FIXME: proper error
+		err = createError("Freeing libxl context: ", ret)
+	}
+	return
+}
+
+func (Ctx *Context) CheckOpen() (err error) {
+	if Ctx.ctx == nil {
+		err = fmt.Errorf("Context not opened")
+	}
+	return
+}
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
@ 2017-01-18 19:56 ` Ronald Rojas
  2017-01-18 22:16   ` Dario Faggioli
  2017-01-19 17:16   ` George Dunlap
  2017-01-18 19:56 ` [PATCH RFC 3/8] golang/xenlight: Add host-related functionality Ronald Rojas
                   ` (10 subsequent siblings)
  11 siblings, 2 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Create error type Errorxl for throwing proper xenlight
errors.

Update Ctx functions to throw Errorxl errors.

Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
 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(&Ctx.ctx), 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


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH RFC 3/8] golang/xenlight: Add host-related functionality
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
@ 2017-01-18 19:56 ` Ronald Rojas
  2017-01-19 17:51   ` George Dunlap
  2017-01-18 19:56 ` [PATCH RFC 4/8] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause Ronald Rojas
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Add calls for the following host-related functionality:
- libxl_get_max_cpus
- libxl_get_online_cpus
- libxl_get_max_nodes
- libxl_get_free_memory
- libxl_get_physinfo
- libxl_get_version_info

Include Golang versions of the following structs:
- libxl_physinfo as Physinfo
- libxl_version_info as VersionInfo
- libxl_hwcap as Hwcap

Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
 tools/golang/xenlight/xenlight.go | 185 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 185 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index d58f8b8..6b04850 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -109,6 +109,62 @@ type Context struct {
 	ctx *C.libxl_ctx
 }
 
+type Hwcap []C.uint32_t
+
+func hwcapCToGo(chwcap C.libxl_hwcap) (ghwcap Hwcap) {
+	// Alloc a Go slice for the bytes
+	size := 8
+	ghwcap = make([]C.uint32_t, size)
+
+	// Make a slice pointing to the C array
+	mapslice := (*[1 << 30]C.uint32_t)(unsafe.Pointer(&chwcap[0]))[:size:size]
+
+	// And copy the C array into the Go array
+	copy(ghwcap, mapslice)
+
+	return
+}
+
+/*
+ * Types: IDL
+ *
+ * FIXME: Generate these automatically from the IDL
+ */
+
+type Physinfo struct {
+	ThreadsPerCore    uint32
+	CoresPerSocket    uint32
+	MaxCpuId          uint32
+	NrCpus            uint32
+	CpuKhz            uint32
+	TotalPages        uint64
+	FreePages         uint64
+	ScrubPages        uint64
+	OutstandingPages  uint64
+	SharingFreedPages uint64
+	SharingUsedFrames uint64
+	NrNodes           uint32
+	HwCap             Hwcap
+	CapHvm            bool
+	CapHvmDirectio    bool
+}
+
+type VersionInfo struct {
+	XenVersionMajor int
+	XenVersionMinor int
+	XenVersionExtra string
+	Compiler        string
+	CompileBy       string
+	CompileDomain   string
+	CompileDate     string
+	Capabilities    string
+	Changeset       string
+	VirtStart       uint64
+	Pagesize        int
+	Commandline     string
+	BuildId         string
+}
+
 /*
  * Context
  */
@@ -147,3 +203,132 @@ func (Ctx *Context) CheckOpen() (err error) {
 	}
 	return
 }
+
+//int libxl_get_max_cpus(libxl_ctx *ctx);
+func (Ctx *Context) GetMaxCpus() (maxCpus int, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_get_max_cpus(Ctx.ctx)
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	maxCpus = int(ret)
+	return
+}
+
+//int libxl_get_online_cpus(libxl_ctx *ctx);
+func (Ctx *Context) GetOnlineCpus() (onCpus int, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_get_online_cpus(Ctx.ctx)
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	onCpus = int(ret)
+	return
+}
+
+//int libxl_get_max_nodes(libxl_ctx *ctx);
+func (Ctx *Context) GetMaxNodes() (maxNodes int, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+	ret := C.libxl_get_max_nodes(Ctx.ctx)
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	maxNodes = int(ret)
+	return
+}
+
+//int libxl_get_free_memory(libxl_ctx *ctx, uint64_t *memkb);
+func (Ctx *Context) GetFreeMemory() (memkb uint64, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+	var cmem C.uint64_t
+	ret := C.libxl_get_free_memory(Ctx.ctx, &cmem)
+
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	memkb = uint64(cmem)
+	return
+
+}
+
+//int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
+func (Ctx *Context) GetPhysinfo() (physinfo *Physinfo, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+	var cphys C.libxl_physinfo
+
+	ret := C.libxl_get_physinfo(Ctx.ctx, &cphys)
+
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	physinfo = &Physinfo{}
+	physinfo.ThreadsPerCore = uint32(cphys.threads_per_core)
+	physinfo.CoresPerSocket = uint32(cphys.cores_per_socket)
+	physinfo.MaxCpuId = uint32(cphys.max_cpu_id)
+	physinfo.NrCpus = uint32(cphys.nr_cpus)
+	physinfo.CpuKhz = uint32(cphys.cpu_khz)
+	physinfo.TotalPages = uint64(cphys.total_pages)
+	physinfo.FreePages = uint64(cphys.free_pages)
+	physinfo.ScrubPages = uint64(cphys.scrub_pages)
+	physinfo.ScrubPages = uint64(cphys.scrub_pages)
+	physinfo.SharingFreedPages = uint64(cphys.sharing_freed_pages)
+	physinfo.SharingUsedFrames = uint64(cphys.sharing_used_frames)
+	physinfo.NrNodes = uint32(cphys.nr_nodes)
+	physinfo.HwCap = hwcapCToGo(cphys.hw_cap)
+	physinfo.CapHvm = bool(cphys.cap_hvm)
+	physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)
+
+	return
+}
+
+//const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx);
+func (Ctx *Context) GetVersionInfo() (info *VersionInfo, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	var cinfo *C.libxl_version_info
+
+	cinfo = C.libxl_get_version_info(Ctx.ctx)
+
+	info = &VersionInfo{}
+	info.XenVersionMajor = int(cinfo.xen_version_major)
+	info.XenVersionMinor = int(cinfo.xen_version_minor)
+	info.XenVersionExtra = C.GoString(cinfo.xen_version_extra)
+	info.Compiler = C.GoString(cinfo.compiler)
+	info.CompileBy = C.GoString(cinfo.compile_by)
+	info.CompileDomain = C.GoString(cinfo.compile_domain)
+	info.CompileDate = C.GoString(cinfo.compile_date)
+	info.Capabilities = C.GoString(cinfo.capabilities)
+	info.Changeset = C.GoString(cinfo.changeset)
+	info.VirtStart = uint64(cinfo.virt_start)
+	info.Pagesize = int(cinfo.pagesize)
+	info.Commandline = C.GoString(cinfo.commandline)
+	info.BuildId = C.GoString(cinfo.build_id)
+
+	return
+}
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH RFC 4/8] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 3/8] golang/xenlight: Add host-related functionality Ronald Rojas
@ 2017-01-18 19:56 ` Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 5/8] golang/xenlight: Implement libxl_bitmap and helper operations Ronald Rojas
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Add calls for the following host-related functionality:
- libxl_domain_info
- libxl_domain_unpause

Include Golang version for the libxl_domain_info as
DomainInfo.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
 tools/golang/xenlight/xenlight.go | 91 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 6b04850..1e25413 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -105,6 +105,12 @@ func (e Errorxl) Error() string {
 /*
  * Types: Builtins
  */
+type Domid uint32
+
+type MemKB uint64
+
+type Uuid C.libxl_uuid
+
 type Context struct {
 	ctx *C.libxl_ctx
 }
@@ -165,6 +171,32 @@ type VersionInfo struct {
 	BuildId         string
 }
 
+type Dominfo struct {
+	Uuid      Uuid
+	Domid     Domid
+	Ssidref   uint32
+	SsidLabel string
+	Running   bool
+	Blocked   bool
+	Paused    bool
+	Shutdown  bool
+	Dying     bool
+	NeverStop bool
+
+	ShutdownReason   int32 // FIXME shutdown_reason enumeration
+	OutstandingMemkb MemKB
+	CurrentMemkb     MemKB
+	SharedMemkb      MemKB
+	PagedMemkb       MemKB
+	MaxMemkb         MemKB
+	CpuTime          time.Duration
+	VcpuMaxId        uint32
+	VcpuOnline       uint32
+	Cpupool          uint32
+	DomainType       int32 //FIXME libxl_domain_type enumeration
+
+}
+
 /*
  * Context
  */
@@ -332,3 +364,62 @@ func (Ctx *Context) GetVersionInfo() (info *VersionInfo, err error) {
 
 	return
 }
+
+func (Ctx *Context) DomainInfo(Id Domid) (di *Dominfo, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	var cdi C.libxl_dominfo
+
+	ret := C.libxl_domain_info(Ctx.ctx, unsafe.Pointer(&cdi), C.uint32_t(Id))
+
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	// We could consider having this boilerplate generated by the
+	// idl, in a function like this:
+	//
+	// di = translateCdomaininfoToGoDomaininfo(cdi)
+	di = &Dominfo{}
+	di.Uuid = Uuid(cdi.uuid)
+	di.Domid = Domid(cdi.domid)
+	di.Ssidref = uint32(cdi.ssidref)
+	di.SsidLabel = C.GoString(cdi.ssid_label)
+	di.Running = bool(cdi.running)
+	di.Blocked = bool(cdi.blocked)
+	di.Paused = bool(cdi.paused)
+	di.Shutdown = bool(cdi.shutdown)
+	di.Dying = bool(cdi.dying)
+	di.NeverStop = bool(cdi.never_stop)
+	di.ShutdownReason = int32(cdi.shutdown_reason)
+	di.OutstandingMemkb = MemKB(cdi.outstanding_memkb)
+	di.CurrentMemkb = MemKB(cdi.current_memkb)
+	di.SharedMemkb = MemKB(cdi.shared_memkb)
+	di.PagedMemkb = MemKB(cdi.paged_memkb)
+	di.MaxMemkb = MemKB(cdi.max_memkb)
+	di.CpuTime = time.Duration(cdi.cpu_time)
+	di.VcpuMaxId = uint32(cdi.vcpu_max_id)
+	di.VcpuOnline = uint32(cdi.vcpu_online)
+	di.Cpupool = uint32(cdi.cpupool)
+	di.DomainType = int32(cdi.domain_type)
+
+	return
+}
+
+func (Ctx *Context) DomainUnpause(Id Domid) (err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_domain_unpause(Ctx.ctx, C.uint32_t(Id))
+
+	if ret != 0 {
+		err = Errorxl(ret)
+	}
+	return
+}
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH RFC 5/8] golang/xenlight: Implement libxl_bitmap and helper operations
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (2 preceding siblings ...)
  2017-01-18 19:56 ` [PATCH RFC 4/8] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause Ronald Rojas
@ 2017-01-18 19:56 ` Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 6/8] tools/xenlight: Implement libxl_scheduler enumeration Ronald Rojas
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Implement Bitmap type, along with helper functions.

The Bitmap type is implemented interllay in a way which makes it
easy to copy into and out of the C libxl_bitmap type.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
 tools/golang/xenlight/xenlight.go | 167 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 167 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 1e25413..8aaca6a 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -131,6 +131,20 @@ func hwcapCToGo(chwcap C.libxl_hwcap) (ghwcap Hwcap) {
 	return
 }
 
+// typedef struct {
+//     uint32_t size;          /* number of bytes in map */
+//     uint8_t *map;
+// } libxl_bitmap;
+
+// Implement the Go bitmap type such that the underlying data can
+// easily be copied in and out.  NB that we still have to do copies
+// both directions, because cgo runtime restrictions forbid passing to
+// a C function a pointer to a Go-allocated structure which contains a
+// pointer.
+type Bitmap struct {
+	bitmap []C.uint8_t
+}
+
 /*
  * Types: IDL
  *
@@ -198,6 +212,159 @@ type Dominfo struct {
 }
 
 /*
+ * Bitmap operations
+ */
+
+// Return a Go bitmap which is a copy of the referred C bitmap.
+func bitmapCToGo(cbm C.libxl_bitmap) (gbm Bitmap) {
+	// Alloc a Go slice for the bytes
+	size := int(cbm.size)
+	gbm.bitmap = make([]C.uint8_t, size)
+
+	// Make a slice pointing to the C array
+	mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+	// And copy the C array into the Go array
+	copy(gbm.bitmap, mapslice)
+
+	return
+}
+
+// Must be C.libxl_bitmap_dispose'd of afterwards
+func bitmapGotoC(gbm Bitmap) (cbm C.libxl_bitmap) {
+	C.libxl_bitmap_init(&cbm)
+
+	size := len(gbm.bitmap)
+	cbm._map = (*C.uint8_t)(C.malloc(C.size_t(size)))
+	cbm.size = C.uint32_t(size)
+	if cbm._map == nil {
+		panic("C.calloc failed!")
+	}
+
+	// Make a slice pointing to the C array
+	mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+	// And copy the Go array into the C array
+	copy(mapslice, gbm.bitmap)
+
+	return
+}
+
+func (bm *Bitmap) Test(bit int) bool {
+	ubit := uint(bit)
+	if bit > bm.Max() || bm.bitmap == nil {
+		return false
+	}
+
+	return (bm.bitmap[bit/8] & (1 << (ubit & 7))) != 0
+}
+
+func (bm *Bitmap) Set(bit int) {
+	ibit := bit / 8
+	if ibit+1 > len(bm.bitmap) {
+		bm.bitmap = append(bm.bitmap, make([]C.uint8_t, ibit+1-len(bm.bitmap))...)
+	}
+
+	bm.bitmap[ibit] |= 1 << (uint(bit) & 7)
+}
+
+func (bm *Bitmap) SetRange(start int, end int) {
+	for i := start; i <= end; i++ {
+		bm.Set(i)
+	}
+}
+
+func (bm *Bitmap) Clear(bit int) {
+	ubit := uint(bit)
+	if bit > bm.Max() || bm.bitmap == nil {
+		return
+	}
+
+	bm.bitmap[bit/8] &= ^(1 << (ubit & 7))
+}
+
+func (bm *Bitmap) ClearRange(start int, end int) {
+	for i := start; i <= end; i++ {
+		bm.Clear(i)
+	}
+}
+
+func (bm *Bitmap) Max() int {
+	return len(bm.bitmap)*8 - 1
+}
+
+func (bm *Bitmap) IsEmpty() bool {
+	for i := 0; i < len(bm.bitmap); i++ {
+		if bm.bitmap[i] != 0 {
+			return false
+		}
+	}
+	return true
+}
+
+func (a Bitmap) And(b Bitmap) (c Bitmap) {
+	var max, min int
+	if len(a.bitmap) > len(b.bitmap) {
+		max = len(a.bitmap)
+		min = len(b.bitmap)
+	} else {
+		max = len(b.bitmap)
+		min = len(a.bitmap)
+	}
+	c.bitmap = make([]C.uint8_t, max)
+
+	for i := 0; i < min; i++ {
+		c.bitmap[i] = a.bitmap[i] & b.bitmap[i]
+	}
+	return
+}
+
+func (bm Bitmap) String() (s string) {
+	lastOnline := false
+	crange := false
+	printed := false
+	var i int
+	/// --x-xxxxx-x -> 2,4-8,10
+	/// --x-xxxxxxx -> 2,4-10
+	for i = 0; i <= bm.Max(); i++ {
+		if bm.Test(i) {
+			if !lastOnline {
+				// Switching offline -> online, print this cpu
+				if printed {
+					s += ","
+				}
+				s += fmt.Sprintf("%d", i)
+				printed = true
+			} else if !crange {
+				// last was online, but we're not in a range; print -
+				crange = true
+				s += "-"
+			} else {
+				// last was online, we're in a range,  nothing else to do
+			}
+			lastOnline = true
+		} else {
+			if lastOnline {
+				// Switching online->offline; do we need to end a range?
+				if crange {
+					s += fmt.Sprintf("%d", i-1)
+				}
+			}
+			lastOnline = false
+			crange = false
+		}
+	}
+	if lastOnline {
+		// Switching online->offline; do we need to end a range?
+		if crange {
+			s += fmt.Sprintf("%d", i-1)
+		}
+	}
+
+	return
+}
+
+/*
  * Context
  */
 var Ctx Context
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH RFC 6/8] tools/xenlight: Implement libxl_scheduler enumeration
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (3 preceding siblings ...)
  2017-01-18 19:56 ` [PATCH RFC 5/8] golang/xenlight: Implement libxl_bitmap and helper operations Ronald Rojas
@ 2017-01-18 19:56 ` Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 7/8] golang/xenlight: Implement cpupool operations Ronald Rojas
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Include both constants and a Stringification for libxl_scheduler.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
 tools/golang/xenlight/xenlight.go | 72 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 8aaca6a..64e867a 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -211,6 +211,78 @@ type Dominfo struct {
 
 }
 
+// # Consistent with values defined in domctl.h
+// # Except unknown which we have made up
+// libxl_scheduler = Enumeration("scheduler", [
+//     (0, "unknown"),
+//     (4, "sedf"),
+//     (5, "credit"),
+//     (6, "credit2"),
+//     (7, "arinc653"),
+//     (8, "rtds"),
+//     ])
+type Scheduler int
+
+var (
+	SchedulerUnknown  Scheduler = C.LIBXL_SCHEDULER_UNKNOWN
+	SchedulerSedf     Scheduler = C.LIBXL_SCHEDULER_SEDF
+	SchedulerCredit   Scheduler = C.LIBXL_SCHEDULER_CREDIT
+	SchedulerCredit2  Scheduler = C.LIBXL_SCHEDULER_CREDIT2
+	SchedulerArinc653 Scheduler = C.LIBXL_SCHEDULER_ARINC653
+	SchedulerRTDS     Scheduler = C.LIBXL_SCHEDULER_RTDS
+)
+
+// const char *libxl_scheduler_to_string(libxl_scheduler p);
+func (s Scheduler) String() string {
+	cs := C.libxl_scheduler_to_string(C.libxl_scheduler(s))
+	// No need to free const return value
+
+	return C.GoString(cs)
+}
+
+// int libxl_scheduler_from_string(const char *s, libxl_scheduler *e);
+func (s *Scheduler) FromString(gstr string) (err error) {
+	cstr := C.CString(gstr)
+	defer C.free(unsafe.Pointer(cstr))
+
+	var cs C.libxl_scheduler
+	ret := C.libxl_scheduler_from_string(cstr, &cs)
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	*s = Scheduler(cs)
+	return
+}
+
+func translateCpupoolInfoCToGo(cci C.libxl_cpupoolinfo) (gci CpupoolInfo) {
+	gci.Poolid = uint32(cci.poolid)
+	gci.PoolName = C.GoString(cci.pool_name)
+	gci.Scheduler = Scheduler(cci.sched)
+	gci.DomainCount = int(cci.n_dom)
+	gci.Cpumap = bitmapCToGo(cci.cpumap)
+
+	return
+}
+
+func SchedulerFromString(name string) (s Scheduler, err error) {
+	cname := C.CString(name)
+	defer C.free(unsafe.Pointer(cname))
+
+	var cs C.libxl_scheduler
+
+	ret := C.libxl_scheduler_from_string(cname, &cs)
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	s = Scheduler(cs)
+
+	return
+}
+
 /*
  * Bitmap operations
  */
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH RFC 7/8] golang/xenlight: Implement cpupool operations
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (4 preceding siblings ...)
  2017-01-18 19:56 ` [PATCH RFC 6/8] tools/xenlight: Implement libxl_scheduler enumeration Ronald Rojas
@ 2017-01-18 19:56 ` Ronald Rojas
  2017-01-18 19:56 ` [PATCH RFC 8/8] golang/xenlight: Add tests host related functinality functions Ronald Rojas
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Include some useful "Utility" functions:
- CpupoolFindByName
- CpupoolMakeFree

Still need to implement the following functions:
- libxl_cpupool_rename
- libxl_cpupool_cpuadd_node
- libxl_cpupool_cpuremove_node
- libxl_cpupool_movedomain
---
 tools/golang/xenlight/xenlight.go | 743 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 743 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 64e867a..215f49c 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -436,6 +436,749 @@ func (bm Bitmap) String() (s string) {
 	return
 }
 
+// libxl_cpupoolinfo = Struct("cpupoolinfo", [
+//     ("poolid",      uint32),
+//     ("pool_name",   string),
+//     ("sched",       libxl_scheduler),
+//     ("n_dom",       uint32),
+//     ("cpumap",      libxl_bitmap)
+//     ], dir=DIR_OUT)
+
+type CpupoolInfo struct {
+	Poolid      uint32
+	PoolName    string
+	Scheduler   Scheduler
+	DomainCount int
+	Cpumap      Bitmap
+}
+
+func translateCpupoolInfoCToGo(cci C.libxl_cpupoolinfo) (gci CpupoolInfo) {
+	gci.Poolid = uint32(cci.poolid)
+	gci.PoolName = C.GoString(cci.pool_name)
+	gci.Scheduler = Scheduler(cci.sched)
+	gci.DomainCount = int(cci.n_dom)
+	gci.Cpumap = bitmapCToGo(cci.cpumap)
+
+	return
+}
+
+// # Consistent with values defined in domctl.h
+// # Except unknown which we have made up
+// libxl_scheduler = Enumeration("scheduler", [
+//     (0, "unknown"),
+//     (4, "sedf"),
+//     (5, "credit"),
+//     (6, "credit2"),
+//     (7, "arinc653"),
+//     (8, "rtds"),
+//     ])
+type Scheduler int
+
+var (
+	SchedulerUnknown  Scheduler = C.LIBXL_SCHEDULER_UNKNOWN
+	SchedulerSedf     Scheduler = C.LIBXL_SCHEDULER_SEDF
+	SchedulerCredit   Scheduler = C.LIBXL_SCHEDULER_CREDIT
+	SchedulerCredit2  Scheduler = C.LIBXL_SCHEDULER_CREDIT2
+	SchedulerArinc653 Scheduler = C.LIBXL_SCHEDULER_ARINC653
+	SchedulerRTDS     Scheduler = C.LIBXL_SCHEDULER_RTDS
+)
+
+// const char *libxl_scheduler_to_string(libxl_scheduler p);
+func (s Scheduler) String() string {
+	cs := C.libxl_scheduler_to_string(C.libxl_scheduler(s))
+	// No need to free const return value
+
+	return C.GoString(cs)
+}
+
+// int libxl_scheduler_from_string(const char *s, libxl_scheduler *e);
+func (s *Scheduler) FromString(gstr string) (err error) {
+	cstr := C.CString(gstr)
+	defer C.free(unsafe.Pointer(cstr))
+
+	var cs C.libxl_scheduler
+	ret := C.libxl_scheduler_from_string(cstr, &cs)
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	*s = Scheduler(cs)
+	return
+}
+
+func translateCpupoolInfoCToGo(cci C.libxl_cpupoolinfo) (gci CpupoolInfo) {
+	gci.Poolid = uint32(cci.poolid)
+	gci.PoolName = C.GoString(cci.pool_name)
+	gci.Scheduler = Scheduler(cci.sched)
+	gci.DomainCount = int(cci.n_dom)
+	gci.Cpumap = bitmapCToGo(cci.cpumap)
+
+	return
+}
+
+func SchedulerFromString(name string) (s Scheduler, err error) {
+	cname := C.CString(name)
+	defer C.free(unsafe.Pointer(cname))
+
+	var cs C.libxl_scheduler
+
+	ret := C.libxl_scheduler_from_string(cname, &cs)
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	s = Scheduler(cs)
+
+	return
+}
+
+// typedef struct {
+//     uint32_t size;          /* number of bytes in map */
+//     uint8_t *map;
+// } libxl_bitmap;
+
+// Implement the Go bitmap type such that the underlying data can
+// easily be copied in and out.  NB that we still have to do copies
+// both directions, because cgo runtime restrictions forbid passing to
+// a C function a pointer to a Go-allocated structure which contains a
+// pointer.
+type Bitmap struct {
+	bitmap []C.uint8_t
+}
+
+/*
+ * Types: IDL
+ *
+ * FIXME: Generate these automatically from the IDL
+ */
+
+type Physinfo struct {
+	ThreadsPerCore    uint32
+	CoresPerSocket    uint32
+	MaxCpuId          uint32
+	NrCpus            uint32
+	CpuKhz            uint32
+	TotalPages        uint64
+	FreePages         uint64
+	ScrubPages        uint64
+	OutstandingPages  uint64
+	SharingFreedPages uint64
+	SharingUsedFrames uint64
+	NrNodes           uint32
+	HwCap             Hwcap
+	CapHvm            bool
+	CapHvmDirectio    bool
+}
+
+type VersionInfo struct {
+	XenVersionMajor int
+	XenVersionMinor int
+	XenVersionExtra string
+	Compiler        string
+	CompileBy       string
+	CompileDomain   string
+	CompileDate     string
+	Capabilities    string
+	Changeset       string
+	VirtStart       uint64
+	Pagesize        int
+	Commandline     string
+	BuildId         string
+}
+
+/*
+ * Bitmap operations
+ */
+
+// Return a Go bitmap which is a copy of the referred C bitmap.
+func bitmapCToGo(cbm C.libxl_bitmap) (gbm Bitmap) {
+	// Alloc a Go slice for the bytes
+	size := int(cbm.size)
+	gbm.bitmap = make([]C.uint8_t, size)
+
+	// Make a slice pointing to the C array
+	mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+	// And copy the C array into the Go array
+	copy(gbm.bitmap, mapslice)
+
+	return
+}
+
+// Must be C.libxl_bitmap_dispose'd of afterwards
+func bitmapGotoC(gbm Bitmap) (cbm C.libxl_bitmap) {
+	C.libxl_bitmap_init(&cbm)
+
+	size := len(gbm.bitmap)
+	cbm._map = (*C.uint8_t)(C.malloc(C.size_t(size)))
+	cbm.size = C.uint32_t(size)
+	if cbm._map == nil {
+		panic("C.calloc failed!")
+	}
+
+	// Make a slice pointing to the C array
+	mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+	// And copy the Go array into the C array
+	copy(mapslice, gbm.bitmap)
+
+	return
+}
+
+func (bm *Bitmap) Test(bit int) bool {
+	ubit := uint(bit)
+	if bit > bm.Max() || bm.bitmap == nil {
+		return false
+	}
+
+	return (bm.bitmap[bit/8] & (1 << (ubit & 7))) != 0
+}
+
+func (bm *Bitmap) Set(bit int) {
+	ibit := bit / 8
+	if ibit+1 > len(bm.bitmap) {
+		bm.bitmap = append(bm.bitmap, make([]C.uint8_t, ibit+1-len(bm.bitmap))...)
+	}
+
+	bm.bitmap[ibit] |= 1 << (uint(bit) & 7)
+}
+
+func (bm *Bitmap) SetRange(start int, end int) {
+	for i := start; i <= end; i++ {
+		bm.Set(i)
+	}
+}
+
+func (bm *Bitmap) Clear(bit int) {
+	ubit := uint(bit)
+	if bit > bm.Max() || bm.bitmap == nil {
+		return
+	}
+
+	bm.bitmap[bit/8] &= ^(1 << (ubit & 7))
+}
+
+func (bm *Bitmap) ClearRange(start int, end int) {
+	for i := start; i <= end; i++ {
+		bm.Clear(i)
+	}
+}
+
+func (bm *Bitmap) Max() int {
+	return len(bm.bitmap)*8 - 1
+}
+
+func (bm *Bitmap) IsEmpty() bool {
+	for i := 0; i < len(bm.bitmap); i++ {
+		if bm.bitmap[i] != 0 {
+			return false
+		}
+	}
+	return true
+}
+
+func (a Bitmap) And(b Bitmap) (c Bitmap) {
+	var max, min int
+	if len(a.bitmap) > len(b.bitmap) {
+		max = len(a.bitmap)
+		min = len(b.bitmap)
+	} else {
+		max = len(b.bitmap)
+		min = len(a.bitmap)
+	}
+	c.bitmap = make([]C.uint8_t, max)
+
+	for i := 0; i < min; i++ {
+		c.bitmap[i] = a.bitmap[i] & b.bitmap[i]
+	}
+	return
+}
+
+func (bm Bitmap) String() (s string) {
+	lastOnline := false
+	crange := false
+	printed := false
+	var i int
+	/// --x-xxxxx-x -> 2,4-8,10
+	/// --x-xxxxxxx -> 2,4-10
+	for i = 0; i <= bm.Max(); i++ {
+		if bm.Test(i) {
+			if !lastOnline {
+				// Switching offline -> online, print this cpu
+				if printed {
+					s += ","
+				}
+				s += fmt.Sprintf("%d", i)
+				printed = true
+			} else if !crange {
+				// last was online, but we're not in a range; print -
+				crange = true
+				s += "-"
+			} else {
+				// last was online, we're in a range,  nothing else to do
+			}
+			lastOnline = true
+		} else {
+			if lastOnline {
+				// Switching online->offline; do we need to end a range?
+				if crange {
+					s += fmt.Sprintf("%d", i-1)
+				}
+			}
+			lastOnline = false
+			crange = false
+		}
+	}
+	if lastOnline {
+		// Switching online->offline; do we need to end a range?
+		if crange {
+			s += fmt.Sprintf("%d", i-1)
+		}
+	}
+
+	return
+}
+
+/*
+ * Context
+ */
+var Ctx Context
+
+func (Ctx *Context) IsOpen() bool {
+	return Ctx.ctx != nil
+}
+
+func (Ctx *Context) Open() (err error) {
+	if Ctx.ctx != nil {
+		return
+	}
+
+	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, nil)
+
+	if ret != 0 {
+		err = Errorxl(ret)
+	}
+	return
+}
+
+func (Ctx *Context) Close() (err error) {
+	ret := C.libxl_ctx_free(unsafe.Pointer(Ctx.ctx))
+	Ctx.ctx = nil
+
+	if ret != 0 {
+		err = Errorxl(ret)
+	}
+	return
+}
+
+func (Ctx *Context) CheckOpen() (err error) {
+	if Ctx.ctx == nil {
+		err = fmt.Errorf("Context not opened")
+	}
+	return
+}
+
+//int libxl_get_max_cpus(libxl_ctx *ctx);
+func (Ctx *Context) GetMaxCpus() (maxCpus int, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_get_max_cpus(Ctx.ctx)
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	maxCpus = int(ret)
+	return
+}
+
+//int libxl_get_online_cpus(libxl_ctx *ctx);
+func (Ctx *Context) GetOnlineCpus() (onCpus int, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_get_online_cpus(Ctx.ctx)
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	onCpus = int(ret)
+	return
+}
+
+//int libxl_get_max_nodes(libxl_ctx *ctx);
+func (Ctx *Context) GetMaxNodes() (maxNodes int, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+	ret := C.libxl_get_max_nodes(Ctx.ctx)
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	maxNodes = int(ret)
+	return
+}
+
+//int libxl_get_free_memory(libxl_ctx *ctx, uint64_t *memkb);
+func (Ctx *Context) GetFreeMemory() (memkb uint64, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+	var cmem C.uint64_t
+	ret := C.libxl_get_free_memory(Ctx.ctx, &cmem)
+
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	memkb = uint64(cmem)
+	return
+
+}
+
+//int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo)
+func (Ctx *Context) GetPhysinfo() (physinfo *Physinfo, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+	var cphys C.libxl_physinfo
+
+	ret := C.libxl_get_physinfo(Ctx.ctx, &cphys)
+
+	if ret < 0 {
+		err = Errorxl(ret)
+		return
+	}
+	physinfo = &Physinfo{}
+	physinfo.ThreadsPerCore = uint32(cphys.threads_per_core)
+	physinfo.CoresPerSocket = uint32(cphys.cores_per_socket)
+	physinfo.MaxCpuId = uint32(cphys.max_cpu_id)
+	physinfo.NrCpus = uint32(cphys.nr_cpus)
+	physinfo.CpuKhz = uint32(cphys.cpu_khz)
+	physinfo.TotalPages = uint64(cphys.total_pages)
+	physinfo.FreePages = uint64(cphys.free_pages)
+	physinfo.ScrubPages = uint64(cphys.scrub_pages)
+	physinfo.ScrubPages = uint64(cphys.scrub_pages)
+	physinfo.SharingFreedPages = uint64(cphys.sharing_freed_pages)
+	physinfo.SharingUsedFrames = uint64(cphys.sharing_used_frames)
+	physinfo.NrNodes = uint32(cphys.nr_nodes)
+	physinfo.HwCap = hwcapCToGo(cphys.hw_cap)
+	physinfo.CapHvm = bool(cphys.cap_hvm)
+	physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)
+
+	return
+}
+
+//const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx);
+func (Ctx *Context) GetVersionInfo() (info *VersionInfo, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	var cinfo *C.libxl_version_info
+
+	cinfo = C.libxl_get_version_info(Ctx.ctx)
+
+	info = &VersionInfo{}
+	info.XenVersionMajor = int(cinfo.xen_version_major)
+	info.XenVersionMinor = int(cinfo.xen_version_minor)
+	info.XenVersionExtra = C.GoString(cinfo.xen_version_extra)
+	info.Compiler = C.GoString(cinfo.compiler)
+	info.CompileBy = C.GoString(cinfo.compile_by)
+	info.CompileDomain = C.GoString(cinfo.compile_domain)
+	info.CompileDate = C.GoString(cinfo.compile_date)
+	info.Capabilities = C.GoString(cinfo.capabilities)
+	info.Changeset = C.GoString(cinfo.changeset)
+	info.VirtStart = uint64(cinfo.virt_start)
+	info.Pagesize = int(cinfo.pagesize)
+	info.Commandline = C.GoString(cinfo.commandline)
+	info.BuildId = C.GoString(cinfo.build_id)
+
+	return
+}
+
+func (Ctx *Context) DomainInfo(Id Domid) (di *Dominfo, err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	var cdi C.libxl_dominfo
+
+	ret := C.libxl_domain_info(Ctx.ctx, unsafe.Pointer(&cdi), C.uint32_t(Id))
+
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	// We could consider having this boilerplate generated by the
+	// idl, in a function like this:
+	//
+	// di = translateCdomaininfoToGoDomaininfo(cdi)
+	di = &Dominfo{}
+	di.Uuid = Uuid(cdi.uuid)
+	di.Domid = Domid(cdi.domid)
+	di.Ssidref = uint32(cdi.ssidref)
+	di.SsidLabel = C.GoString(cdi.ssid_label)
+	di.Running = bool(cdi.running)
+	di.Blocked = bool(cdi.blocked)
+	di.Paused = bool(cdi.paused)
+	di.Shutdown = bool(cdi.shutdown)
+	di.Dying = bool(cdi.dying)
+	di.NeverStop = bool(cdi.never_stop)
+	di.ShutdownReason = int32(cdi.shutdown_reason)
+	di.OutstandingMemkb = MemKB(cdi.outstanding_memkb)
+	di.CurrentMemkb = MemKB(cdi.current_memkb)
+	di.SharedMemkb = MemKB(cdi.shared_memkb)
+	di.PagedMemkb = MemKB(cdi.paged_memkb)
+	di.MaxMemkb = MemKB(cdi.max_memkb)
+	di.CpuTime = time.Duration(cdi.cpu_time)
+	di.VcpuMaxId = uint32(cdi.vcpu_max_id)
+	di.VcpuOnline = uint32(cdi.vcpu_online)
+	di.Cpupool = uint32(cdi.cpupool)
+	di.DomainType = int32(cdi.domain_type)
+
+	return
+}
+
+func (Ctx *Context) DomainUnpause(Id Domid) (err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_domain_unpause(Ctx.ctx, C.uint32_t(Id))
+
+	if ret != 0 {
+		err = Errorxl(ret)
+	}
+	return
+}
+
+// libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool_out);
+// void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nb_pool);
+func (Ctx *Context) ListCpupool() (list []CpupoolInfo) {
+	err := Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	var nbPool C.int
+
+	c_cpupool_list := C.libxl_list_cpupool(Ctx.ctx, &nbPool)
+
+	defer C.libxl_cpupoolinfo_list_free(c_cpupool_list, nbPool)
+
+	if int(nbPool) == 0 {
+		return
+	}
+
+	// Magic
+	cpupoolListSlice := (*[1 << 30]C.libxl_cpupoolinfo)(unsafe.Pointer(c_cpupool_list))[:nbPool:nbPool]
+
+	for i := range cpupoolListSlice {
+		info := translateCpupoolInfoCToGo(cpupoolListSlice[i])
+
+		list = append(list, info)
+	}
+
+	return
+}
+
+// int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t poolid);
+func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool CpupoolInfo) {
+	err := Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	var c_cpupool C.libxl_cpupoolinfo
+
+	ret := C.libxl_cpupool_info(Ctx.ctx, &c_cpupool, C.uint32_t(Poolid))
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+	defer C.libxl_cpupoolinfo_dispose(&c_cpupool)
+
+	pool = translateCpupoolInfoCToGo(c_cpupool)
+
+	return
+}
+
+// int libxl_cpupool_create(libxl_ctx *ctx, const char *name,
+//                          libxl_scheduler sched,
+//                          libxl_bitmap cpumap, libxl_uuid *uuid,
+//                          uint32_t *poolid);
+// FIXME: uuid
+// FIXME: Setting poolid
+func (Ctx *Context) CpupoolCreate(Name string, Scheduler Scheduler, Cpumap Bitmap) (err error, Poolid uint32) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	poolid := C.uint32_t(0)
+	name := C.CString(Name)
+	defer C.free(unsafe.Pointer(name))
+
+	// For now, just do what xl does, and make a new uuid every time we create the pool
+	var uuid C.libxl_uuid
+	C.libxl_uuid_generate(&uuid)
+
+	cbm := bitmapGotoC(Cpumap)
+	defer C.libxl_bitmap_dispose(&cbm)
+
+	ret := C.libxl_cpupool_create(Ctx.ctx, name, C.libxl_scheduler(Scheduler),
+		cbm, &uuid, &poolid)
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	Poolid = uint32(poolid)
+
+	return
+}
+
+// int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
+func (Ctx *Context) CpupoolDestroy(Poolid uint32) (err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_cpupool_destroy(Ctx.ctx, C.uint32_t(Poolid))
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	return
+}
+
+// int libxl_cpupool_cpuadd(libxl_ctx *ctx, uint32_t poolid, int cpu);
+func (Ctx *Context) CpupoolCpuadd(Poolid uint32, Cpu int) (err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_cpupool_cpuadd(Ctx.ctx, C.uint32_t(Poolid), C.int(Cpu))
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	return
+}
+
+// int libxl_cpupool_cpuadd_cpumap(libxl_ctx *ctx, uint32_t poolid,
+//                                 const libxl_bitmap *cpumap);
+func (Ctx *Context) CpupoolCpuaddCpumap(Poolid uint32, Cpumap Bitmap) (err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	cbm := bitmapGotoC(Cpumap)
+	defer C.libxl_bitmap_dispose(&cbm)
+
+	ret := C.libxl_cpupool_cpuadd_cpumap(Ctx.ctx, C.uint32_t(Poolid), &cbm)
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	return
+}
+
+// int libxl_cpupool_cpuremove(libxl_ctx *ctx, uint32_t poolid, int cpu);
+func (Ctx *Context) CpupoolCpuremove(Poolid uint32, Cpu int) (err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	ret := C.libxl_cpupool_cpuremove(Ctx.ctx, C.uint32_t(Poolid), C.int(Cpu))
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	return
+}
+
+// int libxl_cpupool_cpuremove_cpumap(libxl_ctx *ctx, uint32_t poolid,
+//                                    const libxl_bitmap *cpumap);
+func (Ctx *Context) CpupoolCpuremoveCpumap(Poolid uint32, Cpumap Bitmap) (err error) {
+	err = Ctx.CheckOpen()
+	if err != nil {
+		return
+	}
+
+	cbm := bitmapGotoC(Cpumap)
+	defer C.libxl_bitmap_dispose(&cbm)
+
+	ret := C.libxl_cpupool_cpuremove_cpumap(Ctx.ctx, C.uint32_t(Poolid), &cbm)
+	if ret != 0 {
+		err = Errorxl(ret)
+		return
+	}
+
+	return
+}
+
+// int libxl_cpupool_rename(libxl_ctx *ctx, const char *name, uint32_t poolid);
+// int libxl_cpupool_cpuadd_node(libxl_ctx *ctx, uint32_t poolid, int node, int *cpus);
+// int libxl_cpupool_cpuremove_node(libxl_ctx *ctx, uint32_t poolid, int node, int *cpus);
+// int libxl_cpupool_movedomain(libxl_ctx *ctx, uint32_t poolid, uint32_t domid);
+
+//
+// Utility functions
+//
+func (Ctx *Context) CpupoolFindByName(name string) (info CpupoolInfo, found bool) {
+	plist := Ctx.ListCpupool()
+
+	for i := range plist {
+		if plist[i].PoolName == name {
+			found = true
+			info = plist[i]
+			return
+		}
+	}
+	return
+}
+
+func (Ctx *Context) CpupoolMakeFree(Cpumap Bitmap) (err error) {
+	plist := Ctx.ListCpupool()
+
+	for i := range plist {
+		var Intersection Bitmap
+		Intersection = Cpumap.And(plist[i].Cpumap)
+		if !Intersection.IsEmpty() {
+			err = Ctx.CpupoolCpuremoveCpumap(plist[i].Poolid, Intersection)
+			if err != nil {
+				return
+			}
+		}
+	}
+	return
+}
+
 /*
  * Context
  */
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH RFC 8/8] golang/xenlight: Add tests host related functinality functions
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (5 preceding siblings ...)
  2017-01-18 19:56 ` [PATCH RFC 7/8] golang/xenlight: Implement cpupool operations Ronald Rojas
@ 2017-01-18 19:56 ` Ronald Rojas
  2017-01-18 22:10 ` [PATCH RFC 1/8] golang/xenlight: Create stub package Dario Faggioli
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 19:56 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Create tests for the following functions:
- GetVersionInfo
- GetPhysinfo
- GetDominfo
- GetMaxCpus
- GetOnlineCpus
- GetMaxNodes
- GetFreeMemory

Signed-off-by: Ronald Rojas <ronladred@gmail.com>

---
To do:
-Seperate GetMaxCpus, GetOnlineCpus, GetMaxNodes, and
GetFreeMemory into seperate tests
-create tests for cpupool and bitmap operations
---
 tools/golang/xenlight/test/dominfo/Makefile        | 19 ++++++++
 tools/golang/xenlight/test/dominfo/dominfo.c       | 30 ++++++++++++
 tools/golang/xenlight/test/dominfo/dominfo.go      | 48 +++++++++++++++++++
 tools/golang/xenlight/test/funcs/Makefile          | 20 ++++++++
 tools/golang/xenlight/test/funcs/func.c            | 34 ++++++++++++++
 tools/golang/xenlight/test/funcs/func.go           | 54 ++++++++++++++++++++++
 tools/golang/xenlight/test/physinfo/Makefile       | 20 ++++++++
 tools/golang/xenlight/test/physinfo/physinfo.c     | 31 +++++++++++++
 tools/golang/xenlight/test/physinfo/physinfo.go    | 36 +++++++++++++++
 tools/golang/xenlight/test/versioninfo/Makefile    | 20 ++++++++
 .../golang/xenlight/test/versioninfo/versioninfo.c | 18 ++++++++
 .../xenlight/test/versioninfo/versioninfo.go       | 33 +++++++++++++
 12 files changed, 363 insertions(+)
 create mode 100644 tools/golang/xenlight/test/dominfo/Makefile
 create mode 100644 tools/golang/xenlight/test/dominfo/dominfo.c
 create mode 100644 tools/golang/xenlight/test/dominfo/dominfo.go
 create mode 100644 tools/golang/xenlight/test/funcs/Makefile
 create mode 100644 tools/golang/xenlight/test/funcs/func.c
 create mode 100644 tools/golang/xenlight/test/funcs/func.go
 create mode 100644 tools/golang/xenlight/test/physinfo/Makefile
 create mode 100644 tools/golang/xenlight/test/physinfo/physinfo.c
 create mode 100644 tools/golang/xenlight/test/physinfo/physinfo.go
 create mode 100644 tools/golang/xenlight/test/versioninfo/Makefile
 create mode 100644 tools/golang/xenlight/test/versioninfo/versioninfo.c
 create mode 100644 tools/golang/xenlight/test/versioninfo/versioninfo.go

diff --git a/tools/golang/xenlight/test/dominfo/Makefile b/tools/golang/xenlight/test/dominfo/Makefile
new file mode 100644
index 0000000..89ca1b3
--- /dev/null
+++ b/tools/golang/xenlight/test/dominfo/Makefile
@@ -0,0 +1,19 @@
+
+test: clean build
+	./dominfo.out >> c.output
+	go run dominfo.go >> go.output
+	if cmp -s "c.output" "go.output"; then\
+		echo "GetDominfo PASSED";\
+	else\
+		echo "GetDominfo FAILED";\
+	fi\
+
+build:
+	go build -o dominfo.a dominfo.go 
+	gcc dominfo.c -lxenlight -lyajl -o dominfo.out
+
+clean:
+	rm dominfo.a
+	rm dominfo.out
+	rm c.output
+	rm go.output
diff --git a/tools/golang/xenlight/test/dominfo/dominfo.c b/tools/golang/xenlight/test/dominfo/dominfo.c
new file mode 100644
index 0000000..ed0fe78
--- /dev/null
+++ b/tools/golang/xenlight/test/dominfo/dominfo.c
@@ -0,0 +1,30 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+
+char * bool_to_string(bool b);
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+    libxl_dominfo info;
+    int err = libxl_domain_info(context, &info, 0);
+    if(err != 0){
+        return err;
+    }
+    
+    
+	printf("%d\n%d\n", info.domid, info.ssidref);
+	//printf("%s\n", info.ssid_label);
+	printf("%s\n%s\n%s\n%s\n%s\n%s\n", bool_to_string(info.running), bool_to_string(info.blocked ), bool_to_string(info.paused ),bool_to_string(info.shutdown ),bool_to_string(info.dying), bool_to_string(info.never_stop));
+	long cpu_time = info.cpu_time/((long)1<<35);
+	printf("%d\n%lu\n%lu\n%lu\n%lu\n%lu\n%lu\n%d\n%d\n%d\n", info.shutdown_reason, info.outstanding_memkb, info.current_memkb, info.shared_memkb, info.paged_memkb, info.max_memkb, cpu_time, info.vcpu_max_id, info.vcpu_online, info.cpupool);
+	printf("%d\n", info.domain_type);
+
+
+    libxl_ctx_free(context);
+
+}
+char * bool_to_string(bool b){
+    return b ? "true": "false";
+}
diff --git a/tools/golang/xenlight/test/dominfo/dominfo.go b/tools/golang/xenlight/test/dominfo/dominfo.go
new file mode 100644
index 0000000..268b118
--- /dev/null
+++ b/tools/golang/xenlight/test/dominfo/dominfo.go
@@ -0,0 +1,48 @@
+
+package main
+
+/*
+#cgo LDFLAGS: -lxenlight -lyajl
+#include <stdlib.h>
+#include <libxl.h>
+*/
+import "C"
+import (
+	"fmt"
+	"xenlight"
+	"os"
+)
+
+func main() {
+	ctx := xenlight.Ctx
+	err := ctx.Open()
+	if err != nil{
+		os.Exit(-1)
+	}
+	defer ctx.Close()
+	info, err:= ctx.DomainInfo(0)
+	if err != nil{
+		os.Exit(-1)
+	}
+
+	fmt.Printf("%d\n%d\n", info.Domid, info.Ssidref)
+	//fmt.Printf("%s\n", info.SsidLabel)
+	fmt.Printf("%t\n%t\n%t\n%t\n%t\n%t\n", info.Running, info.Blocked, info.Paused, info.Shutdown, info.Dying, info.NeverStop)
+	cpuTime := info.CpuTime/(1<<35)
+	fmt.Printf("%d\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n", info.ShutdownReason, info.OutstandingMemkb, info.CurrentMemkb, info.SharedMemkb, info.PagedMemkb, info.MaxMemkb, cpuTime, info.VcpuMaxId, info.VcpuOnline, info.Cpupool)
+	fmt.Printf("%d\n", info.DomainType)
+
+}
+
+func replaceDomainType(n int32)(ret string){
+	switch n{
+	case -1:
+		ret = "invalid"
+	case 1:
+		ret = "hvm"
+	case 2:
+		ret = "pv"
+	}
+	return
+
+}
diff --git a/tools/golang/xenlight/test/funcs/Makefile b/tools/golang/xenlight/test/funcs/Makefile
new file mode 100644
index 0000000..d8c234c
--- /dev/null
+++ b/tools/golang/xenlight/test/funcs/Makefile
@@ -0,0 +1,20 @@
+
+test: clean build
+	./func.out >> c.output
+	go run func.go >> go.output
+	if cmp -s "c.output" "go.output"; then\
+		echo "same";\
+	else\
+		echo "diff"; \
+	fi\
+
+build:
+	go build -o func.a func.go
+	gcc func.c -lxenlight -lyajl -o func.out
+
+clean:
+	rm func.a
+	rm func.out
+	rm c.output
+	rm go.output
+
diff --git a/tools/golang/xenlight/test/funcs/func.c b/tools/golang/xenlight/test/funcs/func.c
new file mode 100644
index 0000000..3fdbf22
--- /dev/null
+++ b/tools/golang/xenlight/test/funcs/func.c
@@ -0,0 +1,34 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+    libxl_physinfo info ;
+    int err= libxl_get_physinfo(context,&info);
+    if(err != 0){
+        return err;
+    }
+
+    int max_cpus = libxl_get_max_cpus(context);
+    printf("%d\n", max_cpus);
+
+    int online_cpus = libxl_get_online_cpus(context);
+    printf("%d\n", online_cpus);
+
+    int max_nodes = libxl_get_max_nodes(context);
+    printf("%d\n", max_nodes);
+
+    uint64_t free_memory;
+    err = libxl_get_free_memory(context, &free_memory);
+    if(err < 0){
+        printf("%d\n", err);
+    }else{
+        printf("%lu\n", free_memory);
+    }
+    libxl_ctx_free(context);
+
+}
+
diff --git a/tools/golang/xenlight/test/funcs/func.go b/tools/golang/xenlight/test/funcs/func.go
new file mode 100644
index 0000000..fd83356
--- /dev/null
+++ b/tools/golang/xenlight/test/funcs/func.go
@@ -0,0 +1,54 @@
+package main
+
+/*
+#cgo LDFLAGS: -lxenlight -lyajl
+#include <stdlib.h>
+#include <libxl.h>
+*/
+import "C"
+import (
+	"fmt"
+	"os"
+	"xenlight"
+)
+
+func main() {
+	ctx := xenlight.Ctx
+	err := ctx.Open()
+	if err != nil {
+		os.Exit(-1)
+	}
+	defer ctx.Close()
+	if err != nil {
+		os.Exit(-1)
+	}
+
+	max_cpus, err := ctx.GetMaxCpus()
+	if err != nil {
+		fmt.Printf("%d\n", err)
+	} else {
+		fmt.Printf("%d\n", max_cpus)
+	}
+
+	online_cpus, err := ctx.GetOnlineCpus()
+	if err != nil {
+		fmt.Printf("%d\n", err)
+	} else {
+		fmt.Printf("%d\n", online_cpus)
+	}
+
+	max_nodes, err := ctx.GetMaxNodes()
+	if err != nil {
+		fmt.Printf("%d\n", err)
+	} else {
+		fmt.Printf("%d\n", max_nodes)
+	}
+
+	free_memory, err := ctx.GetFreeMemory()
+	if err != nil {
+		fmt.Printf("%d\n", err)
+	} else {
+		fmt.Printf("%d\n", free_memory)
+	}
+
+}
diff --git a/tools/golang/xenlight/test/physinfo/Makefile b/tools/golang/xenlight/test/physinfo/Makefile
new file mode 100644
index 0000000..cef23e7
--- /dev/null
+++ b/tools/golang/xenlight/test/physinfo/Makefile
@@ -0,0 +1,20 @@
+
+test: clean build
+	./physinfo.out >> c.output
+	go run physinfo.go >> go.output
+	if cmp -s "c.output" "go.output"; then\
+		echo "GetPhysinfo PASSED";\
+	else\
+		echo "GetPhysinfo FAILED"; \
+	fi\
+
+build:
+	go build -o physinfo.a physinfo.go 
+	gcc physinfo.c -lxenlight -lyajl -o physinfo.out
+
+clean:
+	rm physinfo.a
+	rm physinfo.out
+	rm c.output
+	rm go.output
+
diff --git a/tools/golang/xenlight/test/physinfo/physinfo.c b/tools/golang/xenlight/test/physinfo/physinfo.c
new file mode 100644
index 0000000..92c8f47
--- /dev/null
+++ b/tools/golang/xenlight/test/physinfo/physinfo.c
@@ -0,0 +1,31 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+
+char *bool_to_string(bool b);
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+    libxl_physinfo info ;
+    int err= libxl_get_physinfo(context,&info);
+    if(err != 0){
+        return err;
+    }
+
+    printf("%d\n%d\n%d\n%d\n%d\n", info.threads_per_core, info.cores_per_socket, info.max_cpu_id, info.nr_cpus, info.cpu_khz);
+    printf("%lu\n%lu\n%lu\n%lu\n%lu\n%lu\n", info.total_pages, info.free_pages, info.scrub_pages, info.outstanding_pages, info.sharing_freed_pages, info.sharing_used_frames);
+    printf("%u\n",info.nr_nodes);
+    printf("%s\n%s\n", bool_to_string(info.cap_hvm), bool_to_string(info.cap_hvm_directio));
+
+    for(int i = 0; i < 8; i++){
+        printf("%u\n", info.hw_cap[i]);
+    }
+
+    libxl_ctx_free(context);
+
+}
+
+char * bool_to_string(bool b){
+    return b ? "true": "false";
+}
diff --git a/tools/golang/xenlight/test/physinfo/physinfo.go b/tools/golang/xenlight/test/physinfo/physinfo.go
new file mode 100644
index 0000000..e0b7a74
--- /dev/null
+++ b/tools/golang/xenlight/test/physinfo/physinfo.go
@@ -0,0 +1,36 @@
+
+package main
+
+/*
+#cgo LDFLAGS: -lxenlight -lyajl
+#include <stdlib.h>
+#include <libxl.h>
+*/
+import "C"
+import (
+	"fmt"
+	"xenlight"
+	"os"
+)
+
+func main() {
+	ctx := xenlight.Ctx
+	err := ctx.Open()
+	if err != nil{
+		os.Exit(-1)
+	}
+	defer ctx.Close()
+	info, err:= ctx.GetPhysinfo()
+	if err != nil{
+		os.Exit(-1)
+	}
+
+	fmt.Printf("%d\n%d\n%d\n%d\n%d\n", info.ThreadsPerCore, info.CoresPerSocket, info.MaxCpuId, info.NrCpus, info.CpuKhz)
+	fmt.Printf("%d\n%d\n%d\n%d\n%d\n%d\n", info.TotalPages, info.FreePages, info.ScrubPages, info.OutstandingPages, info.SharingFreedPages, info.SharingUsedFrames)
+	fmt.Printf("%d\n",info.NrNodes)
+	fmt.Printf("%t\n%t\n", info.CapHvm, info.CapHvmDirectio)
+
+	for i := 0; i < 8; i++ {
+	fmt.Printf("%d\n", info.HwCap[i]);
+	}
+}
diff --git a/tools/golang/xenlight/test/versioninfo/Makefile b/tools/golang/xenlight/test/versioninfo/Makefile
new file mode 100644
index 0000000..e269e2d
--- /dev/null
+++ b/tools/golang/xenlight/test/versioninfo/Makefile
@@ -0,0 +1,20 @@
+
+test: clean build
+	./versioninfo.out >> c.output
+	go run versioninfo.go >> go.output
+	if cmp -s "c.output" "go.output"; then\
+		echo "GetVersionInfo PASSED";\
+	else\
+		echo "GetVersionInfo FAILED"; \
+	fi\
+
+build:
+	go build -o versioninfo.a versioninfo.go 
+	gcc versioninfo.c -lxenlight -lyajl -o versioninfo.out
+
+clean:
+	rm versioninfo.a
+	rm versioninfo.out
+	rm c.output
+	rm go.output
+
diff --git a/tools/golang/xenlight/test/versioninfo/versioninfo.c b/tools/golang/xenlight/test/versioninfo/versioninfo.c
new file mode 100644
index 0000000..f40495d
--- /dev/null
+++ b/tools/golang/xenlight/test/versioninfo/versioninfo.c
@@ -0,0 +1,18 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+
+main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+    libxl_version_info* info = libxl_get_version_info(context);
+
+    printf("%d\n%d\n", info->xen_version_major, info->xen_version_minor);
+    printf("%s\n%s\n%s\n%s\n%s\n%s\n%s\n", info->xen_version_extra, info->compiler, info->compile_by, info->compile_domain, info->compile_date, info->capabilities, info->changeset);
+    printf("%lu\n%d\n", info->virt_start, info->pagesize);
+    printf("%s\n%s\n", info->commandline, info->build_id);
+
+    libxl_ctx_free(context);
+
+}
diff --git a/tools/golang/xenlight/test/versioninfo/versioninfo.go b/tools/golang/xenlight/test/versioninfo/versioninfo.go
new file mode 100644
index 0000000..e455b66
--- /dev/null
+++ b/tools/golang/xenlight/test/versioninfo/versioninfo.go
@@ -0,0 +1,33 @@
+
+package main
+
+/*
+#cgo LDFLAGS: -lxenlight -lyajl
+#include <stdlib.h>
+#include <libxl.h>
+*/
+import "C"
+import (
+	"fmt"
+	"xenlight"
+	"os"
+)
+
+func main() {
+	ctx := xenlight.Ctx
+	err := ctx.Open()
+	if err != nil{
+		os.Exit(-1)
+	}
+	defer ctx.Close()
+	info, err:= ctx.GetVersionInfo()
+	if err != nil{
+		os.Exit(-1)
+	}
+
+	fmt.Printf("%d\n%d\n", info.XenVersionMajor, info.XenVersionMinor);
+	fmt.Printf("%s\n%s\n%s\n%s\n%s\n%s\n%s\n", info.XenVersionExtra, info.Compiler, info.CompileBy, info.CompileDomain, info.CompileDate, info.Capabilities, info.Changeset);
+	fmt.Printf("%d\n%d\n", info.VirtStart, info.Pagesize);
+	fmt.Printf("%s\n%s\n", info.Commandline, info.BuildId);
+
+}
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 1/8] golang/xenlight: Create stub package
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (6 preceding siblings ...)
  2017-01-18 19:56 ` [PATCH RFC 8/8] golang/xenlight: Add tests host related functinality functions Ronald Rojas
@ 2017-01-18 22:10 ` Dario Faggioli
  2017-01-18 22:37   ` Ronald Rojas
  2017-01-19 12:03 ` Wei Liu
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Dario Faggioli @ 2017-01-18 22:10 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, george.dunlap, ian.jackson, wei.liu2


[-- Attachment #1.1: Type: text/plain, Size: 790 bytes --]

Hey,

I'm afraid I can't comment on nothing, as I'm not at all into go.

But there's a thing that I've noticed while skipping the patch out of
curiosity...

On Wed, 2017-01-18 at 14:56 -0500, Ronald Rojas wrote:
> diff --git a/tools/Makefile b/tools/Makefile
> index 77e0723..fd49e7f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -6,12 +6,13 @@ SUBDIRS-y += include
>  SUBDIRS-y += libs
>  SUBDIRS-y += libxc
>  SUBDIRS-y += flask
> -SUBDIRS-y += fuzz
>
Why does this needs to be removed?

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-18 19:56 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
@ 2017-01-18 22:16   ` Dario Faggioli
  2017-01-19 15:13     ` Ronald Rojas
  2017-01-19 17:16   ` George Dunlap
  1 sibling, 1 reply; 24+ messages in thread
From: Dario Faggioli @ 2017-01-18 22:16 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, george.dunlap, ian.jackson, wei.liu2


[-- Attachment #1.1: Type: text/plain, Size: 6397 bytes --]

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 <ronladred@gmail.com>
> ---
>  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(&Ctx.ctx),
> 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 harder and longer to review (the same code
needs to be looked at, understood and, perhaps, commented twice!).

There are situations where that can't be avoided... but in this case,
it seems to me that the series can be reorganized in such a way that
errors/symbols are introduced before being used.

If that's not the case, because of Go specific characteristics that I'm
unaware of, then sorry for the noise (but then I'd say that these
specific characteristics could be at least quickly explained in a
changelog or a cover letter).

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 1/8] golang/xenlight: Create stub package
  2017-01-18 22:10 ` [PATCH RFC 1/8] golang/xenlight: Create stub package Dario Faggioli
@ 2017-01-18 22:37   ` Ronald Rojas
  0 siblings, 0 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-18 22:37 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: wei.liu2, ian.jackson, george.dunlap, xen-devel

On Wed, Jan 18, 2017 at 11:10:32PM +0100, Dario Faggioli wrote:
> Hey,
> 
> I'm afraid I can't comment on nothing, as I'm not at all into go.
> 
> But there's a thing that I've noticed while skipping the patch out of
> curiosity...
> 
> On Wed, 2017-01-18 at 14:56 -0500, Ronald Rojas wrote:
> > diff --git a/tools/Makefile b/tools/Makefile
> > index 77e0723..fd49e7f 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -6,12 +6,13 @@ SUBDIRS-y += include
> >  SUBDIRS-y += libs
> >  SUBDIRS-y += libxc
> >  SUBDIRS-y += flask
> > -SUBDIRS-y += fuzz
> >
> Why does this needs to be removed?
This was a mistake and should not have been removed. I'll
fix it in the next revision.

Thanks, 
Ronald
> 
> Dario
> -- 
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 1/8] golang/xenlight: Create stub package
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (7 preceding siblings ...)
  2017-01-18 22:10 ` [PATCH RFC 1/8] golang/xenlight: Create stub package Dario Faggioli
@ 2017-01-19 12:03 ` Wei Liu
  2017-01-19 15:15   ` Ronald Rojas
  2017-01-19 16:16 ` George Dunlap
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Wei Liu @ 2017-01-19 12:03 UTC (permalink / raw)
  To: Ronald Rojas; +Cc: wei.liu2, ian.jackson, george.dunlap, xen-devel

On Wed, Jan 18, 2017 at 02:56:39PM -0500, Ronald Rojas wrote:
  
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..a45336b
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,29 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlightgo
> +GO = go

GO ?= go

to allow overriding this command.

Other than this, I don't have further comments on the actual go code and
will let George review your series.

Wei.

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-18 22:16   ` Dario Faggioli
@ 2017-01-19 15:13     ` Ronald Rojas
  2017-01-19 16:02       ` George Dunlap
  0 siblings, 1 reply; 24+ messages in thread
From: Ronald Rojas @ 2017-01-19 15:13 UTC (permalink / raw)
  To: Dario Faggioli; +Cc: wei.liu2, ian.jackson, george.dunlap, xen-devel

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 <ronladred@gmail.com>
> > ---
> >  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(&Ctx.ctx),
> > 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 harder and longer to review (the same code
> needs to be looked at, understood and, perhaps, commented twice!).
> 
> There are situations where that can't be avoided... but in this case,
> it seems to me that the series can be reorganized in such a way that
> errors/symbols are introduced before being used.
> 
> If that's not the case, because of Go specific characteristics that I'm
> unaware of, then sorry for the noise (but then I'd say that these
> specific characteristics could be at least quickly explained in a
> changelog or a cover letter).

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.

> 
> Regards,
> Dario
> -- 
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Thanks,
Ronald

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 1/8] golang/xenlight: Create stub package
  2017-01-19 12:03 ` Wei Liu
@ 2017-01-19 15:15   ` Ronald Rojas
  0 siblings, 0 replies; 24+ messages in thread
From: Ronald Rojas @ 2017-01-19 15:15 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, george.dunlap, xen-devel

On Thu, Jan 19, 2017 at 12:03:22PM +0000, Wei Liu wrote:
> On Wed, Jan 18, 2017 at 02:56:39PM -0500, Ronald Rojas wrote:
>   
> > diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> > new file mode 100644
> > index 0000000..a45336b
> > --- /dev/null
> > +++ b/tools/golang/xenlight/Makefile
> > @@ -0,0 +1,29 @@
> > +XEN_ROOT=$(CURDIR)/../../..
> > +include $(XEN_ROOT)/tools/Rules.mk
> > +
> > +BINARY = xenlightgo
> > +GO = go
> 
> GO ?= go
> 
> to allow overriding this command.

Fixed. Thanks!
> 
> Other than this, I don't have further comments on the actual go code and
> will let George review your series.
> 
> Wei.

Thanks,
Ronald

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-19 15:13     ` Ronald Rojas
@ 2017-01-19 16:02       ` George Dunlap
  2017-01-19 16:15         ` Wei Liu
  2017-01-19 16:19         ` Dario Faggioli
  0 siblings, 2 replies; 24+ messages in thread
From: George Dunlap @ 2017-01-19 16:02 UTC (permalink / raw)
  To: Ronald Rojas, Dario Faggioli; +Cc: wei.liu2, ian.jackson, xen-devel

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 <ronladred@gmail.com>
>>> ---
>>>  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(&Ctx.ctx),
>>> 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 harder and longer to review (the same code
>> needs to be looked at, understood and, perhaps, commented twice!).
>>
>> There are situations where that can't be avoided... but in this case,
>> it seems to me that the series can be reorganized in such a way that
>> errors/symbols are introduced before being used.
>>
>> If that's not the case, because of Go specific characteristics that I'm
>> unaware of, then sorry for the noise (but then I'd say that these
>> specific characteristics could be at least quickly explained in a
>> changelog or a cover letter).
> 
> 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.  The only way to avoid this "fix" would be to merge the two
patches into one.

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; but it's Ian and Wei that have the final
word on that.

 -George


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-19 16:02       ` George Dunlap
@ 2017-01-19 16:15         ` Wei Liu
  2017-01-19 16:19         ` Dario Faggioli
  1 sibling, 0 replies; 24+ messages in thread
From: Wei Liu @ 2017-01-19 16:15 UTC (permalink / raw)
  To: George Dunlap
  Cc: Ronald Rojas, Dario Faggioli, ian.jackson, wei.liu2, xen-devel

On Thu, Jan 19, 2017 at 04:02:46PM +0000, 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 <ronladred@gmail.com>
> >>> ---
> >>>  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(&Ctx.ctx),
> >>> 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 harder and longer to review (the same code
> >> needs to be looked at, understood and, perhaps, commented twice!).
> >>
> >> There are situations where that can't be avoided... but in this case,
> >> it seems to me that the series can be reorganized in such a way that
> >> errors/symbols are introduced before being used.
> >>
> >> If that's not the case, because of Go specific characteristics that I'm
> >> unaware of, then sorry for the noise (but then I'd say that these
> >> specific characteristics could be at least quickly explained in a
> >> changelog or a cover letter).
> > 
> > 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.  The only way to avoid this "fix" would be to merge the two
> patches into one.
> 
> 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; but it's Ian and Wei that have the final
> word on that.
> 

I'm fine with it as-is. I would rather avoid folding this patch into
the previous one.

Wei.

>  -George
> 

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 1/8] golang/xenlight: Create stub package
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (8 preceding siblings ...)
  2017-01-19 12:03 ` Wei Liu
@ 2017-01-19 16:16 ` George Dunlap
  2017-01-19 16:40 ` George Dunlap
  2017-01-19 16:45 ` George Dunlap
  11 siblings, 0 replies; 24+ messages in thread
From: George Dunlap @ 2017-01-19 16:16 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, ian.jackson, wei.liu2

On 18/01/17 19:56, Ronald Rojas wrote:
> Create a basic Makefile to build and install libxenlight Golang
> bindings. Also add a stub package which only opens libxl context.
> 
> Include a global xenlight.Ctx variable which can be used as the
> default context by the entire program if desired.
> 
> For now, return simple errors. Proper error handling will be
> added in next patch.
> 
> Signed-off-by: Ronald Rojas <ronladred@gmail.com>
> ---
>  tools/Makefile                    | 15 +++++++-
>  tools/golang/xenlight/Makefile    | 29 ++++++++++++++
>  tools/golang/xenlight/xenlight.go | 80 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+), 1 deletion(-)
>  create mode 100644 tools/golang/xenlight/Makefile
>  create mode 100644 tools/golang/xenlight/xenlight.go
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 77e0723..fd49e7f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -6,12 +6,13 @@ SUBDIRS-y += include
>  SUBDIRS-y += libs
>  SUBDIRS-y += libxc
>  SUBDIRS-y += flask
> -SUBDIRS-y += fuzz
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += misc
>  SUBDIRS-y += examples
>  SUBDIRS-y += hotplug
>  SUBDIRS-y += xentrace
> +#Uncomment line to build Golang libxl
> +#SUBDIRS-y += golang/xenlight
>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>  SUBDIRS-$(CONFIG_X86) += firmware
>  SUBDIRS-y += console
> @@ -322,6 +323,18 @@ subdir-install-debugger/kdd: .phony
>  subdir-all-debugger/kdd: .phony
>  	$(MAKE) -C debugger/kdd all
>  
> +subdir-clean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight clean
> +
> +subdir-distclean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight distclean
> +
> +subdir-install-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight install
> +
> +subdir-all-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight all
> +
>  subdir-distclean-firmware: .phony
>  	$(MAKE) -C firmware distclean
>  
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..a45336b
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,29 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlightgo
> +GO = go
> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: build
> +build: xenlight
> +
> +.PHONY: install
> +install: build
> +	! [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)

This will install xenlight.go in /usr/bin.  What happened to the
GOLANG_SRC variable I had in the sample series I sent you?

> +
> +.PHONY: clean
> +clean:
> +	$(RM) $(BINARY)
> +
> +.PHONY: distclean
> +distclean: clean
> +
> +
> +xenlight: xenlight.go
> +	$(GO) build -o $(BINARY) xenlight.go
> +
> +
> +-include $(DEPS)
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> new file mode 100644
> index 0000000..1f10e51
> --- /dev/null
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (C) 2016 George W. Dunlap, Citrix Systems UK Ltd
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; version 2 of the
> + * License only.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +package xenlight
> +
> +/*
> +#cgo LDFLAGS: -lxenlight -lyajl
> +#include <stdlib.h>
> +#include <libxl.h>
> +*/
> +import "C"
> +
> +import (
> +	"fmt"
> +	"time"
> +	"unsafe"
> +)
> +
> +/*
> + * Types: Builtins
> + */
> +type Context struct {
> +	ctx *C.libxl_ctx
> +}
> +
> +/*
> + * Context
> + */
> +var Ctx Context
> +
> +func (Ctx *Context) IsOpen() bool {
> +	return Ctx.ctx != nil
> +}
> +
> +func (Ctx *Context) Open() (err error) {
> +	if Ctx.ctx != nil {
> +		return
> +	}
> +
> +	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, nil)
> +
> +	if ret != 0 {
> +		//FIXME: proper error
> +		err = createError("Allocating libxl context: ", ret)

Ah, right -- I think I see what Dario was getting at.

At the moment this won't compile, because you haven't declared
createError() anywhere.  You should try to make an effort to make each
commit in the series actually compile and have no regressions.

Since you're already including fmt, I think here I'd say:

    err = fmt.Errorf("Allocating libxl context: %d", ret)

This will compile and behave sensibly before it's replaced with the
error message.

Everything else looks good for now.  (The makefile will go through a lot
of updates once we integrate it into configure.)

 -George


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-19 16:02       ` George Dunlap
  2017-01-19 16:15         ` Wei Liu
@ 2017-01-19 16:19         ` Dario Faggioli
  1 sibling, 0 replies; 24+ messages in thread
From: Dario Faggioli @ 2017-01-19 16:19 UTC (permalink / raw)
  To: George Dunlap, Ronald Rojas; +Cc: ian.jackson, wei.liu2, xen-devel


[-- Attachment #1.1: Type: text/plain, Size: 1739 bytes --]

On Thu, 2017-01-19 at 16:02 +0000, 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
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 1/8] golang/xenlight: Create stub package
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (9 preceding siblings ...)
  2017-01-19 16:16 ` George Dunlap
@ 2017-01-19 16:40 ` George Dunlap
  2017-01-19 16:45 ` George Dunlap
  11 siblings, 0 replies; 24+ messages in thread
From: George Dunlap @ 2017-01-19 16:40 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, ian.jackson, wei.liu2

On 18/01/17 19:56, Ronald Rojas wrote:
> Create a basic Makefile to build and install libxenlight Golang
> bindings. Also add a stub package which only opens libxl context.
> 
> Include a global xenlight.Ctx variable which can be used as the
> default context by the entire program if desired.
> 
> For now, return simple errors. Proper error handling will be
> added in next patch.
> 
> Signed-off-by: Ronald Rojas <ronladred@gmail.com>
> ---
>  tools/Makefile                    | 15 +++++++-
>  tools/golang/xenlight/Makefile    | 29 ++++++++++++++
>  tools/golang/xenlight/xenlight.go | 80 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+), 1 deletion(-)
>  create mode 100644 tools/golang/xenlight/Makefile
>  create mode 100644 tools/golang/xenlight/xenlight.go
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 77e0723..fd49e7f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -6,12 +6,13 @@ SUBDIRS-y += include
>  SUBDIRS-y += libs
>  SUBDIRS-y += libxc
>  SUBDIRS-y += flask
> -SUBDIRS-y += fuzz
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += misc
>  SUBDIRS-y += examples
>  SUBDIRS-y += hotplug
>  SUBDIRS-y += xentrace
> +#Uncomment line to build Golang libxl
> +#SUBDIRS-y += golang/xenlight
>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>  SUBDIRS-$(CONFIG_X86) += firmware
>  SUBDIRS-y += console
> @@ -322,6 +323,18 @@ subdir-install-debugger/kdd: .phony
>  subdir-all-debugger/kdd: .phony
>  	$(MAKE) -C debugger/kdd all
>  
> +subdir-clean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight clean
> +
> +subdir-distclean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight distclean
> +
> +subdir-install-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight install
> +
> +subdir-all-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight all
> +
>  subdir-distclean-firmware: .phony
>  	$(MAKE) -C firmware distclean
>  
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..a45336b
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,29 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlightgo
> +GO = go
> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: build
> +build: xenlight
> +
> +.PHONY: install
> +install: build
> +	! [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
> +
> +.PHONY: clean
> +clean:
> +	$(RM) $(BINARY)
> +
> +.PHONY: distclean
> +distclean: clean
> +
> +
> +xenlight: xenlight.go
> +	$(GO) build -o $(BINARY) xenlight.go

Actually, one more thing here -- this will build using whatever version
of libxl you have installed *on the host system*, not the one you just
build as part of the build process.

You need to point the go compiler to use the version of libxl you've
just built in-tree, like this:

CGO_CFLAGS = -I$(DESTDIR)$(includedir)
CGO_LDFLAGS = -L$(DESTDIR)$(libdir) -Wl,-rpath-link=$(DESTDIR)$(libdir)
xenlight: xenlight.go
	CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" $(GO) build -o
$(BINARY) xenlight.go


 -George

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 1/8] golang/xenlight: Create stub package
  2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
                   ` (10 preceding siblings ...)
  2017-01-19 16:40 ` George Dunlap
@ 2017-01-19 16:45 ` George Dunlap
  11 siblings, 0 replies; 24+ messages in thread
From: George Dunlap @ 2017-01-19 16:45 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, ian.jackson, wei.liu2

On 18/01/17 19:56, Ronald Rojas wrote:
> Create a basic Makefile to build and install libxenlight Golang
> bindings. Also add a stub package which only opens libxl context.
> 
> Include a global xenlight.Ctx variable which can be used as the
> default context by the entire program if desired.
> 
> For now, return simple errors. Proper error handling will be
> added in next patch.
> 
> Signed-off-by: Ronald Rojas <ronladred@gmail.com>
> ---
>  tools/Makefile                    | 15 +++++++-
>  tools/golang/xenlight/Makefile    | 29 ++++++++++++++
>  tools/golang/xenlight/xenlight.go | 80 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+), 1 deletion(-)
>  create mode 100644 tools/golang/xenlight/Makefile
>  create mode 100644 tools/golang/xenlight/xenlight.go
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index 77e0723..fd49e7f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -6,12 +6,13 @@ SUBDIRS-y += include
>  SUBDIRS-y += libs
>  SUBDIRS-y += libxc
>  SUBDIRS-y += flask
> -SUBDIRS-y += fuzz
>  SUBDIRS-y += xenstore
>  SUBDIRS-y += misc
>  SUBDIRS-y += examples
>  SUBDIRS-y += hotplug
>  SUBDIRS-y += xentrace
> +#Uncomment line to build Golang libxl
> +#SUBDIRS-y += golang/xenlight
>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>  SUBDIRS-$(CONFIG_X86) += firmware
>  SUBDIRS-y += console
> @@ -322,6 +323,18 @@ subdir-install-debugger/kdd: .phony
>  subdir-all-debugger/kdd: .phony
>  	$(MAKE) -C debugger/kdd all
>  
> +subdir-clean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight clean
> +
> +subdir-distclean-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight distclean
> +
> +subdir-install-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight install
> +
> +subdir-all-golang/xenlight: .phony
> +	$(MAKE) -C golang/xenlight all
> +
>  subdir-distclean-firmware: .phony
>  	$(MAKE) -C firmware distclean
>  
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..a45336b
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,29 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlightgo
> +GO = go
> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: build
> +build: xenlight
> +
> +.PHONY: install
> +install: build
> +	! [ -f $(BINARY) ] || $(INSTALL_PROG) xenlight.go $(DESTDIR)$(bindir)
> +
> +.PHONY: clean
> +clean:
> +	$(RM) $(BINARY)
> +
> +.PHONY: distclean
> +distclean: clean
> +
> +
> +xenlight: xenlight.go
> +	$(GO) build -o $(BINARY) xenlight.go
> +
> +
> +-include $(DEPS)
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> new file mode 100644
> index 0000000..1f10e51
> --- /dev/null
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (C) 2016 George W. Dunlap, Citrix Systems UK Ltd
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; version 2 of the
> + * License only.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + * 02110-1301, USA.
> + */
> +package xenlight
> +
> +/*
> +#cgo LDFLAGS: -lxenlight -lyajl
> +#include <stdlib.h>
> +#include <libxl.h>
> +*/
> +import "C"
> +
> +import (
> +	"fmt"
> +	"time"
> +	"unsafe"
> +)

And finally -- this won't compile because we're not using the "time"
package yet.  This should be removed and added back in in the patch that
requires it.

 -George


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-18 19:56 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
  2017-01-18 22:16   ` Dario Faggioli
@ 2017-01-19 17:16   ` George Dunlap
  1 sibling, 0 replies; 24+ messages in thread
From: George Dunlap @ 2017-01-19 17:16 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, ian.jackson, wei.liu2

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 <ronladred@gmail.com>

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 (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))

You don't include the strconv package at this point.

I think since you're using fmt already, I'd just use fmt.Sprintf; and
I'd probably say, "libxl error: %d".  (Remembering to negate it again.)

> +}
> +
> +/*
>   * Types: Builtins
>   */
>  type Context struct {
> @@ -55,8 +126,7 @@ func (Ctx *Context) Open() (err error) {
>  	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), 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
>  }
> 

Hmm -- might be nice to have some golang-specific errors, like "Context
not open".  Not sure the best way to deal with that.

 -George

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 3/8] golang/xenlight: Add host-related functionality
  2017-01-18 19:56 ` [PATCH RFC 3/8] golang/xenlight: Add host-related functionality Ronald Rojas
@ 2017-01-19 17:51   ` George Dunlap
  0 siblings, 0 replies; 24+ messages in thread
From: George Dunlap @ 2017-01-19 17:51 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, ian.jackson, wei.liu2

On 18/01/17 19:56, Ronald Rojas wrote:
> Add calls for the following host-related functionality:
> - libxl_get_max_cpus
> - libxl_get_online_cpus
> - libxl_get_max_nodes
> - libxl_get_free_memory
> - libxl_get_physinfo
> - libxl_get_version_info
> 
> Include Golang versions of the following structs:
> - libxl_physinfo as Physinfo
> - libxl_version_info as VersionInfo
> - libxl_hwcap as Hwcap
> 
> Signed-off-by: Ronald Rojas <ronladred@gmail.com>

This looks good, and I think could be checked in once rebased on
previous changes.  One comment for future direction...

> ---
>  tools/golang/xenlight/xenlight.go | 185 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 185 insertions(+)
> 
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index d58f8b8..6b04850 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -109,6 +109,62 @@ type Context struct {
>  	ctx *C.libxl_ctx
>  }
>  
> +type Hwcap []C.uint32_t
> +
> +func hwcapCToGo(chwcap C.libxl_hwcap) (ghwcap Hwcap) {
> +	// Alloc a Go slice for the bytes
> +	size := 8
> +	ghwcap = make([]C.uint32_t, size)
> +
> +	// Make a slice pointing to the C array
> +	mapslice := (*[1 << 30]C.uint32_t)(unsafe.Pointer(&chwcap[0]))[:size:size]
> +
> +	// And copy the C array into the Go array
> +	copy(ghwcap, mapslice)
> +
> +	return
> +}

In my own copy of the library, I was experimenting with using methods to
do this, rather than making standalone functions that embed the name in
the type.  Something like this:

func (c C.libxl_dominfo) toGo() (g Dominfo) {
	g.Uuid = Uuid(c.uuid)
 [etc]
}

That way you can do cdi.toGo() rather than dominfoCToGo(cdi).  The
syntax looks a lot cleaner and more consistent.

What do you think?

 -George


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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-23 16:43 Ronald Rojas
@ 2017-01-23 16:43 ` Ronald Rojas
  2017-01-27 12:22   ` George Dunlap
  0 siblings, 1 reply; 24+ messages in thread
From: Ronald Rojas @ 2017-01-23 16:43 UTC (permalink / raw)
  To: xen-devel, george.dunlap, ian.jackson, wei.liu2; +Cc: Ronald Rojas

Create error type Errorxl for throwing proper xenlight
errors.

Update Ctx functions to throw Errorxl errors.

Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling
  2017-01-23 16:43 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
@ 2017-01-27 12:22   ` George Dunlap
  0 siblings, 0 replies; 24+ messages in thread
From: George Dunlap @ 2017-01-27 12:22 UTC (permalink / raw)
  To: Ronald Rojas, xen-devel, ian.jackson, wei.liu2

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@gmail.com>

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@citrix.com>



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

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2017-01-27 12:22 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-18 19:56 [PATCH RFC 1/8] golang/xenlight: Create stub package Ronald Rojas
2017-01-18 19:56 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
2017-01-18 22:16   ` Dario Faggioli
2017-01-19 15:13     ` Ronald Rojas
2017-01-19 16:02       ` George Dunlap
2017-01-19 16:15         ` Wei Liu
2017-01-19 16:19         ` Dario Faggioli
2017-01-19 17:16   ` George Dunlap
2017-01-18 19:56 ` [PATCH RFC 3/8] golang/xenlight: Add host-related functionality Ronald Rojas
2017-01-19 17:51   ` George Dunlap
2017-01-18 19:56 ` [PATCH RFC 4/8] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause Ronald Rojas
2017-01-18 19:56 ` [PATCH RFC 5/8] golang/xenlight: Implement libxl_bitmap and helper operations Ronald Rojas
2017-01-18 19:56 ` [PATCH RFC 6/8] tools/xenlight: Implement libxl_scheduler enumeration Ronald Rojas
2017-01-18 19:56 ` [PATCH RFC 7/8] golang/xenlight: Implement cpupool operations Ronald Rojas
2017-01-18 19:56 ` [PATCH RFC 8/8] golang/xenlight: Add tests host related functinality functions Ronald Rojas
2017-01-18 22:10 ` [PATCH RFC 1/8] golang/xenlight: Create stub package Dario Faggioli
2017-01-18 22:37   ` Ronald Rojas
2017-01-19 12:03 ` Wei Liu
2017-01-19 15:15   ` Ronald Rojas
2017-01-19 16:16 ` George Dunlap
2017-01-19 16:40 ` George Dunlap
2017-01-19 16:45 ` George Dunlap
  -- strict thread matches above, loose matches on Subject: below --
2017-01-23 16:43 Ronald Rojas
2017-01-23 16:43 ` [PATCH RFC 2/8] golang/xenlight: Add error constants and standard handling Ronald Rojas
2017-01-27 12:22   ` George Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).