Re: [libvirt] ignore vs. error when inappropriate or unrecognized attributes/elements are present in XML
On 08/22/2011 09:10 AM, Eric Blake wrote:
On 08/22/2011 01:36 AM, Laine Stump wrote:
> The problem I was *really* trying to point out is that of a keymap
> attribute being given *at all* when type is something other than "spice"
> or "vnc". The problem is that keymap is stored in a union, and the parts
> of the union that are active when type != "(spice|vnc)" don't have any
> keymap members. So even if keymap has something that would have been
> valid when type="(spice|vnc)" (but not otherwise), it isn't flagged in
> the parser (because the parser ignores keymap when it's not appropriate,
> rather than logging an error) and the driver *can't* do anything about
> it (because it has no way of knowing that a keymap was given - the
> parser can't put keymap into the config object because there's no place
> to put it).
To some degree, rng validation can map unions, if the distinguishing
choice between unions is an attribute also exposed in the xml. It is
done via <choice> and <group>, but it does make the rng larger; it
also helps to have more <define>s in order to avoid some repetition of
subelements that are common between more than one choice.
>
> In particular, to solve
> https://bugzilla.redhat.com/show_bug.cgi?id=638633 according to your
> recommendation, I think what I need to do is:
>
> 1) Remove the script attribute from the bridge & ethernet unions in
> virDomainNetDef and place it in the common part of the struct.
>
> 2) Change the parser to always populate script, no matter what is the
> type of the virDomainNetDef.
Not necessarily. We can instead rewrite the rng to forbid script
attributes from all but bridge and ethernet attributes. However,
there's still the issue that some, but not all, hypervisors can
support scripts with a bridge, so there's still some validation that
will have to be done in drivers to reject unsupported items even
though the rng allows it.
That's all great, but the fact remains that we don't use the RNG
anywhere in the libvirt API, so it does us no good, for example, when
running "virsh edit mydomain".
Until/unless we do that (validate input XML to the relevant RNG), having
RNGs that properly mirror unions in the config objects is just
preventing us from detecting improper usage. (and I'm doubtful this
validation is a good candidate for backporting)
For example, look at <define name="disk"> - it has a large <choice>
with a number of groups, each group defines one value for <attribute
name="type">, along with the sub-<elements> that are appropriate for
that attribute choice. I think we could do the same with <define
name="interface">, and split up <define
name="interface-options"> to
instead list which subelements belong under a given choice of
interface type.
Again, the RNG has a very thorough/correct representation of what should
be allowed, but the only place the RNG is used is when running the test
suite.
>
> 3) In the driver code that uses the virDomainNetDef, check for script in
> *all* cases (not just the cases where the driver *wants* to see a
> script), and log VIR_ERR_CONFIG_UNSUPPORTED when it is specified at an
> inappropriate time.
This part sounds right - anywhere that we allow parsing a script, we
have to then check in the driver whether a script was given, whether
or not the driver can support a script in that use case.