Re: RFC: Early mock-up of a xenopsd based on golang libxl bindings

[Nick Rosbrook <rosbrookn@xxxxxxxxx>]

On Fri, Oct 16, 2020 at 04:29:58PM +0000, George Dunlap wrote:
> https://gitlab.com/martyros/go-xen branch `working/xenops` contains a 
> super-basic mock-up of a unix domain xenopsd based on the golang libxl 
> bindings.
> 
> To use:
> 
> * Install Xen >= 4.14 on your target system
> 
> * Make sure you have go >= 1.11 installed
> 
> * Clone & build the server
> 
> $ git clone https://gitlab.com/martyros/go-xen
> 
> $ cd go-xen
> 
> $ git checkout working/xenops
> 
> Note that this is *not* a fast-forwarding branch.
> 
> $ cd xenops/xenopsd
> 
> $ go build
> 
> $ ./xenopsd
> 
> Theoretically this will now accept jsonrpc v1 calls on `/tmp/xenops`.  I 
> haven’t dug into exactly what the wire protocol looks like, but you can test 
> golang’s version of it by using one of the “client examples”.  In another 
> terminal:
> 
> $ cd xenops/client-examples
> 
> $ go run get-domains-example.co
> 
> It should list the currently-running domains and their domain names.
> 
> The core of the actual implementation is in go-xen/xenops/xenops/xenops.go.  
> Basically, every method you add to the Xenops type of the correct format 
> (described in the “net/rpc” documentation) will be exposed as a method 
> available via RPC.
> 
I haven't had a chance to run it yet, but the code all seems very
straight forward. Looks like a promising approach for prototyping.
> The current code only does a Unix socket, but it could easily be modified to 
> work over http as well.
> 
> Once we have function signatures in the libxl IDL, the xenops methods could 
> all be autogenerated, just like the types are for the golang bindings.
> 
It's on my todo list to get that RFC going again. :)

> It should be noted that at the moment there will be two “layers” of 
> translation both ways here: The golang package will be converting rpc into 
> golang structures, then the libxl libraries will be converting golang 
> structures into C structures; then any return values have to be converted 
> from C structures into golang structures, and then converted a gain from 
> golang structures into json before being sent back over the wire.  This may 
> or may not be a big overhead.
> 
> Two things that are currently sub-optimal about the `xenlight` package for 
> this use case.
> 
> First, although we have a xenlight.Error type, a lot of the xenlight wrappers 
> return a generic “error”.  I’m not sure how that will end up being converted 
> into json, but we might thing about making the xenlight wrappers all return 
> xenlight.Error instead.
>
Returning the "generic error" (i.e. the builtin error interface) is what
we want. Doing otherwise would be very awkward for callers. E.g., if
NameToDomid returned (xenlight.Domid, xenlight.Error), we would have:

  domid, err := ctx.NameToDomid(name)
  if err != nil { // <- Compile-time error; can't compare int to nil
   
  }

So, callers would need to explicitly convert to `error`, or always make
sure they declare `var err error` before assignment. 

This is essentially why the builtin `error` interface exists. 

Re: json, the RPC server will call Error() on any error we return from a
registered function, and set it in the Error field of the response
header (see [1]). So, I think it would be our responsibility to add an
additional "libxl error code" field to our RPC return types.

> Secondly, at the moment the xenlight types are in the same package as the 
> function wrappers.  This means that in order to build even the client, you 
> need to be able to link against an installed libxl library — even though the 
> final binary won’t need to link against libxl at all, and could theoretically 
> be on a completely separate host.
> 
> Unfortunately the way we’ve structured xenlight, it’s not simple to move 
> types.gen.go into its own package, because of the toC and fromC wrappers, 
> which *do* need to link against libxl (for the init and dispose functions).  
> Nick, we might think about whether we should make separate toC and fromC 
> functions for each of the types, rather than making those methods.
> 
Splitting the package would be one option, but we could also just use a
build tag that allows users to say "I don't need to link agaist libxl, I
just want the types." E.g., users could run `go build -tags nolibxl`, 
assuming we put `// +build !nolibxl` in the appropriate .go files.

I can send a proof of concept patch for that.

Thanks,
NR

[1] https://golang.org/pkg/net/rpc/#Response