Re: [PATCH V3 13/13] xl: introduce "xl-json" format

[Wei Liu <wei.liu2@citrix.com>]

On Thu, Apr 24, 2014 at 05:11:11PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH V3 13/13] xl: introduce "xl-json" format"):
> > Originally xl verbatimly copies the domain config file in its user data
> > store. This patch adds the functionality to transform text domain config
> > file to JSON object and save that object in user data store.
> 
> As I said on IRC, I think you should explain your plans for this.  I
> approve of your plans :-).
> 
> > What this patch does:
> > 1. add a mandatory flag to save protocol to indicate whether the saved
> >    config is a JSON object
> > 2. register a new private data type "xl-json" in libxl.h
> > 3. modify xl to save / load "xl-json" file where necessary
> > 
> > After this change xl supports both "xl" format and "xl-json" format.
> > The user-supplied config file is still restricted to normal text config
> > file format ("xl"), as xl has more sanity checks when parsing text
> > config file. "xl-json" format is only used internally. The saved config
> > file is always in "xl-json" format.
> 
> I think it would be very useful to allow people to
>   - retreive the domain's json config

This can already be done. Just that the JSON config is parsed from
stored config file and not up to date.

>   - start a new domain with a json config

This is doable.

> That would let you write a program to adjust a domain's config, for
> example.
> 
> If there is something important error that xl checks for, which can
> also be represented as an error in the domain config IDL struct, but
> which isn't checked for by libxl, I think that's a bug in libxl.
> 

OK, got it. The parser should be able to catch illegal input and aborts.
The concern is that the input is legal but not meaningful to libxl, say,
some fileds require a string and that's the only thing parser checks.
But since you point out any breakage caused by illegal input beyond
parser's scope is classfied as libxl bug, then I think my concern on
sanity check can go away.

Wei.

> > Tests done so far (xl.{new,old} denotes xl with{,out} "xl_json"
> > support):
> > 1. xl.new create then hexdump saved file: domain config saved in JSON format
> > 2. xl.new create then xl.old restore: failed on mandatory flag check
> > 3. xl.new create then xl.new restore: succeeded
> > 4. xl.old create then xl.new restore: succeeded
> > 5. xl.new create then local migrate, receiving end xl.new: succeeded
> > 6. xl.old create then local migrate, receiving end xl.new: succeeded
> 
> Great, thanks.
> 
> Ian.