Re: [PATCH RFC 1/8] golang/xenlight: Create stub package

[George Dunlap <george.dunlap@citrix.com>]

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

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

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

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

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

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

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

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

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

 -George


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