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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 1/28/20 8:41 PM, Nick Rosbrook wrote:
>> 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 *think* to guarantee that libxl_<type>_init() has been called when
> unmarshaling, we would need to generate UnmarshalJSON functions to
> implement json.Unmarshaler. E.g.,:
> 
> func (dd *DeviceDisk) UnmarshalJSON(data []byte) error {
>         if dd == nil { // Or maybe this is the 'initialized' check you 
> mentioned
>                 dd, err := NewDeviceDisk()
>                 if err != nil {
>                         return err
>                 }
>         }
> 
>         return json.Unmarshal(data, dd)
> }
> 
> AFAICT, this would be required for someone to unmarshal a complete
> domain configuration in one go.

So the above will fix an issue like this:

Scenario A
1. Make a structure from version V by calling NewType()
2. Fill in what you want
3. Marshal it into json
4. Marshal it out of json into a structure from version V+1, with new fields

With code as above, the new elements of structure V+1 will be
initialized by the NewType() in the UnmarshalJSON() method.

But the problem I'm worried about is this:

Scenario B
1. Make an empty, uninitialized structure, without calling NewType()
2. Fill in some fields
3. Marshal it into json
4. Marshal it out of json (with the same version)

In the case above, step 3 encodes all the known fields with *golang*'s
zero values, rather than libxl's default values, and so step 4 will
clobber any defaults written by NewType() with golang zero values again.

Of course, something like scenario A might happen without the marshal /
unmarshal, which is why I thought having a private 'initialized' flag
might be helpful.  But then what you'd want to solve B by having the
unmarshaller read the initialized flag and add it to the json / read it
from the json (since the json package itself can't do it).

(Naturally people can directly modify the json to have the 'initialized'
flag set, but if you get to that level of messing around, you get to
keep all the pieces if it breaks.)

>> 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.
> 
> Yes, I plan on using these bindings in redctl (our Redfield toolstack)
> [1], to replace our os/exec calls to xl. To fully make that
> transition, we would need domain start/stop, PCI and network
> attach/detach, as well as some utilities (most of which are either
> implemented, or would be easy to do). But, making that transition is
> relatively low on the priority list right now, so I can't commit to a
> timeline unfortunately.

Sure, nor I; but having a goal always helps, even if it's only best-effort.

Looking at redctl, it seems like actually a pretty full-featured
toolstack -- that seems like a nice complete target to aim at. :-)

>> Re function calls -- do we want to just manually duplicate them as
>> needed, or try to get some sort of IDL support?
> 
> I think it will make more sense to manually duplicate them as needed.
> That way, we can be more particular about function signatures etc. to
> ensure they are idiomatic Go. Also, I am of the opinion that a minimal
> API is a better place to start. Which brings me to another question
> and potential work item:
> 
> Do we want to re-evaluate what is currently implemented in the API? Do
> you have plans to use everything that is currently there? If not, it
> might be nice to trim off things we don't need right now.

I think we should make sure that things actually work.  The very
original golang bindings I wrote to be able to control cpupools, so I
think those functions should stay.  But I'm not sure if anyone's ever
used ConsoleGetTty.  Like `xl.cfg` parsing, it's the sort of thing that
*somebody* will probably want eventually; so I'm inclined to say it
would be less cost to just test it and make sure it works than to remove
it and re-add it when someone decides they need it.

Did you have anything in particular in mind?

I was sort of thinking what we might do is leave `xenlight` as mostly
just a plain wrapper around the libxl C functions, as close to what
might be generated as possible; and then have another package that would
do something more useful.  For instance, having 'Vm' struct, which could
be Start()ed, shut down, and so on; and which would keep track of
if/when the domain died, &c.

>> 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
> 
> I was going to ask about this. You had a vanity URL in place at one
> point, right? Did `go get` ever work with that? In any case, pushing
> to another repo might be desirable.
We never actually had a URL in place, no.  I had simply chosen the URL
based on what would be a good combination of easy to type/remember and
easy to set up effectively.  (This was after having an informal chat
with Ian Jackson, who tends to do most of this sort of technical admin
thing.)  We'd probably want to go back and figure out what kind of
"interface" is possible, how to do versioning &c, and then work
backwards from that to get a workflow from the various Xen branches into
that.

BTW do you guys have a solution to the "install new tools then reboot"
issue?  I guess if you have a daemon then it will retain the old version
of the library until after you reboot?

 -George

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