All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/5] golang/xenlight: Create stub package
@ 2017-03-02 16:07 Ronald Rojas
  2017-03-02 16:07 ` [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling Ronald Rojas
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Ronald Rojas @ 2017-03-02 16:07 UTC (permalink / raw)
  Cc: Ronald Rojas, wei.liu2, ian.jackson, George Dunlap, xen-devel

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>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---

Changes:
- Changed GPL Lisense to LGPL Lisense

- Initialized xentoollog_logger for storing error messages

- Moved manual-enable config option to tools/Rules.mk, use
  CONFIG_GOLANG in tools/Makefile

- Added XEN_GOPATH, pointing to tools/golang

- Modified tools/golang/xenlight makefile to construct necessary $GOPATH

- Added tools/golang/Makefile, so we don't need special rules in tools
  to make tools/golang/xenlight; and so we have a single place to remove the
  $GOPATH build side-effects ($GOPATH/src and $GOPATH/pkg)

- Build of tools/golang/xenlight sets $GOPATH and does a 'go install'

- Use tree-provided CFLAGS_libxenlight and LDLIBS_libxenlight, rather
  than hard-coding our own

- Made a PKGSOURCES variable to track dependencies of all target files
  which need to be part of the output package (i.e., what's put in
  $GOPATH/src).  At the moment this is one file, but it will probably
  be more once we start using the IDL.

CC: xen-devel@lists.xen.org
CC: george.dunlap@citrix.com
CC: ian.jackson@eu.citrix.com
CC: wei.liu2@citrix.com
---
---
 tools/Makefile                    |  1 +
 tools/Rules.mk                    |  6 +++
 tools/golang/Makefile             | 27 +++++++++++++
 tools/golang/xenlight/Makefile    | 49 ++++++++++++++++++++++
 tools/golang/xenlight/xenlight.go | 85 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 168 insertions(+)
 create mode 100644 tools/golang/Makefile
 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..df1fda1 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -31,6 +31,7 @@ endif
 
 SUBDIRS-y += xenpmd
 SUBDIRS-y += libxl
+SUBDIRS-$(CONFIG_GOLANG) += golang
 SUBDIRS-y += helpers
 SUBDIRS-$(CONFIG_X86) += xenpaging
 SUBDIRS-$(CONFIG_X86) += debugger/gdbsx
diff --git a/tools/Rules.mk b/tools/Rules.mk
index b35999b..24e5220 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -30,6 +30,12 @@ XENSTORE_XENSTORED ?= y
 debug ?= y
 debug_symbols ?= $(debug)
 
+# Uncomment to compile with Go
+CONFIG_GOLANG ?= y
+ifeq ($(CONFIG_GOLANG),y)
+XEN_GOPATH        = $(XEN_ROOT)/tools/golang
+endif
+
 ifeq ($(debug_symbols),y)
 CFLAGS += -g3
 endif
diff --git a/tools/golang/Makefile b/tools/golang/Makefile
new file mode 100644
index 0000000..47a9235
--- /dev/null
+++ b/tools/golang/Makefile
@@ -0,0 +1,27 @@
+XEN_ROOT=$(CURDIR)/../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+# In order to link against a package in Go, the package must live in a
+# directory tree in the way that Go expects.  To make this possible,
+# there must be a directory such that we can set GOPATH=${dir}, and
+# the package will be under $GOPATH/src/${full-package-path}.
+
+# So we set XEN_GOPATH to $XEN_ROOT/tools/golang.  The xenlight
+# "package build" directory ($PWD/xenlight) will create the "package
+# source" directory in the proper place.  Go programs can use this
+# package by setting GOPATH=$(XEN_GOPATH).
+
+SUBDIRS-y = xenlight
+
+.PHONY: build all
+all build: subdirs-all
+
+.PHONY: install
+install: subdirs-install
+
+.PHONY: clean
+clean: subdirs-clean
+	$(RM) -r src pkg
+
+.PHONY: distclean
+distclean: clean
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
new file mode 100644
index 0000000..5d578f3
--- /dev/null
+++ b/tools/golang/xenlight/Makefile
@@ -0,0 +1,49 @@
+XEN_ROOT=$(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+# Standing boldly against convention, we insist on installing the
+# package source under $(prefix)/share/gocode
+GOCODE_DIR ?= $(prefix)/share/gocode/
+GOXL_PKG_DIR = /src/xenproject.org/xenlight/
+GOXL_INSTALL_DIR = $(GOCODE_DIR)$(GOXL_PKG_DIR)
+
+# PKGSOURCES: Files which comprise the distributed source package
+PKGSOURCES = xenlight.go
+
+GO ?= go
+
+.PHONY: all
+all: build
+
+.PHONY: package
+package: $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES)
+
+$(XEN_GOPATH)/src/xenproject.org/xenlight/$(PKGSOURCES): $(PKGSOURCES)
+	$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
+	$(INSTALL_DATA) $(PKGSOURCES) $(XEN_GOPATH)$(GOXL_PKG_DIR)
+
+# Go will do its own dependency checking, and not actuall go through
+# with the build if none of the input files have changed.
+#
+# NB that because the users of this library need to be able to
+# recompile the library from source, it needs to include '-lxenlight'
+# in the LDFLAGS; and thus we need to add -L$(XEN_XENLIGHT) here
+# so that it can find the actual library.
+.PHONY: build
+build: package
+	CGO_CFLAGS="$(CFLAGS_libxenlight)" CGO_LDFLAGS="$(LDLIBS_libxenlight) -L$(XEN_XENLIGHT)" GOPATH=$(XEN_GOPATH) $(GO) install -x xenproject.org/xenlight
+
+.PHONY: install
+install: build
+	$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
+	$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES) $(DESTDIR)$(GOXL_INSTALL_DIR)
+
+.PHONY: clean
+clean:
+	$(RM) -r $(XEN_GOPATH)$(GOXL_PKG_DIR)
+	$(RM) $(XEN_GOPATH)/pkg/*/xenproject.org/xenlight.a
+
+.PHONY: distclean
+distclean: clean
+
+-include $(DEPS)
diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
new file mode 100644
index 0000000..0a0cea2
--- /dev/null
+++ b/tools/golang/xenlight/xenlight.go
@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2016 George W. Dunlap, Citrix Systems UK Ltd
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see <http://www.gnu.org/licenses/>.
+ */
+package xenlight
+
+/*
+#cgo LDFLAGS: -lxenlight -lyajl -lxentoollog
+#include <stdlib.h>
+#include <libxl.h>
+*/
+import "C"
+
+/*
+ * Other flags that may be needed at some point:
+ *  -lnl-route-3 -lnl-3
+ *
+ * To get back to static linking:
+ * #cgo LDFLAGS: -lxenlight -lyajl_s -lxengnttab -lxenstore -lxenguest -lxentoollog -lxenevtchn -lxenctrl -lblktapctl -lxenforeignmemory -lxencall -lz -luuid -lutil
+*/
+
+import (
+	"fmt"
+	"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
+	}
+
+	logger := C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0);
+	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, unsafe.Pointer(logger))
+
+	if ret != 0 {
+		err = fmt.Errorf("Error: %d", -ret)
+	}
+	return
+}
+
+func (Ctx *Context) Close() (err error) {
+	ret := C.libxl_ctx_free(unsafe.Pointer(Ctx.ctx))
+	Ctx.ctx = nil
+
+	if ret != 0 {
+		err = fmt.Errorf("Error: %d", -ret)
+	}
+	return
+}
+
+func (Ctx *Context) CheckOpen() (err error) {
+	if Ctx.ctx == nil {
+		err = fmt.Errorf("Context not opened")
+	}
+	return
+}
-- 
2.7.3


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

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

* [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling
  2017-03-02 16:07 [PATCH v2 1/5] golang/xenlight: Create stub package Ronald Rojas
@ 2017-03-02 16:07 ` Ronald Rojas
  2017-03-03 14:47   ` George Dunlap
  2017-03-02 16:07 ` [PATCH v2 3/5] golang/xenlight: Add host-related functionality Ronald Rojas
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Ronald Rojas @ 2017-03-02 16:07 UTC (permalink / raw)
  Cc: Ronald Rojas, wei.liu2, ian.jackson, george.dunlap, xen-devel

Create error type Errorxl for throwing proper xenlight
errors.

Update Ctx functions to throw Errorxl errors.

Signed-off-by: Ronald Rojas <ronladred@gmail.com>
---
Changes since last patch:

- Whitespace fixes

CC: xen-devel@lists.xen.org
CC: george.dunlap@citrix.com
CC: ian.jackson@eu.citrix.com
CC: wei.liu2@citrix.com
---
---
 tools/golang/xenlight/xenlight.go | 81 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 77 insertions(+), 4 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 0a0cea2..cbd3527 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -29,17 +29,79 @@ import "C"
  *
  * To get back to static linking:
  * #cgo LDFLAGS: -lxenlight -lyajl_s -lxengnttab -lxenstore -lxenguest -lxentoollog -lxenevtchn -lxenctrl -lblktapctl -lxenforeignmemory -lxencall -lz -luuid -lutil
-*/
+ */
 
 import (
 	"fmt"
 	"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
 }
@@ -49,6 +111,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
 }
@@ -58,11 +131,11 @@ func (Ctx *Context) Open() (err error) {
 		return
 	}
 
-	logger := C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0);
+	logger := C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
 	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, unsafe.Pointer(logger))
 
 	if ret != 0 {
-		err = fmt.Errorf("Error: %d", -ret)
+		err = Error(-ret)
 	}
 	return
 }
@@ -72,7 +145,7 @@ func (Ctx *Context) Close() (err error) {
 	Ctx.ctx = nil
 
 	if ret != 0 {
-		err = fmt.Errorf("Error: %d", -ret)
+		err = Error(-ret)
 	}
 	return
 }
-- 
2.7.3


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

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

* [PATCH v2 3/5] golang/xenlight: Add host-related functionality
  2017-03-02 16:07 [PATCH v2 1/5] golang/xenlight: Create stub package Ronald Rojas
  2017-03-02 16:07 ` [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling Ronald Rojas
@ 2017-03-02 16:07 ` Ronald Rojas
  2017-03-03 14:54   ` George Dunlap
  2017-03-02 16:07 ` [PATCH v2 4/5] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause Ronald Rojas
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Ronald Rojas @ 2017-03-02 16:07 UTC (permalink / raw)
  Cc: Ronald Rojas, wei.liu2, ian.jackson, george.dunlap, xen-devel

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>
---
Changes since last version

- Refactored creating Physinfo and VersionInfo types into
seperate toGo() functions.

- Used libxl_<type>_init() and libxl_<type>_dispose() on IDL
type physinfo

- Whitespace fixes

CC: xen-devel@lists.xen.org
CC: george.dunlap@citrix.com
CC: ian.jackson@eu.citrix.com
CC: wei.liu2@citrix.com
---
---
 tools/golang/xenlight/xenlight.go | 200 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 200 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index cbd3527..63cc805 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -106,6 +106,103 @@ type Context struct {
 	ctx *C.libxl_ctx
 }
 
+type Hwcap []C.uint32_t
+
+func (chwcap C.libxl_hwcap) CToGo() (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
+}
+
+func (cphys *C.libxl_physinfo) toGo() (physinfo *Physinfo) {
+
+	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 = cphys.hw_cap.CToGo()
+	physinfo.CapHvm = bool(cphys.cap_hvm)
+	physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)
+
+	return
+}
+
+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
+}
+
+func (cinfo *C.libxl_version_info) toGo() (info *VersionInfo) {
+	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
+}
+
 /*
  * Context
  */
@@ -156,3 +253,106 @@ 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 = Error(-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 = Error(-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 = Error(-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 = Error(-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
+	C.libxl_physinfo_init(&cphys)
+
+	ret := C.libxl_get_physinfo(Ctx.ctx, &cphys)
+
+	if ret < 0 {
+		err = Error(ret)
+		return
+	}
+	physinfo = cphys.toGo()
+	C.libxl_physinfo_dispose(&cphys)
+
+	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 = cinfo.toGo()
+
+	return
+}
-- 
2.7.3


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

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

* [PATCH v2 4/5] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause
  2017-03-02 16:07 [PATCH v2 1/5] golang/xenlight: Create stub package Ronald Rojas
  2017-03-02 16:07 ` [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling Ronald Rojas
  2017-03-02 16:07 ` [PATCH v2 3/5] golang/xenlight: Add host-related functionality Ronald Rojas
@ 2017-03-02 16:07 ` Ronald Rojas
  2017-03-03 15:15   ` George Dunlap
  2017-03-02 16:07 ` [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions Ronald Rojas
  2017-03-03 14:42 ` [PATCH v2 1/5] golang/xenlight: Create stub package George Dunlap
  4 siblings, 1 reply; 20+ messages in thread
From: Ronald Rojas @ 2017-03-02 16:07 UTC (permalink / raw)
  Cc: Ronald Rojas, wei.liu2, ian.jackson, George Dunlap, xen-devel

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>
---
Changes since last version
- Created type and enumeration of DomainType and ShutdownReason

- Created String() method for DomainType and ShutdownReason

- Refactored creating DomainInfo from c type into seperate
toGo() function

- Applied libxl_domain_info_init/dispose()

- whitespace fixes

CC: xen-devel@lists.xen.org
CC: george.dunlap@citrix.com
CC: ian.jackson@eu.citrix.com
CC: wei.liu2@citrix.com

---
---
 tools/golang/xenlight/xenlight.go | 132 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 63cc805..18dedcb 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -34,6 +34,7 @@ import "C"
 import (
 	"fmt"
 	"unsafe"
+	"time"
 )
 
 /*
@@ -102,6 +103,12 @@ var errors = [...]string{
  * Types: Builtins
  */
 
+type Domid uint32
+
+type MemKB uint64
+
+type Uuid C.libxl_uuid
+
 type Context struct {
 	ctx *C.libxl_ctx
 }
@@ -203,6 +210,95 @@ func (cinfo *C.libxl_version_info) toGo() (info *VersionInfo) {
 	return
 }
 
+type ShutdownReason int32
+
+const(
+	ShutdownReasonUnknown = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_UNKNOWN)
+	ShutdownReasonPoweroff = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_POWEROFF)
+	ShutdownReasonReboot = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_REBOOT)
+	ShutdownReasonSuspend = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_SUSPEND)
+	ShutdownReasonCrash = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_CRASH)
+	ShutdownReasonWatchdog = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_WATCHDOG)
+	ShutdownReasonSoftReset = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_SOFT_RESET)
+
+)
+
+func (sr ShutdownReason) String()(str string){
+	cstr := C.libxl_shutdown_reason_to_string(C.libxl_shutdown_reason(sr))
+	str = C.GoString(cstr)
+
+	return
+}
+
+type DomainType int32
+
+const(
+	DomainTypeInvalid = DomainType(C.LIBXL_DOMAIN_TYPE_INVALID)
+	DomainTypeHvm = DomainType(C.LIBXL_DOMAIN_TYPE_HVM)
+	DomainTypePv = DomainType(C.LIBXL_DOMAIN_TYPE_PV)
+)
+
+func (dt DomainType) String()(str string){
+	cstr := C.libxl_domain_type_to_string(C.libxl_domain_type(dt))
+	str = C.GoString(cstr)
+
+	return
+}
+
+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
+	OutstandingMemkb MemKB
+	CurrentMemkb     MemKB
+	SharedMemkb      MemKB
+	PagedMemkb       MemKB
+	MaxMemkb         MemKB
+	CpuTime          time.Duration
+	VcpuMaxId       uint32
+	VcpuOnline       uint32
+	Cpupool           uint32
+	DomainType       int32
+
+}
+
+func (cdi *C.libxl_dominfo) toGo()(di *Dominfo){
+
+	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
+}
+
 /*
  * Context
  */
@@ -356,3 +452,39 @@ 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
+	C.libxl_dominfo_init(&cdi)
+
+	ret := C.libxl_domain_info(Ctx.ctx, unsafe.Pointer(&cdi), C.uint32_t(Id))
+
+	if ret != 0 {
+		err = Error(-ret)
+		return
+	}
+
+	di = cdi.toGo()
+	C.libxl_dominfo_dispose(&cdi)
+
+	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 = Error(-ret)
+	}
+	return
+}
-- 
2.7.3


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

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

* [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 16:07 [PATCH v2 1/5] golang/xenlight: Create stub package Ronald Rojas
                   ` (2 preceding siblings ...)
  2017-03-02 16:07 ` [PATCH v2 4/5] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause Ronald Rojas
@ 2017-03-02 16:07 ` Ronald Rojas
  2017-03-02 17:36   ` Ian Jackson
  2017-03-03 16:20   ` George Dunlap
  2017-03-03 14:42 ` [PATCH v2 1/5] golang/xenlight: Create stub package George Dunlap
  4 siblings, 2 replies; 20+ messages in thread
From: Ronald Rojas @ 2017-03-02 16:07 UTC (permalink / raw)
  Cc: Ronald Rojas, wei.liu2, ian.jackson, George Dunlap, xen-devel

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

Signed-off-by: Ronald Rojas <ronladred@gmail.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>

---

- Removed individual Makefiles for each test for a single
Makefile to test all files at once

- Changed all tests to xeninfo directory

- Applied libxl_<type>_{init/dispose} for IDL types in tests

- Applied formating fixes

CC: xen-devel@lists.xen.org
CC: george.dunlap@citrix.com
CC: ian.jackson@eu.citrix.com
CC: wei.liu2@citrix.com
---
---
 tools/golang/xenlight/test/xeninfo/Makefile       | 34 +++++++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/dominfo.c      | 31 +++++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/dominfo.go     | 31 +++++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/freememory.c   | 24 ++++++++++++++++
 tools/golang/xenlight/test/xeninfo/freememory.go  | 28 +++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/maxcpu.c       | 16 +++++++++++
 tools/golang/xenlight/test/xeninfo/maxcpu.go      | 27 ++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/maxnodes.c     | 16 +++++++++++
 tools/golang/xenlight/test/xeninfo/maxnodes.go    | 27 ++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/onlinecpu.c    | 16 +++++++++++
 tools/golang/xenlight/test/xeninfo/onlinecpu.go   | 27 ++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/physinfo.c     | 31 +++++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/physinfo.go    | 32 +++++++++++++++++++++
 tools/golang/xenlight/test/xeninfo/print.h        |  3 ++
 tools/golang/xenlight/test/xeninfo/versioninfo.c  | 20 +++++++++++++
 tools/golang/xenlight/test/xeninfo/versioninfo.go | 28 +++++++++++++++++++
 16 files changed, 391 insertions(+)
 create mode 100644 tools/golang/xenlight/test/xeninfo/Makefile
 create mode 100644 tools/golang/xenlight/test/xeninfo/dominfo.c
 create mode 100644 tools/golang/xenlight/test/xeninfo/dominfo.go
 create mode 100644 tools/golang/xenlight/test/xeninfo/freememory.c
 create mode 100644 tools/golang/xenlight/test/xeninfo/freememory.go
 create mode 100644 tools/golang/xenlight/test/xeninfo/maxcpu.c
 create mode 100644 tools/golang/xenlight/test/xeninfo/maxcpu.go
 create mode 100644 tools/golang/xenlight/test/xeninfo/maxnodes.c
 create mode 100644 tools/golang/xenlight/test/xeninfo/maxnodes.go
 create mode 100644 tools/golang/xenlight/test/xeninfo/onlinecpu.c
 create mode 100644 tools/golang/xenlight/test/xeninfo/onlinecpu.go
 create mode 100644 tools/golang/xenlight/test/xeninfo/physinfo.c
 create mode 100644 tools/golang/xenlight/test/xeninfo/physinfo.go
 create mode 100644 tools/golang/xenlight/test/xeninfo/print.h
 create mode 100644 tools/golang/xenlight/test/xeninfo/versioninfo.c
 create mode 100644 tools/golang/xenlight/test/xeninfo/versioninfo.go

diff --git a/tools/golang/xenlight/test/xeninfo/Makefile b/tools/golang/xenlight/test/xeninfo/Makefile
new file mode 100644
index 0000000..859e4d6
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/Makefile
@@ -0,0 +1,34 @@
+XEN_ROOT = $(CURDIR)/../../../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+GO ?= go
+
+TESTS = dominfo freememory maxcpu onlinecpu physinfo versioninfo
+CBINARIES = $(TESTS:%=%-c)
+GOBINARIES = $(TESTS:%=%-go)
+
+all: build
+
+test: clean build
+	for test in $(TESTS) ; do \
+		./$$test-c >> c.output ; \
+	        ./$$test-go >> go.output ; \
+		if cmp -s "c.output" "go.output"; then\
+			echo "$$test PASSED";\
+		else \
+			echo "$$test FAILED";\
+		fi ; \
+	done
+
+build: $(CBINARIES) $(GOBINARIES)
+
+%-c: %.c
+	gcc $(CFLAGS_libxenlight) -o $@ $< $(LDLIBS_libxenlight)
+
+%-go: %.go
+	GOPATH=$(XEN_ROOT)/tools/golang $(GO) build -o $@ $<
+
+clean:
+	rm -f *-c
+	rm -f *-go
+	rm -f *.output
diff --git a/tools/golang/xenlight/test/xeninfo/dominfo.c b/tools/golang/xenlight/test/xeninfo/dominfo.c
new file mode 100644
index 0000000..85f658d
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/dominfo.c
@@ -0,0 +1,31 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+#include "print.h"
+
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+    libxl_dominfo info;
+    libxl_dominfo_init(&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%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_dominfo_dispose(&info);
+	libxl_ctx_free(context);
+
+}
diff --git a/tools/golang/xenlight/test/xeninfo/dominfo.go b/tools/golang/xenlight/test/xeninfo/dominfo.go
new file mode 100644
index 0000000..bb9257b
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/dominfo.go
@@ -0,0 +1,31 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"xenproject.org/xenlight"
+)
+
+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)
+
+}
diff --git a/tools/golang/xenlight/test/xeninfo/freememory.c b/tools/golang/xenlight/test/xeninfo/freememory.c
new file mode 100644
index 0000000..04ee90d
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/freememory.c
@@ -0,0 +1,24 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+#include "print.h"
+
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+
+    uint64_t free_memory;
+    int 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/xeninfo/freememory.go b/tools/golang/xenlight/test/xeninfo/freememory.go
new file mode 100644
index 0000000..69a7723
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/freememory.go
@@ -0,0 +1,28 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"xenproject.org/xenlight"
+)
+
+func main() {
+	ctx := xenlight.Ctx
+	err := ctx.Open()
+	if err != nil {
+		os.Exit(-1)
+	}
+
+	defer ctx.Close()
+	if err != nil {
+		os.Exit(-1)
+	}
+
+	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/xeninfo/maxcpu.c b/tools/golang/xenlight/test/xeninfo/maxcpu.c
new file mode 100644
index 0000000..6ba4a87
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/maxcpu.c
@@ -0,0 +1,16 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+
+    int max_cpus = libxl_get_max_cpus(context);
+    printf("%d\n", max_cpus);
+
+    libxl_ctx_free(context);
+
+}
+
diff --git a/tools/golang/xenlight/test/xeninfo/maxcpu.go b/tools/golang/xenlight/test/xeninfo/maxcpu.go
new file mode 100644
index 0000000..f847c0d
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/maxcpu.go
@@ -0,0 +1,27 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"xenproject.org/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)
+	}
+
+}
diff --git a/tools/golang/xenlight/test/xeninfo/maxnodes.c b/tools/golang/xenlight/test/xeninfo/maxnodes.c
new file mode 100644
index 0000000..0e79c8d
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/maxnodes.c
@@ -0,0 +1,16 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+
+    int max_nodes = libxl_get_max_nodes(context);
+    printf("%d\n", max_nodes);
+
+   libxl_ctx_free(context);
+
+}
+
diff --git a/tools/golang/xenlight/test/xeninfo/maxnodes.go b/tools/golang/xenlight/test/xeninfo/maxnodes.go
new file mode 100644
index 0000000..906e596
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/maxnodes.go
@@ -0,0 +1,27 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"xenproject.org/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_nodes, err := ctx.GetMaxNodes()
+	if err != nil {
+		fmt.Printf("%d\n", err)
+	} else {
+		fmt.Printf("%d\n", max_nodes)
+	}
+
+}
diff --git a/tools/golang/xenlight/test/xeninfo/onlinecpu.c b/tools/golang/xenlight/test/xeninfo/onlinecpu.c
new file mode 100644
index 0000000..8da3414
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/onlinecpu.c
@@ -0,0 +1,16 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+
+    int online_cpus = libxl_get_online_cpus(context);
+    printf("%d\n", online_cpus);
+
+    libxl_ctx_free(context);
+
+}
+
diff --git a/tools/golang/xenlight/test/xeninfo/onlinecpu.go b/tools/golang/xenlight/test/xeninfo/onlinecpu.go
new file mode 100644
index 0000000..74323c4
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/onlinecpu.go
@@ -0,0 +1,27 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"xenproject.org/xenlight"
+)
+
+func main() {
+	ctx := xenlight.Ctx
+	err := ctx.Open()
+	if err != nil {
+		os.Exit(-1)
+	}
+	defer ctx.Close()
+	if err != nil {
+		os.Exit(-1)
+	}
+
+	online_cpus, err := ctx.GetOnlineCpus()
+	if err != nil {
+		fmt.Printf("%d\n", err)
+	} else {
+		fmt.Printf("%d\n", online_cpus)
+	}
+
+}
diff --git a/tools/golang/xenlight/test/xeninfo/physinfo.c b/tools/golang/xenlight/test/xeninfo/physinfo.c
new file mode 100644
index 0000000..ba3aa44
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/physinfo.c
@@ -0,0 +1,31 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <libxl.h>
+#include "print.h"
+
+int main(){
+
+    libxl_ctx *context;
+    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
+    libxl_physinfo info;
+    libxl_physinfo_init(&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));
+
+    int i;
+    for(i = 0; i < 8; i++){
+        printf("%u\n", info.hw_cap[i]);
+    }
+
+    libxl_physinfo_init(&info);
+    libxl_ctx_free(context);
+
+}
+
diff --git a/tools/golang/xenlight/test/xeninfo/physinfo.go b/tools/golang/xenlight/test/xeninfo/physinfo.go
new file mode 100644
index 0000000..cf7bdd4
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/physinfo.go
@@ -0,0 +1,32 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"xenproject.org/xenlight"
+)
+
+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/xeninfo/print.h b/tools/golang/xenlight/test/xeninfo/print.h
new file mode 100644
index 0000000..6aaa29c
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/print.h
@@ -0,0 +1,3 @@
+char* bool_to_string(a){
+       	return (a? "true" : "false");
+}
diff --git a/tools/golang/xenlight/test/xeninfo/versioninfo.c b/tools/golang/xenlight/test/xeninfo/versioninfo.c
new file mode 100644
index 0000000..6d807c0
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/versioninfo.c
@@ -0,0 +1,20 @@
+#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/xeninfo/versioninfo.go b/tools/golang/xenlight/test/xeninfo/versioninfo.go
new file mode 100644
index 0000000..5545472
--- /dev/null
+++ b/tools/golang/xenlight/test/xeninfo/versioninfo.go
@@ -0,0 +1,28 @@
+package main
+
+import (
+	"fmt"
+	"os"
+	"xenproject.org/xenlight"
+)
+
+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.3


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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 16:07 ` [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions Ronald Rojas
@ 2017-03-02 17:36   ` Ian Jackson
  2017-03-02 17:53     ` George Dunlap
  2017-03-03 16:20   ` George Dunlap
  1 sibling, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2017-03-02 17:36 UTC (permalink / raw)
  To: Ronald Rojas; +Cc: wei.liu2, George Dunlap, xen-devel

Ronald Rojas writes ("[PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
> Create tests for the following functions:
> - GetVersionInfo
> - GetPhysinfo
> - GetDominfo
> - GetMaxCpus
> - GetOnlineCpus
> - GetMaxNodes
> - GetFreeMemory

I assume this whole series is RFC still ?

I don't feel competent to review the golang code.  But I did spot some
C to review :-)

> diff --git a/tools/golang/xenlight/test/xeninfo/freememory.c b/tools/golang/xenlight/test/xeninfo/freememory.c
> new file mode 100644
> index 0000000..04ee90d
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/freememory.c
> @@ -0,0 +1,24 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +#include "print.h"
> +
> +int main(){

This is an unusual definition of main.  (I think it's still legal, but
probably not what you meant.)

> +    libxl_ctx *context;
> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
> +
> +    uint64_t free_memory;
> +    int err = libxl_get_free_memory(context, &free_memory);
> +    if (err < 0)
> +    {
> +        printf("%d\n", err);
> +    }
> +    else
> +    {
> +        printf("%lu\n", free_memory);
> +    }

This output is ambiguous.

> +    libxl_ctx_free(context);
> +
> +}

Returns from main without returning a value.  Error code is lost.

Thanks,
Ian.

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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 17:36   ` Ian Jackson
@ 2017-03-02 17:53     ` George Dunlap
  2017-03-02 17:55       ` Ian Jackson
  2017-03-02 18:22       ` Ronald Rojas
  0 siblings, 2 replies; 20+ messages in thread
From: George Dunlap @ 2017-03-02 17:53 UTC (permalink / raw)
  To: Ian Jackson, Ronald Rojas; +Cc: wei.liu2, xen-devel

On 02/03/17 17:36, Ian Jackson wrote:
> Ronald Rojas writes ("[PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
>> Create tests for the following functions:
>> - GetVersionInfo
>> - GetPhysinfo
>> - GetDominfo
>> - GetMaxCpus
>> - GetOnlineCpus
>> - GetMaxNodes
>> - GetFreeMemory
> 
> I assume this whole series is RFC still ?

I think the earlier patches looked pretty close to being checked in.  I
think having a basic chunk of functionality checked in will make it
easier to actually collaborate on improving things.

> I don't feel competent to review the golang code.  But I did spot some
> C to review :-)
> 
>> diff --git a/tools/golang/xenlight/test/xeninfo/freememory.c b/tools/golang/xenlight/test/xeninfo/freememory.c
>> new file mode 100644
>> index 0000000..04ee90d
>> --- /dev/null
>> +++ b/tools/golang/xenlight/test/xeninfo/freememory.c
>> @@ -0,0 +1,24 @@
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <libxl.h>
>> +#include "print.h"
>> +
>> +int main(){
> 
> This is an unusual definition of main.  (I think it's still legal, but
> probably not what you meant.)
> 
>> +    libxl_ctx *context;
>> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
>> +
>> +    uint64_t free_memory;
>> +    int err = libxl_get_free_memory(context, &free_memory);
>> +    if (err < 0)
>> +    {
>> +        printf("%d\n", err);
>> +    }
>> +    else
>> +    {
>> +        printf("%lu\n", free_memory);
>> +    }
> 
> This output is ambiguous.
> 
>> +    libxl_ctx_free(context);
>> +
>> +}
> 
> Returns from main without returning a value.  Error code is lost.

He's not testing whether the call succeeds; he's testing if the call has
the same result as the golang function.

But this won't quite work anymore, because as of v2 the golang return
values are positive constants (to make it easy to use them for indexes).
 So if there were an identical error, the output would be different even
if the error number were identical.

That needs to be fixed; but I also agree it would probably be better to
print something that distinguishes between success and failure.

 -George

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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 17:53     ` George Dunlap
@ 2017-03-02 17:55       ` Ian Jackson
  2017-03-02 18:01         ` George Dunlap
  2017-03-02 18:22       ` Ronald Rojas
  1 sibling, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2017-03-02 17:55 UTC (permalink / raw)
  To: George Dunlap; +Cc: Ronald Rojas, wei.liu2, xen-devel

George Dunlap writes ("Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
> On 02/03/17 17:36, Ian Jackson wrote:
> > I assume this whole series is RFC still ?
> 
> I think the earlier patches looked pretty close to being checked in.  I
> think having a basic chunk of functionality checked in will make it
> easier to actually collaborate on improving things.

There is a lot of hand-crafted code here, whose semantics (eg, lists
of enum values and fields) which is copied from the libxl idl.

What this means is that the golang code may stop building, or (worse)
start to produce broken results, when the idl is updated.

Ian.

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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 17:55       ` Ian Jackson
@ 2017-03-02 18:01         ` George Dunlap
  2017-03-03 15:02           ` Ian Jackson
  0 siblings, 1 reply; 20+ messages in thread
From: George Dunlap @ 2017-03-02 18:01 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Ronald Rojas, wei.liu2, xen-devel

On 02/03/17 17:55, Ian Jackson wrote:
> George Dunlap writes ("Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
>> On 02/03/17 17:36, Ian Jackson wrote:
>>> I assume this whole series is RFC still ?
>>
>> I think the earlier patches looked pretty close to being checked in.  I
>> think having a basic chunk of functionality checked in will make it
>> easier to actually collaborate on improving things.
> 
> There is a lot of hand-crafted code here, whose semantics (eg, lists
> of enum values and fields) which is copied from the libxl idl.
> 
> What this means is that the golang code may stop building, or (worse)
> start to produce broken results, when the idl is updated.

Right.  The purpose of hand-crafting the code was to get a feel for what
a good Go-like output would look like before investing in the IDL.  It
sounds like you're suggesting that having IDL support would be a
prerequisite to getting anything checked in?

I'd definitely say havind IDL support would be a prerequisite for
declaring the bindings "supported".  I don't think the structures for
these functions change so often that it would be a hardship for Ronald
or I to change them whenever they broke; and so I would argue it
shouldn't be a blocker for getting things into the tree if the code
looks good.

But in the end it's your & Wei's call. :-)

 -George

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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 17:53     ` George Dunlap
  2017-03-02 17:55       ` Ian Jackson
@ 2017-03-02 18:22       ` Ronald Rojas
  1 sibling, 0 replies; 20+ messages in thread
From: Ronald Rojas @ 2017-03-02 18:22 UTC (permalink / raw)
  To: George Dunlap; +Cc: wei.liu2, Ian Jackson, xen-devel

On Thu, Mar 02, 2017 at 05:53:00PM +0000, George Dunlap wrote:
> On 02/03/17 17:36, Ian Jackson wrote:
> > Ronald Rojas writes ("[PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
> >> Create tests for the following functions:
> >> - GetVersionInfo
> >> - GetPhysinfo
> >> - GetDominfo
> >> - GetMaxCpus
> >> - GetOnlineCpus
> >> - GetMaxNodes
> >> - GetFreeMemory
> > 
> > I assume this whole series is RFC still ?
> 
> I think the earlier patches looked pretty close to being checked in.  I
> think having a basic chunk of functionality checked in will make it
> easier to actually collaborate on improving things.
> 
> > I don't feel competent to review the golang code.  But I did spot some
> > C to review :-)
> > 
> >> diff --git a/tools/golang/xenlight/test/xeninfo/freememory.c b/tools/golang/xenlight/test/xeninfo/freememory.c
> >> new file mode 100644
> >> index 0000000..04ee90d
> >> --- /dev/null
> >> +++ b/tools/golang/xenlight/test/xeninfo/freememory.c
> >> @@ -0,0 +1,24 @@
> >> +#include <stdio.h>
> >> +#include <stdlib.h>
> >> +#include <libxl.h>
> >> +#include "print.h"
> >> +
> >> +int main(){
> > 
> > This is an unusual definition of main.  (I think it's still legal, but
> > probably not what you meant.)
> > 

I did this because I'm not expecting any arguments  to be passed into
main. I can change it to the standard definition instead.

> >> +    libxl_ctx *context;
> >> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
> >> +
> >> +    uint64_t free_memory;
> >> +    int err = libxl_get_free_memory(context, &free_memory);
> >> +    if (err < 0)
> >> +    {
> >> +        printf("%d\n", err);
> >> +    }
> >> +    else
> >> +    {
> >> +        printf("%lu\n", free_memory);
> >> +    }
> > 
> > This output is ambiguous.
> > 
> >> +    libxl_ctx_free(context);
> >> +
> >> +}
> > 
> > Returns from main without returning a value.  Error code is lost.
> 
> He's not testing whether the call succeeds; he's testing if the call has
> the same result as the golang function.
> 
> But this won't quite work anymore, because as of v2 the golang return
> values are positive constants (to make it easy to use them for indexes).
>  So if there were an identical error, the output would be different even
> if the error number were identical.

You are right. I need to negate the value that I print in the go file.

> 
> That needs to be fixed; but I also agree it would probably be better to
> print something that distinguishes between success and failure.

I think it's always clear whether the function failed or succeeded. The
error code will always be a negative number while free_memory can only
be non-negative because it is an unsigned long. There is no overlap
between those two values.


Ronald

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

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

* Re: [PATCH v2 1/5] golang/xenlight: Create stub package
  2017-03-02 16:07 [PATCH v2 1/5] golang/xenlight: Create stub package Ronald Rojas
                   ` (3 preceding siblings ...)
  2017-03-02 16:07 ` [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions Ronald Rojas
@ 2017-03-03 14:42 ` George Dunlap
  4 siblings, 0 replies; 20+ messages in thread
From: George Dunlap @ 2017-03-03 14:42 UTC (permalink / raw)
  To: Ronald Rojas; +Cc: wei.liu2, ian.jackson, xen-devel

On 02/03/17 16:07, 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>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

Getting close!  A few more changes.

> ---
> 
> Changes:
> - Changed GPL Lisense to LGPL Lisense
> 
> - Initialized xentoollog_logger for storing error messages
> 
> - Moved manual-enable config option to tools/Rules.mk, use
>   CONFIG_GOLANG in tools/Makefile
> 
> - Added XEN_GOPATH, pointing to tools/golang
> 
> - Modified tools/golang/xenlight makefile to construct necessary $GOPATH
> 
> - Added tools/golang/Makefile, so we don't need special rules in tools
>   to make tools/golang/xenlight; and so we have a single place to remove the
>   $GOPATH build side-effects ($GOPATH/src and $GOPATH/pkg)
> 
> - Build of tools/golang/xenlight sets $GOPATH and does a 'go install'
> 
> - Use tree-provided CFLAGS_libxenlight and LDLIBS_libxenlight, rather
>   than hard-coding our own
> 
> - Made a PKGSOURCES variable to track dependencies of all target files
>   which need to be part of the output package (i.e., what's put in
>   $GOPATH/src).  At the moment this is one file, but it will probably
>   be more once we start using the IDL.

Good summary of changes, thanks.

> 
> CC: xen-devel@lists.xen.org
> CC: george.dunlap@citrix.com
> CC: ian.jackson@eu.citrix.com
> CC: wei.liu2@citrix.com
> ---
> ---
>  tools/Makefile                    |  1 +
>  tools/Rules.mk                    |  6 +++
>  tools/golang/Makefile             | 27 +++++++++++++
>  tools/golang/xenlight/Makefile    | 49 ++++++++++++++++++++++
>  tools/golang/xenlight/xenlight.go | 85 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 168 insertions(+)
>  create mode 100644 tools/golang/Makefile
>  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..df1fda1 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -31,6 +31,7 @@ endif
>  
>  SUBDIRS-y += xenpmd
>  SUBDIRS-y += libxl
> +SUBDIRS-$(CONFIG_GOLANG) += golang
>  SUBDIRS-y += helpers
>  SUBDIRS-$(CONFIG_X86) += xenpaging
>  SUBDIRS-$(CONFIG_X86) += debugger/gdbsx
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index b35999b..24e5220 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -30,6 +30,12 @@ XENSTORE_XENSTORED ?= y
>  debug ?= y
>  debug_symbols ?= $(debug)
>  
> +# Uncomment to compile with Go
> +CONFIG_GOLANG ?= y

You forgot to comment this out again before submitting it. :-)  (This is
something that can be fixed up on check-in if necessary.)

> +ifeq ($(CONFIG_GOLANG),y)
> +XEN_GOPATH        = $(XEN_ROOT)/tools/golang
> +endif
> +
>  ifeq ($(debug_symbols),y)
>  CFLAGS += -g3
>  endif
> diff --git a/tools/golang/Makefile b/tools/golang/Makefile
> new file mode 100644
> index 0000000..47a9235
> --- /dev/null
> +++ b/tools/golang/Makefile
> @@ -0,0 +1,27 @@
> +XEN_ROOT=$(CURDIR)/../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +# In order to link against a package in Go, the package must live in a
> +# directory tree in the way that Go expects.  To make this possible,
> +# there must be a directory such that we can set GOPATH=${dir}, and
> +# the package will be under $GOPATH/src/${full-package-path}.
> +
> +# So we set XEN_GOPATH to $XEN_ROOT/tools/golang.  The xenlight
> +# "package build" directory ($PWD/xenlight) will create the "package
> +# source" directory in the proper place.  Go programs can use this
> +# package by setting GOPATH=$(XEN_GOPATH).
> +
> +SUBDIRS-y = xenlight
> +
> +.PHONY: build all
> +all build: subdirs-all
> +
> +.PHONY: install
> +install: subdirs-install
> +
> +.PHONY: clean
> +clean: subdirs-clean
> +	$(RM) -r src pkg
> +
> +.PHONY: distclean
> +distclean: clean
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..5d578f3
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,49 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +# Standing boldly against convention, we insist on installing the
> +# package source under $(prefix)/share/gocode
> +GOCODE_DIR ?= $(prefix)/share/gocode/
> +GOXL_PKG_DIR = /src/xenproject.org/xenlight/

So I had a chat with Ian Jackson about the logistics of setting up a
repo such that "go get" would Just Work (TM).  What we agreed (if I
remember right) was to use the hostname "golang.xenproject.org" for
those mirrored repos, to make sure there was never any name collisions
between web pages at xenproject.org

So we should probably make a variable (maybe XEN_GOCODE_URL), set it to
"golang.xenproject.org", and then replace all instances of
"xenproject.org" in this makefile with $(XEN_GOCODE_URL).

> +GOXL_INSTALL_DIR = $(GOCODE_DIR)$(GOXL_PKG_DIR)
> +
> +# PKGSOURCES: Files which comprise the distributed source package
> +PKGSOURCES = xenlight.go
> +
> +GO ?= go
> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: package
> +package: $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES)
> +
> +$(XEN_GOPATH)/src/xenproject.org/xenlight/$(PKGSOURCES): $(PKGSOURCES)
> +	$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
> +	$(INSTALL_DATA) $(PKGSOURCES) $(XEN_GOPATH)$(GOXL_PKG_DIR)
> +
> +# Go will do its own dependency checking, and not actuall go through
> +# with the build if none of the input files have changed.
> +#
> +# NB that because the users of this library need to be able to
> +# recompile the library from source, it needs to include '-lxenlight'
> +# in the LDFLAGS; and thus we need to add -L$(XEN_XENLIGHT) here
> +# so that it can find the actual library.
> +.PHONY: build
> +build: package
> +	CGO_CFLAGS="$(CFLAGS_libxenlight)" CGO_LDFLAGS="$(LDLIBS_libxenlight) -L$(XEN_XENLIGHT)" GOPATH=$(XEN_GOPATH) $(GO) install -x xenproject.org/xenlight

Now that we -lxentoollog, we also need to add the appropriate CFLAGS,
LDLIBS, and library path: "$(CFLAGS_libxentoollog)" and
"$(LDLIBS_libxentoolllog) -L$(XEN_LIBXENTOOLLOG)" respectively.

> +
> +.PHONY: install
> +install: build
> +	$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
> +	$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES) $(DESTDIR)$(GOXL_INSTALL_DIR)
> +
> +.PHONY: clean
> +clean:
> +	$(RM) -r $(XEN_GOPATH)$(GOXL_PKG_DIR)
> +	$(RM) $(XEN_GOPATH)/pkg/*/xenproject.org/xenlight.a
> +
> +.PHONY: distclean
> +distclean: clean
> +
> +-include $(DEPS)
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> new file mode 100644
> index 0000000..0a0cea2
> --- /dev/null
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright (C) 2016 George W. Dunlap, Citrix Systems UK Ltd
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation;
> + * version 2.1 of the License.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; If not, see <http://www.gnu.org/licenses/>.
> + */
> +package xenlight
> +
> +/*
> +#cgo LDFLAGS: -lxenlight -lyajl -lxentoollog
> +#include <stdlib.h>
> +#include <libxl.h>
> +*/
> +import "C"
> +
> +/*
> + * Other flags that may be needed at some point:
> + *  -lnl-route-3 -lnl-3
> + *
> + * To get back to static linking:
> + * #cgo LDFLAGS: -lxenlight -lyajl_s -lxengnttab -lxenstore -lxenguest -lxentoollog -lxenevtchn -lxenctrl -lblktapctl -lxenforeignmemory -lxencall -lz -luuid -lutil
> +*/
> +
> +import (
> +	"fmt"
> +	"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
> +	}
> +
> +	logger := C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0);

You don't check the return value of logger to make sure that it succeeds.

Also -- xtl_create_logger() will allocate a struct; and libxl_ctx_free()
will not free that struct; which means that repeated calls to
Context.Open() and Context.Close() will leak C memory.  (Yay for no
garbage collection!)

So you need to add the logger to the Context struct and call
C.xtl_logger_destroy(Ctx.logger) on it in Context.Close().

Also -- you forgot to re-run `go fmt`, and so there's a stray semicolon
at the end of the line. :-)

> +	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, unsafe.Pointer(logger))

Actually, I realized looking at your code that a lot of the
unsafe.Pointer() casts (which you inherited from the code I wrote
originally) are unnecessary.  We need to use it for logger because it's
doing C's version of polymorphism.  But for libxl_ctx_alloc, it's
expecting the first argument to be a pointer to C.libxl_ctx, so we don't
need any casts.  Same with libxl_ctx_free().

Finally, in other parts of the tree we limit line lenghth to 80
characters unless there's a good reason to do otherwise.  It seems like
in this case breaking it after the '0,' would be a good idea.

(It will be nice once this is checked in and I can just post a patch to
fix a lot of these types of issues, rather than asking you to do it.)

Thanks,
 -George


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

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

* Re: [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling
  2017-03-02 16:07 ` [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling Ronald Rojas
@ 2017-03-03 14:47   ` George Dunlap
  0 siblings, 0 replies; 20+ messages in thread
From: George Dunlap @ 2017-03-03 14:47 UTC (permalink / raw)
  To: Ronald Rojas; +Cc: wei.liu2, ian.jackson, xen-devel

On 02/03/17 16:07, 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>

There are a couple of `go fmt` changes which should be in the previous
patch (including the stray semicolon and a space for a c-style comment).
 With that fixed:

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

> ---
> Changes since last patch:
> 
> - Whitespace fixes
> 
> CC: xen-devel@lists.xen.org
> CC: george.dunlap@citrix.com
> CC: ian.jackson@eu.citrix.com
> CC: wei.liu2@citrix.com
> ---
> ---
>  tools/golang/xenlight/xenlight.go | 81 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 77 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index 0a0cea2..cbd3527 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -29,17 +29,79 @@ import "C"
>   *
>   * To get back to static linking:
>   * #cgo LDFLAGS: -lxenlight -lyajl_s -lxengnttab -lxenstore -lxenguest -lxentoollog -lxenevtchn -lxenctrl -lblktapctl -lxenforeignmemory -lxencall -lz -luuid -lutil
> -*/
> + */
>  
>  import (
>  	"fmt"
>  	"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
>  }
> @@ -49,6 +111,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
>  }
> @@ -58,11 +131,11 @@ func (Ctx *Context) Open() (err error) {
>  		return
>  	}
>  
> -	logger := C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0);
> +	logger := C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
>  	ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, unsafe.Pointer(logger))
>  
>  	if ret != 0 {
> -		err = fmt.Errorf("Error: %d", -ret)
> +		err = Error(-ret)
>  	}
>  	return
>  }
> @@ -72,7 +145,7 @@ func (Ctx *Context) Close() (err error) {
>  	Ctx.ctx = nil
>  
>  	if ret != 0 {
> -		err = fmt.Errorf("Error: %d", -ret)
> +		err = Error(-ret)
>  	}
>  	return
>  }
> 


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

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

* Re: [PATCH v2 3/5] golang/xenlight: Add host-related functionality
  2017-03-02 16:07 ` [PATCH v2 3/5] golang/xenlight: Add host-related functionality Ronald Rojas
@ 2017-03-03 14:54   ` George Dunlap
  2017-03-03 18:49     ` Ronald Rojas
  0 siblings, 1 reply; 20+ messages in thread
From: George Dunlap @ 2017-03-03 14:54 UTC (permalink / raw)
  To: Ronald Rojas; +Cc: wei.liu2, ian.jackson, xen-devel

On 02/03/17 16:07, 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>

Looks good -- just two minor comments...

> ---
> Changes since last version
> 
> - Refactored creating Physinfo and VersionInfo types into
> seperate toGo() functions.
> 
> - Used libxl_<type>_init() and libxl_<type>_dispose() on IDL
> type physinfo
> 
> - Whitespace fixes
> 
> CC: xen-devel@lists.xen.org
> CC: george.dunlap@citrix.com
> CC: ian.jackson@eu.citrix.com
> CC: wei.liu2@citrix.com
> ---
> ---
>  tools/golang/xenlight/xenlight.go | 200 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 200 insertions(+)
> 
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index cbd3527..63cc805 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -106,6 +106,103 @@ type Context struct {
>  	ctx *C.libxl_ctx
>  }
>  
> +type Hwcap []C.uint32_t
> +
> +func (chwcap C.libxl_hwcap) CToGo() (ghwcap Hwcap) {

Was there a reason you left this as CToGo() rather than just toGo()?

> +	// 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
> +}
> +
> +func (cphys *C.libxl_physinfo) toGo() (physinfo *Physinfo) {
> +
> +	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 = cphys.hw_cap.CToGo()
> +	physinfo.CapHvm = bool(cphys.cap_hvm)
> +	physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)
> +
> +	return
> +}
> +
> +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
> +}
> +
> +func (cinfo *C.libxl_version_info) toGo() (info *VersionInfo) {
> +	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
> +}
> +
>  /*
>   * Context
>   */
> @@ -156,3 +253,106 @@ 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 = Error(-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 = Error(-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 = Error(-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 = Error(-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
> +	C.libxl_physinfo_init(&cphys)
> +
> +	ret := C.libxl_get_physinfo(Ctx.ctx, &cphys)
> +
> +	if ret < 0 {
> +		err = Error(ret)
> +		return
> +	}
> +	physinfo = cphys.toGo()
> +	C.libxl_physinfo_dispose(&cphys)

I think it would probably be more idiomatic to write "defer
C.libxl_physinfo_dispose()" just after the physinfo_init.

If the init() actually allocated any memory, the current code would
return without disposing of it if there was an error.  `defer` avoids
that by ensuring that *all* return paths call the clean-up function.

Other than that, looks great, thanks!

 -George


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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 18:01         ` George Dunlap
@ 2017-03-03 15:02           ` Ian Jackson
  2017-03-03 15:10             ` George Dunlap
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2017-03-03 15:02 UTC (permalink / raw)
  To: George Dunlap; +Cc: Ronald Rojas, wei.liu2, xen-devel

George Dunlap writes ("Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
> Right.  The purpose of hand-crafting the code was to get a feel for what
> a good Go-like output would look like before investing in the IDL.

That makes sense.

>  It sounds like you're suggesting that having IDL support would be a
> prerequisite to getting anything checked in?

I think it's a prerequisite for it to be hooked into the build by
default, unless someone can convince me that foreseeable changes to
the libxl idl will not cause the golang build to break.

> I'd definitely say havind IDL support would be a prerequisite for
> declaring the bindings "supported".  I don't think the structures for
> these functions change so often that it would be a hardship for Ronald
> or I to change them whenever they broke; and so I would argue it
> shouldn't be a blocker for getting things into the tree if the code
> looks good.

I don't think it's sensible to ask other contributors who change the
IDL to have to liase with golang experts in order to not break the
build.  The whole point of the idl system is precisely to make this
kind of thing automatic.

OTOH I don't mind it being in-tree if it's not built by default.  Of
course then it will probably rot, so that's not a very stable
situation, but if having it in-tree makes the development easier then
that's fine by me.

Ian.

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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-03 15:02           ` Ian Jackson
@ 2017-03-03 15:10             ` George Dunlap
  2017-03-03 15:41               ` Ian Jackson
  0 siblings, 1 reply; 20+ messages in thread
From: George Dunlap @ 2017-03-03 15:10 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Ronald Rojas, wei.liu2, xen-devel

On 03/03/17 15:02, Ian Jackson wrote:
> George Dunlap writes ("Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
>> Right.  The purpose of hand-crafting the code was to get a feel for what
>> a good Go-like output would look like before investing in the IDL.
> 
> That makes sense.
> 
>>  It sounds like you're suggesting that having IDL support would be a
>> prerequisite to getting anything checked in?
> 
> I think it's a prerequisite for it to be hooked into the build by
> default, unless someone can convince me that foreseeable changes to
> the libxl idl will not cause the golang build to break.
> 
>> I'd definitely say havind IDL support would be a prerequisite for
>> declaring the bindings "supported".  I don't think the structures for
>> these functions change so often that it would be a hardship for Ronald
>> or I to change them whenever they broke; and so I would argue it
>> shouldn't be a blocker for getting things into the tree if the code
>> looks good.
> 
> I don't think it's sensible to ask other contributors who change the
> IDL to have to liase with golang experts in order to not break the
> build.  The whole point of the idl system is precisely to make this
> kind of thing automatic.
> 
> OTOH I don't mind it being in-tree if it's not built by default.  Of
> course then it will probably rot, so that's not a very stable
> situation, but if having it in-tree makes the development easier then
> that's fine by me.

Yes, the plan was to have it off by default at very least until we got
configure to detect the presence of a go compiler.  Ronald has already
spent a decent chunk of his time doing Makefile work; I didn't want to
make him spend a big chunk of time poking autoconf as well on a project
which was advertised as programming Go. :-)  Adding "won't break on IDL
changes" is another criteria it would be good to add.

Once the core patches are in-tree, I could start working on some of that
stuff if I have time.  In addition, I keep finding random improvements
to make in Ronald's patches which aren't actually bugs; it would be
nicer for both of us if I could just post patches for those improvements
rather than commenting on his patches and making him re-submit them.

 -George

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

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

* Re: [PATCH v2 4/5] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause
  2017-03-02 16:07 ` [PATCH v2 4/5] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause Ronald Rojas
@ 2017-03-03 15:15   ` George Dunlap
  2017-03-03 19:31     ` Ronald Rojas
  0 siblings, 1 reply; 20+ messages in thread
From: George Dunlap @ 2017-03-03 15:15 UTC (permalink / raw)
  To: Ronald Rojas; +Cc: wei.liu2, ian.jackson, xen-devel

On 02/03/17 16:07, Ronald Rojas wrote:
> 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>
> ---
> Changes since last version
> - Created type and enumeration of DomainType and ShutdownReason
> 
> - Created String() method for DomainType and ShutdownReason
> 
> - Refactored creating DomainInfo from c type into seperate
> toGo() function
> 
> - Applied libxl_domain_info_init/dispose()
> 
> - whitespace fixes
> 
> CC: xen-devel@lists.xen.org
> CC: george.dunlap@citrix.com
> CC: ian.jackson@eu.citrix.com
> CC: wei.liu2@citrix.com
> 
> ---
> ---
>  tools/golang/xenlight/xenlight.go | 132 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 132 insertions(+)
> 
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index 63cc805..18dedcb 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -34,6 +34,7 @@ import "C"
>  import (
>  	"fmt"
>  	"unsafe"
> +	"time"
>  )
>  
>  /*
> @@ -102,6 +103,12 @@ var errors = [...]string{
>   * Types: Builtins
>   */
>  
> +type Domid uint32
> +
> +type MemKB uint64
> +
> +type Uuid C.libxl_uuid
> +
>  type Context struct {
>  	ctx *C.libxl_ctx
>  }
> @@ -203,6 +210,95 @@ func (cinfo *C.libxl_version_info) toGo() (info *VersionInfo) {
>  	return
>  }
>  
> +type ShutdownReason int32
> +
> +const(
> +	ShutdownReasonUnknown = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_UNKNOWN)
> +	ShutdownReasonPoweroff = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_POWEROFF)
> +	ShutdownReasonReboot = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_REBOOT)
> +	ShutdownReasonSuspend = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_SUSPEND)
> +	ShutdownReasonCrash = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_CRASH)
> +	ShutdownReasonWatchdog = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_WATCHDOG)
> +	ShutdownReasonSoftReset = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_SOFT_RESET)

Looks like this could use having `go fmt` run.

> +
> +)
> +
> +func (sr ShutdownReason) String()(str string){
> +	cstr := C.libxl_shutdown_reason_to_string(C.libxl_shutdown_reason(sr))
> +	str = C.GoString(cstr)
> +
> +	return
> +}
> +
> +type DomainType int32
> +
> +const(
> +	DomainTypeInvalid = DomainType(C.LIBXL_DOMAIN_TYPE_INVALID)
> +	DomainTypeHvm = DomainType(C.LIBXL_DOMAIN_TYPE_HVM)
> +	DomainTypePv = DomainType(C.LIBXL_DOMAIN_TYPE_PV)
> +)
> +
> +func (dt DomainType) String()(str string){
> +	cstr := C.libxl_domain_type_to_string(C.libxl_domain_type(dt))
> +	str = C.GoString(cstr)
> +
> +	return
> +}
> +
> +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
> +	OutstandingMemkb MemKB
> +	CurrentMemkb     MemKB
> +	SharedMemkb      MemKB
> +	PagedMemkb       MemKB
> +	MaxMemkb         MemKB
> +	CpuTime          time.Duration
> +	VcpuMaxId       uint32
> +	VcpuOnline       uint32
> +	Cpupool           uint32
> +	DomainType       int32
> +
> +}
> +
> +func (cdi *C.libxl_dominfo) toGo()(di *Dominfo){
> +
> +	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
> +}
> +
>  /*
>   * Context
>   */
> @@ -356,3 +452,39 @@ 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
> +	C.libxl_dominfo_init(&cdi)

As before, put the 'defer libxl_dominfo_dispose()' here.

> +
> +	ret := C.libxl_domain_info(Ctx.ctx, unsafe.Pointer(&cdi), C.uint32_t(Id))

unsafe.Pointer() isn't needed here.

Other than that, looks good, thanks.

 -George


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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-03 15:10             ` George Dunlap
@ 2017-03-03 15:41               ` Ian Jackson
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Jackson @ 2017-03-03 15:41 UTC (permalink / raw)
  To: George Dunlap; +Cc: Ronald Rojas, wei.liu2, xen-devel

George Dunlap writes ("Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions"):
> Yes, the plan was to have it off by default at very least until we got
> configure to detect the presence of a go compiler.  Ronald has already
> spent a decent chunk of his time doing Makefile work; I didn't want to
> make him spend a big chunk of time poking autoconf as well on a project
> which was advertised as programming Go. :-)  Adding "won't break on IDL
> changes" is another criteria it would be good to add.

Right.  OK.  I'm content then.

> Once the core patches are in-tree, I could start working on some of that
> stuff if I have time.  In addition, I keep finding random improvements
> to make in Ronald's patches which aren't actually bugs; it would be
> nicer for both of us if I could just post patches for those improvements
> rather than commenting on his patches and making him re-submit them.

Sure.

FWIW I have no other comments, and on this basis I'm happy for it to
go in with your review.

Ian.

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

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

* Re: [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
  2017-03-02 16:07 ` [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions Ronald Rojas
  2017-03-02 17:36   ` Ian Jackson
@ 2017-03-03 16:20   ` George Dunlap
  1 sibling, 0 replies; 20+ messages in thread
From: George Dunlap @ 2017-03-03 16:20 UTC (permalink / raw)
  To: Ronald Rojas; +Cc: wei.liu2, ian.jackson, xen-devel

On 02/03/17 16:07, Ronald Rojas wrote:
> Create tests for the following functions:
> - GetVersionInfo
> - GetPhysinfo
> - GetDominfo
> - GetMaxCpus
> - GetOnlineCpus
> - GetMaxNodes
> - GetFreeMemory
> 
> Signed-off-by: Ronald Rojas <ronladred@gmail.com>
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>
> 
> ---
> 
> - Removed individual Makefiles for each test for a single
> Makefile to test all files at once
> 
> - Changed all tests to xeninfo directory
> 
> - Applied libxl_<type>_{init/dispose} for IDL types in tests
> 
> - Applied formating fixes

Looks a lot better!  Still some more things to improve...

> 
> CC: xen-devel@lists.xen.org
> CC: george.dunlap@citrix.com
> CC: ian.jackson@eu.citrix.com
> CC: wei.liu2@citrix.com
> ---
> ---
>  tools/golang/xenlight/test/xeninfo/Makefile       | 34 +++++++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/dominfo.c      | 31 +++++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/dominfo.go     | 31 +++++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/freememory.c   | 24 ++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/freememory.go  | 28 +++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/maxcpu.c       | 16 +++++++++++
>  tools/golang/xenlight/test/xeninfo/maxcpu.go      | 27 ++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/maxnodes.c     | 16 +++++++++++
>  tools/golang/xenlight/test/xeninfo/maxnodes.go    | 27 ++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/onlinecpu.c    | 16 +++++++++++
>  tools/golang/xenlight/test/xeninfo/onlinecpu.go   | 27 ++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/physinfo.c     | 31 +++++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/physinfo.go    | 32 +++++++++++++++++++++
>  tools/golang/xenlight/test/xeninfo/print.h        |  3 ++
>  tools/golang/xenlight/test/xeninfo/versioninfo.c  | 20 +++++++++++++
>  tools/golang/xenlight/test/xeninfo/versioninfo.go | 28 +++++++++++++++++++
>  16 files changed, 391 insertions(+)
>  create mode 100644 tools/golang/xenlight/test/xeninfo/Makefile
>  create mode 100644 tools/golang/xenlight/test/xeninfo/dominfo.c
>  create mode 100644 tools/golang/xenlight/test/xeninfo/dominfo.go
>  create mode 100644 tools/golang/xenlight/test/xeninfo/freememory.c
>  create mode 100644 tools/golang/xenlight/test/xeninfo/freememory.go
>  create mode 100644 tools/golang/xenlight/test/xeninfo/maxcpu.c
>  create mode 100644 tools/golang/xenlight/test/xeninfo/maxcpu.go
>  create mode 100644 tools/golang/xenlight/test/xeninfo/maxnodes.c
>  create mode 100644 tools/golang/xenlight/test/xeninfo/maxnodes.go
>  create mode 100644 tools/golang/xenlight/test/xeninfo/onlinecpu.c
>  create mode 100644 tools/golang/xenlight/test/xeninfo/onlinecpu.go
>  create mode 100644 tools/golang/xenlight/test/xeninfo/physinfo.c
>  create mode 100644 tools/golang/xenlight/test/xeninfo/physinfo.go
>  create mode 100644 tools/golang/xenlight/test/xeninfo/print.h
>  create mode 100644 tools/golang/xenlight/test/xeninfo/versioninfo.c
>  create mode 100644 tools/golang/xenlight/test/xeninfo/versioninfo.go
> 
> diff --git a/tools/golang/xenlight/test/xeninfo/Makefile b/tools/golang/xenlight/test/xeninfo/Makefile
> new file mode 100644
> index 0000000..859e4d6
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/Makefile
> @@ -0,0 +1,34 @@
> +XEN_ROOT = $(CURDIR)/../../../../..
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +GO ?= go
> +
> +TESTS = dominfo freememory maxcpu onlinecpu physinfo versioninfo
> +CBINARIES = $(TESTS:%=%-c)
> +GOBINARIES = $(TESTS:%=%-go)
> +
> +all: build
> +
> +test: clean build
> +	for test in $(TESTS) ; do \
> +		./$$test-c >> c.output ; \
> +	        ./$$test-go >> go.output ; \
> +		if cmp -s "c.output" "go.output"; then\
> +			echo "$$test PASSED";\
> +		else \
> +			echo "$$test FAILED";\
> +		fi ; \
> +	done
> +
> +build: $(CBINARIES) $(GOBINARIES)
> +
> +%-c: %.c
> +	gcc $(CFLAGS_libxenlight) -o $@ $< $(LDLIBS_libxenlight)

So I think here we need to define CFLAGS and LDLIBS; and we want to add
-Werror, thus:

CFLAGS += -Werror
CFLAGS += $(CFLAGS_libxenlight)

LDLIBS += $(LDLIBS_libxenlight)

And then we want to build things like this:

    $(CC) $(CFLAGS) -o $@ $< $(LDLIBS)

And when you do that, gcc will tell you a long list of improvements in
your C code. :-)  (If you run into any problems with how to fix these,
just give us a shout on the IRC channel.)

> +
> +%-go: %.go
> +	GOPATH=$(XEN_ROOT)/tools/golang $(GO) build -o $@ $<
> +
> +clean:
> +	rm -f *-c
> +	rm -f *-go
> +	rm -f *.output
> diff --git a/tools/golang/xenlight/test/xeninfo/dominfo.c b/tools/golang/xenlight/test/xeninfo/dominfo.c
> new file mode 100644
> index 0000000..85f658d
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/dominfo.c
> @@ -0,0 +1,31 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +#include "print.h"
> +
> +int main(){
> +
> +    libxl_ctx *context;
> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);

Hmm, you don't create a logger for this -- so if it manages to run on a
system that's booted with a different version of Xen, the golang version
will print an error, but the C version will crash with a null pointer
deference.

You probably want to make a function (maybe add it to your 'print.h')
that both makes a logger and calls ctx_alloc.

> +    libxl_dominfo info;
> +    libxl_dominfo_init(&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%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_dominfo_dispose(&info);
> +	libxl_ctx_free(context);

These seem to have tabs instead of spaces, which is verboten in C code
in the Xen tree.  You might want to ask Wei and/or Anthony how to get
vim to only make spaces.

> +
> +}
> diff --git a/tools/golang/xenlight/test/xeninfo/dominfo.go b/tools/golang/xenlight/test/xeninfo/dominfo.go
> new file mode 100644
> index 0000000..bb9257b
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/dominfo.go
> @@ -0,0 +1,31 @@
> +package main
> +
> +import (
> +	"fmt"
> +	"os"
> +	"xenproject.org/xenlight"
> +)
> +
> +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)

I think I commented on this before -- what's this for?

> +	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)
> +
> +}
> diff --git a/tools/golang/xenlight/test/xeninfo/freememory.c b/tools/golang/xenlight/test/xeninfo/freememory.c
> new file mode 100644
> index 0000000..04ee90d
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/freememory.c
> @@ -0,0 +1,24 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +#include "print.h"
> +
> +int main(){
> +
> +    libxl_ctx *context;
> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
> +
> +    uint64_t free_memory;
> +    int 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/xeninfo/freememory.go b/tools/golang/xenlight/test/xeninfo/freememory.go
> new file mode 100644
> index 0000000..69a7723
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/freememory.go
> @@ -0,0 +1,28 @@
> +package main
> +
> +import (
> +	"fmt"
> +	"os"
> +	"xenproject.org/xenlight"
> +)
> +
> +func main() {
> +	ctx := xenlight.Ctx
> +	err := ctx.Open()
> +	if err != nil {
> +		os.Exit(-1)
> +	}
> +
> +	defer ctx.Close()
> +	if err != nil {
> +		os.Exit(-1)
> +	}

Why are you checking err a second time? :-)

> +
> +	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/xeninfo/maxcpu.c b/tools/golang/xenlight/test/xeninfo/maxcpu.c
> new file mode 100644
> index 0000000..6ba4a87
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/maxcpu.c
> @@ -0,0 +1,16 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +
> +int main(){
> +
> +    libxl_ctx *context;
> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
> +
> +    int max_cpus = libxl_get_max_cpus(context);
> +    printf("%d\n", max_cpus);
> +
> +    libxl_ctx_free(context);
> +
> +}
> +
> diff --git a/tools/golang/xenlight/test/xeninfo/maxcpu.go b/tools/golang/xenlight/test/xeninfo/maxcpu.go
> new file mode 100644
> index 0000000..f847c0d
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/maxcpu.go
> @@ -0,0 +1,27 @@
> +package main
> +
> +import (
> +	"fmt"
> +	"os"
> +	"xenproject.org/xenlight"
> +)
> +
> +func main() {
> +	ctx := xenlight.Ctx
> +	err := ctx.Open()
> +	if err != nil {
> +		os.Exit(-1)
> +	}
> +	defer ctx.Close()
> +	if err != nil {
> +		os.Exit(-1)
> +	}

Redundant error check.

> +
> +	max_cpus, err := ctx.GetMaxCpus()
> +	if err != nil {
> +		fmt.Printf("%d\n", err)
> +	} else {
> +		fmt.Printf("%d\n", max_cpus)
> +	}
> +
> +}
> diff --git a/tools/golang/xenlight/test/xeninfo/maxnodes.c b/tools/golang/xenlight/test/xeninfo/maxnodes.c
> new file mode 100644
> index 0000000..0e79c8d
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/maxnodes.c
> @@ -0,0 +1,16 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +
> +int main(){
> +
> +    libxl_ctx *context;
> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
> +
> +    int max_nodes = libxl_get_max_nodes(context);
> +    printf("%d\n", max_nodes);
> +
> +   libxl_ctx_free(context);

Indentation.

> +
> +}
> +
> diff --git a/tools/golang/xenlight/test/xeninfo/maxnodes.go b/tools/golang/xenlight/test/xeninfo/maxnodes.go
> new file mode 100644
> index 0000000..906e596
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/maxnodes.go
> @@ -0,0 +1,27 @@
> +package main
> +
> +import (
> +	"fmt"
> +	"os"
> +	"xenproject.org/xenlight"
> +)
> +
> +func main() {
> +	ctx := xenlight.Ctx
> +	err := ctx.Open()
> +	if err != nil {
> +		os.Exit(-1)
> +	}
> +	defer ctx.Close()
> +	if err != nil {
> +		os.Exit(-1)
> +	}

Redundant error check.

> +
> +	max_nodes, err := ctx.GetMaxNodes()
> +	if err != nil {
> +		fmt.Printf("%d\n", err)
> +	} else {
> +		fmt.Printf("%d\n", max_nodes)
> +	}
> +
> +}
> diff --git a/tools/golang/xenlight/test/xeninfo/onlinecpu.c b/tools/golang/xenlight/test/xeninfo/onlinecpu.c
> new file mode 100644
> index 0000000..8da3414
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/onlinecpu.c
> @@ -0,0 +1,16 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +
> +int main(){
> +
> +    libxl_ctx *context;
> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
> +
> +    int online_cpus = libxl_get_online_cpus(context);
> +    printf("%d\n", online_cpus);
> +
> +    libxl_ctx_free(context);
> +
> +}
> +
> diff --git a/tools/golang/xenlight/test/xeninfo/onlinecpu.go b/tools/golang/xenlight/test/xeninfo/onlinecpu.go
> new file mode 100644
> index 0000000..74323c4
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/onlinecpu.go
> @@ -0,0 +1,27 @@
> +package main
> +
> +import (
> +	"fmt"
> +	"os"
> +	"xenproject.org/xenlight"
> +)
> +
> +func main() {
> +	ctx := xenlight.Ctx
> +	err := ctx.Open()
> +	if err != nil {
> +		os.Exit(-1)
> +	}
> +	defer ctx.Close()
> +	if err != nil {
> +		os.Exit(-1)
> +	}

Redundant error check.

> +
> +	online_cpus, err := ctx.GetOnlineCpus()
> +	if err != nil {
> +		fmt.Printf("%d\n", err)
> +	} else {
> +		fmt.Printf("%d\n", online_cpus)
> +	}
> +
> +}
> diff --git a/tools/golang/xenlight/test/xeninfo/physinfo.c b/tools/golang/xenlight/test/xeninfo/physinfo.c
> new file mode 100644
> index 0000000..ba3aa44
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/physinfo.c
> @@ -0,0 +1,31 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <libxl.h>
> +#include "print.h"
> +
> +int main(){
> +
> +    libxl_ctx *context;
> +    libxl_ctx_alloc(&context,LIBXL_VERSION, 0, NULL);
> +    libxl_physinfo info;
> +    libxl_physinfo_init(&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));
> +
> +    int i;
> +    for(i = 0; i < 8; i++){
> +        printf("%u\n", info.hw_cap[i]);
> +    }
> +
> +    libxl_physinfo_init(&info);
> +    libxl_ctx_free(context);
> +
> +}
> +
> diff --git a/tools/golang/xenlight/test/xeninfo/physinfo.go b/tools/golang/xenlight/test/xeninfo/physinfo.go
> new file mode 100644
> index 0000000..cf7bdd4
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/physinfo.go
> @@ -0,0 +1,32 @@
> +package main
> +
> +import (
> +	"fmt"
> +	"os"
> +	"xenproject.org/xenlight"
> +)
> +
> +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/xeninfo/print.h b/tools/golang/xenlight/test/xeninfo/print.h
> new file mode 100644
> index 0000000..6aaa29c
> --- /dev/null
> +++ b/tools/golang/xenlight/test/xeninfo/print.h
> @@ -0,0 +1,3 @@
> +char* bool_to_string(a){
> +       	return (a? "true" : "false");
> +}

It's not considered good form to define C functions in a .h file; this
should at least be a static inline.

And as gcc will tell you if you add -Werror, you need to give a a type.

Thanks,
 -George


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

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

* Re: [PATCH v2 3/5] golang/xenlight: Add host-related functionality
  2017-03-03 14:54   ` George Dunlap
@ 2017-03-03 18:49     ` Ronald Rojas
  0 siblings, 0 replies; 20+ messages in thread
From: Ronald Rojas @ 2017-03-03 18:49 UTC (permalink / raw)
  To: George Dunlap; +Cc: wei.liu2, ian.jackson, xen-devel

On Fri, Mar 03, 2017 at 02:54:56PM +0000, George Dunlap wrote:
> On 02/03/17 16:07, 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>
> 
> Looks good -- just two minor comments...
> 
> > ---
> > Changes since last version
> > 
> > - Refactored creating Physinfo and VersionInfo types into
> > seperate toGo() functions.
> > 
> > - Used libxl_<type>_init() and libxl_<type>_dispose() on IDL
> > type physinfo
> > 
> > - Whitespace fixes
> > 
> > CC: xen-devel@lists.xen.org
> > CC: george.dunlap@citrix.com
> > CC: ian.jackson@eu.citrix.com
> > CC: wei.liu2@citrix.com
> > ---
> > ---
> >  tools/golang/xenlight/xenlight.go | 200 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 200 insertions(+)
> > 
> > diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> > index cbd3527..63cc805 100644
> > --- a/tools/golang/xenlight/xenlight.go
> > +++ b/tools/golang/xenlight/xenlight.go
> > @@ -106,6 +106,103 @@ type Context struct {
> >  	ctx *C.libxl_ctx
> >  }
> >  
> > +type Hwcap []C.uint32_t
> > +
> > +func (chwcap C.libxl_hwcap) CToGo() (ghwcap Hwcap) {
> 
> Was there a reason you left this as CToGo() rather than just toGo()?
> 

No. This should be changed to toGo()

[snip]

> > +//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
> > +	C.libxl_physinfo_init(&cphys)
> > +
> > +	ret := C.libxl_get_physinfo(Ctx.ctx, &cphys)
> > +
> > +	if ret < 0 {
> > +		err = Error(ret)
> > +		return
> > +	}
> > +	physinfo = cphys.toGo()
> > +	C.libxl_physinfo_dispose(&cphys)
> 
> I think it would probably be more idiomatic to write "defer
> C.libxl_physinfo_dispose()" just after the physinfo_init.
> 
> If the init() actually allocated any memory, the current code would
> return without disposing of it if there was an error.  `defer` avoids
> that by ensuring that *all* return paths call the clean-up function.

Yep. Using defer is the better approach here. I will change
it in all instances where I use the init/dispose functions.

> 
> Other than that, looks great, thanks!
> 
>  -George
> 

Thanks,
Ronald

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

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

* Re: [PATCH v2 4/5] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause
  2017-03-03 15:15   ` George Dunlap
@ 2017-03-03 19:31     ` Ronald Rojas
  0 siblings, 0 replies; 20+ messages in thread
From: Ronald Rojas @ 2017-03-03 19:31 UTC (permalink / raw)
  To: George Dunlap; +Cc: wei.liu2, ian.jackson, xen-devel

On Fri, Mar 03, 2017 at 03:15:51PM +0000, George Dunlap wrote:
> On 02/03/17 16:07, Ronald Rojas wrote:
> > 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>
> > ---
> > Changes since last version
> > - Created type and enumeration of DomainType and ShutdownReason
> > 
> > - Created String() method for DomainType and ShutdownReason
> > 
> > - Refactored creating DomainInfo from c type into seperate
> > toGo() function
> > 
> > - Applied libxl_domain_info_init/dispose()
> > 
> > - whitespace fixes
> > 
> > CC: xen-devel@lists.xen.org
> > CC: george.dunlap@citrix.com
> > CC: ian.jackson@eu.citrix.com
> > CC: wei.liu2@citrix.com
> > 
> > ---
> > ---
> >  tools/golang/xenlight/xenlight.go | 132 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 132 insertions(+)
> > 
> > diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> > index 63cc805..18dedcb 100644
> > --- a/tools/golang/xenlight/xenlight.go
> > +++ b/tools/golang/xenlight/xenlight.go
> > @@ -34,6 +34,7 @@ import "C"
> >  import (
> >  	"fmt"
> >  	"unsafe"
> > +	"time"
> >  )
> >  
> >  /*
> > @@ -102,6 +103,12 @@ var errors = [...]string{
> >   * Types: Builtins
> >   */
> >  
> > +type Domid uint32
> > +
> > +type MemKB uint64
> > +
> > +type Uuid C.libxl_uuid
> > +
> >  type Context struct {
> >  	ctx *C.libxl_ctx
> >  }
> > @@ -203,6 +210,95 @@ func (cinfo *C.libxl_version_info) toGo() (info *VersionInfo) {
> >  	return
> >  }
> >  
> > +type ShutdownReason int32
> > +
> > +const(
> > +	ShutdownReasonUnknown = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_UNKNOWN)
> > +	ShutdownReasonPoweroff = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_POWEROFF)
> > +	ShutdownReasonReboot = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_REBOOT)
> > +	ShutdownReasonSuspend = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_SUSPEND)
> > +	ShutdownReasonCrash = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_CRASH)
> > +	ShutdownReasonWatchdog = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_WATCHDOG)
> > +	ShutdownReasonSoftReset = ShutdownReason(C.LIBXL_SHUTDOWN_REASON_SOFT_RESET)
> 
> Looks like this could use having `go fmt` run.
> 
> > +
> > +)
> > +
> > +func (sr ShutdownReason) String()(str string){
> > +	cstr := C.libxl_shutdown_reason_to_string(C.libxl_shutdown_reason(sr))
> > +	str = C.GoString(cstr)
> > +
> > +	return
> > +}
> > +
> > +type DomainType int32
> > +
> > +const(
> > +	DomainTypeInvalid = DomainType(C.LIBXL_DOMAIN_TYPE_INVALID)
> > +	DomainTypeHvm = DomainType(C.LIBXL_DOMAIN_TYPE_HVM)
> > +	DomainTypePv = DomainType(C.LIBXL_DOMAIN_TYPE_PV)
> > +)
> > +
> > +func (dt DomainType) String()(str string){
> > +	cstr := C.libxl_domain_type_to_string(C.libxl_domain_type(dt))
> > +	str = C.GoString(cstr)
> > +
> > +	return
> > +}
> > +
> > +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
> > +	OutstandingMemkb MemKB
> > +	CurrentMemkb     MemKB
> > +	SharedMemkb      MemKB
> > +	PagedMemkb       MemKB
> > +	MaxMemkb         MemKB
> > +	CpuTime          time.Duration
> > +	VcpuMaxId       uint32
> > +	VcpuOnline       uint32
> > +	Cpupool           uint32
> > +	DomainType       int32
> > +
> > +}
> > +
> > +func (cdi *C.libxl_dominfo) toGo()(di *Dominfo){
> > +
> > +	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
> > +}
> > +
> >  /*
> >   * Context
> >   */
> > @@ -356,3 +452,39 @@ 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
> > +	C.libxl_dominfo_init(&cdi)
> 
> As before, put the 'defer libxl_dominfo_dispose()' here.
> 

Fixed.

> > +
> > +	ret := C.libxl_domain_info(Ctx.ctx, unsafe.Pointer(&cdi), C.uint32_t(Id))
> 
> unsafe.Pointer() isn't needed here.

Fixed. 

> 
> Other than that, looks good, thanks.
> 
>  -George
> 

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

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

end of thread, other threads:[~2017-03-03 19:31 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-02 16:07 [PATCH v2 1/5] golang/xenlight: Create stub package Ronald Rojas
2017-03-02 16:07 ` [PATCH v2 2/5] golang/xenlight: Add error constants and standard handling Ronald Rojas
2017-03-03 14:47   ` George Dunlap
2017-03-02 16:07 ` [PATCH v2 3/5] golang/xenlight: Add host-related functionality Ronald Rojas
2017-03-03 14:54   ` George Dunlap
2017-03-03 18:49     ` Ronald Rojas
2017-03-02 16:07 ` [PATCH v2 4/5] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause Ronald Rojas
2017-03-03 15:15   ` George Dunlap
2017-03-03 19:31     ` Ronald Rojas
2017-03-02 16:07 ` [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions Ronald Rojas
2017-03-02 17:36   ` Ian Jackson
2017-03-02 17:53     ` George Dunlap
2017-03-02 17:55       ` Ian Jackson
2017-03-02 18:01         ` George Dunlap
2017-03-03 15:02           ` Ian Jackson
2017-03-03 15:10             ` George Dunlap
2017-03-03 15:41               ` Ian Jackson
2017-03-02 18:22       ` Ronald Rojas
2017-03-03 16:20   ` George Dunlap
2017-03-03 14:42 ` [PATCH v2 1/5] golang/xenlight: Create stub package George Dunlap

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.