Re: [PATCH v2 1/1] golang/xenlight: add NameToDomid and DomidToName util functions

[Nick Rosbrook <rosbrookn@xxxxxxxxx>]

On Mon, Apr 27, 2020 at 8:59 AM George Dunlap <George.Dunlap@xxxxxxxxxx> wrote:
>
>
>
> > On Apr 24, 2020, at 4:02 AM, Nick Rosbrook <rosbrookn@xxxxxxxxx> wrote:
> >
> > Many exported functions in xenlight require a domid as an argument. Make
> > it easier for package users to use these functions by adding wrappers
> > for the libxl utility functions libxl_name_to_domid and
> > libxl_domid_to_name.
> >
> > Signed-off-by: Nick Rosbrook <rosbrookn@xxxxxxxxxxxx>
> > ---
> > tools/golang/xenlight/xenlight.go | 38 ++++++++++++++++++++++++++++++-
> > 1 file changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/golang/xenlight/xenlight.go 
> > b/tools/golang/xenlight/xenlight.go
> > index 6b4f492550..d1d30b63e1 100644
> > --- a/tools/golang/xenlight/xenlight.go
> > +++ b/tools/golang/xenlight/xenlight.go
> > @@ -21,13 +21,13 @@ package xenlight
> > #cgo LDFLAGS: -lxenlight -lyajl -lxentoollog
> > #include <stdlib.h>
> > #include <libxl.h>
> > +#include <libxl_utils.h>
> >
> > static const libxl_childproc_hooks childproc_hooks = { .chldowner = 
> > libxl_sigchld_owner_mainloop };
> >
> > void xenlight_set_chldproc(libxl_ctx *ctx) {
> >       libxl_childproc_setmode(ctx, &childproc_hooks, NULL);
> > }
> > -
> > */
> > import "C"
> >
> > @@ -75,6 +75,10 @@ var libxlErrors = map[Error]string{
> >       ErrorFeatureRemoved:               "Feature removed",
> > }
> >
> > +const (
> > +     DomidInvalid Domid = ^Domid(0)
>
> Not to be a stickler, but are we positive that this will always result in the 
> same value as C.INVALID_DOMID?
>
> There are some functions that will return INVALID_DOMID, or accept 
> INVALID_DOMID as an argument.  Users of the `xenlight` package will 
> presumably need to compare against this value and/or pass it in.
>
> It seems like we could:
>
> 1. Rely on language lawyering to justify our assumption that ^Domid(0) will 
> always be the same as C “~0”
>
> 2. On marshalling into / out of C, convert C.INVALID_DOMID to DomidInvalid
>
> 3. Set Domid = C.INVALID_DOMID

I just tested this way, and it does not work as expected. Since
C.INVALID_DOMID is #define'd, cgo does not know it is intended to be
used as uint32_t, and decides to declare C.INVALID_DOMID as int. So,
C.INVALID_DOMID = ^int(0) = -1, which overflows Domid.

I tried a few things in the cgo preamble without any luck.
Essentially, one cannot define a const uint32_t in C space, and use
that to define a const in Go space.

Any ideas?

-NR