[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Nick Rosbrook <rosbrookn@xxxxxxxxx>
  • From: George Dunlap <George.Dunlap@xxxxxxxxxx>
  • Date: Tue, 12 May 2020 13:23:18 +0000
  • Accept-language: en-GB, en-US
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=George.Dunlap@xxxxxxxxxx; spf=Pass smtp.mailfrom=George.Dunlap@xxxxxxxxxx; spf=None smtp.helo=postmaster@xxxxxxxxxxxxxxx; dmarc=pass (p=none dis=none) d=citrix.com
  • Cc: Nick Rosbrook <rosbrookn@xxxxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxx>
  • Delivery-date: Tue, 12 May 2020 13:23:33 +0000
  • Ironport-sdr: aJxO6Y5oMvLWSh28PJ4mUV68jCAICCitJYcw7yacTTsmlKSc/XKWXYG3626LXMDar0FfgnuRmo Ud28dy0F7UmquHV6bxHttpSO5wYJnucxrJbcgSyuHyDhllJ25ID1xTq3qu37m7t01KNAK4dUJK 1WUsAZUpu/U2eOELRZQ5NrmkOgQPGBgtCroPX5CCLdw5MnwwKASmsblmVYkU/AGaky7GbZN+sl cJQCMBJCt0LvU6FHoHpFNjpc/YS3brejpY27pQp/BUCXqudWrwscytO1HkPuxQxtYn+Gjfo3Qg Pls=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHWGeTLlIhJvo8WaUufFBMrVdiAt6iM0gqAgABiaYCAFzc7AA==
  • Thread-topic: [PATCH v2 1/1] golang/xenlight: add NameToDomid and DomidToName util functions


> 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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.