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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 23/01/17 16:43, 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.

So this is not true at the moment; and as a result the patch doesn't build.

As I said in response to Dario's suggestion, I think in this
chicken-and-egg situation it makes sense to just use fmt.Errorf() for
this patch, and then change them to Error(-ret) in the next patch.

> 
> Signed-off-by: Ronald Rojas <ronladred@xxxxxxxxx>
> ---
>  tools/Makefile                    | 17 ++++++++
>  tools/golang/xenlight/Makefile    | 31 ++++++++++++++
>  tools/golang/xenlight/xenlight.go | 86 
> +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+)
>  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..c1e975f 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -11,6 +11,8 @@ SUBDIRS-y += xenstore
>  SUBDIRS-y += misc
>  SUBDIRS-y += examples
>  SUBDIRS-y += hotplug
> +#Uncomment line to build Golang libxl
> +#SUBDIRS-y += golang/xenlight

This won't build from a clean tree, because at this point libxl hasn't
been built yet; and if you modify libxl and re-build, it will use the
old version of libxl rather than the new one you just modified.  It
needs to go a bit further down, under "SUBDIRS-y += libxl".

(Actually this whole list could use some reordering, but that's not your
problem.)

>  SUBDIRS-y += xentrace
>  SUBDIRS-$(CONFIG_XCUTILS) += xcutils
>  SUBDIRS-$(CONFIG_X86) += firmware
> @@ -303,6 +305,21 @@ subdir-clean-qemu-xen-dir:
>               $(MAKE) -C qemu-xen-dir clean; \
>       fi
>  
> +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
> +
>  subdir-clean-debugger/gdbsx subdir-distclean-debugger/gdbsx: .phony
>       $(MAKE) -C debugger/gdbsx clean
>  
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> new file mode 100644
> index 0000000..1c2a2b7
> --- /dev/null
> +++ b/tools/golang/xenlight/Makefile
> @@ -0,0 +1,31 @@
> +XEN_ROOT=$(CURDIR)/../../..
> +GOLANG_SRC=$(GOPATH)/src/xenproject.org/xenlight

So GOPATH is actually like the PATH variable: you can have several
directories in it.

I think this should probably be:

GOCODEDIR ?= $(prefix)/share/cogode
GOLANG_SRC = $(GOCODEDIR)/src/xenproject.org/xenlight

Other than that, looks good -- I think with those three changes, this
patch should be ready to go in. :-)

 -George

> +CGO_CFLAGS = -I$(DESTDIR)$(includedir)
> +CGO_LDFLAGS = -L$(DESTDIR)$(libdir) -Wl,-rpath-link=$(DESTDIR)$(libdir)
> +include $(XEN_ROOT)/tools/Rules.mk
> +
> +BINARY = xenlight.a
> +GO ?= go
> +
> +.PHONY: all
> +all: build
> +
> +.PHONY: build
> +build: xenlight.a
> +
> +.PHONY: install
> +install: build
> +     $(INSTALL_DIR) $(DESTDIR)$(GOLANG_SRC)
> +     $(INSTALL_DATA) xenlight.go $(DESTDIR)$(GOLANG_SRC)
> +
> +.PHONY: clean
> +clean:
> +     $(RM) $(BINARY)
> +
> +.PHONY: distclean
> +distclean: clean
> +
> +xenlight.a: xenlight.go
> +     CGO_CFLAGS="$(CGO_CFLAGS)" CGO_LDFLAGS="$(CGO_LDFLAGS)" $(GO) build -o 
> $@ $<
> +
> +-include $(DEPS)
> diff --git a/tools/golang/xenlight/xenlight.go 
> b/tools/golang/xenlight/xenlight.go
> new file mode 100644
> index 0000000..f82e14e
> --- /dev/null
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -0,0 +1,86 @@
> +/*
> + * 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"
> +
> +/*
> + * 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
> +     }
> +
> +     ret := C.libxl_ctx_alloc(unsafe.Pointer(&Ctx.ctx), C.LIBXL_VERSION, 0, 
> nil)
> +
> +     if ret != 0 {
> +             err = Error(-ret)
> +     }
> +     return
> +}
> +
> +func (Ctx *Context) Close() (err error) {
> +     ret := C.libxl_ctx_free(unsafe.Pointer(Ctx.ctx))
> +     Ctx.ctx = nil
> +
> +     if ret != 0 {
> +             err = Error(-ret)
> +     }
> +     return
> +}
> +
> +func (Ctx *Context) CheckOpen() (err error) {
> +     if Ctx.ctx == nil {
> +             err = fmt.Errorf("Context not opened")
> +     }
> +     return
> +}
> \ No newline at end of file
> 


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