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

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Apr 27, 2020, at 7:51 PM, Nick Rosbrook <rosbrookn@xxxxxxxxx> wrote:
> 
> 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?

The following seems to work for me.  In the C section:

// #define INVALID_DOMID_TYPED ((uint32_t)INVALID_DOMID)

Then:

    DomidInvalid Domid = C.INVALID_DOMID_TYPED

Attached is a PoC.  What do you think?

 -George

Attachment: invalid-domid-test.go
Description: invalid-domid-test.go