|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 5/5] golang/xenlight: Add tests host related functionality functions
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@xxxxxxxxx>
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
>
> ---
>
> - 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@xxxxxxxxxxxxx
> CC: george.dunlap@xxxxxxxxxx
> CC: ian.jackson@xxxxxxxxxxxxx
> CC: wei.liu2@xxxxxxxxxx
> ---
> ---
> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |