Re: [Xen-devel] [PATCH 0/4] xl: consolidate adhoc parsers

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Feb 17, 2016 at 10:01:37AM +0000, Ian Campbell wrote:
> On Tue, 2016-02-16 at 23:20 +0000, Wei Liu wrote:
> > On Tue, Feb 16, 2016 at 03:09:37PM -0700, Jim Fehlig wrote:
> > > Wei Liu wrote:
> > > > This patch series consolidates adhoc parsers in xl.
> > > 
> > > Hi Wei,
> > > 
> > > I never tested or reviewed this series after seeing Ian's comments. Did
> > > you have
> > > time to work on a V2? Or did I miss a V2? :-) Let me know if I can
> > > help.
> > > 
> > 
> > Sorry, this series fell through the crack -- it was only the work of
> > one afternoon when I had some free cycles. Now I'm busy with other
> > stuff and haven't had the time to pick it up again.
> > 
> > One thing I'm not entirely happy with writing this in plain C is that
> > there is subtle semantics difference from the ones generated by
> > flex/bison even if they expose similar APIs. For example, the
> > functions to parse disk spec won't allow you to set same attribute
> > twice, while the work present in this function allows you to do
> > that. It's cumbersome to implement the same functionality with open
> > coding in C IMHO.
> 
> Take care -- the xlu disk parser has a lot of additional complexity and
> scaffolding to support the "legacy" xm style device specifications with
> positional parameters and strange (ignored by xl) prefixes, trying to merge
> that with other device types is likely to result in unwanted/unmanageable
> complexity IMHO.
> 
> I'm also not sure that switching to flex+bison for the rest is going to
> solve the problems you mention above (disallowing setting something twice
> etc). flex+bison will take care nicely of things like tokenising and
> parsing the input (i.e. it is a nice replacement for strtok and friends),

Yes, this is what I'm after as first step -- it's all very vague idea in
my head at the moment. I was thinking we should use flex and bison to
tokenise the input, instead of doing everything by hand.

> but ultimately the keys and values need to be exposed to the user somehow
> i.e. put into some data structure.
> 
> That data structure could either be completely abstract (i.e. just provide
> "lookup by key" functionality, via some sort of hash or list etc, which is
> basically what the xlu cfg stuff does) or device type specific (i.e. fills
> in a given struct based on a knowledge of the value keys for that struct,
> which is what the disk stuff does) and it's in that code where you would
> need to handle things like what to do with repeated keys.
> 
> That is essentially just done by the C snippets in the bison rules or the
> scaffolding surrounding the bison, it's not something that bison inherently
> deals with.
> 
> So ultimately you need to deal with the question of what the semantics of
> setting a given key a second time are (error, take the first value, take
> the last value, per-key settings etc) in C code, even if you are using
> bison. That's slightly easier if you have a parser per device type, since
> you can just check the specific field in the specific struct for being
> unset or not before making a choice, but even in abstract code it's just a
> case of checking in the list/table/hash you are constructing during parsing
> for the key and reacting accordingly, but in both cases you are doing so in
> C not in "bison".
> 

Right, I do understand this.

I'm heavily biased towards formalising everything in BNF -- everyone who
provides a new device type should provide a parser for the configuration
string in BNF, as well as some test cases in our suite. I know this is a
bit unsocial so I'm trying to figure out a way if we can provide some
sensible framework to do that. It's not going to be easy. And whether
the return of investment is worthy of the effort is unclear. That's why
I propose it to be a GSoC project.

Again, perfection is the enemy of good. I think having everything done
in open coding C is fine. But since we need to have similar semantics
for all parsers we need to disallow setting same value twice, the
boilerplate for that is just tedious to write.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel