Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew

From: George Dunlap <george.dunlap@citrix.com>
To: Nick Rosbrook <rosbrookn@gmail.com>
Cc: Nick Rosbrook <rosbrookn@ainfosec.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH v3 8/8] RFC: Sketch constructors, DomainCreateNew
Date: Mon, 27 Jan 2020 18:08:15 +0000	[thread overview]
Message-ID: <4db0f4fa-98db-33d6-5be5-f6ea59096166@citrix.com> (raw)
In-Reply-To: <CAEBZRSc_+G6itzyNGMd7GO5eC6aOZ3zE7vopQmTiQ5CnG+6VYw@mail.gmail.com>

On 1/24/20 7:32 PM, Nick Rosbrook wrote:
> Sorry for the late reply on this one.
> 
>> +func NewDomainConfig(t DomainType) (*DomainConfig, error) {
>> +       var cconfig C.libxl_domain_config
>> +
>> +       C.libxl_domain_config_init(&cconfig)
>> +       C.libxl_domain_build_info_init_type(&cconfig.b_info, C.libxl_domain_type(t))
>> +
>> +       gconfig := &DomainConfig{}
>> +       err := gconfig.fromC(&cconfig)
>> +       if err != nil {
>> +               return nil, err
>> +       }
>> +
>> +       return gconfig, nil
>> +}
> 
> I think this is sufficient for the "New" functions; simple is probably
> better here. I appreciate the idea of the `populate func` approach you
> mentioned in another email, but I think in practice people will want
> to leverage encoding/json or otherwise to unmarshal some data into a
> DomainConfig etc. Or, we would hopefully be able to unmarshal an
> xl.cfg. All that is to say, I think the approach you have shown here
> gives us and package users the most flexibility.

I think marshaling and unmarshalling should be fine, *as long as* the
structure being unmarshalled actually went through the
libxl_<type>_init() function first.

In fact, I was kicking around the idea of adding a non-exported field to
all the generated structs -- maybe "bool initalized" -- and having the
.fromC() methods set this to 'true', and the .toC() methods return an
error if it wasn't set.  But then we'd need to write custom JSON
marshallers to handle these.

I'm not personally very interested in parsing xl.cfg files, but libxl
has library functions to do that, so it should be something very easy to
add if you want.  (Although in fact, it looks like a lot of the code to
actually interpret the results of the parsing is in xl; we might want to
see about moving some of that functionality into libxlu.)

> If we stick with this outline for constructors, they will be easy to
> generate. I'm happy to work on that, unless you were already planning
> on it.

I'm afraid my 1 day a week of coding is going to have to be diverted to
something else for a month or so; so please go ahead if you have the time.

As far as further steps -- do you have a clear idea what kind of
functionality you'd like to see possible by the time of the feature
freeze (probably in May)?  Do you have plans to use these bindings
yourself, and if so, how?

For my part, I want to start and reap guests.  The latter will require
adding event callback functionality which will require more thought (and
perhaps expose more libxl issues).  But I don't yet have a clear target
beyond that.

Re function calls -- do we want to just manually duplicate them as
needed, or try to get some sort of IDL support?

Other work items include:

* modifying configure to detect the availability of go and enable the
bindings if it's available

* Enabling go build testing in the gitlab CI loop

* Making `go get` work, which might involve having code to push
post-generated code to a repo and tagging as appropriate

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel