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

[Nick Rosbrook <rosbrookn@xxxxxxxxx>]

> 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.)

Yeah, that makes sense. I'll add that.

> > +     }
> > +
> > +     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.

I considered returning an error if C.GoString(cname) == "". But, with
these functions (as well as the others in these series), I opted to
keep the signatures aligned with their libxl counterparts since we're
keeping the package API mostly one-to-one with libxl. I can add a
second return value if you prefer, otherwise I'll just add a note in
the comment.

> 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.

That's good to know, thanks. I'll add that to the comments as well.

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

Thanks!

-NR