[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: Mon, 27 Apr 2020 12:59:25 +0000
  • Accept-language: en-GB, en-US
  • Authentication-results: esa2.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
  • Cc: Nick Rosbrook <rosbrookn@xxxxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxx>
  • Delivery-date: Mon, 27 Apr 2020 12:59:48 +0000
  • Ironport-sdr: ZwvWdwgWnd+9EzfoTUuAOIANHIVr10hEtBYsA/jQDLNX75wNgBIP6xnPQR6cXSQwI9PZTY9Zry i6wqmGQSwJiM5zYuFK6ibVjpwGr9uWBf+o5hlhdRD9Q8yIOSjwKEf8RNFP7FocgOvtDQ+7yILR tqDwDVhG9tlI/O1a3YjEg+Kp8slWd1SdG9nUe+6EnKqyeRJZn6oI5XrEdKRRSxav9fuklbSSXm mb9UUcQE8m6LSXsjRQvQRsGJR+x/UoNiJmR8Sjt+HHtVIChI7Cr56BrXdqebeU0vSEdToL1/nL UrQ=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHWGeTLlIhJvo8WaUufFBMrVdiAt6iM0gqA
  • Thread-topic: [PATCH v2 1/1] golang/xenlight: add NameToDomid and DomidToName util functions


> 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

If you’re confident in your language lawyering skills, I could accept #1; but 
I’d prefer a comment with some sort of reference.  But if it were me I’d just 
go with #3; particularly given that this value could theoretically change 
(libxl has a stable API, not a stable ABI).

Everything else looks good.

If you want I could s/~Domid(0)/C.INVALID_DOMID/; myself, add my R-b and check 
it in.

 -George


 


Rackspace

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