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

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Apr 12, 2020, at 11:02 PM, 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 | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
> 
> diff --git a/tools/golang/xenlight/xenlight.go 
> b/tools/golang/xenlight/xenlight.go
> index 3f1b0baa0c..8492bcec4e 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -20,6 +20,7 @@ package xenlight
> #cgo LDFLAGS: -lxenlight -lyajl -lxentoollog
> #include <stdlib.h>
> #include <libxl.h>
> +#include <libxl_utils.h>
> */
> import "C"
> 
> @@ -124,6 +125,28 @@ func (ctx *Context) Close() error {
> 
> type Domid uint32
> 
> +// NameToDomid returns the Domid for a domain, given its name, if it exists.
> +func (Ctx *Context) NameToDomid(name string) (Domid, error) {
> +     var domid C.uint32_t
> +
> +     cname := C.CString(name)
> +     defer C.free(unsafe.Pointer(cname))
> +
> +     if ret := C.libxl_name_to_domid(Ctx.ctx, cname, &domid); ret != 0 {
> +             return ^Domid(0), Error(ret)

libxl.h defines INVALID_DOMID — do we want to define an exported constant with 
the same name and use that here?  (Although part of me wonders if DOMID_INVALID 
would be a better option.)

> +     }
> +
> +     return Domid(domid), nil
> +}
> +
> +// DomidToName returns the name for a domain, given its domid.
> +func (Ctx *Context) DomidToName(domid Domid) string {
> +     cname := C.libxl_domid_to_name(Ctx.ctx, C.uint32_t(domid))
> +     defer C.free(unsafe.Pointer(cname))
> +
> +     return C.GoString(cname)
> +}

It looks to me like if the domid doesn’t exist, libxl_domid_to_name() will 
return NULL; and then DomidToName will return “”.  Is that what we want?

If so, it should probably be documented.

One thing that might be worth pointing out is that both of these functions are 
actually racy: There’s no guarantee that by the time libxl_domid_to_name() 
returns that the domain with that name has died, and another domain with a 
different name has re-used the same domid.  That’s partly why Xen has the whole 
“domain reaper” system, like for Unix processes (which so far isn’t 
implementable yet with the golang wrappers).  I think when we make our “vm” 
library, we’re going to want to try to come up with something like an object 
that makes it easy to avoid this sort of race.

But that’s a discussion for another day. :-)  Everything else looks good, 
thanks.

 -George